diff --git a/clang-tools-extra/clangd/ClangdLSPServer.cpp b/clang-tools-extra/clangd/ClangdLSPServer.cpp --- a/clang-tools-extra/clangd/ClangdLSPServer.cpp +++ b/clang-tools-extra/clangd/ClangdLSPServer.cpp @@ -794,7 +794,8 @@ void ClangdLSPServer::onPrepareRename(const TextDocumentPositionParams &Params, Callback> Reply) { Server->prepareRename( - Params.textDocument.uri.file(), Params.position, Opts.Rename, + Params.textDocument.uri.file(), Params.position, /*NewName*/ llvm::None, + Opts.Rename, [Reply = std::move(Reply)](llvm::Expected Result) mutable { if (!Result) return Reply(Result.takeError()); 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 @@ -273,7 +273,10 @@ StringRef TriggerText, Callback> CB); /// Test the validity of a rename operation. + /// + /// If NewName is provided, it peforms a name validation. void prepareRename(PathRef File, Position Pos, + llvm::Optional NewName, const RenameOptions &RenameOpts, Callback CB); 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 @@ -399,9 +399,11 @@ } void ClangdServer::prepareRename(PathRef File, Position Pos, + llvm::Optional NewName, const RenameOptions &RenameOpts, Callback CB) { - auto Action = [Pos, File = File.str(), CB = std::move(CB), RenameOpts, + auto Action = [Pos, File = File.str(), CB = std::move(CB), + NewName = std::move(NewName), RenameOpts, this](llvm::Expected InpAST) mutable { if (!InpAST) return CB(InpAST.takeError()); @@ -413,7 +415,7 @@ // the cost, thus the result may be incomplete as it only contains // main-file occurrences; auto Results = clangd::rename( - {Pos, /*NewName=*/"__clangd_rename_dummy", InpAST->AST, File, + {Pos, NewName.getValueOr("__clangd_rename_dummy"), InpAST->AST, File, RenameOpts.AllowCrossFile ? nullptr : Index, RenameOpts}); if (!Results) { // LSP says to return null on failure, but that will result in a generic 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 @@ -730,8 +730,8 @@ runAddDocument(Server, FooHPath, FooH.code()); runAddDocument(Server, FooCCPath, FooCC.code()); - auto Results = - runPrepareRename(Server, FooCCPath, FooCC.point(), {/*CrossFile=*/true}); + auto Results = runPrepareRename(Server, FooCCPath, FooCC.point(), + /*NewName=*/llvm::None, {/*CrossFile=*/true}); // verify that for multi-file rename, we only return main-file occurrences. ASSERT_TRUE(bool(Results)) << Results.takeError(); // We don't know the result is complete in prepareRename (passing a nullptr @@ -740,9 +740,17 @@ EXPECT_THAT(FooCC.ranges(), testing::UnorderedElementsAreArray(Results->LocalChanges)); - // single-file rename on global symbols, we should report an error. + // verify name validation. Results = - runPrepareRename(Server, FooCCPath, FooCC.point(), {/*CrossFile=*/false}); + runPrepareRename(Server, FooCCPath, FooCC.point(), + /*NewName=*/std::string("int"), {/*CrossFile=*/true}); + EXPECT_FALSE(Results); + EXPECT_THAT(llvm::toString(Results.takeError()), + testing::HasSubstr("keyword")); + + // single-file rename on global symbols, we should report an error. + Results = runPrepareRename(Server, FooCCPath, FooCC.point(), + /*NewName=*/llvm::None, {/*CrossFile=*/false}); EXPECT_FALSE(Results); EXPECT_THAT(llvm::toString(Results.takeError()), testing::HasSubstr("is used outside")); diff --git a/clang-tools-extra/clangd/unittests/SyncAPI.h b/clang-tools-extra/clangd/unittests/SyncAPI.h --- a/clang-tools-extra/clangd/unittests/SyncAPI.h +++ b/clang-tools-extra/clangd/unittests/SyncAPI.h @@ -46,6 +46,7 @@ llvm::Expected runPrepareRename(ClangdServer &Server, PathRef File, Position Pos, + llvm::Optional NewName, const clangd::RenameOptions &RenameOpts); llvm::Expected diff --git a/clang-tools-extra/clangd/unittests/SyncAPI.cpp b/clang-tools-extra/clangd/unittests/SyncAPI.cpp --- a/clang-tools-extra/clangd/unittests/SyncAPI.cpp +++ b/clang-tools-extra/clangd/unittests/SyncAPI.cpp @@ -105,11 +105,12 @@ return std::move(*Result); } -llvm::Expected runPrepareRename(ClangdServer &Server, - PathRef File, Position Pos, - const RenameOptions &RenameOpts) { +llvm::Expected +runPrepareRename(ClangdServer &Server, PathRef File, Position Pos, + llvm::Optional NewName, + const RenameOptions &RenameOpts) { llvm::Optional> Result; - Server.prepareRename(File, Pos, RenameOpts, capture(Result)); + Server.prepareRename(File, Pos, NewName, RenameOpts, capture(Result)); return std::move(*Result); }