hokein created this revision.
hokein added a reviewer: sammccall.
Herald added subscribers: usaxena95, kadircet, arphaman.
Herald added a project: clang.
hokein requested review of this revision.
Herald added subscribers: MaskRay, ilya-biryukov.
so that our embedders can implement customized behavior on top of it.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D88881
Files:
clang-tools-extra/clangd/ClangdLSPServer.cpp
clang-tools-extra/clangd/ClangdServer.cpp
clang-tools-extra/clangd/ClangdServer.h
clang-tools-extra/clangd/unittests/RenameTests.cpp
clang-tools-extra/clangd/unittests/SyncAPI.cpp
clang-tools-extra/clangd/unittests/SyncAPI.h
Index: clang-tools-extra/clangd/unittests/SyncAPI.h
===================================================================
--- clang-tools-extra/clangd/unittests/SyncAPI.h
+++ clang-tools-extra/clangd/unittests/SyncAPI.h
@@ -46,6 +46,7 @@
llvm::Expected<RenameResult>
runPrepareRename(ClangdServer &Server, PathRef File, Position Pos,
+ llvm::StringRef NewName,
const clangd::RenameOptions &RenameOpts);
llvm::Expected<tooling::Replacements>
Index: clang-tools-extra/clangd/unittests/SyncAPI.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/SyncAPI.cpp
+++ clang-tools-extra/clangd/unittests/SyncAPI.cpp
@@ -107,9 +107,10 @@
llvm::Expected<RenameResult> runPrepareRename(ClangdServer &Server,
PathRef File, Position Pos,
+ llvm::StringRef NewName,
const RenameOptions &RenameOpts) {
llvm::Optional<llvm::Expected<RenameResult>> Result;
- Server.prepareRename(File, Pos, RenameOpts, capture(Result));
+ Server.prepareRename(File, Pos, NewName, RenameOpts, capture(Result));
return std::move(*Result);
}
Index: clang-tools-extra/clangd/unittests/RenameTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/RenameTests.cpp
+++ 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=*/"", {/*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,16 @@
EXPECT_THAT(FooCC.ranges(),
testing::UnorderedElementsAreArray(Results->LocalChanges));
+ // verify name validation.
+ Results = runPrepareRename(Server, FooCCPath, FooCC.point(),
+ /*NewName=*/"int", {/*CrossFile=*/true});
+ EXPECT_FALSE(Results);
+ EXPECT_THAT(llvm::toString(Results.takeError()),
+ testing::HasSubstr("renamed to a reserved keyword"));
+
// single-file rename on global symbols, we should report an error.
- Results =
- runPrepareRename(Server, FooCCPath, FooCC.point(), {/*CrossFile=*/false});
+ Results = runPrepareRename(Server, FooCCPath, FooCC.point(), /*NewName=*/"",
+ {/*CrossFile=*/false});
EXPECT_FALSE(Results);
EXPECT_THAT(llvm::toString(Results.takeError()),
testing::HasSubstr("is used outside"));
Index: clang-tools-extra/clangd/ClangdServer.h
===================================================================
--- clang-tools-extra/clangd/ClangdServer.h
+++ clang-tools-extra/clangd/ClangdServer.h
@@ -274,9 +274,11 @@
/// Test the validity of a rename operation.
///
+ /// If NewName is not empty, it peforms a name validation.
+ ///
/// The returned result describes edits in the main-file only (all
/// occurrences of the renamed symbol are simply deleted.
- void prepareRename(PathRef File, Position Pos,
+ void prepareRename(PathRef File, Position Pos, llvm::StringRef NewName,
const RenameOptions &RenameOpts,
Callback<RenameResult> CB);
Index: clang-tools-extra/clangd/ClangdServer.cpp
===================================================================
--- clang-tools-extra/clangd/ClangdServer.cpp
+++ clang-tools-extra/clangd/ClangdServer.cpp
@@ -399,9 +399,11 @@
}
void ClangdServer::prepareRename(PathRef File, Position Pos,
+ llvm::StringRef NewName,
const RenameOptions &RenameOpts,
Callback<RenameResult> CB) {
- auto Action = [Pos, File = File.str(), CB = std::move(CB), RenameOpts,
+ auto Action = [Pos, File = File.str(), CB = std::move(CB),
+ NewName = NewName.str(), RenameOpts,
this](llvm::Expected<InputsAndAST> InpAST) mutable {
if (!InpAST)
return CB(InpAST.takeError());
@@ -412,7 +414,7 @@
// - for cross-file rename, we deliberately pass a nullptr index to save
// the cost, thus the result may be incomplete as it only contains
// main-file occurrences;
- auto Results = clangd::rename({Pos, /*NewName*/ "", InpAST->AST, File,
+ auto Results = clangd::rename({Pos, NewName, InpAST->AST, File,
RenameOpts.AllowCrossFile ? nullptr : Index,
RenameOpts});
if (!Results) {
Index: clang-tools-extra/clangd/ClangdLSPServer.cpp
===================================================================
--- clang-tools-extra/clangd/ClangdLSPServer.cpp
+++ clang-tools-extra/clangd/ClangdLSPServer.cpp
@@ -794,7 +794,8 @@
void ClangdLSPServer::onPrepareRename(const TextDocumentPositionParams &Params,
Callback<llvm::Optional<Range>> Reply) {
Server->prepareRename(
- Params.textDocument.uri.file(), Params.position, Opts.Rename,
+ Params.textDocument.uri.file(), Params.position, /*NewName*/ "",
+ Opts.Rename,
[Reply = std::move(Reply)](llvm::Expected<RenameResult> Result) mutable {
if (!Result)
return Reply(Result.takeError());
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
