diff --git a/clang-tools-extra/clangd/ClangdServer.h b/clang-tools-extra/clangd/ClangdServer.h --- a/clang-tools-extra/clangd/ClangdServer.h +++ b/clang-tools-extra/clangd/ClangdServer.h @@ -142,8 +142,8 @@ /// Enable semantic highlighting features. bool SemanticHighlighting = false; - /// Enable cross-file rename feature. - bool CrossFileRename = false; + /// Options for rename. + RenameOptions RenameOpts; /// Returns true if the tweak should be enabled. std::function TweakFilter = [](const Tweak &T) { @@ -340,7 +340,7 @@ // can be caused by missing includes (e.g. member access in incomplete type). bool SuggestMissingIncludes = false; - bool CrossFileRename = false; + RenameOptions RenameOpts; std::function TweakFilter; diff --git a/clang-tools-extra/clangd/ClangdServer.cpp b/clang-tools-extra/clangd/ClangdServer.cpp --- a/clang-tools-extra/clangd/ClangdServer.cpp +++ b/clang-tools-extra/clangd/ClangdServer.cpp @@ -131,7 +131,7 @@ : nullptr), GetClangTidyOptions(Opts.GetClangTidyOptions), SuggestMissingIncludes(Opts.SuggestMissingIncludes), - CrossFileRename(Opts.CrossFileRename), TweakFilter(Opts.TweakFilter), + RenameOpts(Opts.RenameOpts), TweakFilter(Opts.TweakFilter), WorkspaceRoot(Opts.WorkspaceRoot), // Pass a callback into `WorkScheduler` to extract symbols from a newly // parsed file and rebuild the file index synchronously each time an AST @@ -338,14 +338,13 @@ SM, CharSourceRange::getCharRange(TouchingIdentifier->location(), TouchingIdentifier->endLocation())); - if (CrossFileRename) + if (RenameOpts.AllowCrossFile) // FIXME: we now assume cross-file rename always succeeds, revisit this. return CB(Range); // Performing the local rename isn't substantially more expensive than // doing an AST-based check, so we just rename and throw away the results. - auto Changes = clangd::rename({Pos, "dummy", AST, File, Index, - /*AllowCrossFile=*/false, + auto Changes = clangd::rename({Pos, "dummy", AST, File, Index, RenameOpts, /*GetDirtyBuffer=*/nullptr}); if (!Changes) { // LSP says to return null on failure, but that will result in a generic @@ -374,8 +373,8 @@ return llvm::None; return It->second; }; - auto Edits = clangd::rename({Pos, NewName, InpAST->AST, File, Index, - CrossFileRename, GetDirtyBuffer}); + auto Edits = clangd::rename( + {Pos, NewName, InpAST->AST, File, Index, RenameOpts, GetDirtyBuffer}); if (!Edits) return CB(Edits.takeError()); diff --git a/clang-tools-extra/clangd/refactor/Rename.h b/clang-tools-extra/clangd/refactor/Rename.h --- a/clang-tools-extra/clangd/refactor/Rename.h +++ b/clang-tools-extra/clangd/refactor/Rename.h @@ -26,6 +26,14 @@ using DirtyBufferGetter = llvm::function_ref(PathRef AbsPath)>; +struct RenameOptions { + bool AllowCrossFile = false; + /// The mamimum number of affected files (0 means no limit), only meaningful + /// when AllowCrossFile = true. + /// If the actual number exceeds the limit, rename is forbidden. + size_t LimitFiles = 50; +}; + struct RenameInputs { Position Pos; // the position triggering the rename llvm::StringRef NewName; @@ -35,7 +43,7 @@ const SymbolIndex *Index = nullptr; - bool AllowCrossFile = false; + RenameOptions Opts = {}; // When set, used by the rename to get file content for all rename-related // files. // If there is no corresponding dirty buffer, we will use the file content 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 @@ -322,7 +322,8 @@ // grouped by the absolute file path. llvm::Expected>> findOccurrencesOutsideFile(const NamedDecl &RenameDecl, - llvm::StringRef MainFile, const SymbolIndex &Index) { + llvm::StringRef MainFile, const SymbolIndex &Index, + size_t MaxLimitFiles) { trace::Span Tracer("FindOccurrencesOutsideFile"); RefsRequest RQuest; RQuest.IDs.insert(*getSymbolID(&RenameDecl)); @@ -337,8 +338,6 @@ // Absolute file path => rename occurrences in that file. llvm::StringMap> AffectedFiles; - // FIXME: Make the limit customizable. - static constexpr size_t MaxLimitFiles = 50; bool HasMore = Index.refs(RQuest, [&](const Ref &R) { if (AffectedFiles.size() > MaxLimitFiles) return; @@ -387,11 +386,11 @@ // there is no dirty buffer. llvm::Expected renameOutsideFile( const NamedDecl &RenameDecl, llvm::StringRef MainFilePath, - llvm::StringRef NewName, const SymbolIndex &Index, + llvm::StringRef NewName, const SymbolIndex &Index, size_t MaxLimitFiles, llvm::function_ref(PathRef)> GetFileContent) { trace::Span Tracer("RenameOutsideFile"); - auto AffectedFiles = - findOccurrencesOutsideFile(RenameDecl, MainFilePath, Index); + auto AffectedFiles = findOccurrencesOutsideFile(RenameDecl, MainFilePath, + Index, MaxLimitFiles); if (!AffectedFiles) return AffectedFiles.takeError(); FileEdits Results; @@ -473,6 +472,7 @@ llvm::Expected rename(const RenameInputs &RInputs) { trace::Span Tracer("Rename flow"); + const auto &Opts = RInputs.Opts; ParsedAST &AST = RInputs.AST; const SourceManager &SM = AST.getSourceManager(); llvm::StringRef MainFileCode = SM.getBufferData(SM.getMainFileID()); @@ -520,7 +520,7 @@ const auto &RenameDecl = llvm::cast(*(*DeclsUnderCursor.begin())->getCanonicalDecl()); auto Reject = renameable(RenameDecl, RInputs.MainFilePath, RInputs.Index, - RInputs.AllowCrossFile); + Opts.AllowCrossFile); if (Reject) return makeError(*Reject); @@ -537,7 +537,7 @@ if (!MainFileRenameEdit) return MainFileRenameEdit.takeError(); - if (!RInputs.AllowCrossFile) { + if (!Opts.AllowCrossFile) { // Within-file rename: just return the main file results. return FileEdits( {std::make_pair(RInputs.MainFilePath, @@ -548,9 +548,11 @@ // Renameable safely guards us that at this point we are renaming a local // symbol if we don't have index. if (RInputs.Index) { - auto OtherFilesEdits = - renameOutsideFile(RenameDecl, RInputs.MainFilePath, RInputs.NewName, - *RInputs.Index, GetFileContent); + auto OtherFilesEdits = renameOutsideFile( + RenameDecl, RInputs.MainFilePath, RInputs.NewName, *RInputs.Index, + Opts.LimitFiles == 0 ? std::numeric_limits::max() + : Opts.LimitFiles - 1, + GetFileContent); if (!OtherFilesEdits) return OtherFilesEdits.takeError(); Results = std::move(*OtherFilesEdits); diff --git a/clang-tools-extra/clangd/tool/ClangdMain.cpp b/clang-tools-extra/clangd/tool/ClangdMain.cpp --- a/clang-tools-extra/clangd/tool/ClangdMain.cpp +++ b/clang-tools-extra/clangd/tool/ClangdMain.cpp @@ -628,7 +628,7 @@ } Opts.StaticIndex = StaticIdx.get(); Opts.AsyncThreadsCount = WorkerThreadsCount; - Opts.CrossFileRename = CrossFileRename; + Opts.RenameOpts.AllowCrossFile = CrossFileRename; clangd::CodeCompleteOptions CCOpts; CCOpts.IncludeIneligibleResults = IncludeIneligibleResults; 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 @@ -714,8 +714,13 @@ TestTU TU = TestTU::withCode(MainCode.code()); auto AST = TU.build(); llvm::StringRef NewName = "newName"; - auto Results = rename({MainCode.point(), NewName, AST, MainFilePath, - Index.get(), /*CrossFile=*/true, GetDirtyBuffer}); + auto Results = rename({MainCode.point(), + NewName, + AST, + MainFilePath, + Index.get(), + {/*CrossFile=*/true}, + GetDirtyBuffer}); ASSERT_TRUE(bool(Results)) << Results.takeError(); EXPECT_THAT( applyEdits(std::move(*Results)), @@ -730,8 +735,13 @@ // Set a file "bar.cc" on disk. TU.AdditionalFiles["bar.cc"] = std::string(BarCode.code()); AST = TU.build(); - Results = rename({MainCode.point(), NewName, AST, MainFilePath, Index.get(), - /*CrossFile=*/true, GetDirtyBuffer}); + Results = rename({MainCode.point(), + NewName, + AST, + MainFilePath, + Index.get(), + {/*CrossFile=*/true}, + GetDirtyBuffer}); ASSERT_TRUE(bool(Results)) << Results.takeError(); EXPECT_THAT( applyEdits(std::move(*Results)), @@ -761,8 +771,13 @@ Callback) const override {} size_t estimateMemoryUsage() const override { return 0; } } PIndex; - Results = rename({MainCode.point(), NewName, AST, MainFilePath, &PIndex, - /*CrossFile=*/true, GetDirtyBuffer}); + Results = rename({MainCode.point(), + NewName, + AST, + MainFilePath, + &PIndex, + {/*CrossFile=*/true}, + GetDirtyBuffer}); EXPECT_FALSE(Results); EXPECT_THAT(llvm::toString(Results.takeError()), testing::HasSubstr("too many occurrences")); @@ -803,9 +818,12 @@ RefsRequest *Out; } RIndex(&Req); - auto Results = - rename({MainCode.point(), "NewName", AST, testPath("main.cc"), &RIndex, - /*CrossFile=*/true}); + auto Results = rename({MainCode.point(), + "NewName", + AST, + testPath("main.cc"), + &RIndex, + {/*CrossFile=*/true}}); ASSERT_TRUE(bool(Results)) << Results.takeError(); const auto HeaderSymbols = TU.headerSymbols(); EXPECT_THAT(Req.IDs, @@ -850,8 +868,9 @@ Ref ReturnedRef; } DIndex(XRefInBarCC); llvm::StringRef NewName = "newName"; - auto Results = rename({MainCode.point(), NewName, AST, MainFilePath, &DIndex, - /*CrossFile=*/true}); + RenameOptions RenameOpts{true, 50}; + auto Results = rename( + {MainCode.point(), NewName, AST, MainFilePath, &DIndex, RenameOpts}); ASSERT_TRUE(bool(Results)) << Results.takeError(); EXPECT_THAT( applyEdits(std::move(*Results)), @@ -1021,7 +1040,7 @@ FS.Files[FooCCPath] = std::string(FooCC.code()); auto ServerOpts = ClangdServer::optsForTest(); - ServerOpts.CrossFileRename = true; + ServerOpts.RenameOpts.AllowCrossFile = true; ServerOpts.BuildDynamicSymbolIndex = true; ClangdServer Server(CDB, FS, ServerOpts);