CppTools: Fix nextToSurroundingDefinitions

Now it is checked if the adjacent declaration is also defined and one
can define the destination file.

Change-Id: Ifff59c49fc2ab3e2f36f41df42ae4b7d7ff8dd35
Reviewed-by: Nikolai Kosjar <nikolai.kosjar@digia.com>

src/plugins/cppeditor/cppquickfix_test.cpp

@@ -1791,21 +1791,31 @@ void CppEditorPlugin::test_quickfix_MoveFuncDefOutside_MemberFuncOutside()
 {
     QByteArray original =
         "class Foo {\n"
-        "  inline int numbe@r() const\n"
+        "    void f1();\n"
+        "    inline int f2@() const\n"
         "    {\n"
-        "    return 5;\n"
+        "        return 1;\n"
         "    }\n"
-        "};";
+        "    void f3();\n"
+        "    void f4();\n"
+        "};\n"
+        "\n"
+        "void Foo::f4() {}\n";
     QByteArray expected =
         "class Foo {\n"
-        "  inline int number() const;\n"
+        "    void f1();\n"
+        "    inline int f2@() const;\n"
+        "    void f3();\n"
+        "    void f4();\n"
         "};\n"
         "\n"
-        "int Foo::number() const\n"
+        "int Foo::f2() const\n"
         "{\n"
-        "    return 5;\n"
-        "}"
-        "\n\n";
+        "    return 1;\n"
+        "}\n"
+        "\n"
+        "\n"
+        "void Foo::f4() {}\n\n";
 
     MoveFuncDefOutside factory;
     TestCase data(original, expected);

src/plugins/cpptools/cppcodegen_test.cpp

@@ -621,3 +621,150 @@ void CppToolsPlugin::test_codegen_definition_middle_member()
     QCOMPARE(loc.prefix(), QLatin1String("\n\n"));
     QCOMPARE(loc.suffix(), QString());
 }
