CppEditor: Avoid duplicate "Add #include XYZ"

Task-number: QTCREATORBUG-13422
Change-Id: I3648bf44760fdac4e8e1e79652519136af6032c8
Reviewed-by: Erik Verbruggen <erik.verbruggen@theqtcompany.com>

src/plugins/cppeditor/cppeditorplugin.h

@@ -182,6 +182,7 @@ private slots:
     void test_quickfix_AddIncludeForUndefinedIdentifier_inserting_veryFirstIncludeCppStyleCommentOnTop();
     void test_quickfix_AddIncludeForUndefinedIdentifier_inserting_veryFirstIncludeCStyleCommentOnTop();
     void test_quickfix_AddIncludeForUndefinedIdentifier_inserting_checkQSomethingInQtIncludePaths();
+    void test_quickfix_AddIncludeForUndefinedIdentifier_noDoubleQtHeaderInclude();
 
     void test_quickfix_MoveFuncDefOutside_MemberFuncToCpp();
     void test_quickfix_MoveFuncDefOutside_MemberFuncToCppInsideNS();

src/plugins/cppeditor/cppquickfix_test.cpp

@@ -245,6 +245,26 @@ void QuickFixOperationTest::run(const QList<QuickFixTestDocument::Ptr> &testDocu
     QuickFixOperationTest(testDocuments, factory, headerPaths, operationIndex);
 }
 
+QuickFixOfferedOperationsTest::QuickFixOfferedOperationsTest(
+        const QList<QuickFixTestDocument::Ptr> &testDocuments,
+        CppQuickFixFactory *factory,
+        const ProjectPart::HeaderPaths &headerPaths,
+        const QStringList &expectedOperations)
+    : BaseQuickFixTestCase(testDocuments, headerPaths)
+{
+    // Get operations
+    CppQuickFixInterface quickFixInterface(m_documentWithMarker->m_editorWidget, ExplicitlyInvoked);
+    TextEditor::QuickFixOperations actualOperations;
+    factory->match(quickFixInterface, actualOperations);
+
+    // Convert to QStringList
+    QStringList actualOperationsAsStringList;
+    foreach (const QuickFixOperation::Ptr &operation, actualOperations)
+        actualOperationsAsStringList << operation->description();
+
+    QCOMPARE(actualOperationsAsStringList, expectedOperations);
+}
+
 /// Delegates directly to AddIncludeForUndefinedIdentifierOp for easier testing.
 class AddIncludeForUndefinedIdentifierTestFactory : public CppQuickFixFactory
 {
@@ -3090,7 +3110,32 @@ void CppEditorPlugin::test_quickfix_AddIncludeForUndefinedIdentifier_inserting_c
 
     AddIncludeForUndefinedIdentifier factory;
     QuickFixOperationTest::run(testFiles, &factory, TestIncludePaths::globalQtCoreIncludePath());
+}
 
+void CppEditorPlugin::test_quickfix_AddIncludeForUndefinedIdentifier_noDoubleQtHeaderInclude()
+{
+    QList<QuickFixTestDocument::Ptr> testFiles;
+
+    QByteArray original;
+    QByteArray expected;
+
+    const QByteArray base = TestIncludePaths::directoryOfTestFile().toUtf8();
+
+    // This file makes the QDir definition available so that locator finds it.
+    original = expected = "#include <QDir>\n"
+                          "void avoidBeingRecognizedAsForwardingHeader();";
+    testFiles << QuickFixTestDocument::create(base + "/fileUsingQDir.cpp", original, expected);
+
+    original = expected = "@QDir dir;\n";
+    testFiles << QuickFixTestDocument::create(base + "/fileWantsToUseQDir.cpp", original, expected);
+
+    ProjectPart::HeaderPaths headerPaths;
+    headerPaths += ProjectPart::HeaderPath(TestIncludePaths::globalQtCoreIncludePath(),
+                                           ProjectPart::HeaderPath::IncludePath);
+
+    AddIncludeForUndefinedIdentifier factory;
+    const QStringList expectedOperations = QStringList() << QLatin1String("Add #include <QDir>");
+    QuickFixOfferedOperationsTest(testFiles, &factory, headerPaths, expectedOperations);
 }
 
 /// Check: Move definition from header to cpp.

