[clang-tools-extra] 24bb2d1 - [clangd] Add a tweak for adding "using" statement.
Sam McCall via cfe-commits
cfe-commits at lists.llvm.org
Thu Apr 2 08:37:47 PDT 2020
Author: Adam Czachorowski
Date: 2020-04-02T17:37:38+02:00
New Revision: 24bb2d1e776897c3a93856d2ca76decb4cfd0562
URL: https://github.com/llvm/llvm-project/commit/24bb2d1e776897c3a93856d2ca76decb4cfd0562
DIFF: https://github.com/llvm/llvm-project/commit/24bb2d1e776897c3a93856d2ca76decb4cfd0562.diff
LOG: [clangd] Add a tweak for adding "using" statement.
Summary:
This triggers on types and function calls with namespace qualifiers. The
action is to remove the qualifier and instead add a "using" statement at
appropriate place.
It is not always clear where to add the "using" line. Right now we find
the nearest "using" line and add it there, thus keeping with local
convention. If there are no usings, we put it at the deepest relevant
namespace level.
This is an initial version only. There are several improvements that
can be made:
* Support for qualifiers that are not purely namespace (e.g. record
types, etc).
* Removing qualifier from other instances of the same type/call.
* Smarter placement of the "using" line.
Reviewers: sammccall
Reviewed By: sammccall
Subscribers: nridge, mgorny, ilya-biryukov, MaskRay, jkorous, mgrang, arphaman, kadircet, usaxena95, cfe-commits
Tags: #clang
Differential Revision: https://reviews.llvm.org/D76432
Added:
clang-tools-extra/clangd/refactor/tweaks/AddUsing.cpp
Modified:
clang-tools-extra/clangd/refactor/tweaks/CMakeLists.txt
clang-tools-extra/clangd/unittests/TweakTests.cpp
Removed:
################################################################################
diff --git a/clang-tools-extra/clangd/refactor/tweaks/AddUsing.cpp b/clang-tools-extra/clangd/refactor/tweaks/AddUsing.cpp
new file mode 100644
index 000000000000..1ecec6674b02
--- /dev/null
+++ b/clang-tools-extra/clangd/refactor/tweaks/AddUsing.cpp
@@ -0,0 +1,286 @@
+//===--- AddUsing.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 "Logger.h"
+#include "refactor/Tweak.h"
+#include "clang/AST/Decl.h"
+#include "clang/AST/RecursiveASTVisitor.h"
+
+namespace clang {
+namespace clangd {
+namespace {
+
+// Tweak for removing full namespace qualifier under cursor on DeclRefExpr and
+// types and adding "using" statement instead.
+//
+// Only qualifiers that refer exclusively to namespaces (no record types) are
+// supported. There is some guessing of appropriate place to insert the using
+// declaration. If we find any existing usings, we insert it there. If not, we
+// insert right after the inner-most relevant namespace declaration. If there is
+// none, or there is, but it was declared via macro, we insert above the first
+// top level decl.
+//
+// Currently this only removes qualifier from under the cursor. In the future,
+// we should improve this to remove qualifier from all occurences of this
+// symbol.
+class AddUsing : 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:
+ // The qualifier to remove. Set by prepare().
+ NestedNameSpecifierLoc QualifierToRemove;
+ // The name following QualifierToRemove. Set by prepare().
+ llvm::StringRef Name;
+};
+REGISTER_TWEAK(AddUsing)
+
+std::string AddUsing::title() const {
+ return std::string(llvm::formatv(
+ "Add using-declaration for {0} and remove qualifier.", Name));
+}
+
+// Locates all "using" statements relevant to SelectionDeclContext.
+class UsingFinder : public RecursiveASTVisitor<UsingFinder> {
+public:
+ UsingFinder(std::vector<const UsingDecl *> &Results,
+ const DeclContext *SelectionDeclContext, const SourceManager &SM)
+ : Results(Results), SelectionDeclContext(SelectionDeclContext), SM(SM) {}
+
+ bool VisitUsingDecl(UsingDecl *D) {
+ auto Loc = D->getUsingLoc();
+ if (SM.getFileID(Loc) != SM.getMainFileID()) {
+ return true;
+ }
+ if (D->getDeclContext()->Encloses(SelectionDeclContext)) {
+ Results.push_back(D);
+ }
+ return true;
+ }
+
+ bool TraverseDecl(Decl *Node) {
+ // There is no need to go deeper into nodes that do not enclose selection,
+ // since "using" there will not affect selection, nor would it make a good
+ // insertion point.
+ if (Node->getDeclContext()->Encloses(SelectionDeclContext)) {
+ return RecursiveASTVisitor<UsingFinder>::TraverseDecl(Node);
+ }
+ return true;
+ }
+
+private:
+ std::vector<const UsingDecl *> &Results;
+ const DeclContext *SelectionDeclContext;
+ const SourceManager &SM;
+};
+
+struct InsertionPointData {
+ // Location to insert the "using" statement. If invalid then the statement
+ // should not be inserted at all (it already exists).
+ SourceLocation Loc;
+ // Extra suffix to place after the "using" statement. Depending on what the
+ // insertion point is anchored to, we may need one or more \n to ensure
+ // proper formatting.
+ std::string Suffix;
+};
+
+// Finds the best place to insert the "using" statement. Returns invalid
+// SourceLocation if the "using" statement already exists.
+//
+// The insertion point might be a little awkward if the decl we're anchoring to
+// has a comment in an unfortunate place (e.g. directly above function or using
+// decl, or immediately following "namespace {". We should add some helpers for
+// dealing with that and use them in other code modifications as well.
+llvm::Expected<InsertionPointData>
+findInsertionPoint(const Tweak::Selection &Inputs,
+ const NestedNameSpecifierLoc &QualifierToRemove,
+ const llvm::StringRef Name) {
+ auto &SM = Inputs.AST->getSourceManager();
+
+ // Search for all using decls that affect this point in file. We need this for
+ // two reasons: to skip adding "using" if one already exists and to find best
+ // place to add it, if it doesn't exist.
+ SourceLocation LastUsingLoc;
+ std::vector<const UsingDecl *> Usings;
+ UsingFinder(Usings, &Inputs.ASTSelection.commonAncestor()->getDeclContext(),
+ SM)
+ .TraverseAST(Inputs.AST->getASTContext());
+
+ for (auto &U : Usings) {
+ if (SM.isBeforeInTranslationUnit(Inputs.Cursor, U->getUsingLoc()))
+ // "Usings" is sorted, so we're done.
+ break;
+ if (U->getQualifier()->getAsNamespace()->getCanonicalDecl() ==
+ QualifierToRemove.getNestedNameSpecifier()
+ ->getAsNamespace()
+ ->getCanonicalDecl() &&
+ U->getName() == Name) {
+ return InsertionPointData();
+ }
+ // Insertion point will be before last UsingDecl that affects cursor
+ // position. For most cases this should stick with the local convention of
+ // add using inside or outside namespace.
+ LastUsingLoc = U->getUsingLoc();
+ }
+ if (LastUsingLoc.isValid()) {
+ InsertionPointData Out;
+ Out.Loc = LastUsingLoc;
+ return Out;
+ }
+
+ // No relevant "using" statements. Try the nearest namespace level.
+ const auto *NS = Inputs.ASTSelection.commonAncestor()
+ ->getDeclContext()
+ .getEnclosingNamespaceContext();
+ if (auto *ND = dyn_cast<NamespaceDecl>(NS)) {
+ auto Toks = Inputs.AST->getTokens().expandedTokens(ND->getSourceRange());
+ const auto *Tok = llvm::find_if(Toks, [](const syntax::Token &Tok) {
+ return Tok.kind() == tok::l_brace;
+ });
+ if (Tok == Toks.end() || Tok->endLocation().isInvalid()) {
+ return llvm::createStringError(llvm::inconvertibleErrorCode(),
+ "Namespace with no {");
+ }
+ if (!Tok->endLocation().isMacroID()) {
+ InsertionPointData Out;
+ Out.Loc = Tok->endLocation();
+ Out.Suffix = "\n";
+ return Out;
+ }
+ }
+ // No using, no namespace, no idea where to insert. Try above the first
+ // top level decl.
+ auto TLDs = Inputs.AST->getLocalTopLevelDecls();
+ if (TLDs.empty()) {
+ return llvm::createStringError(llvm::inconvertibleErrorCode(),
+ "Cannot find place to insert \"using\"");
+ }
+ InsertionPointData Out;
+ Out.Loc = SM.getExpansionLoc(TLDs[0]->getBeginLoc());
+ Out.Suffix = "\n\n";
+ return Out;
+}
+
+bool AddUsing::prepare(const Selection &Inputs) {
+ auto &SM = Inputs.AST->getSourceManager();
+ auto *Node = Inputs.ASTSelection.commonAncestor();
+ if (Node == nullptr)
+ return false;
+
+ // If we're looking at a type or NestedNameSpecifier, walk up the tree until
+ // we find the "main" node we care about, which would be ElaboratedTypeLoc or
+ // DeclRefExpr.
+ for (; Node->Parent; Node = Node->Parent) {
+ if (Node->ASTNode.get<NestedNameSpecifierLoc>()) {
+ continue;
+ } else if (auto *T = Node->ASTNode.get<TypeLoc>()) {
+ if (T->getAs<ElaboratedTypeLoc>()) {
+ break;
+ } else if (Node->Parent->ASTNode.get<TypeLoc>() ||
+ Node->Parent->ASTNode.get<NestedNameSpecifierLoc>()) {
+ // Node is TypeLoc, but it's parent is either TypeLoc or
+ // NestedNameSpecifier. In both cases, we want to go up, to find
+ // the outermost TypeLoc.
+ continue;
+ }
+ }
+ break;
+ }
+ if (Node == nullptr)
+ return false;
+
+ if (auto *D = Node->ASTNode.get<DeclRefExpr>()) {
+ QualifierToRemove = D->getQualifierLoc();
+ Name = D->getDecl()->getName();
+ } else if (auto *T = Node->ASTNode.get<TypeLoc>()) {
+ if (auto E = T->getAs<ElaboratedTypeLoc>()) {
+ QualifierToRemove = E.getQualifierLoc();
+ Name =
+ E.getType().getUnqualifiedType().getBaseTypeIdentifier()->getName();
+ }
+ }
+
+ // FIXME: This only supports removing qualifiers that are made up of just
+ // namespace names. If qualifier contains a type, we could take the longest
+ // namespace prefix and remove that.
+ if (!QualifierToRemove.hasQualifier() ||
+ !QualifierToRemove.getNestedNameSpecifier()->getAsNamespace() ||
+ Name.empty()) {
+ 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.
+ // On the other hand, if the qualifier is part of the macro argument, we
+ // should still support that.
+ if (SM.isMacroBodyExpansion(QualifierToRemove.getBeginLoc()) ||
+ !SM.isWrittenInSameFile(QualifierToRemove.getBeginLoc(),
+ QualifierToRemove.getEndLoc())) {
+ return false;
+ }
+
+ return true;
+}
+
+Expected<Tweak::Effect> AddUsing::apply(const Selection &Inputs) {
+ auto &SM = Inputs.AST->getSourceManager();
+ auto &TB = Inputs.AST->getTokens();
+
+ // Determine the length of the qualifier under the cursor, then remove it.
+ auto SpelledTokens = TB.spelledForExpanded(
+ TB.expandedTokens(QualifierToRemove.getSourceRange()));
+ if (!SpelledTokens) {
+ return llvm::createStringError(
+ llvm::inconvertibleErrorCode(),
+ "Could not determine length of the qualifier");
+ }
+ unsigned Length =
+ syntax::Token::range(SM, SpelledTokens->front(), SpelledTokens->back())
+ .length();
+ tooling::Replacements R;
+ if (auto Err = R.add(tooling::Replacement(
+ SM, SpelledTokens->front().location(), Length, ""))) {
+ return std::move(Err);
+ }
+
+ auto InsertionPoint = findInsertionPoint(Inputs, QualifierToRemove, Name);
+ if (!InsertionPoint) {
+ return InsertionPoint.takeError();
+ }
+
+ if (InsertionPoint->Loc.isValid()) {
+ // Add the using statement at appropriate location.
+ std::string UsingText;
+ llvm::raw_string_ostream UsingTextStream(UsingText);
+ UsingTextStream << "using ";
+ QualifierToRemove.getNestedNameSpecifier()->print(
+ UsingTextStream, Inputs.AST->getASTContext().getPrintingPolicy());
+ UsingTextStream << Name << ";" << InsertionPoint->Suffix;
+
+ assert(SM.getFileID(InsertionPoint->Loc) == SM.getMainFileID());
+ if (auto Err = R.add(tooling::Replacement(SM, InsertionPoint->Loc, 0,
+ UsingTextStream.str()))) {
+ return std::move(Err);
+ }
+ }
+
+ return Effect::mainFileEdit(Inputs.AST->getASTContext().getSourceManager(),
+ std::move(R));
+}
+
+} // namespace
+} // namespace clangd
+} // namespace clang
diff --git a/clang-tools-extra/clangd/refactor/tweaks/CMakeLists.txt b/clang-tools-extra/clangd/refactor/tweaks/CMakeLists.txt
index 5817830b7ea3..995288bca2cf 100644
--- a/clang-tools-extra/clangd/refactor/tweaks/CMakeLists.txt
+++ b/clang-tools-extra/clangd/refactor/tweaks/CMakeLists.txt
@@ -12,6 +12,7 @@ set(LLVM_LINK_COMPONENTS
# $<TARGET_OBJECTS:obj.clangDaemonTweaks> to a list of sources, see
# clangd/tool/CMakeLists.txt for an example.
add_clang_library(clangDaemonTweaks OBJECT
+ AddUsing.cpp
AnnotateHighlightings.cpp
DumpAST.cpp
DefineInline.cpp
diff --git a/clang-tools-extra/clangd/unittests/TweakTests.cpp b/clang-tools-extra/clangd/unittests/TweakTests.cpp
index cae922ffcb95..b5d6117217b6 100644
--- a/clang-tools-extra/clangd/unittests/TweakTests.cpp
+++ b/clang-tools-extra/clangd/unittests/TweakTests.cpp
@@ -2390,6 +2390,250 @@ TEST_F(DefineOutlineTest, FailsMacroSpecifier) {
EXPECT_EQ(apply(Case.first), Case.second);
}
}
+
+TWEAK_TEST(AddUsing);
+TEST_F(AddUsingTest, Prepare) {
+ const std::string Header = R"cpp(
+#define NS(name) one::two::name
+namespace one {
+void oo() {}
+namespace two {
+enum ee {};
+void ff() {}
+class cc {
+public:
+ struct st {};
+ static void mm() {}
+};
+}
+})cpp";
+
+ EXPECT_AVAILABLE(Header + "void fun() { o^n^e^:^:^t^w^o^:^:^f^f(); }");
+ EXPECT_AVAILABLE(Header + "void fun() { o^n^e^::^o^o(); }");
+ EXPECT_AVAILABLE(Header + "void fun() { o^n^e^:^:^t^w^o^:^:^e^e E; }");
+ EXPECT_AVAILABLE(Header + "void fun() { o^n^e^:^:^t^w^o:^:^c^c C; }");
+ EXPECT_UNAVAILABLE(Header +
+ "void fun() { o^n^e^:^:^t^w^o^:^:^c^c^:^:^m^m(); }");
+ EXPECT_UNAVAILABLE(Header +
+ "void fun() { o^n^e^:^:^t^w^o^:^:^c^c^:^:^s^t inst; }");
+ EXPECT_UNAVAILABLE(Header +
+ "void fun() { o^n^e^:^:^t^w^o^:^:^c^c^:^:^s^t inst; }");
+ EXPECT_UNAVAILABLE(Header + "void fun() { N^S(c^c) inst; }");
+}
+
+TEST_F(AddUsingTest, Apply) {
+ FileName = "test.cpp";
+ struct {
+ llvm::StringRef TestSource;
+ llvm::StringRef ExpectedSource;
+ } Cases[]{{
+ // Function, no other using, namespace.
+ R"cpp(
+#include "test.hpp"
+namespace {
+void fun() {
+ ^o^n^e^:^:^t^w^o^:^:^f^f();
+}
+})cpp",
+ R"cpp(
+#include "test.hpp"
+namespace {using one::two::ff;
+
+void fun() {
+ ff();
+}
+})cpp",
+ },
+ // Type, no other using, namespace.
+ {
+ R"cpp(
+#include "test.hpp"
+namespace {
+void fun() {
+ ::on^e::t^wo::c^c inst;
+}
+})cpp",
+ R"cpp(
+#include "test.hpp"
+namespace {using ::one::two::cc;
+
+void fun() {
+ cc inst;
+}
+})cpp",
+ },
+ // Type, no other using, no namespace.
+ {
+ R"cpp(
+#include "test.hpp"
+
+void fun() {
+ on^e::t^wo::e^e inst;
+})cpp",
+ R"cpp(
+#include "test.hpp"
+
+using one::two::ee;
+
+void fun() {
+ ee inst;
+})cpp"},
+ // Function, other usings.
+ {
+ R"cpp(
+#include "test.hpp"
+
+using one::two::cc;
+using one::two::ee;
+
+namespace {
+void fun() {
+ one::two::f^f();
+}
+})cpp",
+ R"cpp(
+#include "test.hpp"
+
+using one::two::cc;
+using one::two::ff;using one::two::ee;
+
+namespace {
+void fun() {
+ ff();
+}
+})cpp",
+ },
+ // Function, other usings inside namespace.
+ {
+ R"cpp(
+#include "test.hpp"
+
+using one::two::cc;
+
+namespace {
+
+using one::two::ff;
+
+void fun() {
+ o^ne::o^o();
+}
+})cpp",
+ R"cpp(
+#include "test.hpp"
+
+using one::two::cc;
+
+namespace {
+
+using one::oo;using one::two::ff;
+
+void fun() {
+ oo();
+}
+})cpp"},
+ // Using comes after cursor.
+ {
+ R"cpp(
+#include "test.hpp"
+
+namespace {
+
+void fun() {
+ one::t^wo::ff();
+}
+
+using one::two::cc;
+
+})cpp",
+ R"cpp(
+#include "test.hpp"
+
+namespace {using one::two::ff;
+
+
+void fun() {
+ ff();
+}
+
+using one::two::cc;
+
+})cpp"},
+ // Pointer type.
+ {R"cpp(
+#include "test.hpp"
+
+void fun() {
+ one::two::c^c *p;
+})cpp",
+ R"cpp(
+#include "test.hpp"
+
+using one::two::cc;
+
+void fun() {
+ cc *p;
+})cpp"},
+ // Namespace declared via macro.
+ {R"cpp(
+#include "test.hpp"
+#define NS_BEGIN(name) namespace name {
+
+NS_BEGIN(foo)
+
+void fun() {
+ one::two::f^f();
+}
+})cpp",
+ R"cpp(
+#include "test.hpp"
+#define NS_BEGIN(name) namespace name {
+
+using one::two::ff;
+
+NS_BEGIN(foo)
+
+void fun() {
+ ff();
+}
+})cpp"},
+ // Inside macro argument.
+ {R"cpp(
+#include "test.hpp"
+#define CALL(name) name()
+
+void fun() {
+ CALL(one::t^wo::ff);
+})cpp",
+ R"cpp(
+#include "test.hpp"
+#define CALL(name) name()
+
+using one::two::ff;
+
+void fun() {
+ CALL(ff);
+})cpp"}};
+ llvm::StringMap<std::string> EditedFiles;
+ for (const auto &Case : Cases) {
+ for (const auto &SubCase : expandCases(Case.TestSource)) {
+ ExtraFiles["test.hpp"] = R"cpp(
+namespace one {
+void oo() {}
+namespace two {
+enum ee {};
+void ff() {}
+class cc {
+public:
+ struct st { struct nested {}; };
+ static void mm() {}
+};
+}
+})cpp";
+ EXPECT_EQ(apply(SubCase, &EditedFiles), Case.ExpectedSource);
+ }
+ }
+}
+
} // namespace
} // namespace clangd
} // namespace clang
More information about the cfe-commits
mailing list