diff --git a/clang-tools-extra/clangd/refactor/Rename.cpp b/clang-tools-extra/clangd/refactor/Rename.cpp --- a/clang-tools-extra/clangd/refactor/Rename.cpp +++ b/clang-tools-extra/clangd/refactor/Rename.cpp @@ -15,8 +15,14 @@ #include "index/SymbolCollector.h" #include "support/Logger.h" #include "support/Trace.h" +#include "clang/AST/ASTContext.h" +#include "clang/AST/ASTTypeTraits.h" +#include "clang/AST/Decl.h" #include "clang/AST/DeclCXX.h" #include "clang/AST/DeclTemplate.h" +#include "clang/AST/ParentMapContext.h" +#include "clang/AST/Stmt.h" +#include "clang/Basic/LLVM.h" #include "clang/Basic/SourceLocation.h" #include "clang/Tooling/Syntax/Tokens.h" #include "llvm/ADT/None.h" @@ -318,13 +324,101 @@ return Results; } +// Detect name conflict with othter DeclStmts in the same enclosing scope. +const NamedDecl *lookupSiblingWithinEnclosingScope(ASTContext &Ctx, + const NamedDecl &RenamedDecl, + StringRef NewName) { + // Store Parents list outside of GetSingleParent, so that returned pointer is + // not invalidated. + DynTypedNodeList Storage(DynTypedNode::create(RenamedDecl)); + auto GetSingleParent = [&](const DynTypedNode &Node) -> const DynTypedNode * { + Storage = Ctx.getParents(Node); + return (Storage.size() == 1) ? Storage.begin() : nullptr; + }; + + // We need to get to the enclosing scope: NamedDecl's parent is typically + // DeclStmt, so enclosing scope would be the second order parent. + const auto *Parent = GetSingleParent(DynTypedNode::create(RenamedDecl)); + if (!Parent || !Parent->get()) + return nullptr; + Parent = GetSingleParent(*Parent); + + // The following helpers check corresponding AST nodes for variable + // declarations with the name collision. + auto CheckDeclStmt = [&](const DeclStmt *DS, + StringRef Name) -> const NamedDecl * { + if (!DS) + return nullptr; + for (const auto &Child : DS->getDeclGroup()) + if (const auto *ND = dyn_cast(Child)) + if (ND != &RenamedDecl && ND->getName() == Name) + return ND; + return nullptr; + }; + auto CheckCompoundStmt = [&](const Stmt *S, + StringRef Name) -> const NamedDecl * { + if (const auto *CS = dyn_cast_or_null(S)) + for (const auto *Node : CS->children()) + if (const auto *Result = CheckDeclStmt(dyn_cast(Node), Name)) + return Result; + return nullptr; + }; + auto CheckConditionVariable = [&](const auto *Scope, + StringRef Name) -> const NamedDecl * { + if (!Scope) + return nullptr; + return CheckDeclStmt(Scope->getConditionVariableDeclStmt(), Name); + }; + + // CompoundStmt is the most common enclosing scope for function-local symbols + // In the simplest case we just iterate through sibling DeclStmts and check + // for collisions. + if (const auto *EnclosingCS = Parent->get()) { + if (const auto *Result = CheckCompoundStmt(EnclosingCS, NewName)) + return Result; + const auto *ScopeParent = GetSingleParent(*Parent); + // CompoundStmt may be found within if/while/for. In these cases, rename can + // collide with the init-statement variable decalaration, they should be + // checked. + if (const auto *Result = + CheckConditionVariable(ScopeParent->get(), NewName)) + return Result; + if (const auto *Result = + CheckConditionVariable(ScopeParent->get(), NewName)) + return Result; + if (const auto *For = ScopeParent->get()) + if (const auto *Result = CheckDeclStmt( + dyn_cast_or_null(For->getInit()), NewName)) + return Result; + // Also check if there is a name collision with function arguments. + if (const auto *Function = ScopeParent->get()) + for (const auto *Parameter : Function->parameters()) + if (Parameter->getName() == NewName) + return Parameter; + return nullptr; + } + + // When renaming a variable within init-statement within if/while/for + // condition, also check the CompoundStmt in the body. + if (const auto *EnclosingIf = Parent->get()) { + if (const auto *Result = CheckCompoundStmt(EnclosingIf->getElse(), NewName)) + return Result; + return CheckCompoundStmt(EnclosingIf->getThen(), NewName); + } + if (const auto *EnclosingWhile = Parent->get()) + return CheckCompoundStmt(EnclosingWhile->getBody(), NewName); + if (const auto *EnclosingFor = Parent->get()) + return CheckCompoundStmt(EnclosingFor->getBody(), NewName); + + return nullptr; +} + // Lookup the declarations (if any) with the given Name in the context of // RenameDecl. -const NamedDecl *lookupSiblingWithName(const ASTContext &Ctx, - const NamedDecl &RenamedDecl, - llvm::StringRef Name) { - trace::Span Tracer("LookupSiblingWithName"); - const auto &II = Ctx.Idents.get(Name); +const NamedDecl *lookupSiblingsWithinContext(ASTContext &Ctx, + const NamedDecl &RenamedDecl, + llvm::StringRef NewName) { + const auto &II = Ctx.Idents.get(NewName); DeclarationName LookupName(&II); DeclContextLookupResult LookupResult; const auto *DC = RenamedDecl.getDeclContext(); @@ -356,6 +450,16 @@ return nullptr; } +const NamedDecl *lookupSiblingWithName(ASTContext &Ctx, + const NamedDecl &RenamedDecl, + llvm::StringRef NewName) { + trace::Span Tracer("LookupSiblingWithName"); + if (const auto *Result = + lookupSiblingsWithinContext(Ctx, RenamedDecl, NewName)) + return Result; + return lookupSiblingWithinEnclosingScope(Ctx, RenamedDecl, NewName); +} + struct InvalidName { enum Kind { Keywords, diff --git a/clang-tools-extra/clangd/unittests/RenameTests.cpp b/clang-tools-extra/clangd/unittests/RenameTests.cpp --- a/clang-tools-extra/clangd/unittests/RenameTests.cpp +++ b/clang-tools-extra/clangd/unittests/RenameTests.cpp @@ -1010,13 +1010,69 @@ )cpp", "conflict", !HeaderFile, nullptr, "Conflict"}, - {R"cpp(// FIXME: detecting local variables is not supported yet. + {R"cpp( void func() { - int Conflict; - int [[V^ar]]; + bool Whatever; + int V^ar; + char Conflict; + } + )cpp", + "conflict", !HeaderFile, nullptr, "Conflict"}, + + {R"cpp( + void func() { + if (int Conflict = 42) { + int V^ar; + } + } + )cpp", + "conflict", !HeaderFile, nullptr, "Conflict"}, + + {R"cpp( + void func() { + if (int Conflict = 42) { + } else { + bool V^ar; + } } )cpp", - nullptr, !HeaderFile, nullptr, "Conflict"}, + "conflict", !HeaderFile, nullptr, "Conflict"}, + + {R"cpp( + void func() { + if (int V^ar = 42) { + } else { + bool Conflict; + } + } + )cpp", + "conflict", !HeaderFile, nullptr, "Conflict"}, + + {R"cpp( + void func() { + while (int V^ar = 10) { + bool Conflict = true; + } + } + )cpp", + "conflict", !HeaderFile, nullptr, "Conflict"}, + + {R"cpp( + void func() { + for (int Something = 9000, Anything = 14, Conflict = 42; Anything > 9; + ++Something) { + int V^ar; + } + } + )cpp", + "conflict", !HeaderFile, nullptr, "Conflict"}, + + {R"cpp( + void func(int Conflict) { + bool V^ar; + } + )cpp", + "conflict", !HeaderFile, nullptr, "Conflict"}, {R"cpp(// Trying to rename into the same name, SameName == SameName. void func() {