From 2fc4acbc356cbb310663f6568a6e73a66b649a8e Mon Sep 17 00:00:00 2001 From: Nikolai Kosjar <nikolai.kosjar@theqtcompany.com> Date: Thu, 27 Nov 2014 18:00:10 +0100 Subject: [PATCH] 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 | 1 + src/plugins/cppeditor/cppquickfix_test.cpp | 45 +++++++++++++++++++ src/plugins/cppeditor/cppquickfix_test.h | 11 +++++ src/plugins/cppeditor/cppquickfixes.cpp | 15 ++++++- .../data/include-data/QtCore/QDir | 3 ++ 5 files changed, 73 insertions(+), 2 deletions(-) diff --git a/src/plugins/cppeditor/cppeditorplugin.h b/src/plugins/cppeditor/cppeditorplugin.h index ffb9d2c9547..927d4ff9a0c 100644 --- a/src/plugins/cppeditor/cppeditorplugin.h +++ b/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(); diff --git a/src/plugins/cppeditor/cppquickfix_test.cpp b/src/plugins/cppeditor/cppquickfix_test.cpp index 2403cdd3b19..c32e96bf586 100644 --- a/src/plugins/cppeditor/cppquickfix_test.cpp +++ b/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. diff --git a/src/plugins/cppeditor/cppquickfix_test.h b/src/plugins/cppeditor/cppquickfix_test.h index 43119cf94b8..2a7ea4c3b5b 100644 --- a/src/plugins/cppeditor/cppquickfix_test.h +++ b/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); diff --git a/src/plugins/cppeditor/cppquickfixes.cpp b/src/plugins/cppeditor/cppquickfixes.cpp index 36a0d41d5b5..691d7687dec 100644 --- a/src/plugins/cppeditor/cppquickfixes.cpp +++ b/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)); diff --git a/tests/auto/cplusplus/preprocessor/data/include-data/QtCore/QDir b/tests/auto/cplusplus/preprocessor/data/include-data/QtCore/QDir index fef83a9cfe6..8710edd2d37 100644 --- a/tests/auto/cplusplus/preprocessor/data/include-data/QtCore/QDir +++ b/tests/auto/cplusplus/preprocessor/data/include-data/QtCore/QDir @@ -1 +1,4 @@ // comment + +class QDir {}; + -- GitLab