[clang-tools-extra] r374982 - [clangd] Add RemoveUsingNamespace tweak.
Utkarsh Saxena via cfe-commits
cfe-commits at lists.llvm.org
Wed Oct 16 02:53:59 PDT 2019
Author: usaxena95
Date: Wed Oct 16 02:53:59 2019
New Revision: 374982
URL: http://llvm.org/viewvc/llvm-project?rev=374982&view=rev
Log:
[clangd] Add RemoveUsingNamespace tweak.
Summary:
Removes the 'using namespace' under the cursor and qualifies all accesses in the current file.
E.g.:
using namespace std;
vector<int> foo(std::map<int, int>);
Would become:
std::vector<int> foo(std::map<int, int>);
Subscribers: mgorny, ilya-biryukov, MaskRay, jkorous, mgrang, arphaman, kadircet, cfe-commits
Tags: #clang
Differential Revision: https://reviews.llvm.org/D68562
Added:
clang-tools-extra/trunk/clangd/refactor/tweaks/RemoveUsingNamespace.cpp
Modified:
clang-tools-extra/trunk/clangd/AST.cpp
clang-tools-extra/trunk/clangd/AST.h
clang-tools-extra/trunk/clangd/refactor/tweaks/CMakeLists.txt
clang-tools-extra/trunk/clangd/unittests/TweakTests.cpp
Modified: clang-tools-extra/trunk/clangd/AST.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/AST.cpp?rev=374982&r1=374981&r2=374982&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/AST.cpp (original)
+++ clang-tools-extra/trunk/clangd/AST.cpp Wed Oct 16 02:53:59 2019
@@ -115,6 +115,18 @@ static NestedNameSpecifier *getQualifier
return nullptr;
}
+std::string printUsingNamespaceName(const ASTContext &Ctx,
+ const UsingDirectiveDecl &D) {
+ PrintingPolicy PP(Ctx.getLangOpts());
+ std::string Name;
+ llvm::raw_string_ostream Out(Name);
+
+ if (auto *Qual = D.getQualifier())
+ Qual->print(Out, PP);
+ D.getNominatedNamespaceAsWritten()->printName(Out);
+ return Out.str();
+}
+
std::string printName(const ASTContext &Ctx, const NamedDecl &ND) {
std::string Name;
llvm::raw_string_ostream Out(Name);
Modified: clang-tools-extra/trunk/clangd/AST.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/AST.h?rev=374982&r1=374981&r2=374982&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/AST.h (original)
+++ clang-tools-extra/trunk/clangd/AST.h Wed Oct 16 02:53:59 2019
@@ -42,6 +42,12 @@ std::string printQualifiedName(const Nam
/// Returns the first enclosing namespace scope starting from \p DC.
std::string printNamespaceScope(const DeclContext &DC);
+/// Returns the name of the namespace inside the 'using namespace' directive, as
+/// written in the code. E.g., passing 'using namespace ::std' will result in
+/// '::std'.
+std::string printUsingNamespaceName(const ASTContext &Ctx,
+ const UsingDirectiveDecl &D);
+
/// Prints unqualified name of the decl for the purpose of displaying it to the
/// user. Anonymous decls return names of the form "(anonymous {kind})", e.g.
/// "(anonymous struct)" or "(anonymous namespace)".
Modified: clang-tools-extra/trunk/clangd/refactor/tweaks/CMakeLists.txt
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/refactor/tweaks/CMakeLists.txt?rev=374982&r1=374981&r2=374982&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/refactor/tweaks/CMakeLists.txt (original)
+++ clang-tools-extra/trunk/clangd/refactor/tweaks/CMakeLists.txt Wed Oct 16 02:53:59 2019
@@ -19,6 +19,7 @@ add_clang_library(clangDaemonTweaks OBJE
ExtractFunction.cpp
ExtractVariable.cpp
RawStringLiteral.cpp
+ RemoveUsingNamespace.cpp
SwapIfBranches.cpp
LINK_LIBS
Added: clang-tools-extra/trunk/clangd/refactor/tweaks/RemoveUsingNamespace.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/refactor/tweaks/RemoveUsingNamespace.cpp?rev=374982&view=auto
==============================================================================
--- clang-tools-extra/trunk/clangd/refactor/tweaks/RemoveUsingNamespace.cpp (added)
+++ clang-tools-extra/trunk/clangd/refactor/tweaks/RemoveUsingNamespace.cpp Wed Oct 16 02:53:59 2019
@@ -0,0 +1,206 @@
+//===--- RemoveUsingNamespace.cpp --------------------------------*- C++-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+#include "AST.h"
+#include "FindTarget.h"
+#include "Selection.h"
+#include "SourceCode.h"
+#include "refactor/Tweak.h"
+#include "clang/AST/Decl.h"
+#include "clang/AST/DeclBase.h"
+#include "clang/AST/DeclCXX.h"
+#include "clang/AST/RecursiveASTVisitor.h"
+#include "clang/Basic/SourceLocation.h"
+#include "clang/Tooling/Core/Replacement.h"
+#include "clang/Tooling/Refactoring/RecursiveSymbolVisitor.h"
+#include "llvm/ADT/ScopeExit.h"
+
+namespace clang {
+namespace clangd {
+namespace {
+/// Removes the 'using namespace' under the cursor and qualifies all accesses in
+/// the current file. E.g.,
+/// using namespace std;
+/// vector<int> foo(std::map<int, int>);
+/// Would become:
+/// std::vector<int> foo(std::map<int, int>);
+/// Currently limited to using namespace directives inside global namespace to
+/// simplify implementation. Also the namespace must not contain using
+/// directives.
+class RemoveUsingNamespace : public Tweak {
+public:
+ const char *id() const override;
+
+ bool prepare(const Selection &Inputs) override;
+ Expected<Effect> apply(const Selection &Inputs) override;
+ std::string title() const override;
+ Intent intent() const override { return Refactor; }
+
+private:
+ const UsingDirectiveDecl *TargetDirective = nullptr;
+};
+REGISTER_TWEAK(RemoveUsingNamespace)
+
+class FindSameUsings : public RecursiveASTVisitor<FindSameUsings> {
+public:
+ FindSameUsings(const UsingDirectiveDecl &Target,
+ std::vector<const UsingDirectiveDecl *> &Results)
+ : TargetNS(Target.getNominatedNamespace()),
+ TargetCtx(Target.getDeclContext()), Results(Results) {}
+
+ bool VisitUsingDirectiveDecl(UsingDirectiveDecl *D) {
+ if (D->getNominatedNamespace() != TargetNS ||
+ D->getDeclContext() != TargetCtx)
+ return true;
+ Results.push_back(D);
+ return true;
+ }
+
+private:
+ const NamespaceDecl *TargetNS;
+ const DeclContext *TargetCtx;
+ std::vector<const UsingDirectiveDecl *> &Results;
+};
+
+/// Produce edit removing 'using namespace xxx::yyy' and the trailing semicolon.
+llvm::Expected<tooling::Replacement>
+removeUsingDirective(ASTContext &Ctx, const UsingDirectiveDecl *D) {
+ auto &SM = Ctx.getSourceManager();
+ llvm::Optional<Token> NextTok =
+ Lexer::findNextToken(D->getEndLoc(), SM, Ctx.getLangOpts());
+ if (!NextTok || NextTok->isNot(tok::semi))
+ return llvm::createStringError(llvm::inconvertibleErrorCode(),
+ "no semicolon after using-directive");
+ // FIXME: removing the semicolon may be invalid in some obscure cases, e.g.
+ // if (x) using namespace std; else using namespace bar;
+ return tooling::Replacement(
+ SM,
+ CharSourceRange::getTokenRange(D->getBeginLoc(), NextTok->getLocation()),
+ "", Ctx.getLangOpts());
+}
+
+// Returns true iff the parent of the Node is a TUDecl.
+bool isTopLevelDecl(const SelectionTree::Node *Node) {
+ return Node->Parent && Node->Parent->ASTNode.get<TranslationUnitDecl>();
+}
+
+// Returns the first visible context that contains this DeclContext.
+// For example: Returns ns1 for S1 and a.
+// namespace ns1 {
+// inline namespace ns2 { struct S1 {}; }
+// enum E { a, b, c, d };
+// }
+const DeclContext *visibleContext(const DeclContext *D) {
+ while (D->isInlineNamespace() || D->isTransparentContext())
+ D = D->getParent();
+ return D;
+}
+
+bool RemoveUsingNamespace::prepare(const Selection &Inputs) {
+ // Find the 'using namespace' directive under the cursor.
+ auto *CA = Inputs.ASTSelection.commonAncestor();
+ if (!CA)
+ return false;
+ TargetDirective = CA->ASTNode.get<UsingDirectiveDecl>();
+ if (!TargetDirective)
+ return false;
+ if (!dyn_cast<Decl>(TargetDirective->getDeclContext()))
+ return false;
+ // FIXME: Unavailable for namespaces containing using-namespace decl.
+ // It is non-trivial to deal with cases where identifiers come from the inner
+ // namespace. For example map has to be changed to aa::map.
+ // namespace aa {
+ // namespace bb { struct map {}; }
+ // using namespace bb;
+ // }
+ // using namespace a^a;
+ // int main() { map m; }
+ // We need to make this aware of the transitive using-namespace decls.
+ if (!TargetDirective->getNominatedNamespace()->using_directives().empty())
+ return false;
+ return isTopLevelDecl(CA);
+}
+
+Expected<Tweak::Effect> RemoveUsingNamespace::apply(const Selection &Inputs) {
+ auto &Ctx = Inputs.AST.getASTContext();
+ auto &SM = Ctx.getSourceManager();
+ // First, collect *all* using namespace directives that redeclare the same
+ // namespace.
+ std::vector<const UsingDirectiveDecl *> AllDirectives;
+ FindSameUsings(*TargetDirective, AllDirectives).TraverseAST(Ctx);
+
+ SourceLocation FirstUsingDirectiveLoc;
+ for (auto *D : AllDirectives) {
+ if (FirstUsingDirectiveLoc.isInvalid() ||
+ SM.isBeforeInTranslationUnit(D->getBeginLoc(), FirstUsingDirectiveLoc))
+ FirstUsingDirectiveLoc = D->getBeginLoc();
+ }
+
+ // Collect all references to symbols from the namespace for which we're
+ // removing the directive.
+ std::vector<SourceLocation> IdentsToQualify;
+ for (auto &D : Inputs.AST.getLocalTopLevelDecls()) {
+ findExplicitReferences(D, [&](ReferenceLoc Ref) {
+ if (Ref.Qualifier)
+ return; // This reference is already qualified.
+
+ for (auto *T : Ref.Targets) {
+ if (!visibleContext(T->getDeclContext())
+ ->Equals(TargetDirective->getNominatedNamespace()))
+ return;
+ }
+ SourceLocation Loc = Ref.NameLoc;
+ if (Loc.isMacroID()) {
+ // Avoid adding qualifiers before macro expansions, it's probably
+ // incorrect, e.g.
+ // namespace std { int foo(); }
+ // #define FOO 1 + foo()
+ // using namespace foo; // provides matrix
+ // auto x = FOO; // Must not changed to auto x = std::FOO
+ if (!SM.isMacroArgExpansion(Loc))
+ return; // FIXME: report a warning to the users.
+ Loc = SM.getFileLoc(Ref.NameLoc);
+ }
+ assert(Loc.isFileID());
+ if (SM.getFileID(Loc) != SM.getMainFileID())
+ return; // FIXME: report these to the user as warnings?
+ if (SM.isBeforeInTranslationUnit(Loc, FirstUsingDirectiveLoc))
+ return; // Directive was not visible before this point.
+ IdentsToQualify.push_back(Loc);
+ });
+ }
+ // Remove duplicates.
+ llvm::sort(IdentsToQualify);
+ IdentsToQualify.erase(
+ std::unique(IdentsToQualify.begin(), IdentsToQualify.end()),
+ IdentsToQualify.end());
+
+ // Produce replacements to remove the using directives.
+ tooling::Replacements R;
+ for (auto *D : AllDirectives) {
+ auto RemoveUsing = removeUsingDirective(Ctx, D);
+ if (!RemoveUsing)
+ return RemoveUsing.takeError();
+ if (auto Err = R.add(*RemoveUsing))
+ return std::move(Err);
+ }
+ // Produce replacements to add the qualifiers.
+ std::string Qualifier = printUsingNamespaceName(Ctx, *TargetDirective) + "::";
+ for (auto Loc : IdentsToQualify) {
+ if (auto Err = R.add(tooling::Replacement(Ctx.getSourceManager(), Loc,
+ /*Length=*/0, Qualifier)))
+ return std::move(Err);
+ }
+ return Effect::mainFileEdit(SM, std::move(R));
+}
+
+std::string RemoveUsingNamespace::title() const {
+ return llvm::formatv("Remove using namespace, re-qualify names instead.");
+}
+} // namespace
+} // namespace clangd
+} // namespace clang
Modified: clang-tools-extra/trunk/clangd/unittests/TweakTests.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/unittests/TweakTests.cpp?rev=374982&r1=374981&r2=374982&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/unittests/TweakTests.cpp (original)
+++ clang-tools-extra/trunk/clangd/unittests/TweakTests.cpp Wed Oct 16 02:53:59 2019
@@ -69,7 +69,8 @@ TEST_F(SwapIfBranchesTest, Test) {
EXPECT_EQ(apply("^if (true) {return 100;} else {continue;}"),
"if (true) {continue;} else {return 100;}");
EXPECT_EQ(apply("^if () {return 100;} else {continue;}"),
- "if () {continue;} else {return 100;}") << "broken condition";
+ "if () {continue;} else {return 100;}")
+ << "broken condition";
EXPECT_AVAILABLE("^i^f^^(^t^r^u^e^) { return 100; } ^e^l^s^e^ { continue; }");
EXPECT_UNAVAILABLE("if (true) {^return ^100;^ } else { ^continue^;^ }");
// Available in subexpressions of the condition;
@@ -100,7 +101,7 @@ TEST_F(RawStringLiteralTest, Test) {
EXPECT_UNAVAILABLE(R"cpp(R"(multi )" ^"token " u8"str\ning")cpp"); // nonascii
EXPECT_UNAVAILABLE(R"cpp(^R^"^(^multi )" "token " "str\ning")cpp"); // raw
EXPECT_UNAVAILABLE(R"cpp(^"token\n" __FILE__)cpp"); // chunk is macro
- EXPECT_UNAVAILABLE(R"cpp(^"a\r\n";)cpp"); // forbidden escape char
+ EXPECT_UNAVAILABLE(R"cpp(^"a\r\n";)cpp"); // forbidden escape char
const char *Input = R"cpp(R"(multi
token)" "\nst^ring\n" "literal")cpp";
@@ -286,11 +287,11 @@ TEST_F(ExtractVariableTest, Test) {
void f(int a) {
int y = PLUS([[1+a]]);
})cpp",
- /*FIXME: It should be extracted like this.
- R"cpp(#define PLUS(x) x++
- void f(int a) {
- auto dummy = 1+a; int y = PLUS(dummy);
- })cpp"},*/
+ /*FIXME: It should be extracted like this.
+ R"cpp(#define PLUS(x) x++
+ void f(int a) {
+ auto dummy = 1+a; int y = PLUS(dummy);
+ })cpp"},*/
R"cpp(#define PLUS(x) x++
void f(int a) {
auto dummy = PLUS(1+a); int y = dummy;
@@ -301,13 +302,13 @@ TEST_F(ExtractVariableTest, Test) {
if(1)
LOOP(5 + [[3]])
})cpp",
- /*FIXME: It should be extracted like this. SelectionTree needs to be
- * fixed for macros.
- R"cpp(#define LOOP(x) while (1) {a = x;}
- void f(int a) {
- auto dummy = 3; if(1)
- LOOP(5 + dummy)
- })cpp"},*/
+ /*FIXME: It should be extracted like this. SelectionTree needs to be
+ * fixed for macros.
+ R"cpp(#define LOOP(x) while (1) {a = x;}
+ void f(int a) {
+ auto dummy = 3; if(1)
+ LOOP(5 + dummy)
+ })cpp"},*/
R"cpp(#define LOOP(x) while (1) {a = x;}
void f(int a) {
auto dummy = LOOP(5 + 3); if(1)
@@ -403,8 +404,8 @@ TEST_F(ExtractVariableTest, Test) {
void f() {
auto dummy = S(2) + S(3) + S(4); S x = S(1) + dummy + S(5);
})cpp"},
- // Don't try to analyze across macro boundaries
- // FIXME: it'd be nice to do this someday (in a safe way)
+ // Don't try to analyze across macro boundaries
+ // FIXME: it'd be nice to do this someday (in a safe way)
{R"cpp(#define ECHO(X) X
void f() {
int x = 1 + [[ECHO(2 + 3) + 4]] + 5;
@@ -519,7 +520,7 @@ TEST_F(ExpandAutoTypeTest, Test) {
StartsWith("fail: Could not expand type of lambda expression"));
// inline namespaces
EXPECT_EQ(apply("au^to x = inl_ns::Visible();"),
- "Visible x = inl_ns::Visible();");
+ "Visible x = inl_ns::Visible();");
// local class
EXPECT_EQ(apply("namespace x { void y() { struct S{}; ^auto z = S(); } }"),
"namespace x { void y() { struct S{}; S z = S(); } }");
@@ -663,6 +664,222 @@ TEST_F(ExtractFunctionTest, ControlFlow)
EXPECT_THAT(apply(" for(;;) { [[while(1) break; break;]] }"),
StartsWith("fail"));
}
+
+TWEAK_TEST(RemoveUsingNamespace);
+TEST_F(RemoveUsingNamespaceTest, All) {
+ std::pair<llvm::StringRef /*Input*/, llvm::StringRef /*Expected*/> Cases[] = {
+ {// Remove all occurrences of ns. Qualify only unqualified.
+ R"cpp(
+ namespace ns1 { struct vector {}; }
+ namespace ns2 { struct map {}; }
+ using namespace n^s1;
+ using namespace ns2;
+ using namespace ns1;
+ int main() {
+ ns1::vector v1;
+ vector v2;
+ map m1;
+ }
+ )cpp",
+ R"cpp(
+ namespace ns1 { struct vector {}; }
+ namespace ns2 { struct map {}; }
+
+ using namespace ns2;
+
+ int main() {
+ ns1::vector v1;
+ ns1::vector v2;
+ map m1;
+ }
+ )cpp"},
+ {// Ident to be qualified is a macro arg.
+ R"cpp(
+ #define DECLARE(x, y) x y
+ namespace ns { struct vector {}; }
+ using namespace n^s;
+ int main() {
+ DECLARE(ns::vector, v1);
+ DECLARE(vector, v2);
+ }
+ )cpp",
+ R"cpp(
+ #define DECLARE(x, y) x y
+ namespace ns { struct vector {}; }
+
+ int main() {
+ DECLARE(ns::vector, v1);
+ DECLARE(ns::vector, v2);
+ }
+ )cpp"},
+ {// Nested namespace: Fully qualify ident from inner ns.
+ R"cpp(
+ namespace aa { namespace bb { struct map {}; }}
+ using namespace aa::b^b;
+ int main() {
+ map m;
+ }
+ )cpp",
+ R"cpp(
+ namespace aa { namespace bb { struct map {}; }}
+
+ int main() {
+ aa::bb::map m;
+ }
+ )cpp"},
+ {// Nested namespace: Fully qualify ident from inner ns.
+ R"cpp(
+ namespace aa { namespace bb { struct map {}; }}
+ using namespace a^a;
+ int main() {
+ bb::map m;
+ }
+ )cpp",
+ R"cpp(
+ namespace aa { namespace bb { struct map {}; }}
+
+ int main() {
+ aa::bb::map m;
+ }
+ )cpp"},
+ {// Typedef.
+ R"cpp(
+ namespace aa { namespace bb { struct map {}; }}
+ using namespace a^a;
+ typedef bb::map map;
+ int main() { map M; }
+ )cpp",
+ R"cpp(
+ namespace aa { namespace bb { struct map {}; }}
+
+ typedef aa::bb::map map;
+ int main() { map M; }
+ )cpp"},
+ {// FIXME: Nested namespaces: Not aware of using ns decl of outer ns.
+ R"cpp(
+ namespace aa { namespace bb { struct map {}; }}
+ using name[[space aa::b]]b;
+ using namespace aa;
+ int main() {
+ map m;
+ }
+ )cpp",
+ R"cpp(
+ namespace aa { namespace bb { struct map {}; }}
+
+ using namespace aa;
+ int main() {
+ aa::bb::map m;
+ }
+ )cpp"},
+ {// Does not qualify ident from inner namespace.
+ R"cpp(
+ namespace aa { namespace bb { struct map {}; }}
+ using namespace aa::bb;
+ using namespace a^a;
+ int main() {
+ map m;
+ }
+ )cpp",
+ R"cpp(
+ namespace aa { namespace bb { struct map {}; }}
+ using namespace aa::bb;
+
+ int main() {
+ map m;
+ }
+ )cpp"},
+ {// Available only for top level namespace decl.
+ R"cpp(
+ namespace aa {
+ namespace bb { struct map {}; }
+ using namespace b^b;
+ }
+ int main() { aa::map m; }
+ )cpp",
+ "unavailable"},
+ {// FIXME: Unavailable for namespaces containing using-namespace decl.
+ R"cpp(
+ namespace aa {
+ namespace bb { struct map {}; }
+ using namespace bb;
+ }
+ using namespace a^a;
+ int main() {
+ map m;
+ }
+ )cpp",
+ "unavailable"},
+ {R"cpp(
+ namespace a::b { struct Foo {}; }
+ using namespace a;
+ using namespace a::[[b]];
+ using namespace b;
+ int main() { Foo F;}
+ )cpp",
+ R"cpp(
+ namespace a::b { struct Foo {}; }
+ using namespace a;
+
+
+ int main() { a::b::Foo F;}
+ )cpp"},
+ {R"cpp(
+ namespace a::b { struct Foo {}; }
+ using namespace a;
+ using namespace a::b;
+ using namespace [[b]];
+ int main() { Foo F;}
+ )cpp",
+ R"cpp(
+ namespace a::b { struct Foo {}; }
+ using namespace a;
+
+
+ int main() { b::Foo F;}
+ )cpp"},
+ {// Enumerators.
+ R"cpp(
+ namespace tokens {
+ enum Token {
+ comma, identifier, numeric
+ };
+ }
+ using namespace tok^ens;
+ int main() {
+ auto x = comma;
+ }
+ )cpp",
+ R"cpp(
+ namespace tokens {
+ enum Token {
+ comma, identifier, numeric
+ };
+ }
+
+ int main() {
+ auto x = tokens::comma;
+ }
+ )cpp"},
+ {// inline namespaces.
+ R"cpp(
+ namespace std { inline namespace ns1 { inline namespace ns2 { struct vector {}; }}}
+ using namespace st^d;
+ int main() {
+ vector V;
+ }
+ )cpp",
+ R"cpp(
+ namespace std { inline namespace ns1 { inline namespace ns2 { struct vector {}; }}}
+
+ int main() {
+ std::vector V;
+ }
+ )cpp"}};
+ for (auto C : Cases)
+ EXPECT_EQ(C.second, apply(C.first)) << C.first;
+}
+
} // namespace
} // namespace clangd
} // namespace clang
More information about the cfe-commits
mailing list