CppEditor: Improve insert position for AddIncludeForUndefinedIdentifier

Includes of moc files are not considered while determining the new
include's position. Further the new include is inserted at its
"alphabeticaly sorted" position.

Task-number: QTCREATORBUG-8871
Change-Id: I67b40a6f538112753298b960da488e24f4643808
Reviewed-by: Nikolai Kosjar <nikolai.kosjar@digia.com>

src/plugins/cppeditor/cppeditorplugin.h

@@ -161,6 +161,10 @@ private slots:
     void test_quickfix_InsertDeclFromDef();
 
     void test_quickfix_AddIncludeForUndefinedIdentifier_normal();
+    void test_quickfix_AddIncludeForUndefinedIdentifier_ignoremoc();
+    void test_quickfix_AddIncludeForUndefinedIdentifier_sortingTop();
+    void test_quickfix_AddIncludeForUndefinedIdentifier_sortingMiddle();
+    void test_quickfix_AddIncludeForUndefinedIdentifier_sortingBottom();
     void test_quickfix_AddIncludeForUndefinedIdentifier_noinclude();
     void test_quickfix_AddIncludeForUndefinedIdentifier_noincludeComment01();
     void test_quickfix_AddIncludeForUndefinedIdentifier_noincludeComment02();

src/plugins/cppeditor/cppquickfix_test.cpp

@@ -969,13 +969,176 @@ void CppEditorPlugin::test_quickfix_AddIncludeForUndefinedIdentifier_normal()
         "}\n"
         ;
     expected =
+        "#include \"file.h\"\n"
         "#include \"someheader.h\"\n"
