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/8] [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/8] [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/8] [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 >From 5ae3e840539652580e31473ea3d4c394609b80ea Mon Sep 17 00:00:00 2001 From: Dimitri Ratz <[email protected]> Date: Wed, 24 Jun 2026 08:49:30 +0200 Subject: [PATCH 4/8] [clangd]: in tweak ReplaceQualifiedWithAlias, consider using declarations. --- .../refactor/tweaks/ReplaceQualifiedWithAlias.cpp | 10 ++++++++-- .../tweaks/ReplaceQualifiedWithAliasTests.cpp | 5 +++++ 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/clang-tools-extra/clangd/refactor/tweaks/ReplaceQualifiedWithAlias.cpp b/clang-tools-extra/clangd/refactor/tweaks/ReplaceQualifiedWithAlias.cpp index 46b2fd8598dc8..804d27dd1afb4 100644 --- a/clang-tools-extra/clangd/refactor/tweaks/ReplaceQualifiedWithAlias.cpp +++ b/clang-tools-extra/clangd/refactor/tweaks/ReplaceQualifiedWithAlias.cpp @@ -102,9 +102,15 @@ bool ReplaceQualifiedWithAlias::prepare(const Selection &Inputs) { return false; for (auto *N = Node; N; N = N->Parent) { - const auto *Alias = N->ASTNode.get<TypeAliasDecl>(); - if (!Alias) + const NamedDecl *Alias = nullptr; + if (const auto *TAD = N->ASTNode.get<TypeAliasDecl>(); TAD) { + Alias = TAD; + } else if (const auto *UD = N->ASTNode.get<UsingDecl>(); UD) { + Alias = UD; + } else { continue; + } + if (!Alias->getIdentifier()) return false; diff --git a/clang-tools-extra/clangd/unittests/tweaks/ReplaceQualifiedWithAliasTests.cpp b/clang-tools-extra/clangd/unittests/tweaks/ReplaceQualifiedWithAliasTests.cpp index c35037216267d..0b16e58e97168 100644 --- a/clang-tools-extra/clangd/unittests/tweaks/ReplaceQualifiedWithAliasTests.cpp +++ b/clang-tools-extra/clangd/unittests/tweaks/ReplaceQualifiedWithAliasTests.cpp @@ -24,6 +24,11 @@ namespace ns { struct Foo {}; } EXPECT_AVAILABLE(std::string(Header) + R"cpp( using Y = ns::F^oo; ns::Foo A; +)cpp"); + + EXPECT_AVAILABLE(std::string(Header) + R"cpp( +using ns::F^oo; +ns::Foo A; )cpp"); EXPECT_UNAVAILABLE(std::string(Header) + R"cpp( >From d5ed26a9bdaf035d94d1a81c61d59d46d6870820 Mon Sep 17 00:00:00 2001 From: Dimitri Ratz <[email protected]> Date: Thu, 25 Jun 2026 09:16:59 +0200 Subject: [PATCH 5/8] [clangd]: Enhance ReplaceQualifiedWithAlias to handle using declarations Improves the `ReplaceQualifiedWithAlias` tweak by adding support for analyzing using declarations when replacing qualified references with aliases. Updates include extended logic for detection, validation, and processing of using declarations, along with new unit tests to ensure correctness. --- .../tweaks/ReplaceQualifiedWithAlias.cpp | 142 +++++++++++++++--- .../tweaks/ReplaceQualifiedWithAliasTests.cpp | 67 +++++++++ 2 files changed, 192 insertions(+), 17 deletions(-) diff --git a/clang-tools-extra/clangd/refactor/tweaks/ReplaceQualifiedWithAlias.cpp b/clang-tools-extra/clangd/refactor/tweaks/ReplaceQualifiedWithAlias.cpp index 804d27dd1afb4..83c2981819d73 100644 --- a/clang-tools-extra/clangd/refactor/tweaks/ReplaceQualifiedWithAlias.cpp +++ b/clang-tools-extra/clangd/refactor/tweaks/ReplaceQualifiedWithAlias.cpp @@ -16,6 +16,7 @@ #include "clang/Lex/Lexer.h" #include "clang/Tooling/Core/Replacement.h" #include "llvm/ADT/DenseSet.h" +#include "llvm/ADT/SmallVector.h" #include "llvm/ADT/StringRef.h" #include "llvm/Support/FormatVariadic.h" #include <string> @@ -51,6 +52,16 @@ bool isNamespaceQualifier(NestedNameSpecifierLoc Qualifier) { NestedNameSpecifier::Kind::Namespace; } +void collectUsingDecls(const DeclContext *DC, + llvm::SmallVectorImpl<const UsingDecl *> &Out) { + for (const Decl *D : DC->decls()) { + if (const auto *UD = llvm::dyn_cast<UsingDecl>(D)) + Out.push_back(UD); + if (const auto *Nested = llvm::dyn_cast<DeclContext>(D)) + collectUsingDecls(Nested, Out); + } +} + SourceLocation getReferenceEnd(SourceLocation NameLoc, const SourceManager &SM, const LangOptions &LangOpts) { std::optional<Token> Tok = Lexer::findNextToken(NameLoc, SM, LangOpts); @@ -101,19 +112,12 @@ bool ReplaceQualifiedWithAlias::prepare(const Selection &Inputs) { if (!Node) return false; - for (auto *N = Node; N; N = N->Parent) { - const NamedDecl *Alias = nullptr; - if (const auto *TAD = N->ASTNode.get<TypeAliasDecl>(); TAD) { - Alias = TAD; - } else if (const auto *UD = N->ASTNode.get<UsingDecl>(); UD) { - Alias = UD; - } else { - continue; - } - + auto SetFromAlias = [&](const NamedDecl *Alias) -> bool { if (!Alias->getIdentifier()) return false; + llvm::DenseSet<const NamedDecl *> Targets; + ReferenceLoc QualifiedRef; bool FoundQualifiedRef = false; findExplicitReferences( @@ -128,19 +132,123 @@ bool ReplaceQualifiedWithAlias::prepare(const Selection &Inputs) { }, Inputs.AST->getHeuristicResolver()); - if (!FoundQualifiedRef) - return false; - - for (const auto *Target : QualifiedRef.Targets) { - if (const auto *Canonical = canonicalDecl(Target)) - RewriteTargets.insert(Canonical); + if (FoundQualifiedRef) { + for (const auto *Target : QualifiedRef.Targets) { + if (const auto *Canonical = canonicalDecl(Target)) + Targets.insert(Canonical); + } + } else if (const auto *UD = llvm::dyn_cast<UsingDecl>(Alias)) { + for (const auto *Shadow : UD->shadows()) { + if (const auto *Canonical = canonicalDecl(Shadow->getTargetDecl())) + Targets.insert(Canonical); + } } - if (RewriteTargets.empty()) + + if (Targets.empty()) return false; + RewriteTargets = std::move(Targets); AliasName = Alias->getNameAsString(); AliasDeclToSkip = Alias->getSourceRange(); return true; + }; + + for (auto *N = Node; N; N = N->Parent) { + if (const auto *TAD = N->ASTNode.get<TypeAliasDecl>(); TAD) { + if (SetFromAlias(TAD)) + return true; + } else if (const auto *UD = N->ASTNode.get<UsingDecl>(); UD) { + if (SetFromAlias(UD)) + return true; + } + } + + llvm::DenseSet<const NamedDecl *> SelectedTargets; + std::string SelectedName; + bool FoundSelectedQualifiedRef = false; + for (const auto &D : Inputs.AST->getLocalTopLevelDecls()) { + findExplicitReferences( + D, + [&](ReferenceLoc Ref) { + if (FoundSelectedQualifiedRef || !Ref.Qualifier || Ref.Targets.empty()) + return; + if (!isNamespaceQualifier(Ref.Qualifier)) + return; + + auto &SM = Inputs.AST->getSourceManager(); + const auto &LangOpts = Inputs.AST->getLangOpts(); + + 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; + + unsigned BeginOffset = SM.getFileOffset(QualifierLoc); + SourceLocation EndLoc = getReferenceEnd(NameLoc, SM, LangOpts); + if (BeginOffset > SM.getFileOffset(EndLoc)) + return; + + if (Inputs.SelectionBegin < BeginOffset || + Inputs.SelectionBegin > SM.getFileOffset(EndLoc)) + return; + + SelectedName = + Lexer::getSourceText(CharSourceRange::getTokenRange(NameLoc), + SM, LangOpts) + .str(); + for (const auto *Target : Ref.Targets) { + if (const auto *Canonical = canonicalDecl(Target)) + SelectedTargets.insert(Canonical); + } + FoundSelectedQualifiedRef = true; + }, + Inputs.AST->getHeuristicResolver()); + if (FoundSelectedQualifiedRef) + break; + } + + if (!FoundSelectedQualifiedRef) + return false; + + llvm::SmallVector<const UsingDecl *> VisibleUsingDecls; + collectUsingDecls(Inputs.AST->getASTContext().getTranslationUnitDecl(), + VisibleUsingDecls); + auto &SM = Inputs.AST->getSourceManager(); + for (const auto *UD : VisibleUsingDecls) { + if (!UD->getIdentifier()) + continue; + if (!SelectedName.empty() && UD->getName() != SelectedName) + continue; + SourceLocation UDLoc = UD->getLocation(); + if (UDLoc.isInvalid() || + SM.getFileID(SM.getExpansionLoc(UDLoc)) != SM.getMainFileID()) + continue; + + bool MatchesSelection = false; + for (const auto *Shadow : UD->shadows()) { + const auto *Canonical = canonicalDecl(Shadow->getTargetDecl()); + if (Canonical && + (SelectedTargets.empty() || SelectedTargets.contains(Canonical))) { + MatchesSelection = true; + break; + } + } + if (!MatchesSelection) + continue; + + if (SetFromAlias(UD)) + return true; } return false; diff --git a/clang-tools-extra/clangd/unittests/tweaks/ReplaceQualifiedWithAliasTests.cpp b/clang-tools-extra/clangd/unittests/tweaks/ReplaceQualifiedWithAliasTests.cpp index 0b16e58e97168..28f9fdbc2e938 100644 --- a/clang-tools-extra/clangd/unittests/tweaks/ReplaceQualifiedWithAliasTests.cpp +++ b/clang-tools-extra/clangd/unittests/tweaks/ReplaceQualifiedWithAliasTests.cpp @@ -158,6 +158,73 @@ Y A; )cpp"); } +TEST_F(ReplaceQualifiedWithAliasTest, MustNotReplaceExistingUsingDecl) { + EXPECT_EQ(apply(R"cpp( +namespace b { + int test1; +} + +namespace d { + int test1; +} + +namespace a { + using b::test1; + + void foo() { + b::test1 = 1; + b::test1 = 1; + } +} // namespace a + +namespace c { + using d::test1; +} // namespace c + +using namespace a; +using namespace b; + +void foo() { + b::te^st1 = 1; + a::test1 = 1; + c::test1 = 1; +} + +)cpp"), + R"cpp( +namespace b { + int test1; +} + +namespace d { + int test1; +} + +namespace a { + using b::test1; + + void foo() { + test1 = 1; + test1 = 1; + } +} // namespace a + +namespace c { + using d::test1; +} // namespace c + +using namespace a; +using namespace b; + +void foo() { + test1 = 1; + a::test1 = 1; + c::test1 = 1; +} + +)cpp"); +} + } // namespace } // namespace clangd } // namespace clang >From 990750705424239f581dfe12fed7cdcb9da64b4d Mon Sep 17 00:00:00 2001 From: Dimitri Ratz <[email protected]> Date: Thu, 25 Jun 2026 09:37:24 +0200 Subject: [PATCH 6/8] [clangd] Enhance AddUsingReplaceAll to handle insertion and replacement errors Improves error handling in the `AddUsingReplaceAll` tweak by logging detailed error messages for insertion and replacement failures and propagating errors accordingly. --- .../refactor/tweaks/AddUsingReplaceAll.cpp | 22 +++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-) diff --git a/clang-tools-extra/clangd/refactor/tweaks/AddUsingReplaceAll.cpp b/clang-tools-extra/clangd/refactor/tweaks/AddUsingReplaceAll.cpp index b0141bb4da6b1..ac8c06337ee6f 100644 --- a/clang-tools-extra/clangd/refactor/tweaks/AddUsingReplaceAll.cpp +++ b/clang-tools-extra/clangd/refactor/tweaks/AddUsingReplaceAll.cpp @@ -367,8 +367,11 @@ Expected<Tweak::Effect> AddUsingReplaceAll::apply(const Selection &Inputs) { auto InsertionPoint = findInsertionPoint(Inputs, QualifierToRemove, SpelledName, MustInsertAfterLoc); - if (!InsertionPoint) - return InsertionPoint.takeError(); + if (!InsertionPoint) { + std::string Msg = llvm::toString(InsertionPoint.takeError()); + elog("AddUsingReplaceAll: {0}", Msg); + return error(Msg); + } if (InsertionPoint->Loc.isValid()) { std::string UsingText; @@ -386,10 +389,14 @@ Expected<Tweak::Effect> AddUsingReplaceAll::apply(const Selection &Inputs) { return std::move(Err); } + bool HasReplacementError = false; + std::string ReplacementError; for (const auto &D : Inputs.AST->getLocalTopLevelDecls()) { findExplicitReferences( D, [&](ReferenceLoc Ref) { + if (HasReplacementError) + return; if (!Ref.Qualifier || Ref.Targets.empty() || Ref.IsDecl) return; if (!getNamespaceFromQualifier( @@ -428,13 +435,20 @@ Expected<Tweak::Effect> AddUsingReplaceAll::apply(const Selection &Inputs) { if (BeginOffset >= EndOffset) return; - if (Repls.add(tooling::Replacement(SM, QualifierLoc, - EndOffset - BeginOffset, ""))) + if (auto Err = Repls.add(tooling::Replacement( + SM, QualifierLoc, EndOffset - BeginOffset, ""))) { + ReplacementError = llvm::toString(std::move(Err)); + elog("AddUsingReplaceAll: {0}", ReplacementError); + HasReplacementError = true; return; + } }, Inputs.AST->getHeuristicResolver()); } + if (HasReplacementError) + return error(ReplacementError); + return Effect::mainFileEdit(SM, std::move(Repls)); } >From 26b2214a88a691df69d294b4df9501972b4ba588 Mon Sep 17 00:00:00 2001 From: Dimitri Ratz <[email protected]> Date: Thu, 25 Jun 2026 09:51:27 +0200 Subject: [PATCH 7/8] [clangd] Add comments to clarify logic in ReplaceQualifiedWithAlias and AddUsingReplaceAll tweaks Adds detailed comments throughout the implementation of `ReplaceQualifiedWithAlias` and `AddUsingReplaceAll` tweaks to explain the purpose, logic, and key decisions made in the code. Improves maintainability and readability of the tweaks. --- .../refactor/tweaks/AddUsingReplaceAll.cpp | 26 +++++++++++++++++++ .../tweaks/ReplaceQualifiedWithAlias.cpp | 22 ++++++++++++++++ 2 files changed, 48 insertions(+) diff --git a/clang-tools-extra/clangd/refactor/tweaks/AddUsingReplaceAll.cpp b/clang-tools-extra/clangd/refactor/tweaks/AddUsingReplaceAll.cpp index ac8c06337ee6f..bf4de7e03b3da 100644 --- a/clang-tools-extra/clangd/refactor/tweaks/AddUsingReplaceAll.cpp +++ b/clang-tools-extra/clangd/refactor/tweaks/AddUsingReplaceAll.cpp @@ -53,7 +53,10 @@ class AddUsingReplaceAll : public Tweak { std::string QualifierToSpell; llvm::StringRef SpelledQualifier; llvm::StringRef SpelledName; + // The selected declaration must stay after the inserted using-declaration. SourceLocation MustInsertAfterLoc; + // Canonical declarations whose explicit qualified references will be + // rewritten in the main file. llvm::DenseSet<const NamedDecl *> RewriteTargets; }; REGISTER_TWEAK(AddUsingReplaceAll) @@ -214,6 +217,8 @@ bool AddUsingReplaceAll::prepare(const Selection &Inputs) { auto &SM = Inputs.AST->getSourceManager(); const auto &TB = Inputs.AST->getTokens(); + // Clear any state from a previous invocation before analyzing the new + // selection. QualifierToRemove = NestedNameSpecifierLoc(); QualifierToSpell.clear(); SpelledQualifier = llvm::StringRef(); @@ -225,6 +230,8 @@ bool AddUsingReplaceAll::prepare(const Selection &Inputs) { Inputs.AST->getLangOpts())) return false; + // Normalize the selection to the outermost AST node that still represents + // the qualified spelling we want to rewrite. auto *Node = Inputs.ASTSelection.commonAncestor(); if (!Node) return false; @@ -247,6 +254,8 @@ bool AddUsingReplaceAll::prepare(const Selection &Inputs) { return false; SourceRange SpelledNameRange; + // Capture the qualified spelling, the namespace qualifier to remove, and + // the declaration that constrains where a new using-declaration may go. if (auto *D = Node->ASTNode.get<DeclRefExpr>()) { if (D->getDecl()->getIdentifier()) { QualifierToRemove = D->getQualifierLoc(); @@ -322,16 +331,22 @@ bool AddUsingReplaceAll::prepare(const Selection &Inputs) { default: break; } + // For type spellings, the qualifier is part of the type name range rather + // than a separate expression node. if (QualifierToRemove) SpelledNameRange.setBegin(QualifierToRemove.getBeginLoc()); } + // Only handle fully namespace-qualified spellings that can be rewritten + // consistently across the file. if (!QualifierToRemove || RewriteTargets.empty() || QualifierToRemove.getNestedNameSpecifier().getKind() != NestedNameSpecifier::Kind::Namespace || isNamespaceForbidden(Inputs, QualifierToRemove.getNestedNameSpecifier())) return false; + // Macro body expansions and cross-file spellings are too ambiguous for this + // refactoring. if (SM.isMacroBodyExpansion(QualifierToRemove.getBeginLoc()) || !SM.isWrittenInSameFile(QualifierToRemove.getBeginLoc(), QualifierToRemove.getEndLoc())) @@ -345,6 +360,8 @@ bool AddUsingReplaceAll::prepare(const Selection &Inputs) { syntax::Token::range(SM, SpelledTokens->front(), SpelledTokens->back()); std::tie(SpelledQualifier, SpelledName) = splitQualifiedName(SpelledRange.text(SM)); + // Keep the exact source spelling for the qualifier so the inserted using + // matches the style of the original code. QualifierToSpell = getNNSLAsString( QualifierToRemove, Inputs.AST->getASTContext().getPrintingPolicy()); if (!llvm::StringRef(QualifierToSpell).ends_with(SpelledQualifier) || @@ -356,6 +373,7 @@ bool AddUsingReplaceAll::prepare(const Selection &Inputs) { Expected<Tweak::Effect> AddUsingReplaceAll::apply(const Selection &Inputs) { auto &SM = Inputs.AST->getSourceManager(); + // Fail loudly if apply() is reached without the state produced by prepare(). const auto *TargetNamespace = getNamespaceFromQualifier(QualifierToRemove.getNestedNameSpecifier()); if (!TargetNamespace) @@ -365,6 +383,8 @@ Expected<Tweak::Effect> AddUsingReplaceAll::apply(const Selection &Inputs) { tooling::Replacements Repls; + // Find a legal insertion point based on nearby using-declarations, namespace + // scope, or the start of the file. auto InsertionPoint = findInsertionPoint(Inputs, QualifierToRemove, SpelledName, MustInsertAfterLoc); if (!InsertionPoint) { @@ -374,6 +394,8 @@ Expected<Tweak::Effect> AddUsingReplaceAll::apply(const Selection &Inputs) { } if (InsertionPoint->Loc.isValid()) { + // Emit the new using-declaration with any extra qualification required by + // existing local style. std::string UsingText; llvm::raw_string_ostream UsingTextStream(UsingText); UsingTextStream << "using "; @@ -391,6 +413,8 @@ Expected<Tweak::Effect> AddUsingReplaceAll::apply(const Selection &Inputs) { bool HasReplacementError = false; std::string ReplacementError; + // Walk every top-level declaration in the file and rewrite only the + // explicit references that resolve to one of the prepared targets. for (const auto &D : Inputs.AST->getLocalTopLevelDecls()) { findExplicitReferences( D, @@ -435,6 +459,8 @@ Expected<Tweak::Effect> AddUsingReplaceAll::apply(const Selection &Inputs) { if (BeginOffset >= EndOffset) return; + // Drop the namespace qualifier, leaving the referenced name in + // place. if (auto Err = Repls.add(tooling::Replacement( SM, QualifierLoc, EndOffset - BeginOffset, ""))) { ReplacementError = llvm::toString(std::move(Err)); diff --git a/clang-tools-extra/clangd/refactor/tweaks/ReplaceQualifiedWithAlias.cpp b/clang-tools-extra/clangd/refactor/tweaks/ReplaceQualifiedWithAlias.cpp index 83c2981819d73..acc9cab430037 100644 --- a/clang-tools-extra/clangd/refactor/tweaks/ReplaceQualifiedWithAlias.cpp +++ b/clang-tools-extra/clangd/refactor/tweaks/ReplaceQualifiedWithAlias.cpp @@ -62,6 +62,8 @@ void collectUsingDecls(const DeclContext *DC, } } +// Returns the end of a qualified reference, extending past any template +// argument list so rewrites replace the full spelled name. SourceLocation getReferenceEnd(SourceLocation NameLoc, const SourceManager &SM, const LangOptions &LangOpts) { std::optional<Token> Tok = Lexer::findNextToken(NameLoc, SM, LangOpts); @@ -104,6 +106,7 @@ std::string ReplaceQualifiedWithAlias::title() const { } bool ReplaceQualifiedWithAlias::prepare(const Selection &Inputs) { + // Reset all cached state before inspecting the new selection. AliasName.clear(); AliasDeclToSkip = SourceRange(); RewriteTargets.clear(); @@ -120,6 +123,8 @@ bool ReplaceQualifiedWithAlias::prepare(const Selection &Inputs) { ReferenceLoc QualifiedRef; bool FoundQualifiedRef = false; + // Prefer the alias's own qualified reference when it exists, because it + // gives us the exact namespace spelling that should be rewritten. findExplicitReferences( Alias, [&](ReferenceLoc Ref) { @@ -147,12 +152,15 @@ bool ReplaceQualifiedWithAlias::prepare(const Selection &Inputs) { if (Targets.empty()) return false; + // Cache the alias spelling and the canonical declarations it stands for. RewriteTargets = std::move(Targets); AliasName = Alias->getNameAsString(); AliasDeclToSkip = Alias->getSourceRange(); return true; }; + // First look for an alias declaration in the current selection or one of + // its ancestors. This is the narrowest and most reliable source of truth. for (auto *N = Node; N; N = N->Parent) { if (const auto *TAD = N->ASTNode.get<TypeAliasDecl>(); TAD) { if (SetFromAlias(TAD)) @@ -166,6 +174,9 @@ bool ReplaceQualifiedWithAlias::prepare(const Selection &Inputs) { llvm::DenseSet<const NamedDecl *> SelectedTargets; std::string SelectedName; bool FoundSelectedQualifiedRef = false; + // If the selection is a qualified reference instead of an alias, derive the + // target set from that reference and then search for a matching alias in the + // file. for (const auto &D : Inputs.AST->getLocalTopLevelDecls()) { findExplicitReferences( D, @@ -203,6 +214,8 @@ bool ReplaceQualifiedWithAlias::prepare(const Selection &Inputs) { Inputs.SelectionBegin > SM.getFileOffset(EndLoc)) return; + // Record the referred-to name so we can prefer aliases with the same + // identifier when several are visible. SelectedName = Lexer::getSourceText(CharSourceRange::getTokenRange(NameLoc), SM, LangOpts) @@ -225,6 +238,8 @@ bool ReplaceQualifiedWithAlias::prepare(const Selection &Inputs) { collectUsingDecls(Inputs.AST->getASTContext().getTranslationUnitDecl(), VisibleUsingDecls); auto &SM = Inputs.AST->getSourceManager(); + // Search visible using-declarations for one that shadows the same target + // set, then reuse its alias spelling. for (const auto *UD : VisibleUsingDecls) { if (!UD->getIdentifier()) continue; @@ -259,9 +274,12 @@ ReplaceQualifiedWithAlias::apply(const Selection &Inputs) { auto &SM = Inputs.AST->getSourceManager(); const auto &LangOpts = Inputs.AST->getLangOpts(); + // prepare() must have populated the alias name and target set. if (AliasName.empty() || RewriteTargets.empty()) return error("Incomplete prepared state for ReplaceQualifiedWithAlias"); + // Skip the selected alias declaration itself so we do not rewrite the alias + // name inside its own definition. SourceLocation SkipBegin = AliasDeclToSkip.getBegin(); SourceLocation SkipEnd = AliasDeclToSkip.getEnd(); if (SkipBegin.isValid()) @@ -270,6 +288,8 @@ ReplaceQualifiedWithAlias::apply(const Selection &Inputs) { SkipEnd = SM.getExpansionLoc(SkipEnd); tooling::Replacements Repls; + // Rewrite every explicit namespace-qualified reference that resolves to one + // of the canonical declarations represented by the alias. for (const auto &D : Inputs.AST->getLocalTopLevelDecls()) { findExplicitReferences( D, @@ -306,6 +326,7 @@ ReplaceQualifiedWithAlias::apply(const Selection &Inputs) { SM.getFileID(NameLoc) != SM.getMainFileID()) return; + // Leave the selected alias definition untouched. if (SkipBegin.isValid() && SkipEnd.isValid() && SM.isPointWithin(NameLoc, SkipBegin, SkipEnd)) return; @@ -315,6 +336,7 @@ ReplaceQualifiedWithAlias::apply(const Selection &Inputs) { if (BeginOffset > SM.getFileOffset(EndLoc)) return; + // Replace the qualified spelling with the alias name only. if (auto Err = Repls.add(tooling::Replacement( SM, QualifierLoc, SM.getFileOffset(EndLoc) - BeginOffset, AliasName))) >From dbdbb2bff97a2d9ddfe526e802633213f16c067a Mon Sep 17 00:00:00 2001 From: Dimitri Ratz <[email protected]> Date: Thu, 25 Jun 2026 10:08:02 +0200 Subject: [PATCH 8/8] [clangd] Fix format. --- .../clangd/refactor/tweaks/ReplaceQualifiedWithAlias.cpp | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/clang-tools-extra/clangd/refactor/tweaks/ReplaceQualifiedWithAlias.cpp b/clang-tools-extra/clangd/refactor/tweaks/ReplaceQualifiedWithAlias.cpp index acc9cab430037..cf9d7c0495da8 100644 --- a/clang-tools-extra/clangd/refactor/tweaks/ReplaceQualifiedWithAlias.cpp +++ b/clang-tools-extra/clangd/refactor/tweaks/ReplaceQualifiedWithAlias.cpp @@ -181,7 +181,8 @@ bool ReplaceQualifiedWithAlias::prepare(const Selection &Inputs) { findExplicitReferences( D, [&](ReferenceLoc Ref) { - if (FoundSelectedQualifiedRef || !Ref.Qualifier || Ref.Targets.empty()) + if (FoundSelectedQualifiedRef || !Ref.Qualifier || + Ref.Targets.empty()) return; if (!isNamespaceQualifier(Ref.Qualifier)) return; @@ -217,8 +218,8 @@ bool ReplaceQualifiedWithAlias::prepare(const Selection &Inputs) { // Record the referred-to name so we can prefer aliases with the same // identifier when several are visible. SelectedName = - Lexer::getSourceText(CharSourceRange::getTokenRange(NameLoc), - SM, LangOpts) + Lexer::getSourceText(CharSourceRange::getTokenRange(NameLoc), SM, + LangOpts) .str(); for (const auto *Target : Ref.Targets) { if (const auto *Canonical = canonicalDecl(Target)) _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
