Author: Adam Czachorowski Date: 2020-09-18T16:46:09+02:00 New Revision: c894bfd1f580e5807fc98cc353b0834e0c5ddc21
URL: https://github.com/llvm/llvm-project/commit/c894bfd1f580e5807fc98cc353b0834e0c5ddc21 DIFF: https://github.com/llvm/llvm-project/commit/c894bfd1f580e5807fc98cc353b0834e0c5ddc21.diff LOG: [clangd] Add option for disabling AddUsing tweak on some namespaces. For style guides forbid "using" declarations for namespaces like "std". With this new config option, AddUsing can be selectively disabled on those. Differential Revision: https://reviews.llvm.org/D87775 Added: Modified: clang-tools-extra/clangd/Config.h clang-tools-extra/clangd/ConfigCompile.cpp clang-tools-extra/clangd/ConfigFragment.h clang-tools-extra/clangd/ConfigYAML.cpp clang-tools-extra/clangd/refactor/tweaks/AddUsing.cpp clang-tools-extra/clangd/unittests/TweakTests.cpp Removed: ################################################################################ diff --git a/clang-tools-extra/clangd/Config.h b/clang-tools-extra/clangd/Config.h index f8dc2df51814..087dc4fb805d 100644 --- a/clang-tools-extra/clangd/Config.h +++ b/clang-tools-extra/clangd/Config.h @@ -62,6 +62,14 @@ struct Config { /// Whether this TU should be indexed. BackgroundPolicy Background = BackgroundPolicy::Build; } Index; + + /// Style of the codebase. + struct { + // Namespaces that should always be fully qualified, meaning no "using" + // declarations, always spell out the whole name (with or without leading + // ::). All nested namespaces are affected as well. + std::vector<std::string> FullyQualifiedNamespaces; + } Style; }; } // namespace clangd diff --git a/clang-tools-extra/clangd/ConfigCompile.cpp b/clang-tools-extra/clangd/ConfigCompile.cpp index 9b8a48fdaf7b..62fda5c9eacc 100644 --- a/clang-tools-extra/clangd/ConfigCompile.cpp +++ b/clang-tools-extra/clangd/ConfigCompile.cpp @@ -208,6 +208,25 @@ struct FragmentCompiler { } } + void compile(Fragment::StyleBlock &&F) { + if (!F.FullyQualifiedNamespaces.empty()) { + std::vector<std::string> FullyQualifiedNamespaces; + for (auto &N : F.FullyQualifiedNamespaces) { + // Normalize the data by dropping both leading and trailing :: + StringRef Namespace(*N); + Namespace.consume_front("::"); + Namespace.consume_back("::"); + FullyQualifiedNamespaces.push_back(Namespace.str()); + } + 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 index 330f157326f2..5b4865e48f3a 100644 --- a/clang-tools-extra/clangd/ConfigFragment.h +++ b/clang-tools-extra/clangd/ConfigFragment.h @@ -161,6 +161,16 @@ struct Fragment { llvm::Optional<Located<std::string>> Background; }; IndexBlock Index; + + // Describes the style of the codebase, beyond formatting. + struct StyleBlock { + // Namespaces that should always be fully qualified, meaning no "using" + // declarations, always spell out the whole name (with or without leading + // ::). All nested namespaces are affected as well. + // Affects availability of the AddUsing tweak. + std::vector<Located<std::string>> FullyQualifiedNamespaces; + }; + StyleBlock Style; }; } // namespace config diff --git a/clang-tools-extra/clangd/ConfigYAML.cpp b/clang-tools-extra/clangd/ConfigYAML.cpp index 9988fe376648..bc7d4fdfe85b 100644 --- a/clang-tools-extra/clangd/ConfigYAML.cpp +++ b/clang-tools-extra/clangd/ConfigYAML.cpp @@ -39,6 +39,7 @@ class Parser { Dict.handle("If", [&](Node &N) { parse(F.If, N); }); Dict.handle("CompileFlags", [&](Node &N) { parse(F.CompileFlags, N); }); Dict.handle("Index", [&](Node &N) { parse(F.Index, N); }); + Dict.handle("Style", [&](Node &N) { parse(F.Style, N); }); Dict.parse(N); return !(N.failed() || HadError); } @@ -72,6 +73,15 @@ class Parser { 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 index d5e6e12b31aa..8c69b64c5aff 100644 --- 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,19 @@ findInsertionPoint(const Tweak::Selection &Inputs, return Out; } +bool isNamespaceForbidden(const Tweak::Selection &Inputs, + const NestedNameSpecifier &Namespace) { + std::string NamespaceStr = printNamespaceScope(*Namespace.getAsNamespace()); + + for (StringRef Banned : Config::current().Style.FullyQualifiedNamespaces) { + StringRef PrefixMatch = NamespaceStr; + if (PrefixMatch.consume_front(Banned) && PrefixMatch.consume_front("::")) + return true; + } + + return false; +} + bool AddUsing::prepare(const Selection &Inputs) { auto &SM = Inputs.AST->getSourceManager(); @@ -248,6 +262,9 @@ bool AddUsing::prepare(const Selection &Inputs) { return false; } + if (isNamespaceForbidden(Inputs, *QualifierToRemove.getNestedNameSpecifier())) + return false; + // Macros are diff icult. 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 index 5626b7530599..53d574146ab5 100644 --- 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 @@ TEST_F(DefineOutlineTest, FailsMacroSpecifier) { 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<typename TT> class tt {}; @@ -2506,6 +2513,10 @@ class cc { // 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"; _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