+        "\n"
+        "void f()\n"
+        "{\n"
+        "    Foo foo;\n"
+        "}\n"
+        "\n"
+        ;
+    testFiles << TestDocument::create(original, expected, QLatin1String("file.cpp"));
+
+    AddIncludeForUndefinedIdentifier factory;
+    TestCase data(testFiles);
+    data.run(&factory);
+}
+
+/// Check add include, ignoring any moc includes.
+void CppEditorPlugin::test_quickfix_AddIncludeForUndefinedIdentifier_ignoremoc()
+{
+    QList<TestDocumentPtr> testFiles;
+
+    QByteArray original;
+    QByteArray expected;
+
+    // Header File
+    original = "class Foo {};\n";
+    expected = original + "\n";
+    testFiles << TestDocument::create(original, expected, QLatin1String("file.h"));
+
+    // Source File
+    original =
+        "void f()\n"
+        "{\n"
+        "    Fo@o foo;\n"
+        "}\n"
+        "#include \"file.moc\";\n"
+        ;
+    expected =
         "#include \"file.h\"\n"
         "\n"
         "void f()\n"
         "{\n"
         "    Foo foo;\n"
         "}\n"
+        "#include \"file.moc\";\n"
+        "\n"
+        ;
+    testFiles << TestDocument::create(original, expected, QLatin1String("file.cpp"));
+
+    AddIncludeForUndefinedIdentifier factory;
+    TestCase data(testFiles);
+    data.run(&factory);
+}
+
+/// Check add include sorting top
+void CppEditorPlugin::test_quickfix_AddIncludeForUndefinedIdentifier_sortingTop()
+{
+    QList<TestDocumentPtr> testFiles;
+
+    QByteArray original;
+    QByteArray expected;
+
+    // Header File
+    original = "class Foo {};\n";
+    expected = original + "\n";
+    testFiles << TestDocument::create(original, expected, QLatin1String("file.h"));
+
+    // Source File
+    original =
+        "#include \"y.h\"\n"
+        "#include \"z.h\"\n"
+        "void f()\n"
+        "{\n"
+        "    Fo@o foo;\n"
+        "}\n"
+        "#include \"file.moc\";\n"
+        ;
+    expected =
+        "#include \"file.h\"\n"
+        "#include \"y.h\"\n"
+        "#include \"z.h\"\n"
+        "void f()\n"
+        "{\n"
+        "    Foo foo;\n"
+        "}\n"
+        "#include \"file.moc\";\n"
+        "\n"
+        ;
+    testFiles << TestDocument::create(original, expected, QLatin1String("file.cpp"));
+
+    AddIncludeForUndefinedIdentifier factory;
+    TestCase data(testFiles);
+    data.run(&factory);
+}
+
+/// Check add include sorting middle
+void CppEditorPlugin::test_quickfix_AddIncludeForUndefinedIdentifier_sortingMiddle()
+{
+    QList<TestDocumentPtr> testFiles;
+
+    QByteArray original;
+    QByteArray expected;
+
+    // Header File
+    original = "class Foo {};\n";
+    expected = original + "\n";
+    testFiles << TestDocument::create(original, expected, QLatin1String("file.h"));
+
+    // Source File
+    original =
+        "#include \"a.h\"\n"
+        "#include \"z.h\"\n"
+        "void f()\n"
+        "{\n"
+        "    Fo@o foo;\n"
+        "}\n"
+        "#include \"file.moc\";\n"
+        ;
+    expected =
+        "#include \"a.h\"\n"
+        "#include \"file.h\"\n"
+        "#include \"z.h\"\n"
+        "void f()\n"
+        "{\n"
+        "    Foo foo;\n"
+        "}\n"
+        "#include \"file.moc\";\n"
+        "\n"
+        ;
+    testFiles << TestDocument::create(original, expected, QLatin1String("file.cpp"));
+
+    AddIncludeForUndefinedIdentifier factory;
+    TestCase data(testFiles);
+    data.run(&factory);
+}
+
+
+
+/// Check add include sorting bottom
+void CppEditorPlugin::test_quickfix_AddIncludeForUndefinedIdentifier_sortingBottom()
+{
+    QList<TestDocumentPtr> testFiles;
+
+    QByteArray original;
+    QByteArray expected;
+
+    // Header File
+    original = "class Foo {};\n";
+    expected = original + "\n";
+    testFiles << TestDocument::create(original, expected, QLatin1String("file.h"));
+
+    // Source File
+    original =
+        "#include \"a.h\"\n"
+        "#include \"b.h\"\n"
+        "void f()\n"
+        "{\n"
+        "    Fo@o foo;\n"
+        "}\n"
+        "#include \"file.moc\";\n"
+        ;
+    expected =
+        "#include \"a.h\"\n"
+        "#include \"b.h\"\n"
+        "#include \"file.h\"\n"
+        "void f()\n"
+        "{\n"
+        "    Foo foo;\n"
+        "}\n"
+        "#include \"file.moc\";\n"
         "\n"
         ;
     testFiles << TestDocument::create(original, expected, QLatin1String("file.cpp"));

src/plugins/cppeditor/cppquickfixes.cpp

@@ -1730,62 +1730,80 @@ public:
         CppRefactoringFilePtr file = refactoring.file(fileName());
 
         QList<Document::Include> includes = file->cppDocument()->includes();
-        if (includes.isEmpty()) {
-            // No includes, find possible first/multi line comment
-            int insertPos = 0;
-            QTextBlock block = file->document()->firstBlock();
-            while (block.isValid()) {
-                const QString trimmedText = block.text().trimmed();
-
-                // Only skip the first comment!
-                if (trimmedText.startsWith(QLatin1String("/*"))) {
-                    do {
-                        const int pos = block.text().indexOf(QLatin1String("*/"));
-                        if (pos > -1) {
-                            insertPos = block.position() + pos + 2;
-                            break;
-                        }
-                        block = block.next();
-                    } while (block.isValid());
-                    break;
-                } else if (trimmedText.startsWith(QLatin1String("//"))) {
-                    block = block.next();
-                    while (block.isValid()) {
-                        if (!block.text().trimmed().startsWith(QLatin1String("//"))) {
-                            insertPos = block.position() - 1;
-                            break;
-                        }
-                        block = block.next();
-                    }
-                    break;
-                }
+        if (!includes.isEmpty()) {
+            QHash<QString, unsigned> includePositions;
+            foreach (const Document::Include &include, includes) {
+                if (include.fileName().endsWith(QLatin1String(".moc")))
+                    continue;
+                includePositions.insert(include.fileName(), include.line());
+            }
 
-                if (!trimmedText.isEmpty())
-                    break;
-                block = block.next();
+            if (!includePositions.isEmpty()) {
+                const QString include = m_include.mid(1, m_include.length() - 2);
+                QList<QString> keys = includePositions.keys();
+                keys << include;
+                qSort(keys);
+                const int pos = keys.indexOf(include);
+
+                ChangeSet changes;
+                if (pos + 1 != keys.count()) {
+                    const int insertPos = qMax(0, file->position(
+                                                   includePositions.value(keys.at(pos + 1)), 1));
+                    changes.insert(insertPos,
+                                   QLatin1String("#include ") + m_include + QLatin1String("\n"));
+
+                } else {
+                    const int insertPos = qMax(0, file->position(includePositions.value(
+                                                                     keys.at(pos - 1)) + 1, 1) - 1);
+                    changes.insert(insertPos, QLatin1String("\n#include ") + m_include);
+                }
+                file->setChangeSet(changes);
+                file->apply();
+                return;
             }
+        }
 
-            ChangeSet changes;
-            if (insertPos != 0)
-                changes.insert(insertPos, QLatin1String("\n\n#include ") + m_include);
-            else
-                changes.insert(insertPos, QString::fromLatin1("#include %1\n\n").arg(m_include));
-            file->setChangeSet(changes);
-            file->apply();
-        } else {
-            // find location of last include in file
-            unsigned lastIncludeLine = 0;
-            foreach (const Document::Include &include, includes) {
-                if (include.line() > lastIncludeLine)
-                    lastIncludeLine = include.line();
+        // No includes or no matching include, find possible first/multi line comment
+        int insertPos = 0;
+        QTextBlock block = file->document()->firstBlock();
+        while (block.isValid()) {
+            const QString trimmedText = block.text().trimmed();
+
+            // Only skip the first comment!
+            if (trimmedText.startsWith(QLatin1String("/*"))) {
+                do {
+                    const int pos = block.text().indexOf(QLatin1String("*/"));
+                    if (pos > -1) {
+                        insertPos = block.position() + pos + 2;
+                        break;
+                    }
+                    block = block.next();
+                } while (block.isValid());
+                break;
+            } else if (trimmedText.startsWith(QLatin1String("//"))) {
+                block = block.next();
+                while (block.isValid()) {
+                    if (!block.text().trimmed().startsWith(QLatin1String("//"))) {
+                        insertPos = block.position() - 1;
+                        break;
+                    }
+                    block = block.next();
+                }
+                break;
             }
 
-            const int insertPos = qMax(0, file->position(lastIncludeLine + 1, 1) - 1);
-            ChangeSet changes;
-            changes.insert(insertPos, QLatin1String("\n#include ") + m_include);
-            file->setChangeSet(changes);
-            file->apply();
+            if (!trimmedText.isEmpty())
+                break;
+            block = block.next();
         }
+
+        ChangeSet changes;
+        if (insertPos != 0)
+            changes.insert(insertPos, QLatin1String("\n\n#include ") + m_include);
+        else
+            changes.insert(insertPos, QString::fromLatin1("#include %1\n\n").arg(m_include));
+        file->setChangeSet(changes);
+        file->apply();
     }
 
 private: