adamcz created this revision.
Herald added subscribers: cfe-commits, usaxena95, kadircet, arphaman, mgrang,
jkorous, MaskRay, ilya-biryukov, mgorny.
Herald added a project: clang.
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.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D76432
Files:
clang-tools-extra/clangd/refactor/tweaks/AddUsing.cpp
clang-tools-extra/clangd/refactor/tweaks/CMakeLists.txt
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
@@ -2390,6 +2390,191 @@
EXPECT_EQ(apply(Case.first), Case.second);
}
}
+
+TWEAK_TEST(AddUsing);
+TEST_F(AddUsingTest, Prepare) {
+ const std::string Header = R"cpp(
+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; }");
+}
+
+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"}};
+ 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 {};
+ static void mm() {}
+};
+}
+})cpp";
+ EXPECT_EQ(apply(SubCase, &EditedFiles), Case.ExpectedSource);
+ }
+ }
+}
+
} // namespace
} // namespace clangd
} // namespace clang
Index: clang-tools-extra/clangd/refactor/tweaks/CMakeLists.txt
===================================================================
--- clang-tools-extra/clangd/refactor/tweaks/CMakeLists.txt
+++ clang-tools-extra/clangd/refactor/tweaks/CMakeLists.txt
@@ -12,6 +12,7 @@
# $<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
Index: clang-tools-extra/clangd/refactor/tweaks/AddUsing.cpp
===================================================================
--- /dev/null
+++ clang-tools-extra/clangd/refactor/tweaks/AddUsing.cpp
@@ -0,0 +1,238 @@
+//===--- 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 function calls
+// and types and adding "using" statement instead.
+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:
+ struct InsertionPointData {
+ // True iff "using" already exists and we should not add it.
+ bool IdenticalUsingFound = false;
+ // Location to insert the "using" statement.
+ 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.
+ llvm::Expected<InsertionPointData>
+ findInsertionPoint(const Selection &Inputs);
+
+ // The qualifier to remove. Set by prepare().
+ NestedNameSpecifierLoc NNSL;
+ // The name following NNSL. Set by prepare().
+ llvm::StringRef Name;
+};
+REGISTER_TWEAK(AddUsing)
+
+std::string AddUsing::title() const {
+ return std::string(
+ llvm::formatv("Add using statement and remove full qualifier."));
+}
+
+// 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 (Loc.isInvalid() || Loc.isMacroID() ||
+ SM.getFileID(Loc) != SM.getMainFileID()) {
+ return true;
+ }
+ if (D->getDeclContext()->Encloses(SelectionDeclContext)) {
+ Results.push_back(D);
+ }
+ return true;
+ }
+
+private:
+ std::vector<const UsingDecl *> &Results;
+ const DeclContext *SelectionDeclContext;
+ const SourceManager &SM;
+};
+
+bool AddUsing::prepare(const Selection &Inputs) {
+ 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.
+ while (Node->Parent) {
+ if (Node->ASTNode.get<NestedNameSpecifierLoc>()) {
+ Node = Node->Parent;
+ } else if (Node->ASTNode.get<TypeLoc>() &&
+ (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.
+ Node = Node->Parent;
+ } else {
+ break;
+ }
+ }
+ if (Node == nullptr)
+ return false;
+
+ if (auto *D = Node->ASTNode.get<DeclRefExpr>()) {
+ NNSL = D->getQualifierLoc();
+ Name = D->getDecl()->getName();
+ } else if (auto *T = Node->ASTNode.get<TypeLoc>()) {
+ if (auto E = T->getAs<ElaboratedTypeLoc>()) {
+ NNSL = 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 (!NNSL.hasQualifier() ||
+ !NNSL.getNestedNameSpecifier()->getAsNamespace()) {
+ return false;
+ }
+
+ return true;
+}
+
+llvm::Expected<AddUsing::InsertionPointData>
+AddUsing::findInsertionPoint(const Selection &Inputs) {
+ 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());
+
+ llvm::sort(Usings, [&SM](const UsingDecl *L, const UsingDecl *R) {
+ return SM.isBeforeInTranslationUnit(L->getUsingLoc(), R->getUsingLoc());
+ });
+ for (auto &U : Usings) {
+ if (SM.isBeforeInTranslationUnit(Inputs.Cursor, U->getUsingLoc()))
+ break;
+ if (U->getQualifier()->getAsNamespace() ==
+ NNSL.getNestedNameSpecifier()->getAsNamespace() &&
+ U->getName() == Name) {
+ InsertionPointData Out;
+ Out.IdenticalUsingFound = true;
+ return Out;
+ }
+ // 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.
+ // We could be smarter and check if the deepest block of "using" is sorted
+ // alphabetically and if so, insert at appropriate place. For now, users can
+ // just reformat the file with clang-format or similar.
+ 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 {");
+ }
+ 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 = TLDs[0]->getBeginLoc();
+ Out.Suffix = "\n\n";
+ return Out;
+}
+
+Expected<Tweak::Effect> AddUsing::apply(const Selection &Inputs) {
+ auto &SM = Inputs.AST->getSourceManager();
+ // Remove the qualifier from under cursor.
+ tooling::Replacements R;
+ if (auto Err = R.add(tooling::Replacement(
+ SM, CharSourceRange::getTokenRange(NNSL.getSourceRange()), "",
+ Inputs.AST->getLangOpts()))) {
+ return std::move(Err);
+ }
+
+ auto InsertionPoint = findInsertionPoint(Inputs);
+ if (!InsertionPoint) {
+ return InsertionPoint.takeError();
+ }
+
+ if (!InsertionPoint->IdenticalUsingFound) {
+ // Add the using statement at appropriate location.
+ std::string UsingText;
+ llvm::raw_string_ostream UsingTextStream(UsingText);
+ UsingTextStream << "using ";
+ NNSL.getNestedNameSpecifier()->print(
+ UsingTextStream, Inputs.AST->getASTContext().getPrintingPolicy());
+ UsingTextStream << Name << ";" << InsertionPoint->Suffix;
+
+ 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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
