hokein updated this revision to Diff 296622.
hokein marked an inline comment as done.
hokein added a comment.
make NewName optional.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D88881/new/
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::Optional<std::string> 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
@@ -105,11 +105,12 @@
return std::move(*Result);
}
-llvm::Expected<RenameResult> runPrepareRename(ClangdServer &Server,
- PathRef File, Position Pos,
- const RenameOptions &RenameOpts) {
+llvm::Expected<RenameResult>
+runPrepareRename(ClangdServer &Server, PathRef File, Position Pos,
+ llvm::Optional<std::string> 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=*/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"));
Index: clang-tools-extra/clangd/ClangdServer.h
===================================================================
--- clang-tools-extra/clangd/ClangdServer.h
+++ clang-tools-extra/clangd/ClangdServer.h
@@ -273,7 +273,10 @@
StringRef TriggerText, Callback<std::vector<TextEdit>> 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<std::string> 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::Optional<std::string> 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 = std::move(NewName), RenameOpts,
this](llvm::Expected<InputsAndAST> 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
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*/ llvm::None,
+ 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
