diff --git a/clang-tools-extra/clangd/refactor/Rename.cpp b/clang-tools-extra/clangd/refactor/Rename.cpp index 9518d848eff51..83f3a02d80d7a 100644 --- a/clang-tools-extra/clangd/refactor/Rename.cpp +++ b/clang-tools-extra/clangd/refactor/Rename.cpp @@ -75,8 +75,8 @@ llvm::Optional getOtherRefFile(const Decl &D, StringRef MainFile, return OtherFile; } -llvm::DenseSet locateDeclAt(ParsedAST &AST, - SourceLocation TokenStartLoc) { +llvm::DenseSet locateDeclAt(ParsedAST &AST, + SourceLocation TokenStartLoc) { unsigned Offset = AST.getSourceManager().getDecomposedSpellingLoc(TokenStartLoc).second; @@ -85,7 +85,7 @@ llvm::DenseSet locateDeclAt(ParsedAST &AST, if (!SelectedNode) return {}; - llvm::DenseSet Result; + llvm::DenseSet Result; for (const NamedDecl *D : targetDecl(SelectedNode->ASTNode, DeclRelation::Alias | DeclRelation::TemplatePattern)) { @@ -96,6 +96,15 @@ llvm::DenseSet locateDeclAt(ParsedAST &AST, return Result; } +bool isBlacklisted(const NamedDecl &RenameDecl) { + static const auto *StdSymbols = new llvm::DenseSet({ +#define SYMBOL(Name, NameSpace, Header) {#NameSpace #Name}, +#include "StdSymbolMap.inc" +#undef SYMBOL + }); + return StdSymbols->count(RenameDecl.getQualifiedNameAsString()); +} + enum ReasonToReject { NoSymbolFound, NoIndexProvided, @@ -105,7 +114,7 @@ enum ReasonToReject { AmbiguousSymbol, }; -llvm::Optional renameable(const Decl &RenameDecl, +llvm::Optional renameable(const NamedDecl &RenameDecl, StringRef MainFilePath, const SymbolIndex *Index, bool CrossFile) { @@ -120,6 +129,9 @@ llvm::Optional renameable(const Decl &RenameDecl, if (RenameDecl.getParentFunctionOrMethod()) return None; + if (isBlacklisted(RenameDecl)) + return ReasonToReject::UnsupportedSymbol; + // Check whether the symbol being rename is indexable. auto &ASTCtx = RenameDecl.getASTContext(); bool MainFileIsHeader = isHeaderFile(MainFilePath, ASTCtx.getLangOpts()); @@ -131,12 +143,10 @@ llvm::Optional renameable(const Decl &RenameDecl, IsMainFileOnly = false; else if (!DeclaredInMainFile) IsMainFileOnly = false; - bool IsIndexable = - isa(RenameDecl) && - SymbolCollector::shouldCollectSymbol( - cast(RenameDecl), RenameDecl.getASTContext(), - SymbolCollector::Options(), IsMainFileOnly); - if (!IsIndexable) // If the symbol is not indexable, we disallow rename. + // If the symbol is not indexable, we disallow rename. + if (!SymbolCollector::shouldCollectSymbol( + RenameDecl, RenameDecl.getASTContext(), SymbolCollector::Options(), + IsMainFileOnly)) return ReasonToReject::NonIndexable; if (!CrossFile) { @@ -467,13 +477,10 @@ llvm::Expected rename(const RenameInputs &RInputs) { if (DeclsUnderCursor.size() > 1) return makeError(ReasonToReject::AmbiguousSymbol); - const auto *RenameDecl = llvm::dyn_cast(*DeclsUnderCursor.begin()); - if (!RenameDecl) - return makeError(ReasonToReject::UnsupportedSymbol); - - auto Reject = - renameable(*RenameDecl->getCanonicalDecl(), RInputs.MainFilePath, - RInputs.Index, RInputs.AllowCrossFile); + const auto &RenameDecl = + llvm::cast(*(*DeclsUnderCursor.begin())->getCanonicalDecl()); + auto Reject = renameable(RenameDecl, RInputs.MainFilePath, RInputs.Index, + RInputs.AllowCrossFile); if (Reject) return makeError(*Reject); @@ -486,7 +493,7 @@ llvm::Expected rename(const RenameInputs &RInputs) { // To make cross-file rename work for local symbol, we use a hybrid solution: // - run AST-based rename on the main file; // - run index-based rename on other affected files; - auto MainFileRenameEdit = renameWithinFile(AST, *RenameDecl, RInputs.NewName); + auto MainFileRenameEdit = renameWithinFile(AST, RenameDecl, RInputs.NewName); if (!MainFileRenameEdit) return MainFileRenameEdit.takeError(); @@ -502,7 +509,7 @@ llvm::Expected rename(const RenameInputs &RInputs) { // symbol if we don't have index. if (RInputs.Index) { auto OtherFilesEdits = - renameOutsideFile(*RenameDecl, RInputs.MainFilePath, RInputs.NewName, + renameOutsideFile(RenameDecl, RInputs.MainFilePath, RInputs.NewName, *RInputs.Index, GetFileContent); if (!OtherFilesEdits) return OtherFilesEdits.takeError(); diff --git a/clang-tools-extra/clangd/unittests/RenameTests.cpp b/clang-tools-extra/clangd/unittests/RenameTests.cpp index 95b960132a44a..cb7911a4d94d7 100644 --- a/clang-tools-extra/clangd/unittests/RenameTests.cpp +++ b/clang-tools-extra/clangd/unittests/RenameTests.cpp @@ -383,12 +383,12 @@ TEST(RenameTest, WithinFileRename) { // Typedef. R"cpp( - namespace std { + namespace ns { class basic_string {}; typedef basic_string [[s^tring]]; - } // namespace std + } // namespace ns - std::[[s^tring]] foo(); + ns::[[s^tring]] foo(); )cpp", // Variable. @@ -540,6 +540,13 @@ TEST(RenameTest, Renameable) { void fo^o() {})cpp", "used outside main file", !HeaderFile, nullptr /*no index*/}, + {R"cpp(// disallow rename on blacklisted symbols (e.g. std symbols) + namespace std { + class str^ing {}; + } + )cpp", + "not a supported kind", !HeaderFile, Index}, + {R"cpp( void foo(int); void foo(char);