https://github.com/ratzdi updated https://github.com/llvm/llvm-project/pull/204151
>From 6724b163ac711f6c0ad641b0668b9f6395613929 Mon Sep 17 00:00:00 2001 From: Dimitri Ratz <[email protected]> Date: Tue, 16 Jun 2026 13:48:16 +0200 Subject: [PATCH 1/3] [clangd] Add AddUsingReplaceAll tweak to replace all qualified references Adds a new refactoring code action that is triggered on a fully qualified name and removes its namespace qualifier from every occurrence in the file, inserting a using-declaration at an appropriate location. This complements the existing AddUsing tweak, which only rewrites the single reference under the cursor. Implementation notes: - collectEnclosingUsings() replaces a RecursiveASTVisitor-based traversal by walking the DeclContext chain directly, reducing traversal cost from O(AST nodes) to O(scope depth * decls per scope). - Qualifier extraction is guarded via getNamespaceFromQualifier() to reject non-namespace qualifiers (e.g. record-type qualifiers) defensively. - apply() validates the prepared state before building replacements and only rewrites references whose canonical target matches the selection. - References inside macro arguments are rewritten; macro-body expansions are left untouched. Tests cover: basic multi-reference rewrites, macro arguments, global (::) qualifiers with nested-namespace syntax, multiple nested macro layers, and unavailability for non-namespace qualifiers such as record-type qualifiers. --- .../refactor/tweaks/AddUsingReplaceAll.cpp | 445 ++++++++++++++++++ .../clangd/refactor/tweaks/CMakeLists.txt | 1 + .../clangd/unittests/CMakeLists.txt | 1 + .../tweaks/AddUsingReplaceAllTests.cpp | 174 +++++++ 4 files changed, 621 insertions(+) create mode 100644 clang-tools-extra/clangd/refactor/tweaks/AddUsingReplaceAll.cpp create mode 100644 clang-tools-extra/clangd/unittests/tweaks/AddUsingReplaceAllTests.cpp diff --git a/clang-tools-extra/clangd/refactor/tweaks/AddUsingReplaceAll.cpp b/clang-tools-extra/clangd/refactor/tweaks/AddUsingReplaceAll.cpp new file mode 100644 index 0000000000000..d2dc4f793cf12 --- /dev/null +++ b/clang-tools-extra/clangd/refactor/tweaks/AddUsingReplaceAll.cpp @@ -0,0 +1,445 @@ +//===--- AddUsingReplaceAll.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 "Config.h" +#include "FindTarget.h" +#include "SourceCode.h" +#include "refactor/Tweak.h" +#include "support/Logger.h" + +#include "clang/AST/Decl.h" +#include "clang/AST/DeclBase.h" +#include "clang/AST/Expr.h" +#include "clang/AST/NestedNameSpecifier.h" +#include "clang/AST/Type.h" +#include "clang/AST/TypeLoc.h" +#include "clang/Basic/SourceLocation.h" +#include "clang/Tooling/Core/Replacement.h" +#include "clang/Tooling/Syntax/Tokens.h" +#include "llvm/ADT/DenseSet.h" +#include "llvm/ADT/StringRef.h" +#include "llvm/Support/FormatVariadic.h" +#include "llvm/Support/raw_ostream.h" +#include <string> +#include <tuple> +#include <utility> + +namespace clang { +namespace clangd { +namespace { + +/// Tweak for removing full namespace qualifier under cursor on DeclRefExpr and +/// types and adding "using" statement instead. This tweak replaces all +/// occurrences of the qualified symbol in the file, not just the one under the +/// cursor, but it also requires more preparation and is more expensive to +/// compute, so it is hidden behind a separate action from AddUsing, which only +/// removes the qualifier under the cursor. +class AddUsingReplaceAll : 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; + llvm::StringLiteral kind() const override { + return CodeAction::REFACTOR_KIND; + } + +private: + NestedNameSpecifierLoc QualifierToRemove; + std::string QualifierToSpell; + llvm::StringRef SpelledQualifier; + llvm::StringRef SpelledName; + SourceLocation MustInsertAfterLoc; + llvm::DenseSet<const NamedDecl *> RewriteTargets; +}; +REGISTER_TWEAK(AddUsingReplaceAll) + +std::string AddUsingReplaceAll::title() const { + return std::string(llvm::formatv( + "Add using-declaration for {0} and replace qualified references", + SpelledName)); +} + +/// Collects all UsingDecls visible at the selection by walking the enclosing +/// DeclContext chain directly, sorted by source location. This avoids a full +/// AST traversal and only visits O(depth * declarations-per-scope) nodes. +/// +/// A using-declaration declared in a scope S is visible at SelectionCtx iff +/// S is an ancestor of SelectionCtx, i.e. exactly the contexts we walk when +/// ascending via getLexicalParent(). +std::vector<const UsingDecl *> +collectEnclosingUsings(const DeclContext *SelectionCtx, + const SourceManager &SM) { + std::vector<const UsingDecl *> Result; + for (const DeclContext *Ctx = SelectionCtx; Ctx; + Ctx = Ctx->getLexicalParent()) { + for (const Decl *D : Ctx->decls()) { + const auto *UD = dyn_cast<UsingDecl>(D); + if (!UD) + continue; + if (SM.getFileID(UD->getUsingLoc()) != SM.getMainFileID()) + continue; + Result.push_back(UD); + } + } + // Merge into a single sorted list so findInsertionPoint can break early. + llvm::sort(Result, [&](const UsingDecl *A, const UsingDecl *B) { + return SM.isBeforeInTranslationUnit(A->getUsingLoc(), B->getUsingLoc()); + }); + return Result; +} + +struct InsertionPointData { + SourceLocation Loc; + std::string Suffix; + bool AlwaysFullyQualify = false; +}; + +const NamespaceDecl *getNamespaceFromQualifier(NestedNameSpecifier Qualifier) { + if (!Qualifier || Qualifier.getKind() != NestedNameSpecifier::Kind::Namespace) + return nullptr; + return dyn_cast_or_null<NamespaceDecl>( + Qualifier.getAsNamespaceAndPrefix().Namespace); +} + +llvm::Expected<InsertionPointData> +findInsertionPoint(const Tweak::Selection &Inputs, + const NestedNameSpecifierLoc &QualifierToRemove, + const llvm::StringRef Name, + const SourceLocation MustInsertAfterLoc) { + auto &SM = Inputs.AST->getSourceManager(); + + const auto *TargetNamespace = + getNamespaceFromQualifier(QualifierToRemove.getNestedNameSpecifier()); + if (!TargetNamespace) + return error("Qualifier is not a valid namespace qualifier"); + + SourceLocation LastUsingLoc; + const std::vector<const UsingDecl *> Usings = collectEnclosingUsings( + &Inputs.ASTSelection.commonAncestor()->getDeclContext(), SM); + + auto IsValidPoint = [&](const SourceLocation Loc) { + return MustInsertAfterLoc.isInvalid() || + SM.isBeforeInTranslationUnit(MustInsertAfterLoc, Loc); + }; + + bool AlwaysFullyQualify = true; + for (const auto *U : Usings) { + if (!U->getQualifier().isFullyQualified()) + AlwaysFullyQualify = false; + + if (const auto *Namespace = getNamespaceFromQualifier(U->getQualifier())) { + if (Namespace->getCanonicalDecl() == + TargetNamespace->getCanonicalDecl() && + U->getName() == Name) + return InsertionPointData(); + } + + LastUsingLoc = U->getUsingLoc(); + } + if (LastUsingLoc.isValid() && IsValidPoint(LastUsingLoc)) { + InsertionPointData Out; + Out.Loc = LastUsingLoc; + Out.AlwaysFullyQualify = AlwaysFullyQualify; + return Out; + } + + const DeclContext *ParentDeclCtx = + &Inputs.ASTSelection.commonAncestor()->getDeclContext(); + while (ParentDeclCtx && !ParentDeclCtx->isFileContext()) + ParentDeclCtx = ParentDeclCtx->getLexicalParent(); + + if (auto *ND = llvm::dyn_cast_or_null<NamespaceDecl>(ParentDeclCtx)) { + 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 error("Namespace with no {{"); + if (!Tok->endLocation().isMacroID() && IsValidPoint(Tok->endLocation())) { + InsertionPointData Out; + Out.Loc = Tok->endLocation(); + Out.Suffix = "\n"; + return Out; + } + } + + auto TLDs = Inputs.AST->getLocalTopLevelDecls(); + for (const auto &TLD : TLDs) { + if (!IsValidPoint(TLD->getBeginLoc())) + continue; + InsertionPointData Out; + Out.Loc = SM.getExpansionLoc(TLD->getBeginLoc()); + Out.Suffix = "\n\n"; + return Out; + } + return error("Cannot find place to insert \"using\""); +} + +bool isNamespaceForbidden(const Tweak::Selection &Inputs, + NestedNameSpecifier Namespace) { + const auto *NS = + dyn_cast<NamespaceDecl>(Namespace.getAsNamespaceAndPrefix().Namespace); + if (!NS) + return true; + std::string NamespaceStr = printNamespaceScope(*NS); + + for (StringRef Banned : Config::current().Style.FullyQualifiedNamespaces) { + StringRef PrefixMatch = NamespaceStr; + if (PrefixMatch.consume_front(Banned) && PrefixMatch.consume_front("::")) + return true; + } + + return false; +} + +std::string getNNSLAsString(NestedNameSpecifierLoc NNSL, + const PrintingPolicy &Policy) { + std::string Out; + llvm::raw_string_ostream OutStream(Out); + NNSL.getNestedNameSpecifier().print(OutStream, Policy); + return OutStream.str(); +} + +const NamedDecl *canonicalDecl(const NamedDecl *D) { + return D ? llvm::dyn_cast<NamedDecl>(D->getCanonicalDecl()) : nullptr; +} + +bool AddUsingReplaceAll::prepare(const Selection &Inputs) { + auto &SM = Inputs.AST->getSourceManager(); + const auto &TB = Inputs.AST->getTokens(); + + QualifierToRemove = NestedNameSpecifierLoc(); + QualifierToSpell.clear(); + SpelledQualifier = llvm::StringRef(); + SpelledName = llvm::StringRef(); + MustInsertAfterLoc = SourceLocation(); + RewriteTargets.clear(); + + if (isHeaderFile(SM.getFileEntryRefForID(SM.getMainFileID())->getName(), + Inputs.AST->getLangOpts())) + return false; + + auto *Node = Inputs.ASTSelection.commonAncestor(); + if (!Node) + return false; + + for (; Node->Parent; Node = Node->Parent) { + if (Node->ASTNode.get<NestedNameSpecifierLoc>()) + continue; + if (auto *T = Node->ASTNode.get<TypeLoc>()) { + if (Node->Parent->ASTNode.get<NestedNameSpecifierLoc>()) + continue; + if (isa<TagType, TemplateSpecializationType, TypedefType, UsingType, + UnresolvedUsingType>(T->getTypePtr())) + break; + if (Node->Parent->ASTNode.get<TypeLoc>()) + continue; + } + break; + } + if (!Node) + return false; + + SourceRange SpelledNameRange; + if (auto *D = Node->ASTNode.get<DeclRefExpr>()) { + if (D->getDecl()->getIdentifier()) { + QualifierToRemove = D->getQualifierLoc(); + SpelledNameRange = D->getSourceRange(); + if (auto AngleLoc = D->getLAngleLoc(); AngleLoc.isValid()) + SpelledNameRange.setEnd(AngleLoc.getLocWithOffset(-1)); + MustInsertAfterLoc = D->getDecl()->getBeginLoc(); + if (const auto *Canonical = canonicalDecl(D->getDecl())) + RewriteTargets.insert(Canonical); + } + } else if (auto *T = Node->ASTNode.get<TypeLoc>()) { + switch (T->getTypeLocClass()) { + case TypeLoc::TemplateSpecialization: { + auto TL = T->castAs<TemplateSpecializationTypeLoc>(); + QualifierToRemove = TL.getQualifierLoc(); + if (!QualifierToRemove) + break; + SpelledNameRange = TL.getTemplateNameLoc(); + if (auto *TD = TL.getTypePtr()->getTemplateName().getAsTemplateDecl( + /*IgnoreDeduced=*/true)) { + MustInsertAfterLoc = TD->getBeginLoc(); + if (const auto *Canonical = canonicalDecl(TD)) + RewriteTargets.insert(Canonical); + } + break; + } + case TypeLoc::Enum: + case TypeLoc::Record: + case TypeLoc::InjectedClassName: { + auto TL = T->castAs<TagTypeLoc>(); + QualifierToRemove = TL.getQualifierLoc(); + if (!QualifierToRemove) + break; + SpelledNameRange = TL.getNameLoc(); + MustInsertAfterLoc = TL.getDecl()->getBeginLoc(); + if (const auto *Canonical = canonicalDecl(TL.getDecl())) + RewriteTargets.insert(Canonical); + break; + } + case TypeLoc::Typedef: { + auto TL = T->castAs<TypedefTypeLoc>(); + QualifierToRemove = TL.getQualifierLoc(); + if (!QualifierToRemove) + break; + SpelledNameRange = TL.getNameLoc(); + MustInsertAfterLoc = TL.getDecl()->getBeginLoc(); + if (const auto *Canonical = canonicalDecl(TL.getDecl())) + RewriteTargets.insert(Canonical); + break; + } + case TypeLoc::UnresolvedUsing: { + auto TL = T->castAs<UnresolvedUsingTypeLoc>(); + QualifierToRemove = TL.getQualifierLoc(); + if (!QualifierToRemove) + break; + SpelledNameRange = TL.getNameLoc(); + MustInsertAfterLoc = TL.getDecl()->getBeginLoc(); + if (const auto *Canonical = canonicalDecl(TL.getDecl())) + RewriteTargets.insert(Canonical); + break; + } + case TypeLoc::Using: { + auto TL = T->castAs<UsingTypeLoc>(); + QualifierToRemove = TL.getQualifierLoc(); + if (!QualifierToRemove) + break; + SpelledNameRange = TL.getNameLoc(); + MustInsertAfterLoc = TL.getDecl()->getBeginLoc(); + if (const auto *Canonical = canonicalDecl(TL.getDecl())) + RewriteTargets.insert(Canonical); + break; + } + default: + break; + } + if (QualifierToRemove) + SpelledNameRange.setBegin(QualifierToRemove.getBeginLoc()); + } + + if (!QualifierToRemove || RewriteTargets.empty() || + QualifierToRemove.getNestedNameSpecifier().getKind() != + NestedNameSpecifier::Kind::Namespace || + isNamespaceForbidden(Inputs, QualifierToRemove.getNestedNameSpecifier())) + return false; + + if (SM.isMacroBodyExpansion(QualifierToRemove.getBeginLoc()) || + !SM.isWrittenInSameFile(QualifierToRemove.getBeginLoc(), + QualifierToRemove.getEndLoc())) + return false; + + auto SpelledTokens = + TB.spelledForExpanded(TB.expandedTokens(SpelledNameRange)); + if (!SpelledTokens) + return false; + auto SpelledRange = + syntax::Token::range(SM, SpelledTokens->front(), SpelledTokens->back()); + std::tie(SpelledQualifier, SpelledName) = + splitQualifiedName(SpelledRange.text(SM)); + QualifierToSpell = getNNSLAsString( + QualifierToRemove, Inputs.AST->getASTContext().getPrintingPolicy()); + if (!llvm::StringRef(QualifierToSpell).ends_with(SpelledQualifier) || + SpelledName.empty()) + return false; + return true; +} + +Expected<Tweak::Effect> AddUsingReplaceAll::apply(const Selection &Inputs) { + auto &SM = Inputs.AST->getSourceManager(); + + const auto *TargetNamespace = + getNamespaceFromQualifier(QualifierToRemove.getNestedNameSpecifier()); + if (!TargetNamespace) + return error("Invalid namespace qualifier in prepared state"); + if (SpelledName.empty() || QualifierToSpell.empty() || RewriteTargets.empty()) + return error("Incomplete prepared state for AddUsingReplaceAll"); + + tooling::Replacements Repls; + + auto InsertionPoint = findInsertionPoint(Inputs, QualifierToRemove, + SpelledName, MustInsertAfterLoc); + if (!InsertionPoint) + return InsertionPoint.takeError(); + + if (InsertionPoint->Loc.isValid()) { + std::string UsingText; + llvm::raw_string_ostream UsingTextStream(UsingText); + UsingTextStream << "using "; + if (InsertionPoint->AlwaysFullyQualify && + !QualifierToRemove.getNestedNameSpecifier().isFullyQualified()) + UsingTextStream << "::"; + UsingTextStream << QualifierToSpell << SpelledName << ";" + << InsertionPoint->Suffix; + + assert(SM.getFileID(InsertionPoint->Loc) == SM.getMainFileID()); + if (auto Err = Repls.add(tooling::Replacement(SM, InsertionPoint->Loc, 0, + UsingTextStream.str()))) + return std::move(Err); + } + + for (const auto &D : Inputs.AST->getLocalTopLevelDecls()) { + findExplicitReferences( + D, + [&](ReferenceLoc Ref) { + if (!Ref.Qualifier || Ref.Targets.empty() || Ref.IsDecl) + return; + if (!getNamespaceFromQualifier( + Ref.Qualifier.getNestedNameSpecifier())) + return; + + SourceLocation QualifierLoc = Ref.Qualifier.getBeginLoc(); + SourceLocation NameLoc = Ref.NameLoc; + if (QualifierLoc.isMacroID()) { + if (!SM.isMacroArgExpansion(QualifierLoc)) + return; + QualifierLoc = SM.getFileLoc(QualifierLoc); + } + if (NameLoc.isMacroID()) { + if (!SM.isMacroArgExpansion(NameLoc)) + return; + NameLoc = SM.getFileLoc(NameLoc); + } + if (SM.getFileID(QualifierLoc) != SM.getMainFileID() || + SM.getFileID(NameLoc) != SM.getMainFileID()) + return; + + bool MatchesTarget = false; + for (const auto *Target : Ref.Targets) { + if (const auto *Canonical = canonicalDecl(Target); + Canonical && RewriteTargets.contains(Canonical)) { + MatchesTarget = true; + break; + } + } + if (!MatchesTarget) + return; + + unsigned BeginOffset = SM.getFileOffset(QualifierLoc); + unsigned EndOffset = SM.getFileOffset(NameLoc); + if (BeginOffset >= EndOffset) + return; + + if (Repls.add(tooling::Replacement(SM, QualifierLoc, + EndOffset - BeginOffset, ""))) + return; + }, + Inputs.AST->getHeuristicResolver()); + } + + return Effect::mainFileEdit(SM, std::move(Repls)); +} + +} // 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 1d6e38088ad67..8ed40760fef97 100644 --- a/clang-tools-extra/clangd/refactor/tweaks/CMakeLists.txt +++ b/clang-tools-extra/clangd/refactor/tweaks/CMakeLists.txt @@ -13,6 +13,7 @@ set(LLVM_LINK_COMPONENTS # clangd/tool/CMakeLists.txt for an example. add_clang_library(clangDaemonTweaks OBJECT AddUsing.cpp + AddUsingReplaceAll.cpp AnnotateHighlightings.cpp DefineInline.cpp DefineOutline.cpp diff --git a/clang-tools-extra/clangd/unittests/CMakeLists.txt b/clang-tools-extra/clangd/unittests/CMakeLists.txt index d596ba77efd4a..6b27865fbfd46 100644 --- a/clang-tools-extra/clangd/unittests/CMakeLists.txt +++ b/clang-tools-extra/clangd/unittests/CMakeLists.txt @@ -120,6 +120,7 @@ add_unittest(ClangdUnitTests ClangdTests support/TraceTests.cpp tweaks/AddUsingTests.cpp + tweaks/AddUsingReplaceAllTests.cpp tweaks/AnnotateHighlightingsTests.cpp tweaks/DefineInlineTests.cpp tweaks/DefineOutlineTests.cpp diff --git a/clang-tools-extra/clangd/unittests/tweaks/AddUsingReplaceAllTests.cpp b/clang-tools-extra/clangd/unittests/tweaks/AddUsingReplaceAllTests.cpp new file mode 100644 index 0000000000000..741f94c910244 --- /dev/null +++ b/clang-tools-extra/clangd/unittests/tweaks/AddUsingReplaceAllTests.cpp @@ -0,0 +1,174 @@ +//===-- AddUsingReplaceAllTests.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 "TweakTesting.h" +#include "gtest/gtest.h" + +namespace clang { +namespace clangd { +namespace { + +TWEAK_TEST(AddUsingReplaceAll); + +TEST_F(AddUsingReplaceAllTest, Prepare) { + const char *Header = R"cpp(namespace ns { struct SomeStruct {}; })cpp"; + + EXPECT_AVAILABLE(std::string(Header) + R"cpp( +ns::SomeStruct bar() { + n^s::SomeStruct x; + return x; +})cpp"); + EXPECT_UNAVAILABLE(std::string(Header) + R"cpp( +ns::SomeStruct bar() { + ns::SomeStruct ^x; + return x; +})cpp"); + + EXPECT_UNAVAILABLE(R"cpp( +struct S { + static void f(); +}; +void fun() { + S::f^(); +} +)cpp"); +} + +TEST_F(AddUsingReplaceAllTest, Apply) { + ExtraFiles["test.hpp"] = R"cpp( +namespace one { +namespace two { +void ff(); +} +})cpp"; + + EXPECT_EQ(apply(R"cpp( +#include "test.hpp" + +void fun() { + one::two::f^f(); + one::two::ff(); +} + +void other() { + one::two::ff(); +} +)cpp"), + R"cpp( +#include "test.hpp" + +using one::two::ff; + +void fun() { + ff(); + ff(); +} + +void other() { + ff(); +} +)cpp"); +} + +TEST_F(AddUsingReplaceAllTest, ApplyInsideMacroArgument) { + ExtraFiles["test.hpp"] = R"cpp( +namespace one { +namespace two { +void ff(); +} +})cpp"; + + EXPECT_EQ(apply(R"cpp( +#include "test.hpp" +#define CALL(name) name() + +void fun() { + CALL(one::two::f^f); + one::two::ff(); +} +)cpp"), + R"cpp( +#include "test.hpp" +#define CALL(name) name() + +using one::two::ff; + +void fun() { + CALL(ff); + ff(); +} +)cpp"); +} + +TEST_F(AddUsingReplaceAllTest, ApplyWithGlobalQualifierAndNestedNamespaceDecl) { + ExtraFiles["test.hpp"] = R"cpp( +namespace one::two { +void ff(); +} +)cpp"; + + EXPECT_EQ(apply(R"cpp( +#include "test.hpp" + +void fun() { + ::one::two::f^f(); + one::two::ff(); +} +)cpp"), + R"cpp( +#include "test.hpp" + +using ::one::two::ff; + +void fun() { + ff(); + ff(); +} +)cpp"); +} + +TEST_F(AddUsingReplaceAllTest, ApplyWithMultipleMacros) { + ExtraFiles["test.hpp"] = R"cpp( +namespace one { +namespace two { +void ff(); +} +} +)cpp"; + + EXPECT_EQ(apply(R"cpp( +#include "test.hpp" +#define ID(X) X +#define CALL(name) name() +#define CALL_FF one::two::ff() + +void fun() { + CALL(ID(one::two::f^f)); + CALL_FF; + one::two::ff(); +} +)cpp"), + R"cpp( +#include "test.hpp" +#define ID(X) X +#define CALL(name) name() +#define CALL_FF one::two::ff() + +using one::two::ff; + +void fun() { + CALL(ID(ff)); + CALL_FF; + ff(); +} +)cpp"); +} + +} // namespace +} // namespace clangd +} // namespace clang >From f700f3602450a12909fb0ce08d4de03cb2dbfcbf Mon Sep 17 00:00:00 2001 From: Dimitri Ratz <[email protected]> Date: Thu, 18 Jun 2026 09:25:20 +0200 Subject: [PATCH 2/3] [clangd] Refactor AddUsingReplaceAll tweak to use static functions and improve clarity --- .../refactor/tweaks/AddUsingReplaceAll.cpp | 43 +++++++++---------- 1 file changed, 20 insertions(+), 23 deletions(-) diff --git a/clang-tools-extra/clangd/refactor/tweaks/AddUsingReplaceAll.cpp b/clang-tools-extra/clangd/refactor/tweaks/AddUsingReplaceAll.cpp index d2dc4f793cf12..b0141bb4da6b1 100644 --- a/clang-tools-extra/clangd/refactor/tweaks/AddUsingReplaceAll.cpp +++ b/clang-tools-extra/clangd/refactor/tweaks/AddUsingReplaceAll.cpp @@ -16,8 +16,6 @@ #include "clang/AST/Decl.h" #include "clang/AST/DeclBase.h" #include "clang/AST/Expr.h" -#include "clang/AST/NestedNameSpecifier.h" -#include "clang/AST/Type.h" #include "clang/AST/TypeLoc.h" #include "clang/Basic/SourceLocation.h" #include "clang/Tooling/Core/Replacement.h" @@ -32,14 +30,13 @@ namespace clang { namespace clangd { -namespace { - -/// Tweak for removing full namespace qualifier under cursor on DeclRefExpr and -/// types and adding "using" statement instead. This tweak replaces all -/// occurrences of the qualified symbol in the file, not just the one under the -/// cursor, but it also requires more preparation and is more expensive to -/// compute, so it is hidden behind a separate action from AddUsing, which only -/// removes the qualifier under the cursor. + +/// Tweak for removing the full namespace qualifier under the cursor on +/// DeclRefExpr and types and adding "using" statement instead. This tweak +/// replaces all occurrences of the qualified symbol in the file, not just the +/// one under the cursor, but it also requires more preparation and is more +/// expensive to compute, so it is hidden behind a separate action from +/// AddUsing, which only removes the qualifier under the cursor. class AddUsingReplaceAll : public Tweak { public: const char *id() const override; @@ -62,9 +59,9 @@ class AddUsingReplaceAll : public Tweak { REGISTER_TWEAK(AddUsingReplaceAll) std::string AddUsingReplaceAll::title() const { - return std::string(llvm::formatv( - "Add using-declaration for {0} and replace qualified references", - SpelledName)); + return std::string(llvm::formatv("Add using-declaration for {0} and replace " + "all qualified references in this file", + SpelledName)); } /// Collects all UsingDecls visible at the selection by walking the enclosing @@ -72,9 +69,9 @@ std::string AddUsingReplaceAll::title() const { /// AST traversal and only visits O(depth * declarations-per-scope) nodes. /// /// A using-declaration declared in a scope S is visible at SelectionCtx iff -/// S is an ancestor of SelectionCtx, i.e. exactly the contexts we walk when +/// S is an ancestor of SelectionCtx, i.e., exactly the contexts we walk when /// ascending via getLexicalParent(). -std::vector<const UsingDecl *> +static std::vector<const UsingDecl *> collectEnclosingUsings(const DeclContext *SelectionCtx, const SourceManager &SM) { std::vector<const UsingDecl *> Result; @@ -102,14 +99,15 @@ struct InsertionPointData { bool AlwaysFullyQualify = false; }; -const NamespaceDecl *getNamespaceFromQualifier(NestedNameSpecifier Qualifier) { +static const NamespaceDecl * +getNamespaceFromQualifier(NestedNameSpecifier Qualifier) { if (!Qualifier || Qualifier.getKind() != NestedNameSpecifier::Kind::Namespace) return nullptr; return dyn_cast_or_null<NamespaceDecl>( Qualifier.getAsNamespaceAndPrefix().Namespace); } -llvm::Expected<InsertionPointData> +static llvm::Expected<InsertionPointData> findInsertionPoint(const Tweak::Selection &Inputs, const NestedNameSpecifierLoc &QualifierToRemove, const llvm::StringRef Name, @@ -183,8 +181,8 @@ findInsertionPoint(const Tweak::Selection &Inputs, return error("Cannot find place to insert \"using\""); } -bool isNamespaceForbidden(const Tweak::Selection &Inputs, - NestedNameSpecifier Namespace) { +static bool isNamespaceForbidden(const Tweak::Selection &Inputs, + NestedNameSpecifier Namespace) { const auto *NS = dyn_cast<NamespaceDecl>(Namespace.getAsNamespaceAndPrefix().Namespace); if (!NS) @@ -200,15 +198,15 @@ bool isNamespaceForbidden(const Tweak::Selection &Inputs, return false; } -std::string getNNSLAsString(NestedNameSpecifierLoc NNSL, - const PrintingPolicy &Policy) { +static std::string getNNSLAsString(NestedNameSpecifierLoc NNSL, + const PrintingPolicy &Policy) { std::string Out; llvm::raw_string_ostream OutStream(Out); NNSL.getNestedNameSpecifier().print(OutStream, Policy); return OutStream.str(); } -const NamedDecl *canonicalDecl(const NamedDecl *D) { +static const NamedDecl *canonicalDecl(const NamedDecl *D) { return D ? llvm::dyn_cast<NamedDecl>(D->getCanonicalDecl()) : nullptr; } @@ -440,6 +438,5 @@ Expected<Tweak::Effect> AddUsingReplaceAll::apply(const Selection &Inputs) { return Effect::mainFileEdit(SM, std::move(Repls)); } -} // namespace } // namespace clangd } // namespace clang >From 91c092e3de06288f99f6f0a934050ae38adf34ed Mon Sep 17 00:00:00 2001 From: Dimitri Ratz <[email protected]> Date: Thu, 18 Jun 2026 12:13:17 +0200 Subject: [PATCH 3/3] [clangd] Add ReplaceQualifiedWithAlias tweak to simplify references Introduces a new refactoring action `ReplaceQualifiedWithAlias` that replaces fully qualified references in the current file with an alias, improving code readability. Includes implementation, tests, and CMake integration. --- .../clangd/refactor/tweaks/CMakeLists.txt | 1 + .../tweaks/ReplaceQualifiedWithAlias.cpp | 217 ++++++++++++++++++ .../clangd/unittests/CMakeLists.txt | 1 + .../tweaks/ReplaceQualifiedWithAliasTests.cpp | 158 +++++++++++++ 4 files changed, 377 insertions(+) create mode 100644 clang-tools-extra/clangd/refactor/tweaks/ReplaceQualifiedWithAlias.cpp create mode 100644 clang-tools-extra/clangd/unittests/tweaks/ReplaceQualifiedWithAliasTests.cpp diff --git a/clang-tools-extra/clangd/refactor/tweaks/CMakeLists.txt b/clang-tools-extra/clangd/refactor/tweaks/CMakeLists.txt index 8ed40760fef97..38a1a16652427 100644 --- a/clang-tools-extra/clangd/refactor/tweaks/CMakeLists.txt +++ b/clang-tools-extra/clangd/refactor/tweaks/CMakeLists.txt @@ -28,6 +28,7 @@ add_clang_library(clangDaemonTweaks OBJECT OverridePureVirtuals.cpp PopulateSwitch.cpp RawStringLiteral.cpp + ReplaceQualifiedWithAlias.cpp RemoveUsingNamespace.cpp ScopifyEnum.cpp SpecialMembers.cpp diff --git a/clang-tools-extra/clangd/refactor/tweaks/ReplaceQualifiedWithAlias.cpp b/clang-tools-extra/clangd/refactor/tweaks/ReplaceQualifiedWithAlias.cpp new file mode 100644 index 0000000000000..46b2fd8598dc8 --- /dev/null +++ b/clang-tools-extra/clangd/refactor/tweaks/ReplaceQualifiedWithAlias.cpp @@ -0,0 +1,217 @@ +//===--- ReplaceQualifiedWithAlias.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 "FindTarget.h" +#include "refactor/Tweak.h" +#include "support/Logger.h" + +#include "clang/AST/Decl.h" +#include "clang/AST/NestedNameSpecifier.h" +#include "clang/Basic/SourceLocation.h" +#include "clang/Lex/Lexer.h" +#include "clang/Tooling/Core/Replacement.h" +#include "llvm/ADT/DenseSet.h" +#include "llvm/ADT/StringRef.h" +#include "llvm/Support/FormatVariadic.h" +#include <string> + +namespace clang { +namespace clangd { +namespace { + +class ReplaceQualifiedWithAlias : 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; + llvm::StringLiteral kind() const override { + return CodeAction::REFACTOR_KIND; + } + +private: + std::string AliasName; + SourceRange AliasDeclToSkip; + llvm::DenseSet<const NamedDecl *> RewriteTargets; +}; +REGISTER_TWEAK(ReplaceQualifiedWithAlias) + +const NamedDecl *canonicalDecl(const NamedDecl *D) { + return D ? llvm::dyn_cast<NamedDecl>(D->getCanonicalDecl()) : nullptr; +} + +bool isNamespaceQualifier(NestedNameSpecifierLoc Qualifier) { + return Qualifier && Qualifier.getNestedNameSpecifier().getKind() == + NestedNameSpecifier::Kind::Namespace; +} + +SourceLocation getReferenceEnd(SourceLocation NameLoc, const SourceManager &SM, + const LangOptions &LangOpts) { + std::optional<Token> Tok = Lexer::findNextToken(NameLoc, SM, LangOpts); + if (!Tok || Tok->isNot(tok::less)) + return Lexer::getLocForEndOfToken(NameLoc, 0, SM, LangOpts); + + unsigned Depth = 0; + while (Tok) { + switch (Tok->getKind()) { + case tok::less: + ++Depth; + break; + case tok::greater: + if (Depth == 0) + return Lexer::getLocForEndOfToken(Tok->getLocation(), 0, SM, LangOpts); + --Depth; + if (Depth == 0) + return Lexer::getLocForEndOfToken(Tok->getLocation(), 0, SM, LangOpts); + break; + case tok::greatergreater: + if (Depth <= 1) + return Lexer::getLocForEndOfToken(Tok->getLocation(), 0, SM, LangOpts); + Depth -= 2; + if (Depth == 0) + return Lexer::getLocForEndOfToken(Tok->getLocation(), 0, SM, LangOpts); + break; + default: + if (Depth == 0) + return Lexer::getLocForEndOfToken(Tok->getLocation(), 0, SM, LangOpts); + break; + } + Tok = Lexer::findNextToken(Tok->getLocation(), SM, LangOpts); + } + return Lexer::getLocForEndOfToken(NameLoc, 0, SM, LangOpts); +} + +std::string ReplaceQualifiedWithAlias::title() const { + return std::string( + llvm::formatv("Replace qualified references with {0}", AliasName)); +} + +bool ReplaceQualifiedWithAlias::prepare(const Selection &Inputs) { + AliasName.clear(); + AliasDeclToSkip = SourceRange(); + RewriteTargets.clear(); + + auto *Node = Inputs.ASTSelection.commonAncestor(); + if (!Node) + return false; + + for (auto *N = Node; N; N = N->Parent) { + const auto *Alias = N->ASTNode.get<TypeAliasDecl>(); + if (!Alias) + continue; + if (!Alias->getIdentifier()) + return false; + + ReferenceLoc QualifiedRef; + bool FoundQualifiedRef = false; + findExplicitReferences( + Alias, + [&](ReferenceLoc Ref) { + if (FoundQualifiedRef || !Ref.Qualifier || Ref.Targets.empty()) + return; + if (!isNamespaceQualifier(Ref.Qualifier)) + return; + QualifiedRef = std::move(Ref); + FoundQualifiedRef = true; + }, + Inputs.AST->getHeuristicResolver()); + + if (!FoundQualifiedRef) + return false; + + for (const auto *Target : QualifiedRef.Targets) { + if (const auto *Canonical = canonicalDecl(Target)) + RewriteTargets.insert(Canonical); + } + if (RewriteTargets.empty()) + return false; + + AliasName = Alias->getNameAsString(); + AliasDeclToSkip = Alias->getSourceRange(); + return true; + } + + return false; +} + +Expected<Tweak::Effect> +ReplaceQualifiedWithAlias::apply(const Selection &Inputs) { + auto &SM = Inputs.AST->getSourceManager(); + const auto &LangOpts = Inputs.AST->getLangOpts(); + + if (AliasName.empty() || RewriteTargets.empty()) + return error("Incomplete prepared state for ReplaceQualifiedWithAlias"); + + SourceLocation SkipBegin = AliasDeclToSkip.getBegin(); + SourceLocation SkipEnd = AliasDeclToSkip.getEnd(); + if (SkipBegin.isValid()) + SkipBegin = SM.getExpansionLoc(SkipBegin); + if (SkipEnd.isValid()) + SkipEnd = SM.getExpansionLoc(SkipEnd); + + tooling::Replacements Repls; + for (const auto &D : Inputs.AST->getLocalTopLevelDecls()) { + findExplicitReferences( + D, + [&](ReferenceLoc Ref) { + if (!Ref.Qualifier || Ref.Targets.empty() || Ref.IsDecl) + return; + if (!isNamespaceQualifier(Ref.Qualifier)) + return; + + bool MatchesTarget = false; + for (const auto *Target : Ref.Targets) { + if (const auto *Canonical = canonicalDecl(Target); + Canonical && RewriteTargets.contains(Canonical)) { + MatchesTarget = true; + break; + } + } + if (!MatchesTarget) + return; + + SourceLocation QualifierLoc = Ref.Qualifier.getBeginLoc(); + SourceLocation NameLoc = Ref.NameLoc; + if (QualifierLoc.isMacroID()) { + if (!SM.isMacroArgExpansion(QualifierLoc)) + return; + QualifierLoc = SM.getFileLoc(QualifierLoc); + } + if (NameLoc.isMacroID()) { + if (!SM.isMacroArgExpansion(NameLoc)) + return; + NameLoc = SM.getFileLoc(NameLoc); + } + if (SM.getFileID(QualifierLoc) != SM.getMainFileID() || + SM.getFileID(NameLoc) != SM.getMainFileID()) + return; + + if (SkipBegin.isValid() && SkipEnd.isValid() && + SM.isPointWithin(NameLoc, SkipBegin, SkipEnd)) + return; + + unsigned BeginOffset = SM.getFileOffset(QualifierLoc); + SourceLocation EndLoc = getReferenceEnd(NameLoc, SM, LangOpts); + if (BeginOffset > SM.getFileOffset(EndLoc)) + return; + + if (auto Err = Repls.add(tooling::Replacement( + SM, QualifierLoc, SM.getFileOffset(EndLoc) - BeginOffset, + AliasName))) + consumeError(std::move(Err)); + }, + Inputs.AST->getHeuristicResolver()); + } + + return Effect::mainFileEdit(SM, std::move(Repls)); +} + +} // namespace +} // namespace clangd +} // namespace clang diff --git a/clang-tools-extra/clangd/unittests/CMakeLists.txt b/clang-tools-extra/clangd/unittests/CMakeLists.txt index 6b27865fbfd46..8eae604ad1e89 100644 --- a/clang-tools-extra/clangd/unittests/CMakeLists.txt +++ b/clang-tools-extra/clangd/unittests/CMakeLists.txt @@ -137,6 +137,7 @@ add_unittest(ClangdUnitTests ClangdTests tweaks/OverridePureVirtualsTests.cpp tweaks/PopulateSwitchTests.cpp tweaks/RawStringLiteralTests.cpp + tweaks/ReplaceQualifiedWithAliasTests.cpp tweaks/RemoveUsingNamespaceTests.cpp tweaks/ScopifyEnumTests.cpp tweaks/ShowSelectionTreeTests.cpp diff --git a/clang-tools-extra/clangd/unittests/tweaks/ReplaceQualifiedWithAliasTests.cpp b/clang-tools-extra/clangd/unittests/tweaks/ReplaceQualifiedWithAliasTests.cpp new file mode 100644 index 0000000000000..c35037216267d --- /dev/null +++ b/clang-tools-extra/clangd/unittests/tweaks/ReplaceQualifiedWithAliasTests.cpp @@ -0,0 +1,158 @@ +//===-- ReplaceQualifiedWithAliasTests.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 "TweakTesting.h" +#include "gtest/gtest.h" + +namespace clang { +namespace clangd { +namespace { + +TWEAK_TEST(ReplaceQualifiedWithAlias); + +TEST_F(ReplaceQualifiedWithAliasTest, Prepare) { + const char *Header = R"cpp( +namespace ns { struct Foo {}; } +)cpp"; + + EXPECT_AVAILABLE(std::string(Header) + R"cpp( +using Y = ns::F^oo; +ns::Foo A; +)cpp"); + + EXPECT_UNAVAILABLE(std::string(Header) + R"cpp( +using Y = ns::Foo;^ +ns::Foo A; +)cpp"); +} + +TEST_F(ReplaceQualifiedWithAliasTest, Apply) { + EXPECT_EQ(apply(R"cpp( +namespace ns { +struct Foo {}; +} + +using Y = ns::F^oo; + +ns::Foo A; +void f() { + ns::Foo B; +} +)cpp"), + R"cpp( +namespace ns { +struct Foo {}; +} + +using Y = ns::Foo; + +Y A; +void f() { + Y B; +} +)cpp"); +} + +TEST_F(ReplaceQualifiedWithAliasTest, ApplyInsideMacroArgument) { + EXPECT_EQ(apply(R"cpp( +namespace ns { +struct Foo {}; +} + +#define ID(X) X +using Y = ns::F^oo; + +ID(ns::Foo) A; +)cpp"), + R"cpp( +namespace ns { +struct Foo {}; +} + +#define ID(X) X +using Y = ns::Foo; + +ID(Y) A; +)cpp"); +} + +TEST_F(ReplaceQualifiedWithAliasTest, KeepAliasDeclarationUnchanged) { + EXPECT_EQ(apply(R"cpp( +namespace ns { +template <typename T> struct Foo {}; +} + +using Y = ns::F^oo<int>; + +ns::Foo<int> A; +)cpp"), + R"cpp( +namespace ns { +template <typename T> struct Foo {}; +} + +using Y = ns::Foo<int>; + +Y A; +)cpp"); +} + +TEST_F(ReplaceQualifiedWithAliasTest, ApplyNestedNamespaces) { + EXPECT_EQ(apply(R"cpp( +namespace a { +namespace b { +struct Foo {}; +} +} + +using Y = a::b::F^oo; + +a::b::Foo A; +::a::b::Foo B; +)cpp"), + R"cpp( +namespace a { +namespace b { +struct Foo {}; +} +} + +using Y = a::b::Foo; + +Y A; +Y B; +)cpp"); +} + +TEST_F(ReplaceQualifiedWithAliasTest, ApplyWithCompetingAlias) { + EXPECT_EQ(apply(R"cpp( +namespace ns { +struct Foo {}; +} + +using Y = ns::F^oo; +using Z = ns::Foo; + +ns::Foo A; +)cpp"), + R"cpp( +namespace ns { +struct Foo {}; +} + +using Y = ns::Foo; +using Z = Y; + +Y A; +)cpp"); +} + +} // namespace +} // namespace clangd +} // namespace clang _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
