From 92667c779cc5cc64542fb86f3c61f0db552c62b9 Mon Sep 17 00:00:00 2001 From: Leander Schulten Date: Sun, 15 Nov 2020 23:52:56 +0100 Subject: CppEditor: Remove using directive at global scope always in all files Previously only using directives in the global scope that followed the quickfixed using directive were removed Change-Id: I330acfe3236a6845fd1667f9fa699ab6c8fb560d Reviewed-by: Christian Kandeler --- src/plugins/cppeditor/cppquickfix_test.cpp | 49 ++++++++----- src/plugins/cppeditor/cppquickfixes.cpp | 111 +++++++++++++++++++++++++++-- 2 files changed, 137 insertions(+), 23 deletions(-) (limited to 'src') diff --git a/src/plugins/cppeditor/cppquickfix_test.cpp b/src/plugins/cppeditor/cppquickfix_test.cpp index 5dd19adbf3..836b51438f 100644 --- a/src/plugins/cppeditor/cppquickfix_test.cpp +++ b/src/plugins/cppeditor/cppquickfix_test.cpp @@ -6663,6 +6663,26 @@ void CppEditorPlugin::test_quickfix_removeUsingNamespace_data() "}\n" "foo foos;\n"; + // like header1 but without "using namespace std;\n" + QByteArray expected1 = "namespace std{\n" + " template\n" + " class vector{};\n" + " namespace chrono{\n" + " using seconds = int;\n" + " }\n" + "}\n" + "namespace test{\n" + " class vector{\n" + " std::vector ints;\n" + " };\n" + "}\n"; + + // like header2 but without "using namespace std;\n" and with std::vector + QByteArray expected2 = "#include \"header1.h\"\n" + "using foo = test::vector;\n" + "using namespace test;\n" + "std::vector others;\n"; + QByteArray expected3 = "#include \"header2.h\"\n" "using namespace std::chrono;\n" "namespace test{\n" @@ -6683,22 +6703,18 @@ void CppEditorPlugin::test_quickfix_removeUsingNamespace_data() QTest::newRow("remove only in one file local") << header1 << header2 << h3 << header1 << header2 << expected3 << 0; QTest::newRow("remove only in one file globally") - << header1 << header2 << h3 << header1 << header2 << expected3 << 1; + << header1 << header2 << h3 << expected1 << expected2 << expected3 << 1; QByteArray h2 = "#include \"header1.h\"\n" "using foo = test::vector;\n" "using namespace s@td;\n" "using namespace test;\n" "vector others;\n"; - QByteArray expected2 = "#include \"header1.h\"\n" - "using foo = test::vector;\n" - "using namespace test;\n" - "std::vector others;\n"; QTest::newRow("remove across two files only this") << header1 << h2 << header3 << header1 << expected2 << header3 << 0; QTest::newRow("remove across two files globally1") - << header1 << h2 << header3 << header1 << expected2 << expected3 << 1; + << header1 << h2 << header3 << expected1 << expected2 << expected3 << 1; QByteArray h1 = "namespace std{\n" " template\n" @@ -6713,18 +6729,6 @@ void CppEditorPlugin::test_quickfix_removeUsingNamespace_data() " std::vector ints;\n" " };\n" "}\n"; - QByteArray expected1 = "namespace std{\n" - " template\n" - " class vector{};\n" - " namespace chrono{\n" - " using seconds = int;\n" - " }\n" - "}\n" - "namespace test{\n" - " class vector{\n" - " std::vector ints;\n" - " };\n" - "}\n"; QTest::newRow("remove across tree files only this") << h1 << header2 << header3 << expected1 << header2 << header3 << 0; @@ -6779,6 +6783,15 @@ void CppEditorPlugin::test_quickfix_removeUsingNamespace_data() QTest::newRow("existing namespace") << header1 << h2 << header3 << header1 << expected2 << header3 << 1; + + // test: remove using directive at global scope in every file + h1 = "using namespace tes@t;"; + h2 = "using namespace test;"; + h3 = "using namespace test;"; + + expected1 = expected2 = expected3 = ""; + QTest::newRow("global scope remove in every file") + << h1 << h2 << h3 << expected1 << expected2 << expected3 << 1; } void CppEditorPlugin::test_quickfix_removeUsingNamespace() diff --git a/src/plugins/cppeditor/cppquickfixes.cpp b/src/plugins/cppeditor/cppquickfixes.cpp index 94bfef1a84..bb0b781c3e 100644 --- a/src/plugins/cppeditor/cppquickfixes.cpp +++ b/src/plugins/cppeditor/cppquickfixes.cpp @@ -57,6 +57,7 @@ #include #include +#include #include #include @@ -7405,6 +7406,7 @@ void removeLine(const CppRefactoringFile *file, AST *ast, ChangeSet &changeSet) class RemoveNamespaceVisitor : public ASTVisitor { public: + constexpr static int SearchGlobalUsingDirectivePos = std::numeric_limits::max(); RemoveNamespaceVisitor(const CppRefactoringFile *file, const Snapshot &snapshot, const Name *namespace_, @@ -7443,8 +7445,16 @@ private: bool preVisit(AST *ast) override { if (!m_start) { + if (ast->asTranslationUnit()) + return true; if (UsingDirectiveAST *usingDirective = ast->asUsingDirective()) { if (nameEqual(usingDirective->name->name, m_namespace)) { + if (m_symbolPos == SearchGlobalUsingDirectivePos) { + // we have found a global using directive, so lets start + m_start = true; + removeLine(m_file, ast, m_changeSet); + return false; + } // ignore the using namespace that should be removed if (m_file->endOf(ast) != m_symbolPos) { if (m_removeAllAtGlobalScope) @@ -7630,6 +7640,17 @@ private: class RemoveUsingNamespaceOperation : public CppQuickFixOperation { + struct Node + { + Document::Ptr document; + bool processed = false; + bool hasGlobalUsingDirective = false; + std::vector> includes; + Node() = default; + Node(const Node &) = delete; + Node(Node &&) = delete; + }; + public: RemoveUsingNamespaceOperation(const CppQuickFixInterface &interface, UsingDirectiveAST *usingDirective, @@ -7654,12 +7675,90 @@ public: } private: + std::map buildIncludeGraph(CppRefactoringChanges &refactoring) + { + using namespace ProjectExplorer; + using namespace Utils; + + const Snapshot &s = refactoring.snapshot(); + std::map includeGraph; + + auto handleFile = [&](const FilePath &filePath, Document::Ptr doc, auto shouldHandle) { + Node &node = includeGraph[filePath]; + node.document = doc; + for (const Document::Include &include : doc->resolvedIncludes()) { + const auto filePath = FilePath::fromString(include.resolvedFileName()); + if (shouldHandle(filePath)) { + Node &includedNode = includeGraph[filePath]; + node.includes.push_back(includedNode); + } + } + }; + + if (const Project *project = SessionManager::projectForFile(filePath())) { + const FilePaths files = project->files(ProjectExplorer::Project::SourceFiles); + QSet projectFiles(files.begin(), files.end()); + for (const auto &file : files) { + const Document::Ptr doc = s.document(file); + if (!doc) + continue; + handleFile(file, doc, [&](const FilePath &file) { + return projectFiles.contains(file); + }); + } + } else { + for (auto i = s.begin(); i != s.end(); ++i) { + if (ProjectFile::classify(i.key().toString()) != ProjectFile::Unsupported) { + handleFile(i.key(), i.value(), [](const FilePath &file) { + return ProjectFile::classify(file.toString()) != ProjectFile::Unsupported; + }); + } + } + } + return includeGraph; + } + + void removeAllUsingsAtGlobalScope(CppRefactoringChanges &refactoring) + { + auto includeGraph = buildIncludeGraph(refactoring); + std::list> unprocessedNodes; + for (auto &[_, node] : includeGraph) { + Q_UNUSED(_) + unprocessedNodes.push_back(node); + } + while (!unprocessedNodes.empty()) { + for (auto i = unprocessedNodes.begin(); i != unprocessedNodes.end();) { + Node &node = *i; + if (Utils::allOf(node.includes, &Node::processed)) { + CppRefactoringFilePtr file = refactoring.file(node.document->fileName()); + const bool parentHasUsing = Utils::anyOf(node.includes, + &Node::hasGlobalUsingDirective); + const int startPos = parentHasUsing + ? 0 + : RemoveNamespaceVisitor::SearchGlobalUsingDirectivePos; + const bool noGlobalUsing = refactorFile(file, refactoring.snapshot(), startPos); + node.hasGlobalUsingDirective = !noGlobalUsing || parentHasUsing; + node.processed = true; + i = unprocessedNodes.erase(i); + } else { + ++i; + } + } + } + } + void perform() override { CppRefactoringChanges refactoring(snapshot()); CppRefactoringFilePtr currentFile = refactoring.file(filePath().toString()); - if (refactorFile(currentFile, refactoring.snapshot(), currentFile->endOf(m_usingDirective), true)) + if (m_removeAllAtGlobalScope) { + removeAllUsingsAtGlobalScope(refactoring); + } else if (refactorFile(currentFile, + refactoring.snapshot(), + currentFile->endOf(m_usingDirective), + true)) { processIncludes(refactoring, filePath().toString()); + } for (auto &file : m_changes) file->apply(); @@ -7687,10 +7786,12 @@ private: Utils::ChangeSet changes = visitor.getChanges(); if (removeUsing) removeLine(file.get(), m_usingDirective, changes); - file->setChangeSet(changes); - // apply changes at the end, otherwise the symbol finder will fail to resolve symbols if - // the using namespace is missing - m_changes.insert(file); + if (!changes.isEmpty()) { + file->setChangeSet(changes); + // apply changes at the end, otherwise the symbol finder will fail to resolve symbols if + // the using namespace is missing + m_changes.insert(file); + } return visitor.isGlobalUsingNamespace() && !visitor.foundGlobalUsingNamespace(); } -- cgit v1.2.3