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 @@ -209,6 +209,57 @@ llvm::inconvertibleErrorCode()); } +const Decl *getRenameRootDecl(const ClassTemplateSpecializationDecl *D) { + return D->getSpecializedTemplate()->getTemplatedDecl(); +} +const Decl *getRenameRootDecl(const TemplateDecl *D) { + return D->getTemplatedDecl(); +} +const Decl *getRenameRootDecl(const CXXMethodDecl *D) { + auto *Result = D; + if (const auto *InstantiatedMethod = D->getInstantiatedFromMemberFunction()) + Result = llvm::cast(InstantiatedMethod); + while (Result->isVirtual() && Result->size_overridden_methods()) + Result = *Result->overridden_methods().begin(); + return Result; +} +const Decl *getRenameRootDecl(const FunctionDecl *D) { + const auto *Definition = D->getDefinition(); + auto *Candidate = Definition ? Definition : D->getMostRecentDecl(); + return Candidate->isTemplateInstantiation() + ? Candidate->getPrimaryTemplate()->getTemplatedDecl() + : Candidate; +} + +// TODO(kirillbobyrev): Replace clang-rename's USRFindingAction with this +// routine to get a better test coverage and reduce technical debt. +// TODO(kirillbobyrev): Write documentation. +const Decl *getRenameRootDecl(const Decl *D) { + auto *Candidate = D; + // TODO(kirillbobyrev): Should this only happen once? + if (const auto *RD = llvm::dyn_cast(D)) { + const auto *Definition = RD->getDefinition(); + Candidate = Definition ? Definition : RD->getMostRecentDecl(); + } + if (const auto *Template = llvm::dyn_cast(Candidate)) { + return getRenameRootDecl(Template); + } else if (const auto *ClassTemplateSpecialization = + llvm::dyn_cast(Candidate)) { + return getRenameRootDecl(ClassTemplateSpecialization); + } else if (const auto *Constructor = + llvm::dyn_cast(Candidate)) { + return getRenameRootDecl(Constructor->getParent()); + } else if (const auto *Destructor = + llvm::dyn_cast(Candidate)) { + return getRenameRootDecl(Destructor->getParent()); + } else if (const auto *Method = llvm::dyn_cast(Candidate)) { + return getRenameRootDecl(Method); + } else if (const auto *Function = llvm::dyn_cast(Candidate)) { + return getRenameRootDecl(Function); + } + return Candidate; +} + // Return all rename occurrences in the main file. std::vector findOccurrencesWithinFile(ParsedAST &AST, const NamedDecl &ND) { @@ -217,14 +268,7 @@ // get the primary template maunally. // getUSRsForDeclaration will find other related symbols, e.g. virtual and its // overriddens, primary template and all explicit specializations. - // FIXME: Get rid of the remaining tooling APIs. - const auto RenameDecl = - ND.getDescribedTemplate() ? ND.getDescribedTemplate() : &ND; - std::vector RenameUSRs = - tooling::getUSRsForDeclaration(RenameDecl, AST.getASTContext()); - llvm::DenseSet TargetIDs; - for (auto &USR : RenameUSRs) - TargetIDs.insert(SymbolID(USR)); + const auto *RenameDeclRoot = getRenameRootDecl(&ND); std::vector Results; for (Decl *TopLevelDecl : AST.getLocalTopLevelDecls()) { @@ -232,8 +276,7 @@ if (Ref.Targets.empty()) return; for (const auto *Target : Ref.Targets) { - auto ID = getSymbolID(Target); - if (!ID || TargetIDs.find(*ID) == TargetIDs.end()) + if (getRenameRootDecl(Target) != RenameDeclRoot) return; } Results.push_back(Ref.NameLoc); 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 @@ -88,10 +88,87 @@ } TEST(RenameTest, WithinFileRename) { - // rename is runnning on all "^" points, and "[[]]" ranges point to the - // identifier that is being renamed. + // For each "^" this test moves cursor to its location and applies renaming + // while checking that all identifiers enclosed in [[]] ranges are handled + // correctly. + // TODO(kirillbobyrev): Add more tests. llvm::StringRef Tests[] = { - // Function. + // Templated static method instantiation. + R"cpp( + template + class Foo { + public: + static T [[f^oo]]() {} + } + + void bar() { + Foo::[[f^oo]](); + } + )cpp", + + // Templated method instantiation. + R"cpp( + template + class Foo { + public: + T [[f^oo]]() {} + } + + void bar() { + Foo().[[f^oo]](); + } + )cpp", + + // Class template (partial) specialization forward declarations. + R"cpp( + template + class [[Foo^]]; + + template + class [[Foo^]] {}; + + template + class [[Foo^]]; + )cpp", + + // Class template (full) specialization forward declaration. + R"cpp( + template + class [[Foo^]]; + + template + class [[Foo^]] {}; + )cpp", + + // Function template specialization forward declaration. + R"cpp( + template + U [[foo^]](); + + template + U [[foo^]]() {}; + )cpp", + + // Function template specialization forward declaration. + R"cpp( + template + U [[foo^]]() {}; + + template + U [[foo^]](); + )cpp", + + // Function template specialization forward declaration without function + // definition. + R"cpp( + template + U [[foo^]](); + + template + U [[foo^]](); + )cpp", + + // Simple recursive function. R"cpp( void [[foo^]]() { [[fo^o]](); @@ -116,15 +193,16 @@ } )cpp", - // Rename class, including constructor/destructor. + // Class, its constructor and destructor. R"cpp( class [[F^oo]] { + public: [[F^oo]](); - ~[[Foo]](); + ~[[Fo^o]](); void foo(int x); }; - [[Foo]]::[[Fo^o]]() {} - void [[Foo]]::foo(int x) {} + [[Fo^o]]::[[Fo^o]]() {} + void [[Fo^o]]::foo(int x) {} )cpp", // Class in template argument. @@ -175,9 +253,9 @@ class [[F^oo]] {}; void test() { - [[Foo]] x; - [[Foo]] y; - [[Foo]] z; + [[F^oo]] x; + [[Fo^o]] y; + [[Foo^]] z; } )cpp", @@ -303,7 +381,7 @@ void qoo() { [[foo]](); - boo([[foo]]()); + boo([[fo^o]]()); M1(); boo(M1()); M2([[foo]]()); @@ -396,7 +474,7 @@ } )cpp", - // template class in template argument list. + // Template class in template argument list. R"cpp( template class [[Fo^o]] {};