+
+void CppToolsPlugin::test_codegen_definition_middle_member_surrounded_by_undefined()
+{
+    const QByteArray srcText = "\n"
+            "class Foo\n"  // line 1
+            "{\n"
+            "void foo();\n" // line 3
+            "void bar();\n" // line 4
+            "void baz();\n" // line 5
+            "void car();\n" // line 6
+            "};\n"
+            "\n";
+
+    const QByteArray dstText = QString::fromLatin1(
+            "\n"
+            "#include \"%1/file.h\"\n" // line 1
+            "int x;\n"
+            "\n"
+            "void Foo::car()\n" // line 4
+            "{\n"
+            "\n"
+            "}\n"
+            "\n"
+            "int y;\n").arg(QDir::tempPath()).toLatin1();
+
+    Document::Ptr src = Document::create(QDir::tempPath() + QLatin1String("/file.h"));
+    Utils::FileSaver srcSaver(src->fileName());
+    srcSaver.write(srcText);
+    srcSaver.finalize();
+    src->setUtf8Source(srcText);
+    src->parse();
+    src->check();
+    QCOMPARE(src->diagnosticMessages().size(), 0);
+    QCOMPARE(src->globalSymbolCount(), 1U);
+
+    Document::Ptr dst = Document::create(QDir::tempPath() + QLatin1String("/file.cpp"));
+    dst->addIncludeFile(Document::Include(QLatin1String("file.h"), src->fileName(), 1,
+                                          Client::IncludeLocal));
+    Utils::FileSaver dstSaver(dst->fileName());
+    dstSaver.write(dstText);
+    dstSaver.finalize();
+    dst->setUtf8Source(dstText);
+    dst->parse();
+    dst->check();
+    QCOMPARE(dst->diagnosticMessages().size(), 0);
+    QCOMPARE(dst->globalSymbolCount(), 3U);
+
+    Snapshot snapshot;
+    snapshot.insert(src);
+    snapshot.insert(dst);
+
+    Class *foo = src->globalSymbolAt(0)->asClass();
+    QVERIFY(foo);
+    QCOMPARE(foo->line(), 1U);
+    QCOMPARE(foo->column(), 7U);
+    QCOMPARE(foo->memberCount(), 4U);
+    Declaration *decl = foo->memberAt(1)->asDeclaration();
+    QVERIFY(decl);
+    QCOMPARE(decl->line(), 4U);
+    QCOMPARE(decl->column(), 6U);
+
+    CppRefactoringChanges changes(snapshot);
+    InsertionPointLocator find(changes);
+    QList<InsertionLocation> locList = find.methodDefinition(decl);
+    QVERIFY(locList.size() == 1);
+    InsertionLocation loc = locList.first();
+    QCOMPARE(loc.fileName(), dst->fileName());
+    QCOMPARE(loc.line(), 4U);
+    QCOMPARE(loc.column(), 1U);
+    QCOMPARE(loc.prefix(), QString());
+    QCOMPARE(loc.suffix(), QLatin1String("\n\n"));
+}
+
+void CppToolsPlugin::test_codegen_definition_member_specific_file()
+{
+    const QByteArray srcText = "\n"
+            "class Foo\n"  // line 1
+            "{\n"
+            "void foo();\n" // line 3
+            "void bar();\n" // line 4
+            "void baz();\n" // line 5
+            "};\n"
+            "\n"
+            "void Foo::bar()\n"
+            "{\n"
+            "\n"
+            "}\n";
+
+    const QByteArray dstText = QString::fromLatin1(
+                "\n"
+                "#include \"%1/file.h\"\n" // line 1
+                "int x;\n"
+                "\n"
+                "void Foo::foo()\n" // line 4
+                "{\n"
+                "\n"
+                "}\n" // line 7
+                "\n"
+                "int y;\n").arg(QDir::tempPath()).toLatin1();
+
+    Document::Ptr src = Document::create(QDir::tempPath() + QLatin1String("/file.h"));
+    Utils::FileSaver srcSaver(src->fileName());
+    srcSaver.write(srcText);
+    srcSaver.finalize();
+    src->setUtf8Source(srcText);
+    src->parse();
+    src->check();
+    QCOMPARE(src->diagnosticMessages().size(), 0);
+    QCOMPARE(src->globalSymbolCount(), 2U);
+
+    Document::Ptr dst = Document::create(QDir::tempPath() + QLatin1String("/file.cpp"));
+    dst->addIncludeFile(Document::Include(QLatin1String("file.h"), src->fileName(), 1,
+                                          Client::IncludeLocal));
+    Utils::FileSaver dstSaver(dst->fileName());
+    dstSaver.write(dstText);
+    dstSaver.finalize();
+    dst->setUtf8Source(dstText);
+    dst->parse();
+    dst->check();
+    QCOMPARE(dst->diagnosticMessages().size(), 0);
+    QCOMPARE(dst->globalSymbolCount(), 3U);
+
+    Snapshot snapshot;
+    snapshot.insert(src);
+    snapshot.insert(dst);
+
+    Class *foo = src->globalSymbolAt(0)->asClass();
+    QVERIFY(foo);
+    QCOMPARE(foo->line(), 1U);
+    QCOMPARE(foo->column(), 7U);
+    QCOMPARE(foo->memberCount(), 3U);
+    Declaration *decl = foo->memberAt(2)->asDeclaration();
+    QVERIFY(decl);
+    QCOMPARE(decl->line(), 5U);
+    QCOMPARE(decl->column(), 6U);
+
+    CppRefactoringChanges changes(snapshot);
+    InsertionPointLocator find(changes);
+    QList<InsertionLocation> locList = find.methodDefinition(decl, true, dst->fileName());
+    QVERIFY(locList.size() == 1);
+    InsertionLocation loc = locList.first();
+    QCOMPARE(loc.fileName(), dst->fileName());
+    QCOMPARE(loc.line(), 7U);
+    QCOMPARE(loc.column(), 2U);
+    QCOMPARE(loc.prefix(), QLatin1String("\n\n"));
+    QCOMPARE(loc.suffix(), QString());
+}

src/plugins/cpptools/cpptoolsplugin.h

@@ -82,6 +82,8 @@ private slots:
     void test_codegen_definition_first_member();
     void test_codegen_definition_last_member();
     void test_codegen_definition_middle_member();
+    void test_codegen_definition_middle_member_surrounded_by_undefined();
+    void test_codegen_definition_member_specific_file();
 
     void test_completion_forward_declarations_present();
     void test_completion_inside_parentheses_c_style_conversion();

src/plugins/cpptools/insertionpointlocator.cpp

