diff --git a/clang-tools-extra/clangd/Config.h b/clang-tools-extra/clangd/Config.h --- a/clang-tools-extra/clangd/Config.h +++ b/clang-tools-extra/clangd/Config.h @@ -62,6 +62,11 @@ /// Whether this TU should be indexed. BackgroundPolicy Background = BackgroundPolicy::Build; } Index; + + /// Style of the codebase. + struct { + std::vector FullyQualifiedNamespaces; + } Style; }; } // namespace clangd diff --git a/clang-tools-extra/clangd/ConfigCompile.cpp b/clang-tools-extra/clangd/ConfigCompile.cpp --- a/clang-tools-extra/clangd/ConfigCompile.cpp +++ b/clang-tools-extra/clangd/ConfigCompile.cpp @@ -208,6 +208,20 @@ } } + void compile(Fragment::StyleBlock &&F) { + if (!F.FullyQualifiedNamespaces.empty()) { + std::vector FullyQualifiedNamespaces; + for (auto &N : F.FullyQualifiedNamespaces) + FullyQualifiedNamespaces.push_back(std::move(*N)); + Out.Apply.push_back([FullyQualifiedNamespaces( + std::move(FullyQualifiedNamespaces))](Config &C) { + C.Style.FullyQualifiedNamespaces.insert( + C.Style.FullyQualifiedNamespaces.begin(), + FullyQualifiedNamespaces.begin(), FullyQualifiedNamespaces.end()); + }); + } + } + constexpr static llvm::SourceMgr::DiagKind Error = llvm::SourceMgr::DK_Error; constexpr static llvm::SourceMgr::DiagKind Warning = llvm::SourceMgr::DK_Warning; diff --git a/clang-tools-extra/clangd/ConfigFragment.h b/clang-tools-extra/clangd/ConfigFragment.h --- a/clang-tools-extra/clangd/ConfigFragment.h +++ b/clang-tools-extra/clangd/ConfigFragment.h @@ -161,6 +161,13 @@ llvm::Optional> Background; }; IndexBlock Index; + + // Describes the style of the codebase, beyond formatting. + struct StyleBlock { + // List of namespaces that should not appear in "using" declarations. + std::vector> FullyQualifiedNamespaces; + }; + StyleBlock Style; }; } // namespace config diff --git a/clang-tools-extra/clangd/ConfigYAML.cpp b/clang-tools-extra/clangd/ConfigYAML.cpp --- a/clang-tools-extra/clangd/ConfigYAML.cpp +++ b/clang-tools-extra/clangd/ConfigYAML.cpp @@ -38,6 +38,7 @@ DictParser Dict("Config", this); Dict.handle("If", [&](Node &N) { parse(F.If, N); }); Dict.handle("CompileFlags", [&](Node &N) { parse(F.CompileFlags, N); }); + Dict.handle("Style", [&](Node &N) { parse(F.Style, N); }); Dict.parse(N); return !(N.failed() || HadError); } @@ -71,6 +72,15 @@ Dict.parse(N); } + void parse(Fragment::StyleBlock &F, Node &N) { + DictParser Dict("Style", this); + Dict.handle("FullyQualifiedNamespaces", [&](Node &N) { + if (auto Values = scalarValues(N)) + F.FullyQualifiedNamespaces = std::move(*Values); + }); + Dict.parse(N); + } + void parse(Fragment::IndexBlock &F, Node &N) { DictParser Dict("Index", this); Dict.handle("Background", diff --git a/clang-tools-extra/clangd/refactor/tweaks/AddUsing.cpp b/clang-tools-extra/clangd/refactor/tweaks/AddUsing.cpp --- a/clang-tools-extra/clangd/refactor/tweaks/AddUsing.cpp +++ b/clang-tools-extra/clangd/refactor/tweaks/AddUsing.cpp @@ -7,6 +7,7 @@ //===----------------------------------------------------------------------===// #include "AST.h" +#include "Config.h" #include "FindTarget.h" #include "refactor/Tweak.h" #include "support/Logger.h" @@ -190,6 +191,29 @@ return Out; } +bool isNamespaceForbidden(const Tweak::Selection &Inputs, + const NestedNameSpecifier &Namespace) { + std::string Buf; + llvm::raw_string_ostream NamespaceStream(Buf); + Namespace.print(NamespaceStream, + Inputs.AST->getASTContext().getPrintingPolicy()); + NamespaceStream << "::"; + StringRef NamespaceStr = NamespaceStream.str(); + NamespaceStr.consume_front("::"); + + for (StringRef Banned : Config::current().Style.FullyQualifiedNamespaces) { + // We want to match regardless of leading ::, so we remove it from both + // NamespaceStr and Banned. We append it at the end of both to make sure + // that we do not prefix-match ::foo to ::foobar. + Banned.consume_front("::"); + Banned.consume_back("::"); + if (NamespaceStr.startswith(Banned.str() + "::")) + return true; + } + + return false; +} + bool AddUsing::prepare(const Selection &Inputs) { auto &SM = Inputs.AST->getSourceManager(); @@ -248,6 +272,9 @@ return false; } + if (isNamespaceForbidden(Inputs, *QualifierToRemove.getNestedNameSpecifier())) + return false; + // Macros are difficult. We only want to offer code action when what's spelled // under the cursor is a namespace qualifier. If it's a macro that expands to // a qualifier, user would not know what code action will actually change. diff --git a/clang-tools-extra/clangd/unittests/TweakTests.cpp b/clang-tools-extra/clangd/unittests/TweakTests.cpp --- a/clang-tools-extra/clangd/unittests/TweakTests.cpp +++ b/clang-tools-extra/clangd/unittests/TweakTests.cpp @@ -7,6 +7,7 @@ //===----------------------------------------------------------------------===// #include "Annotations.h" +#include "Config.h" #include "SourceCode.h" #include "TestFS.h" #include "TestTU.h" @@ -2471,8 +2472,14 @@ TWEAK_TEST(AddUsing); TEST_F(AddUsingTest, Prepare) { + Config Cfg; + Cfg.Style.FullyQualifiedNamespaces.push_back("ban"); + WithContextValue WithConfig(Config::Key, std::move(Cfg)); + const std::string Header = R"cpp( #define NS(name) one::two::name +namespace ban { void foo() {} } +namespace banana { void foo() {} } namespace one { void oo() {} template class tt {}; @@ -2506,6 +2513,10 @@ // Test that we don't crash or misbehave on unnamed DeclRefExpr. EXPECT_UNAVAILABLE(Header + "void fun() { one::two::cc() ^| one::two::cc(); }"); + // Do not offer code action when operating on a banned namespace. + EXPECT_UNAVAILABLE(Header + "void fun() { ban::fo^o(); }"); + EXPECT_UNAVAILABLE(Header + "void fun() { ::ban::fo^o(); }"); + EXPECT_AVAILABLE(Header + "void fun() { banana::fo^o(); }"); // Check that we do not trigger in header files. FileName = "test.h";