src/plugins/cppeditor/cppquickfix_test.h

@@ -106,6 +106,17 @@ public:
                     int operationIndex = 0);
 };
 
+/// Tests the offered operations provided by a given CppQuickFixFactory
+class QuickFixOfferedOperationsTest : public BaseQuickFixTestCase
+{
+public:
+    QuickFixOfferedOperationsTest(const QList<QuickFixTestDocument::Ptr> &testDocuments,
+                                  CppQuickFixFactory *factory,
+                                  const CppTools::ProjectPart::HeaderPaths &headerPaths
+                                    = CppTools::ProjectPart::HeaderPaths(),
+                                  const QStringList &expectedOperations = QStringList());
+};
+
 QList<QuickFixTestDocument::Ptr> singleDocument(const QByteArray &original,
                                                 const QByteArray &expected);
 

src/plugins/cppeditor/cppquickfixes.cpp

@@ -1866,6 +1866,13 @@ Snapshot forwardingHeaders(const CppQuickFixInterface &interface)
     return result;
 }
 
+bool looksLikeAQtClass(const QString &identifier)
+{
+    return identifier.size() > 2
+        && identifier.at(0) == QLatin1Char('Q')
+        && identifier.at(1).isUpper();
+}
+
 } // anonymous namespace
 
 void AddIncludeForUndefinedIdentifier::match(const CppQuickFixInterface &interface,
@@ -1884,6 +1891,7 @@ void AddIncludeForUndefinedIdentifier::match(const CppQuickFixInterface &interfa
 
     const QString currentDocumentFilePath = interface.semanticInfo().doc->fileName();
     const ProjectPart::HeaderPaths headerPaths = relevantHeaderPaths(currentDocumentFilePath);
+    bool qtHeaderFileIncludeOffered = false;
 
     // Find an include file through the locator
     if (CppClassesFilter *classesFilter
@@ -1906,7 +1914,7 @@ void AddIncludeForUndefinedIdentifier::match(const CppQuickFixInterface &interfa
             foreach (const QString &header, headerAndItsForwardingHeaders) {
                 const QString include = findShortestInclude(currentDocumentFilePath, header,
                                                             headerPaths);
-                if (!include.isEmpty()) {
+                if (include.size() > 2) {
                     const QString headerFileName = QFileInfo(info->fileName()).fileName();
                     QTC_ASSERT(!headerFileName.isEmpty(), break);
 
@@ -1916,6 +1924,9 @@ void AddIncludeForUndefinedIdentifier::match(const CppQuickFixInterface &interfa
                     else if (headerFileName.at(1).isUpper())
                         priority = 1;
 
+                    if (looksLikeAQtClass(include.mid(1, include.size() - 2)))
+                        qtHeaderFileIncludeOffered = true;
+
                     result.append(new AddIncludeForUndefinedIdentifierOp(interface, priority,
                                                                          include));
                 }
@@ -1926,7 +1937,7 @@ void AddIncludeForUndefinedIdentifier::match(const CppQuickFixInterface &interfa
     // The header file we are looking for might not be (yet) included in any file we have parsed.
     // As such, it will not be findable via locator. At least for Qt classes, check also for
     // headers with the same name.
-    if (className.size() > 2 && className.at(0) == QLatin1Char('Q') && className.at(1).isUpper()) {
+    if (!qtHeaderFileIncludeOffered && looksLikeAQtClass(className)) {
         const QString include = findQtIncludeWithSameName(className, headerPaths);
         if (!include.isEmpty())
             result.append(new AddIncludeForUndefinedIdentifierOp(interface, 1, include));

tests/auto/cplusplus/preprocessor/data/include-data/QtCore/QDir

@@ -1 +1,4 @@
 // comment
+
+class QDir {};
+