CppEditor: Add a second variant of "move def to decl" quickfix

This time with the cursor on the declaration.

Fixes: QTCREATORBUG-9515
Change-Id: I50b2ac8516f4df98e4cc9e3ffa60a9e5cf079c4e
Reviewed-by: <github-actions-qt-creator@cristianadam.eu>
Reviewed-by: Christian Stenger <christian.stenger@qt.io>

src/plugins/cppeditor/cppquickfix_test.cpp

@@ -7256,7 +7256,7 @@ void QuickfixTest::testMoveFuncDefToDecl_data()
 
     originalHeader =
         "class Foo {\n"
-        "    inline int number() const;\n"
+        "    inline int @number() const;\n"
         "};\n";
     expectedHeader =
         "class Foo {\n"
@@ -7274,7 +7274,7 @@ void QuickfixTest::testMoveFuncDefToDecl_data()
 
     originalSource =
         "class Foo {\n"
-        "  inline int number() const;\n"
+        "  inline int @number() const;\n"
         "};\n"
         "\n"
         "int Foo::num@ber() const\n"
@@ -7295,7 +7295,7 @@ void QuickfixTest::testMoveFuncDefToDecl_data()
     originalHeader =
         "namespace MyNs {\n"
         "class Foo {\n"
-        "  inline int number() const;\n"
+        "  inline int @number() const;\n"
         "};\n"
         "}\n";
     expectedHeader =
@@ -7322,7 +7322,7 @@ void QuickfixTest::testMoveFuncDefToDecl_data()
     originalHeader =
         "namespace MyNs {\n"
         "class Foo {\n"
-        "  inline int number() const;\n"
+        "  inline int numbe@r() const;\n"
         "};\n"
         "}\n";
     expectedHeader =
@@ -7353,7 +7353,7 @@ void QuickfixTest::testMoveFuncDefToDecl_data()
     originalSource =
         "namespace MyNs {\n"
         "class Foo {\n"
-        "  inline int number() const;\n"
+        "  inline int @number() const;\n"
         "};\n"
         "\n"
         "int Foo::numb@er() const\n"
@@ -7373,7 +7373,7 @@ void QuickfixTest::testMoveFuncDefToDecl_data()
     QTest::newRow("member function, one file, namespace")
             << QByteArrayList() << QByteArrayList{originalSource, expectedSource};
 
-    originalHeader = "int number() const;\n";
+    originalHeader = "int nu@mber() const;\n";
     expectedHeader =
         "inline int number() const\n"
         "{\n"
@@ -7393,7 +7393,7 @@ void QuickfixTest::testMoveFuncDefToDecl_data()
 
     originalHeader =
         "namespace MyNamespace {\n"
-        "int number() const;\n"
+        "int n@umber() const;\n"
         "}\n";
     expectedHeader =
         "namespace MyNamespace {\n"
@@ -7418,7 +7418,7 @@ void QuickfixTest::testMoveFuncDefToDecl_data()
     originalHeader =
         "class Foo {\n"
         "public:\n"
-        "    Foo();\n"
+        "    Fo@o();\n"
         "private:\n"
         "    int a;\n"
         "    float b;\n"
@@ -7443,7 +7443,7 @@ void QuickfixTest::testMoveFuncDefToDecl_data()
     originalSource =
         "struct Foo\n"
         "{\n"
-        "    void foo();\n"
+        "    void f@oo();\n"
         "} bar;\n"
         "void Foo::fo@o()\n"
         "{\n"
@@ -7465,7 +7465,7 @@ void QuickfixTest::testMoveFuncDefToDecl_data()
         "    virtual int foo() = 0;\n"
         "};\n"
         "struct Derived : Base {\n"
-        "    int foo() override;\n"
+        "    int @foo() override;\n"
         "};\n"
         "\n"
         "int Derived::fo@o()\n"
@@ -7487,7 +7487,7 @@ void QuickfixTest::testMoveFuncDefToDecl_data()
 
     originalSource =
         "template<class T>\n"
-        "class Foo { void func(); };\n"
+        "class Foo { void @func(); };\n"
         "\n"
         "template<class T>\n"
         "void Foo<T>::fu@nc() {}\n";
@@ -7501,7 +7501,7 @@ void QuickfixTest::testMoveFuncDefToDecl_data()
         "class Foo\n"
         "{\n"
         "    template<class T>\n"
-        "    void func();\n"
+        "    void @func();\n"
         "};\n"
         "\n"
         "template<class T>\n"
@@ -7524,15 +7524,32 @@ void QuickfixTest::testMoveFuncDefToDecl()
     QVERIFY(headers.isEmpty() || headers.size() == 2);
     QVERIFY(sources.size() == 2);
 
+    QByteArray &declDoc = !headers.empty() ? headers.first() : sources.first();
+    const int declCursorPos = declDoc.indexOf('@');
+    QVERIFY(declCursorPos != -1);
+    const int defCursorPos = sources.first().lastIndexOf('@');
+    QVERIFY(defCursorPos != -1);
+    QVERIFY(declCursorPos != defCursorPos);
+
+    declDoc.remove(declCursorPos, 1);
     QList<TestDocumentPtr> testDocuments;
     if (!headers.isEmpty())
         testDocuments << CppTestDocument::create("file.h", headers.first(), headers.last());
     testDocuments << CppTestDocument::create("file.cpp", sources.first(), sources.last());
 
-    MoveFuncDefToDecl factory;
+    MoveFuncDefToDeclPush pushFactory;
-    QuickFixOperationTest(testDocuments, &factory);
+    QuickFixOperationTest(testDocuments, &pushFactory);
-}
 
+    declDoc.insert(declCursorPos, '@');
+    sources.first().remove(defCursorPos, 1);
+    testDocuments.clear();
+    if (!headers.isEmpty())
+        testDocuments << CppTestDocument::create("file.h", headers.first(), headers.last());
+    testDocuments << CppTestDocument::create("file.cpp", sources.first(), sources.last());
+
+    MoveFuncDefToDeclPull pullFactory;
+    QuickFixOperationTest(testDocuments, &pullFactory);
+}
 
 void QuickfixTest::testMoveFuncDefToDeclMacroUses()
 {
@@ -7560,7 +7577,7 @@ void QuickfixTest::testMoveFuncDefToDeclMacroUses()
         "    }\n"
         "};\n\n\n\n";
 
-    MoveFuncDefToDecl factory;
+    MoveFuncDefToDeclPush factory;
     QuickFixOperationTest(singleDocument(original, expected), &factory,
                           ProjectExplorer::HeaderPaths(), 0, "QTCREATORBUG-12314");
 }

src/plugins/cppeditor/cppquickfixes.cpp

@@ -6763,20 +6763,25 @@ namespace {
 class MoveFuncDefToDeclOp : public CppQuickFixOperation
 {
 public:
+    enum Type { Push, Pull };
     MoveFuncDefToDeclOp(const CppQuickFixInterface &interface,
                         const FilePath &fromFilePath, const FilePath &toFilePath,
-                        FunctionDefinitionAST *funcDef, const QString &declText,
+                        FunctionDefinitionAST *funcAst, Function *func, const QString &declText,
                         const ChangeSet::Range &fromRange,
-                        const ChangeSet::Range &toRange)
+                        const ChangeSet::Range &toRange,
+                        Type type)
         : CppQuickFixOperation(interface, 0)
         , m_fromFilePath(fromFilePath)
         , m_toFilePath(toFilePath)
-        , m_funcAST(funcDef)
+        , m_funcAST(funcAst)
+        , m_func(func)
         , m_declarationText(declText)
         , m_fromRange(fromRange)
         , m_toRange(toRange)
     {
-        if (m_toFilePath == m_fromFilePath) {
+        if (type == Type::Pull) {
+            setDescription(Tr::tr("Move Definition Here"));
+        } else if (m_toFilePath == m_fromFilePath) {
             setDescription(Tr::tr("Move Definition to Class"));
         } else {
             const FilePath resolved = m_toFilePath.relativePathFrom(m_fromFilePath.parentDir());
@@ -6784,12 +6789,17 @@ public:
         }
     }
 
+private:
     void perform() override
     {
         CppRefactoringChanges refactoring(snapshot());
         CppRefactoringFilePtr fromFile = refactoring.file(m_fromFilePath);
         CppRefactoringFilePtr toFile = refactoring.file(m_toFilePath);
 
+        ensureFuncDefAstAndRange(*fromFile);
+        if (!m_funcAST)
+            return;
+
         const QString wholeFunctionText = m_declarationText
                 + fromFile->textOf(fromFile->endOf(m_funcAST->declarator),
                                    fromFile->endOf(m_funcAST->function_body));
@@ -6811,18 +6821,44 @@ public:
         }
     }
 
-private:
+    void ensureFuncDefAstAndRange(CppRefactoringFile &defFile)
+    {
+        if (m_funcAST) {
+            QTC_CHECK(m_fromRange.end > m_fromRange.start);
+            return;
+        }
+        QTC_ASSERT(m_func, return);
+        const QList<AST *> astPath = ASTPath(defFile.cppDocument())(m_func->line(),
+                                                                    m_func->column());
+        if (astPath.isEmpty())
+            return;
+        for (auto it = std::rbegin(astPath); it != std::rend(astPath); ++it) {
+            m_funcAST = (*it)->asFunctionDefinition();
+            if (!m_funcAST)
+                continue;
+            AST *astForRange = m_funcAST;
+            const auto prev = std::next(it);
+            if (prev != std::rend(astPath)) {
+                if (const auto templAst = (*prev)->asTemplateDeclaration())
+                    astForRange = templAst;
+            }
+            m_fromRange = defFile.range(astForRange);
+            return;
+        }
+    }
+
     const FilePath m_fromFilePath;
     const FilePath m_toFilePath;
     FunctionDefinitionAST *m_funcAST;
+    Function *m_func;
     const QString m_declarationText;
-    const ChangeSet::Range m_fromRange;
+    ChangeSet::Range m_fromRange;
     const ChangeSet::Range m_toRange;
 };
 
 } // anonymous namespace
 
