adamcz updated this revision to Diff 292786.
adamcz marked 2 inline comments as done.
adamcz added a comment.
addressed review comments
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D87775/new/
https://reviews.llvm.org/D87775
Files:
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
Index: clang-tools-extra/clangd/unittests/TweakTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/TweakTests.cpp
+++ 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<typename TT> 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";
Index: clang-tools-extra/clangd/refactor/tweaks/AddUsing.cpp
===================================================================
--- clang-tools-extra/clangd/refactor/tweaks/AddUsing.cpp
+++ 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 @@
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 @@
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.
Index: clang-tools-extra/clangd/ConfigYAML.cpp
===================================================================
--- clang-tools-extra/clangd/ConfigYAML.cpp
+++ 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",
Index: clang-tools-extra/clangd/ConfigFragment.h
===================================================================
--- clang-tools-extra/clangd/ConfigFragment.h
+++ clang-tools-extra/clangd/ConfigFragment.h
@@ -161,6 +161,16 @@
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
Index: clang-tools-extra/clangd/ConfigCompile.cpp
===================================================================
--- clang-tools-extra/clangd/ConfigCompile.cpp
+++ clang-tools-extra/clangd/ConfigCompile.cpp
@@ -208,6 +208,25 @@
}
}
+ 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;
Index: clang-tools-extra/clangd/Config.h
===================================================================
--- clang-tools-extra/clangd/Config.h
+++ clang-tools-extra/clangd/Config.h
@@ -62,6 +62,14 @@
/// 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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
