From a3664297660edc2320599b7be197b4934f746bc7 Mon Sep 17 00:00:00 2001 From: Nikolai Kosjar Date: Tue, 7 Oct 2014 17:20:25 +0200 Subject: [PATCH] CppEditor: Add very first include after include guard ...when adding an include for an undefined identifier. Change-Id: Ia338e924901262a847d3bd7ed9733d8e66c631dd Task-number: QTCREATORBUG-10391 Reviewed-by: Christian Stenger --- src/libs/cplusplus/FastPreprocessor.cpp | 8 ++ src/libs/cplusplus/FastPreprocessor.h | 2 +- src/plugins/cppeditor/cppeditorplugin.h | 1 + src/plugins/cppeditor/cppquickfix_test.cpp | 30 +++++ src/plugins/cppeditor/cppquickfixes.cpp | 9 +- src/plugins/cpptools/includeutils.cpp | 129 +++++++++++++-------- src/plugins/cpptools/includeutils.h | 5 +- 7 files changed, 132 insertions(+), 52 deletions(-) diff --git a/src/libs/cplusplus/FastPreprocessor.cpp b/src/libs/cplusplus/FastPreprocessor.cpp index 3c5b4312cea..6c8c0ab2d59 100644 --- a/src/libs/cplusplus/FastPreprocessor.cpp +++ b/src/libs/cplusplus/FastPreprocessor.cpp @@ -151,3 +151,11 @@ void FastPreprocessor::startExpandingMacro(unsigned bytesOffset, unsigned utf16c utf16charsOffset, macro.nameToQString().size(), line, actuals); } + +void FastPreprocessor::markAsIncludeGuard(const QByteArray ¯oName) +{ + if (!_currentDoc) + return; + + _currentDoc->setIncludeGuardMacroName(macroName); +} diff --git a/src/libs/cplusplus/FastPreprocessor.h b/src/libs/cplusplus/FastPreprocessor.h index b4a14cfdb42..7b28f303cdd 100644 --- a/src/libs/cplusplus/FastPreprocessor.h +++ b/src/libs/cplusplus/FastPreprocessor.h @@ -73,7 +73,7 @@ public: const Macro &, const QVector &); virtual void stopExpandingMacro(unsigned, const Macro &) {} - virtual void markAsIncludeGuard(const QByteArray &) {} + virtual void markAsIncludeGuard(const QByteArray ¯oName); virtual void startSkippingBlocks(unsigned) {} virtual void stopSkippingBlocks(unsigned) {} diff --git a/src/plugins/cppeditor/cppeditorplugin.h b/src/plugins/cppeditor/cppeditorplugin.h index bf76e9a22a1..45137bb4bf3 100644 --- a/src/plugins/cppeditor/cppeditorplugin.h +++ b/src/plugins/cppeditor/cppeditorplugin.h @@ -169,6 +169,7 @@ private slots: void test_quickfix_AddIncludeForUndefinedIdentifier_inserting_mixedIncludeTypes3(); void test_quickfix_AddIncludeForUndefinedIdentifier_inserting_mixedIncludeTypes4(); void test_quickfix_AddIncludeForUndefinedIdentifier_inserting_noinclude(); + void test_quickfix_AddIncludeForUndefinedIdentifier_inserting_onlyIncludeGuard(); void test_quickfix_AddIncludeForUndefinedIdentifier_inserting_veryFirstIncludeCppStyleCommentOnTop(); void test_quickfix_AddIncludeForUndefinedIdentifier_inserting_veryFirstIncludeCStyleCommentOnTop(); void test_quickfix_AddIncludeForUndefinedIdentifier_inserting_checkQSomethingInQtIncludePaths(); diff --git a/src/plugins/cppeditor/cppquickfix_test.cpp b/src/plugins/cppeditor/cppquickfix_test.cpp index 7a5594344bb..88ed07080e4 100644 --- a/src/plugins/cppeditor/cppquickfix_test.cpp +++ b/src/plugins/cppeditor/cppquickfix_test.cpp @@ -2828,6 +2828,36 @@ void CppEditorPlugin::test_quickfix_AddIncludeForUndefinedIdentifier_inserting_n QuickFixTestCase::run(testFiles, &factory, TestIncludePaths::globalIncludePath()); } +/// Check: Insert very first include after include guard +void CppEditorPlugin::test_quickfix_AddIncludeForUndefinedIdentifier_inserting_onlyIncludeGuard() +{ + QList testFiles; + + QByteArray original; + QByteArray expected; + + original = + "#ifndef FOO_H\n" + "#define FOO_H\n" + "void @f();\n" + "#endif\n" + ; + expected = + "#ifndef FOO_H\n" + "#define FOO_H\n" + "\n" + "#include \"file.h\"\n" + "\n" + "void f();\n" + "#endif\n" + ; + testFiles << QuickFixTestDocument::create(TestIncludePaths::directoryOfTestFile().toUtf8() + + "/file.cpp", original, expected); + + AddIncludeForUndefinedIdentifierTestFactory factory(QLatin1String("\"file.h\"")); + QuickFixTestCase::run(testFiles, &factory, TestIncludePaths::globalIncludePath()); +} + /// Check: Insert very first include if there is a c++ style comment on top void CppEditorPlugin::test_quickfix_AddIncludeForUndefinedIdentifier_inserting_veryFirstIncludeCppStyleCommentOnTop() { diff --git a/src/plugins/cppeditor/cppquickfixes.cpp b/src/plugins/cppeditor/cppquickfixes.cpp index f4923628711..1049cd2fb3c 100644 --- a/src/plugins/cppeditor/cppquickfixes.cpp +++ b/src/plugins/cppeditor/cppquickfixes.cpp @@ -259,11 +259,12 @@ Namespace *isNamespaceFunction(const LookupContext &context, Function *function) } // Given include is e.g. "afile.h" or (quotes/angle brackets included!). -void insertNewIncludeDirective(const QString &include, CppRefactoringFilePtr file) +void insertNewIncludeDirective(const QString &include, CppRefactoringFilePtr file, + const CPlusPlus::Document::Ptr &cppDocument) { // Find optimal position using namespace IncludeUtils; - LineForNewIncludeDirective finder(file->document(), file->cppDocument()->resolvedIncludes(), + LineForNewIncludeDirective finder(file->document(), cppDocument, LineForNewIncludeDirective::IgnoreMocIncludes, LineForNewIncludeDirective::AutoDetect); unsigned newLinesToPrepend = 0; @@ -1583,7 +1584,7 @@ public: best = headerFile; const QString include = QString::fromLatin1("<%1>").arg(QFileInfo(best).fileName()); - insertNewIncludeDirective(include, currentFile); + insertNewIncludeDirective(include, currentFile, semanticInfo().doc); } } @@ -1826,7 +1827,7 @@ void AddIncludeForUndefinedIdentifierOp::perform() CppRefactoringChanges refactoring(snapshot()); CppRefactoringFilePtr file = refactoring.file(fileName()); - insertNewIncludeDirective(m_include, file); + insertNewIncludeDirective(m_include, file, semanticInfo().doc); } namespace { diff --git a/src/plugins/cpptools/includeutils.cpp b/src/plugins/cpptools/includeutils.cpp index 490e9fcb799..2484e9112f6 100644 --- a/src/plugins/cpptools/includeutils.cpp +++ b/src/plugins/cpptools/includeutils.cpp @@ -35,6 +35,7 @@ #include #include +#include #include #include @@ -80,15 +81,57 @@ QString includeDir(const QString &include) return dirPrefix; } +int lineAfterFirstComment(const QTextDocument *textDocument) +{ + int insertLine = -1; + + QTextBlock block = textDocument->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) { + insertLine = block.blockNumber() + 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("//"))) { + insertLine = block.blockNumber() + 1; + break; + } + block = block.next(); + } + break; + } + + if (!trimmedText.isEmpty()) + break; + block = block.next(); + } + + return insertLine; +} + } // anonymous namespace LineForNewIncludeDirective::LineForNewIncludeDirective(const QTextDocument *textDocument, - QList includes, + const Document::Ptr cppDocument, MocIncludeMode mocIncludeMode, IncludeStyle includeStyle) : m_textDocument(textDocument) + , m_cppDocument(cppDocument) , m_includeStyle(includeStyle) { + const QList includes = cppDocument->resolvedIncludes(); + // Ignore *.moc includes if requested if (mocIncludeMode == IgnoreMocIncludes) { foreach (const Document::Include &include, includes) { @@ -129,6 +172,43 @@ LineForNewIncludeDirective::LineForNewIncludeDirective(const QTextDocument *text } } +int LineForNewIncludeDirective::findInsertLineForVeryFirstInclude(unsigned *newLinesToPrepend, + unsigned *newLinesToAppend) +{ + int insertLine = 1; + + // If there is an include guard, insert right after that one + const QByteArray includeGuardMacroName = m_cppDocument->includeGuardMacroName(); + if (!includeGuardMacroName.isEmpty()) { + const QList definedMacros = m_cppDocument->definedMacros(); + foreach (const Macro &definedMacro, definedMacros) { + if (definedMacro.name() == includeGuardMacroName) { + if (newLinesToPrepend) + *newLinesToPrepend = 1; + if (newLinesToAppend) + *newLinesToAppend += 1; + insertLine = definedMacro.line() + 1; + } + } + QTC_CHECK(insertLine != 1); + } else { + // Otherwise, if there is a comment, insert right after it + insertLine = lineAfterFirstComment(m_textDocument); + if (insertLine != -1) { + if (newLinesToPrepend) + *newLinesToPrepend = 1; + + // Otherwise, insert at top of file + } else { + if (newLinesToAppend) + *newLinesToAppend += 1; + insertLine = 1; + } + } + + return insertLine; +} + int LineForNewIncludeDirective::operator()(const QString &newIncludeFileName, unsigned *newLinesToPrepend, unsigned *newLinesToAppend) @@ -144,51 +224,8 @@ int LineForNewIncludeDirective::operator()(const QString &newIncludeFileName, : Client::IncludeGlobal; // Handle no includes - if (m_includes.empty()) { - unsigned insertLine = 0; - - QTextBlock block = m_textDocument->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) { - insertLine = block.blockNumber() + 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("//"))) { - insertLine = block.blockNumber() + 1; - break; - } - block = block.next(); - } - break; - } - - if (!trimmedText.isEmpty()) - break; - block = block.next(); - } - - if (insertLine == 0) { - if (newLinesToAppend) - *newLinesToAppend += 1; - insertLine = 1; - } else { - if (newLinesToPrepend) - *newLinesToPrepend = 1; - } - return insertLine; - } + if (m_includes.empty()) + return findInsertLineForVeryFirstInclude(newLinesToPrepend, newLinesToAppend); typedef QList IncludeGroups; diff --git a/src/plugins/cpptools/includeutils.h b/src/plugins/cpptools/includeutils.h index 980f40a4e6a..e7391c250b9 100644 --- a/src/plugins/cpptools/includeutils.h +++ b/src/plugins/cpptools/includeutils.h @@ -90,7 +90,7 @@ public: enum IncludeStyle { LocalBeforeGlobal, GlobalBeforeLocal, AutoDetect }; LineForNewIncludeDirective(const QTextDocument *textDocument, - QList includes, + const CPlusPlus::Document::Ptr cppDocument, MocIncludeMode mocIncludeMode = IgnoreMocIncludes, IncludeStyle includeStyle = AutoDetect); @@ -100,10 +100,13 @@ public: unsigned *newLinesToAppend = 0); private: + int findInsertLineForVeryFirstInclude(unsigned *newLinesToPrepend, unsigned *newLinesToAppend); QList getGroupsByIncludeType(const QList &groups, IncludeType includeType); const QTextDocument *m_textDocument; + const CPlusPlus::Document::Ptr m_cppDocument; + IncludeStyle m_includeStyle; QList m_includes; };