[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

llvm-svn: 374982
This commit is contained in:
Utkarsh Saxena 2019-10-16 09:53:59 +00:00
parent 1c3ca61294
commit b62b454121
5 changed files with 459 additions and 17 deletions

View File

@ -115,6 +115,18 @@ static NestedNameSpecifier *getQualifier(const NamedDecl &ND) {
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);

View File

@ -42,6 +42,12 @@ std::string printQualifiedName(const NamedDecl &ND);
/// 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)".

View File

@ -19,6 +19,7 @@ add_clang_library(clangDaemonTweaks OBJECT
ExtractFunction.cpp
ExtractVariable.cpp
RawStringLiteral.cpp
RemoveUsingNamespace.cpp
SwapIfBranches.cpp
LINK_LIBS

View File

@ -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

View File

@ -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