diff --git a/clang-tools-extra/clangd/refactor/Rename.cpp b/clang-tools-extra/clangd/refactor/Rename.cpp index 3969f3e2e4e21..fb83083384f95 100644 --- a/clang-tools-extra/clangd/refactor/Rename.cpp +++ b/clang-tools-extra/clangd/refactor/Rename.cpp @@ -8,14 +8,16 @@ #include "refactor/Rename.h" #include "AST.h" +#include "FindTarget.h" #include "Logger.h" #include "ParsedAST.h" +#include "Selection.h" #include "SourceCode.h" #include "index/SymbolCollector.h" -#include "clang/Tooling/Refactoring/Rename/RenamingAction.h" -#include "clang/Tooling/Refactoring/Rename/USRFinder.h" +#include "clang/AST/DeclCXX.h" +#include "clang/AST/DeclTemplate.h" +#include "clang/Basic/SourceLocation.h" #include "clang/Tooling/Refactoring/Rename/USRFindingAction.h" -#include "clang/Tooling/Refactoring/Rename/USRLocFinder.h" namespace clang { namespace clangd { @@ -34,6 +36,17 @@ llvm::Optional filePath(const SymbolLocation &Loc, return *Path; } +// Returns true if the given location is expanded from any macro body. +bool isInMacroBody(const SourceManager &SM, SourceLocation Loc) { + while (Loc.isMacroID()) { + if (SM.isMacroBodyExpansion(Loc)) + return true; + Loc = SM.getImmediateMacroCallerLoc(Loc); + } + + return false; +} + // Query the index to find some other files where the Decl is referenced. llvm::Optional getOtherRefFile(const Decl &D, StringRef MainFile, const SymbolIndex &Index) { @@ -56,12 +69,41 @@ llvm::Optional getOtherRefFile(const Decl &D, StringRef MainFile, return OtherFile; } +llvm::DenseSet locateDeclAt(ParsedAST &AST, + SourceLocation TokenStartLoc) { + unsigned Offset = + AST.getSourceManager().getDecomposedSpellingLoc(TokenStartLoc).second; + + SelectionTree Selection(AST.getASTContext(), AST.getTokens(), Offset); + const SelectionTree::Node *SelectedNode = Selection.commonAncestor(); + if (!SelectedNode) + return {}; + + // If the location points to a Decl, we check it is actually on the name + // range of the Decl. This would avoid allowing rename on unrelated tokens. + // ^class Foo {} // SelectionTree returns CXXRecordDecl, + // // we don't attempt to trigger rename on this position. + // FIXME: make this work on destructors, e.g. "~F^oo()". + if (const auto *D = SelectedNode->ASTNode.get()) { + if (D->getLocation() != TokenStartLoc) + return {}; + } + + llvm::DenseSet Result; + for (const auto *D : + targetDecl(SelectedNode->ASTNode, + DeclRelation::Alias | DeclRelation::TemplatePattern)) + Result.insert(D); + return Result; +} + enum ReasonToReject { NoSymbolFound, NoIndexProvided, NonIndexable, UsedOutsideFile, UnsupportedSymbol, + AmbiguousSymbol, }; // Check the symbol Decl is renameable (per the index) within the file. @@ -125,6 +167,8 @@ llvm::Error makeError(ReasonToReject Reason) { return "symbol may be used in other files (not eligible for indexing)"; case UnsupportedSymbol: return "symbol is not a supported kind (e.g. namespace, macro)"; + case AmbiguousSymbol: + return "there are multiple symbols at the given location"; } llvm_unreachable("unhandled reason kind"); }; @@ -134,22 +178,38 @@ llvm::Error makeError(ReasonToReject Reason) { } // Return all rename occurrences in the main file. -tooling::SymbolOccurrences -findOccurrencesWithinFile(ParsedAST &AST, const NamedDecl *RenameDecl) { - const NamedDecl *CanonicalRenameDecl = - tooling::getCanonicalSymbolDeclaration(RenameDecl); - assert(CanonicalRenameDecl && "RenameDecl must be not null"); - std::vector RenameUSRs = - tooling::getUSRsForDeclaration(CanonicalRenameDecl, AST.getASTContext()); - std::string OldName = CanonicalRenameDecl->getNameAsString(); - tooling::SymbolOccurrences Result; +std::vector findOccurrencesWithinFile(ParsedAST &AST, + const NamedDecl &ND) { + // In theory, locateDeclAt should return the primary template. However, if the + // cursor is under the underlying CXXRecordDecl of the ClassTemplateDecl, ND + // will be the CXXRecordDecl, for this case, we need to get the primary + // template maunally. + const auto &RenameDecl = + ND.getDescribedTemplate() ? *ND.getDescribedTemplate() : ND; + // 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. + std::vector RenameUSRs = tooling::getUSRsForDeclaration( + tooling::getCanonicalSymbolDeclaration(&RenameDecl), AST.getASTContext()); + llvm::DenseSet TargetIDs; + for (auto &USR : RenameUSRs) + TargetIDs.insert(SymbolID(USR)); + + std::vector Results; for (Decl *TopLevelDecl : AST.getLocalTopLevelDecls()) { - tooling::SymbolOccurrences RenameInDecl = - tooling::getOccurrencesOfUSRs(RenameUSRs, OldName, TopLevelDecl); - Result.insert(Result.end(), std::make_move_iterator(RenameInDecl.begin()), - std::make_move_iterator(RenameInDecl.end())); + findExplicitReferences(TopLevelDecl, [&](ReferenceLoc Ref) { + if (Ref.Targets.empty()) + return; + for (const auto *Target : Ref.Targets) { + auto ID = getSymbolID(Target); + if (!ID || TargetIDs.find(*ID) == TargetIDs.end()) + return; + } + Results.push_back(Ref.NameLoc); + }); } - return Result; + + return Results; } } // namespace @@ -165,30 +225,41 @@ renameWithinFile(ParsedAST &AST, llvm::StringRef File, Position Pos, if (locateMacroAt(SourceLocationBeg, AST.getPreprocessor())) return makeError(UnsupportedSymbol); - const auto *RenameDecl = - tooling::getNamedDeclAt(AST.getASTContext(), SourceLocationBeg); - if (!RenameDecl) + auto DeclsUnderCursor = locateDeclAt(AST, SourceLocationBeg); + if (DeclsUnderCursor.empty()) return makeError(NoSymbolFound); + if (DeclsUnderCursor.size() > 1) + return makeError(AmbiguousSymbol); + + const auto *RenameDecl = llvm::dyn_cast(*DeclsUnderCursor.begin()); + if (!RenameDecl) + return makeError(UnsupportedSymbol); if (auto Reject = renamableWithinFile(*RenameDecl->getCanonicalDecl(), File, Index)) return makeError(*Reject); - // Rename sometimes returns duplicate edits (which is a bug). A side-effect of - // adding them to a single Replacements object is these are deduplicated. tooling::Replacements FilteredChanges; - for (const tooling::SymbolOccurrence &Rename : - findOccurrencesWithinFile(AST, RenameDecl)) { - // Currently, we only support normal rename (one range) for C/C++. - // FIXME: support multiple-range rename for objective-c methods. - if (Rename.getNameRanges().size() > 1) + for (SourceLocation Loc : findOccurrencesWithinFile(AST, *RenameDecl)) { + SourceLocation RenameLoc = Loc; + // We don't rename in any macro bodies, but we allow rename the symbol + // spelled in a top-level macro argument in the main file. + if (RenameLoc.isMacroID()) { + if (isInMacroBody(SM, RenameLoc)) + continue; + RenameLoc = SM.getSpellingLoc(Loc); + } + // Filter out locations not from main file. + // We traverse only main file decls, but locations could come from an + // non-preamble #include file e.g. + // void test() { + // int f^oo; + // #include "use_foo.inc" + // } + if (!isInsideMainFile(RenameLoc, SM)) continue; - // We shouldn't have conflicting replacements. If there are conflicts, it - // means that we have bugs either in clangd or in Rename library, therefore - // we refuse to perform the rename. if (auto Err = FilteredChanges.add(tooling::Replacement( - AST.getASTContext().getSourceManager(), - CharSourceRange::getCharRange(Rename.getNameRanges()[0]), NewName))) + SM, CharSourceRange::getTokenRange(RenameLoc), NewName))) return std::move(Err); } return FilteredChanges; diff --git a/clang-tools-extra/clangd/unittests/RenameTests.cpp b/clang-tools-extra/clangd/unittests/RenameTests.cpp index 212d9c089f92e..3c3c60500f9dd 100644 --- a/clang-tools-extra/clangd/unittests/RenameTests.cpp +++ b/clang-tools-extra/clangd/unittests/RenameTests.cpp @@ -38,27 +38,27 @@ std::string expectedResult(Annotations Test, llvm::StringRef NewName) { return Result; } -TEST(RenameTest, SingleFile) { - // "^" points to the position of the rename, and "[[]]" ranges point to the +TEST(RenameTest, WithinFileRename) { + // rename is runnning on all "^" points, and "[[]]" ranges point to the // identifier that is being renamed. llvm::StringRef Tests[] = { - // Rename function. + // Function. R"cpp( - void [[foo]]() { + void [[foo^]]() { [[fo^o]](); } )cpp", - // Rename type. + // Type. R"cpp( - struct [[foo]]{}; + struct [[foo^]] {}; [[foo]] test() { [[f^oo]] x; return x; } )cpp", - // Rename variable. + // Local variable. R"cpp( void bar() { if (auto [[^foo]] = 5) { @@ -66,18 +66,313 @@ TEST(RenameTest, SingleFile) { } } )cpp", + + // Rename class, including constructor/destructor. + R"cpp( + class [[F^oo]] { + [[F^oo]](); + ~[[Foo]](); + void foo(int x); + }; + [[Foo]]::[[Fo^o]]() {} + void [[Foo]]::foo(int x) {} + )cpp", + + // Class in template argument. + R"cpp( + class [[F^oo]] {}; + template void func(); + template class Baz {}; + int main() { + func<[[F^oo]]>(); + Baz<[[F^oo]]> obj; + return 0; + } + )cpp", + + // Forward class declaration without definition. + R"cpp( + class [[F^oo]]; + [[Foo]] *f(); + )cpp", + + // Class methods overrides. + R"cpp( + struct A { + virtual void [[f^oo]]() {} + }; + struct B : A { + void [[f^oo]]() override {} + }; + struct C : B { + void [[f^oo]]() override {} + }; + + void func() { + A().[[f^oo]](); + B().[[f^oo]](); + C().[[f^oo]](); + } + )cpp", + + // Template class (partial) specializations. + R"cpp( + template + class [[F^oo]] {}; + + template<> + class [[F^oo]] {}; + template + class [[F^oo]] {}; + + void test() { + [[Foo]] x; + [[Foo]] y; + [[Foo]] z; + } + )cpp", + + // Template class instantiations. + R"cpp( + template + class [[F^oo]] { + public: + T foo(T arg, T& ref, T* ptr) { + T value; + int number = 42; + value = (T)number; + value = static_cast(number); + return value; + } + static void foo(T value) {} + T member; + }; + + template + void func() { + [[F^oo]] obj; + obj.member = T(); + [[Foo]]::foo(); + } + + void test() { + [[F^oo]] i; + i.member = 0; + [[F^oo]]::foo(0); + + [[F^oo]] b; + b.member = false; + [[Foo]]::foo(false); + } + )cpp", + + // Template class methods. + R"cpp( + template + class A { + public: + void [[f^oo]]() {} + }; + + void func() { + A().[[f^oo]](); + A().[[f^oo]](); + A().[[f^oo]](); + } + )cpp", + + // Complicated class type. + R"cpp( + // Forward declaration. + class [[Fo^o]]; + class Baz { + virtual int getValue() const = 0; + }; + + class [[F^oo]] : public Baz { + public: + [[Foo]](int value = 0) : x(value) {} + + [[Foo]] &operator++(int); + + bool operator<([[Foo]] const &rhs); + int getValue() const; + private: + int x; + }; + + void func() { + [[Foo]] *Pointer = 0; + [[Foo]] Variable = [[Foo]](10); + for ([[Foo]] it; it < Variable; it++); + const [[Foo]] *C = new [[Foo]](); + const_cast<[[Foo]] *>(C)->getValue(); + [[Foo]] foo; + const Baz &BazReference = foo; + const Baz *BazPointer = &foo; + dynamic_cast(BazReference).getValue(); + dynamic_cast(BazPointer)->getValue(); + reinterpret_cast(BazPointer)->getValue(); + static_cast(BazReference).getValue(); + static_cast(BazPointer)->getValue(); + } + )cpp", + + // CXXConstructor initializer list. + R"cpp( + class Baz {}; + class Qux { + Baz [[F^oo]]; + public: + Qux(); + }; + Qux::Qux() : [[F^oo]]() {} + )cpp", + + // DeclRefExpr. + R"cpp( + class C { + public: + static int [[F^oo]]; + }; + + int foo(int x); + #define MACRO(a) foo(a) + + void func() { + C::[[F^oo]] = 1; + MACRO(C::[[Foo]]); + int y = C::[[F^oo]]; + } + )cpp", + + // Macros. + R"cpp( + // no rename inside macro body. + #define M1 foo + #define M2(x) x + int [[fo^o]](); + void boo(int); + + void qoo() { + [[foo]](); + boo([[foo]]()); + M1(); + boo(M1()); + M2([[foo]]()); + M2(M1()); // foo is inside the nested macro body. + } + )cpp", + + // MemberExpr in macros + R"cpp( + class Baz { + public: + int [[F^oo]]; + }; + int qux(int x); + #define MACRO(a) qux(a) + + int main() { + Baz baz; + baz.[[Foo]] = 1; + MACRO(baz.[[Foo]]); + int y = baz.[[Foo]]; + } + )cpp", + + // Template parameters. + R"cpp( + template + class Foo { + [[T]] foo([[T]] arg, [[T]]& ref, [[^T]]* ptr) { + [[T]] value; + int number = 42; + value = ([[T]])number; + value = static_cast<[[^T]]>(number); + return value; + } + static void foo([[T]] value) {} + [[T]] member; + }; + )cpp", + + // Typedef. + R"cpp( + namespace std { + class basic_string {}; + typedef basic_string [[s^tring]]; + } // namespace std + + std::[[s^tring]] foo(); + )cpp", + + // Variable. + R"cpp( + namespace A { + int [[F^oo]]; + } + int Foo; + int Qux = Foo; + int Baz = A::[[^Foo]]; + void fun() { + struct { + int Foo; + } b = {100}; + int Foo = 100; + Baz = Foo; + { + extern int Foo; + Baz = Foo; + Foo = A::[[F^oo]] + Baz; + A::[[Fo^o]] = b.Foo; + } + Foo = b.Foo; + } + )cpp", + + // Namespace alias. + R"cpp( + namespace a { namespace b { void foo(); } } + namespace [[^x]] = a::b; + void bar() { + [[x]]::foo(); + } + )cpp", + + // Scope enums. + R"cpp( + enum class [[K^ind]] { ABC }; + void ff() { + [[K^ind]] s; + s = [[Kind]]::ABC; + } + )cpp", + + // template class in template argument list. + R"cpp( + template + class [[Fo^o]] {}; + template class Z> struct Bar { }; + template <> struct Bar<[[Foo]]> {}; + )cpp", }; for (const auto T : Tests) { Annotations Code(T); auto TU = TestTU::withCode(Code.code()); auto AST = TU.build(); + EXPECT_TRUE(AST.getDiagnostics().empty()) + << AST.getDiagnostics().front() << Code.code(); + llvm::StringRef NewName = "abcde"; - auto RenameResult = - renameWithinFile(AST, testPath(TU.Filename), Code.point(), NewName); - ASSERT_TRUE(bool(RenameResult)) << RenameResult.takeError(); - auto ApplyResult = llvm::cantFail( - tooling::applyAllReplacements(Code.code(), *RenameResult)); - EXPECT_EQ(expectedResult(Code, NewName), ApplyResult); + for (const auto &RenamePos : Code.points()) { + auto RenameResult = + renameWithinFile(AST, testPath(TU.Filename), RenamePos, NewName); + ASSERT_TRUE(bool(RenameResult)) << RenameResult.takeError() << T; + auto ApplyResult = llvm::cantFail( + tooling::applyAllReplacements(Code.code(), *RenameResult)); + EXPECT_EQ(expectedResult(Code, NewName), ApplyResult); + } } } @@ -148,12 +443,32 @@ TEST(RenameTest, Renameable) { {R"cpp(// foo is declared outside the file. void fo^o() {} - )cpp", "used outside main file", !HeaderFile /*cc file*/, Index}, + )cpp", + "used outside main file", !HeaderFile /*cc file*/, Index}, {R"cpp( // We should detect the symbol is used outside the file from the AST. void fo^o() {})cpp", "used outside main file", !HeaderFile, nullptr /*no index*/}, + + {R"cpp( + void foo(int); + void foo(char); + template void f(T t) { + fo^o(t); + })cpp", + "multiple symbols", !HeaderFile, nullptr /*no index*/}, + + {R"cpp(// disallow rename on unrelated token. + cl^ass Foo {}; + )cpp", + "no symbol", !HeaderFile, nullptr}, + + {R"cpp(// disallow rename on unrelated token. + temp^late + class Foo {}; + )cpp", + "no symbol", !HeaderFile, nullptr}, }; for (const auto& Case : Cases) { @@ -191,6 +506,36 @@ TEST(RenameTest, Renameable) { } } +TEST(RenameTest, MainFileReferencesOnly) { + // filter out references not from main file. + llvm::StringRef Test = + R"cpp( + void test() { + int [[fo^o]] = 1; + // rename references not from main file are not included. + #include "foo.inc" + })cpp"; + + Annotations Code(Test); + auto TU = TestTU::withCode(Code.code()); + TU.AdditionalFiles["foo.inc"] = R"cpp( + #define Macro(X) X + &Macro(foo); + &foo; + )cpp"; + auto AST = TU.build(); + EXPECT_TRUE(AST.getDiagnostics().empty()) + << AST.getDiagnostics().front() << Code.code(); + llvm::StringRef NewName = "abcde"; + + auto RenameResult = + renameWithinFile(AST, testPath(TU.Filename), Code.point(), NewName); + ASSERT_TRUE(bool(RenameResult)) << RenameResult.takeError() << Code.point(); + auto ApplyResult = + llvm::cantFail(tooling::applyAllReplacements(Code.code(), *RenameResult)); + EXPECT_EQ(expectedResult(Code, NewName), ApplyResult); +} + } // namespace } // namespace clangd } // namespace clang