@@ -470,7 +470,9 @@ static Declaration *isNonVirtualFunctionDeclaration(Symbol *s)
     return declaration;
 }
 
-static InsertionLocation nextToSurroundingDefinitions(Symbol *declaration, const CppRefactoringChanges &changes)
+static InsertionLocation nextToSurroundingDefinitions(Symbol *declaration,
+                                                      const CppRefactoringChanges &changes,
+                                                      const QString& destinationFile)
 {
     InsertionLocation noResult;
     Class *klass = declaration->enclosingClass();
@@ -489,35 +491,47 @@ static InsertionLocation nextToSurroundingDefinitions(Symbol *declaration, const
     if (declIndex == -1)
         return noResult;
 
-    // scan preceding declarations for a function declaration
+    // scan preceding declarations for a function declaration (and see if it is defined)
+    CppTools::SymbolFinder symbolFinder;
+    Function *definitionFunction = 0;
     QString prefix, suffix;
     Declaration *surroundingFunctionDecl = 0;
     for (int i = declIndex - 1; i >= 0; --i) {
         Symbol *s = klass->memberAt(i);
         surroundingFunctionDecl = isNonVirtualFunctionDeclaration(s);
-        if (surroundingFunctionDecl) {
+        if (!surroundingFunctionDecl)
+            continue;
+        if ((definitionFunction = symbolFinder.findMatchingDefinition(surroundingFunctionDecl,
+                                                                      changes.snapshot())))
+        {
+            if (destinationFile.isEmpty() || destinationFile == QString::fromUtf8(
+                        definitionFunction->fileName(), definitionFunction->fileNameLength())) {
                 prefix = QLatin1String("\n\n");
                 break;
             }
+            definitionFunction = 0;
         }
-    if (!surroundingFunctionDecl) {
+    }
+    if (!definitionFunction) {
         // try to find one below
         for (unsigned i = declIndex + 1; i < klass->memberCount(); ++i) {
             Symbol *s = klass->memberAt(i);
             surroundingFunctionDecl = isNonVirtualFunctionDeclaration(s);
-            if (surroundingFunctionDecl) {
+            if (!surroundingFunctionDecl)
+                continue;
+            if ((definitionFunction = symbolFinder.findMatchingDefinition(surroundingFunctionDecl,
+                                                                          changes.snapshot())))
+            {
+                if (destinationFile.isEmpty() || destinationFile == QString::fromUtf8(
+                            definitionFunction->fileName(), definitionFunction->fileNameLength())) {
                     suffix = QLatin1String("\n\n");
                     break;
                 }
+                definitionFunction = 0;
+            }
         }
-        if (!surroundingFunctionDecl)
-            return noResult;
     }
 
-    // find the declaration's definition
-    CppTools::SymbolFinder symbolFinder;
-    Function *definitionFunction = symbolFinder.findMatchingDefinition(surroundingFunctionDecl,
-                                                                       changes.snapshot());
     if (!definitionFunction)
         return noResult;
 
@@ -546,7 +560,8 @@ static InsertionLocation nextToSurroundingDefinitions(Symbol *declaration, const
 }
 
 QList<InsertionLocation> InsertionPointLocator::methodDefinition(Symbol *declaration,
-                                                                 bool useSymbolFinder) const
+                                                                 bool useSymbolFinder,
+                                                                 const QString &destinationFile) const
 {
     QList<InsertionLocation> result;
     if (!declaration)
@@ -558,7 +573,9 @@ QList<InsertionLocation> InsertionPointLocator::methodDefinition(Symbol *declara
             return result;
     }
 
-    const InsertionLocation location = nextToSurroundingDefinitions(declaration, m_refactoringChanges);
+    const InsertionLocation location = nextToSurroundingDefinitions(declaration,
+                                                                    m_refactoringChanges,
+                                                                    destinationFile);
     if (location.isValid()) {
         result += location;
         return result;

src/plugins/cpptools/insertionpointlocator.h

@@ -99,7 +99,8 @@ public:
                                                AccessSpec xsSpec) const;
 
     QList<InsertionLocation> methodDefinition(CPlusPlus::Symbol *declaration,
-                                              bool useSymbolFinder = true) const;
+                                              bool useSymbolFinder = true,
+                                              const QString &destinationFile = QString()) const;
 
 private:
     CppRefactoringChanges m_refactoringChanges;