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 @@ -67,15 +67,14 @@ } // Query the index to find some other files where the Decl is referenced. -llvm::Optional getOtherRefFile(const NamedDecl &Decl, - StringRef MainFile, +llvm::Optional getOtherRefFile(const Decl &D, StringRef MainFile, const SymbolIndex &Index) { RefsRequest Req; // We limit the number of results, this is a correctness/performance // tradeoff. We expect the number of symbol references in the current file // is smaller than the limit. Req.Limit = 100; - if (auto ID = getSymbolID(&Decl)) + if (auto ID = getSymbolID(&D)) Req.IDs.insert(*ID); llvm::Optional OtherFile; Index.refs(Req, [&](const Ref &R) { @@ -97,16 +96,16 @@ }; // Check the symbol Decl is renameable (per the index) within the file. -llvm::Optional renamableWithinFile(const NamedDecl &Decl, +llvm::Optional renamableWithinFile(const Decl &RenameDecl, StringRef MainFile, const SymbolIndex *Index) { - if (llvm::isa(&Decl)) + if (llvm::isa(&RenameDecl)) return ReasonToReject::UnsupportedSymbol; - auto &ASTCtx = Decl.getASTContext(); + auto &ASTCtx = RenameDecl.getASTContext(); const auto &SM = ASTCtx.getSourceManager(); bool MainFileIsHeader = ASTCtx.getLangOpts().IsHeaderFile; bool DeclaredInMainFile = - SM.isWrittenInMainFile(SM.getExpansionLoc(Decl.getLocation())); + SM.isWrittenInMainFile(SM.getExpansionLoc(RenameDecl.getLocation())); // If the symbol is declared in the main file (which is not a header), we // rename it. @@ -115,18 +114,19 @@ // Below are cases where the symbol is declared in the header. // If the symbol is function-local, we rename it. - if (Decl.getParentFunctionOrMethod()) + if (RenameDecl.getParentFunctionOrMethod()) return None; if (!Index) return ReasonToReject::NoIndexProvided; - bool IsIndexable = - SymbolCollector::shouldCollectSymbol(Decl, ASTCtx, {}, false); + bool IsIndexable = isa(RenameDecl) && + SymbolCollector::shouldCollectSymbol( + cast(RenameDecl), ASTCtx, {}, false); // If the symbol is not indexable, we disallow rename. if (!IsIndexable) return ReasonToReject::NonIndexable; - auto OtherFile = getOtherRefFile(Decl, MainFile, *Index); + auto OtherFile = getOtherRefFile(RenameDecl, MainFile, *Index); // If the symbol is indexable and has no refs from other files in the index, // we rename it. if (!OtherFile) @@ -154,7 +154,8 @@ const auto *RenameDecl = Rename->getRenameDecl(); assert(RenameDecl && "symbol must be found at this point"); - if (auto Reject = renamableWithinFile(*RenameDecl, File, Index)) { + if (auto Reject = + renamableWithinFile(*RenameDecl->getCanonicalDecl(), File, Index)) { auto Message = [](ReasonToReject Reason) { switch (Reason) { case NoIndexProvided: 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 @@ -78,7 +78,6 @@ for (const Test &T : Tests) { Annotations Code(T.Before); auto TU = TestTU::withCode(Code.code()); - TU.HeaderCode = "void foo();"; // outside main file, will not be touched. auto AST = TU.build(); auto RenameResult = renameWithinFile(AST, testPath(TU.Filename), Code.point(), "abcde"); @@ -92,58 +91,68 @@ } TEST(RenameTest, Renameable) { - // Test cases where the symbol is declared in header. struct Case { - const char* HeaderCode; + const char *Code; const char* ErrorMessage; // null if no error + bool IsHeaderFile; }; + const bool HeaderFile = true; Case Cases[] = { {R"cpp(// allow -- function-local void f(int [[Lo^cal]]) { [[Local]] = 2; } )cpp", - nullptr}, + nullptr, HeaderFile}, {R"cpp(// allow -- symbol is indexable and has no refs in index. void [[On^lyInThisFile]](); )cpp", - nullptr}, + nullptr, HeaderFile}, {R"cpp(// disallow -- symbol is indexable and has other refs in index. void f() { Out^side s; } )cpp", - "used outside main file"}, + "used outside main file", HeaderFile}, {R"cpp(// disallow -- symbol is not indexable. namespace { class Unin^dexable {}; } )cpp", - "not eligible for indexing"}, + "not eligible for indexing", HeaderFile}, {R"cpp(// disallow -- namespace symbol isn't supported namespace fo^o {} )cpp", - "not a supported kind"}, + "not a supported kind", HeaderFile}, + + {R"cpp(// foo is declared outside the file. + void fo^o() {} + )cpp", "used outside main file", !HeaderFile/*cc file*/}, }; - const char *CommonHeader = "class Outside {};"; - TestTU OtherFile = TestTU::withCode("Outside s;"); + const char *CommonHeader = R"cpp( + class Outside {}; + void foo(); + )cpp"; + TestTU OtherFile = TestTU::withCode("Outside s; auto ss = &foo;"); OtherFile.HeaderCode = CommonHeader; OtherFile.Filename = "other.cc"; - // The index has a "Outside" reference. + // The index has a "Outside" reference and a "foo" reference. auto OtherFileIndex = OtherFile.index(); for (const auto& Case : Cases) { - Annotations T(Case.HeaderCode); - // We open the .h file as the main file. + Annotations T(Case.Code); TestTU TU = TestTU::withCode(T.code()); - TU.Filename = "test.h"; TU.HeaderCode = CommonHeader; - // Parsing the .h file as C++ include. - TU.ExtraArgs.push_back("-xobjective-c++-header"); + if (Case.IsHeaderFile) { + // We open the .h file as the main file. + TU.Filename = "test.h"; + // Parsing the .h file as C++ include. + TU.ExtraArgs.push_back("-xobjective-c++-header"); + } auto AST = TU.build(); auto Results = renameWithinFile(AST, testPath(TU.Filename), T.point(),