-void MoveFuncDefToDecl::match(const CppQuickFixInterface &interface, QuickFixOperations &result)
+void MoveFuncDefToDeclPush::match(const CppQuickFixInterface &interface, QuickFixOperations &result)
 {
     const QList<AST *> &path = interface.path();
     AST *completeDefAST = nullptr;
@@ -6937,10 +6973,53 @@ void MoveFuncDefToDecl::match(const CppQuickFixInterface &interface, QuickFixOpe
         result << new MoveFuncDefToDeclOp(interface,
                                           interface.filePath(),
                                           declFilePath,
-                                          funcAST, declText,
+                                          funcAST, func, declText,
-                                          defRange, declRange);
+                                          defRange, declRange, MoveFuncDefToDeclOp::Push);
 }
 
+void MoveFuncDefToDeclPull::match(const CppQuickFixInterface &interface,
+                                  QuickFixOperations &result)
+{
+    const QList<AST *> &path = interface.path();
+    for (auto it = std::rbegin(path); it != std::rend(path); ++it) {
+        SimpleDeclarationAST * const simpleDecl = (*it)->asSimpleDeclaration();
+        if (!simpleDecl)
+            continue;
+        const auto prev = std::next(it);
+        if (prev != std::rend(path) && (*prev)->asStatement())
+            return;
+        if (!simpleDecl->symbols || !simpleDecl->symbols->value || simpleDecl->symbols->next)
+            return;
+        Declaration * const decl = simpleDecl->symbols->value->asDeclaration();
+        if (!decl)
+            return;
+        Function * const funcDecl = decl->type()->asFunctionType();
+        if (!funcDecl)
+            return;
+        if (funcDecl->isSignal() || funcDecl->isPureVirtual() || funcDecl->isFriend())
+            return;
+
+        // Is there a definition?
+        SymbolFinder symbolFinder;
+        Function * const funcDef = symbolFinder.findMatchingDefinition(decl, interface.snapshot(),
+                                                                       true);
+        if (!funcDef)
+            return;
+
+        QString declText = interface.currentFile()->textOf(simpleDecl);
+        declText.chop(1); // semicolon
+        declText.prepend(inlinePrefix(interface.filePath(), [funcDecl] {
+            return !funcDecl->enclosingScope()->asClass();
+        }));
+        result << new MoveFuncDefToDeclOp(interface, funcDef->filePath(), decl->filePath(), nullptr,
+                                          funcDef, declText, {},
+                                          interface.currentFile()->range(simpleDecl),
+                                          MoveFuncDefToDeclOp::Pull);
+        return;
+    }
+}
+
+
 namespace {
 
 class AssignToLocalVariableOperation : public CppQuickFixOperation
@@ -9744,7 +9823,8 @@ void createCppQuickFixes()
 
     new MoveFuncDefOutside;
     new MoveAllFuncDefOutside;
-    new MoveFuncDefToDecl;
+    new MoveFuncDefToDeclPush;
+    new MoveFuncDefToDeclPull;
 
     new AssignToLocalVariable;
 

src/plugins/cppeditor/cppquickfixes.h

@@ -520,9 +520,18 @@ public:
 };
 
 /*!
- Moves the definition of a function to its declaration.
+ Moves the definition of a function to its declaration, with the cursor on the definition.
  */
-class MoveFuncDefToDecl: public CppQuickFixFactory
+class MoveFuncDefToDeclPush : public CppQuickFixFactory
+{
+public:
+    void match(const CppQuickFixInterface &interface, TextEditor::QuickFixOperations &result) override;
+};
+
+/*!
+ Moves the definition of a function to its declaration, with the cursor on the declaration.
+ */
+class MoveFuncDefToDeclPull : public CppQuickFixFactory
 {
 public:
     void match(const CppQuickFixInterface &interface, TextEditor::QuickFixOperations &result) override;