[clang-tools-extra] c894bfd - [clangd] Add option for disabling AddUsing tweak on some namespaces.
Adam Czachorowski via cfe-commits
cfe-commits at lists.llvm.org
Fri Sep 18 08:01:11 PDT 2020
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";
More information about the cfe-commits
mailing list