adamcz created this revision.
adamcz added a reviewer: sammccall.
Herald added subscribers: cfe-commits, usaxena95, kadircet, arphaman, jkorous.
Herald added a project: clang.
adamcz requested review of this revision.
Herald added subscribers: MaskRay, ilya-biryukov.
For style guides forbid "using" declarations for namespaces like "std".
With this new config option, AddUsing can be selectively disabled on
those.
Repository:
rG LLVM Github Monorepo
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,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.
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,13 @@
llvm::Optional<Located<std::string>> 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<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,20 @@
}
}
+ void compile(Fragment::StyleBlock &&F) {
+ if (!F.FullyQualifiedNamespaces.empty()) {
+ std::vector<std::string> 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;
Index: clang-tools-extra/clangd/Config.h
===================================================================
--- clang-tools-extra/clangd/Config.h
+++ 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<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
