[clang-tools-extra] 9c09e20 - [clangd] Add a NewName optional parameter to clangdServer::prepareRename.
Haojian Wu via cfe-commits
cfe-commits at lists.llvm.org
Wed Oct 7 12:19:07 PDT 2020
Author: Haojian Wu
Date: 2020-10-07T21:18:51+02:00
New Revision: 9c09e2055ee4d4e3b26e393ab460635825a79538
URL: https://github.com/llvm/llvm-project/commit/9c09e2055ee4d4e3b26e393ab460635825a79538
DIFF: https://github.com/llvm/llvm-project/commit/9c09e2055ee4d4e3b26e393ab460635825a79538.diff
LOG: [clangd] Add a NewName optional parameter to clangdServer::prepareRename.
If the NewName is provided, prepareRename would perform a name
validation.
The motivation is to allow our internal embeder implement the customized
"canRenameInto" functionality on top of prepareRename.
Differential Revision: https://reviews.llvm.org/D88881
Added:
Modified:
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
Removed:
################################################################################
diff --git a/clang-tools-extra/clangd/ClangdLSPServer.cpp b/clang-tools-extra/clangd/ClangdLSPServer.cpp
index 34d5a305494c..e5ea4ccc6b8c 100644
--- a/clang-tools-extra/clangd/ClangdLSPServer.cpp
+++ b/clang-tools-extra/clangd/ClangdLSPServer.cpp
@@ -794,7 +794,8 @@ void ClangdLSPServer::onWorkspaceSymbol(
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());
diff --git a/clang-tools-extra/clangd/ClangdServer.cpp b/clang-tools-extra/clangd/ClangdServer.cpp
index d38e115a6796..68afa49514a9 100644
--- a/clang-tools-extra/clangd/ClangdServer.cpp
+++ b/clang-tools-extra/clangd/ClangdServer.cpp
@@ -399,9 +399,11 @@ void ClangdServer::formatOnType(PathRef File, llvm::StringRef Code,
}
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 @@ void ClangdServer::prepareRename(PathRef File, Position Pos,
// 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/ClangdServer.h b/clang-tools-extra/clangd/ClangdServer.h
index d03f50069746..c52ec007bbdc 100644
--- a/clang-tools-extra/clangd/ClangdServer.h
+++ b/clang-tools-extra/clangd/ClangdServer.h
@@ -273,7 +273,10 @@ class ClangdServer {
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);
diff --git a/clang-tools-extra/clangd/unittests/RenameTests.cpp b/clang-tools-extra/clangd/unittests/RenameTests.cpp
index d925dfa36f50..143e8c6ce1ff 100644
--- a/clang-tools-extra/clangd/unittests/RenameTests.cpp
+++ b/clang-tools-extra/clangd/unittests/RenameTests.cpp
@@ -730,8 +730,8 @@ TEST(RenameTest, PrepareRename) {
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 @@ TEST(RenameTest, PrepareRename) {
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.cpp b/clang-tools-extra/clangd/unittests/SyncAPI.cpp
index 6d6879ab62db..27b6cf33e055 100644
--- a/clang-tools-extra/clangd/unittests/SyncAPI.cpp
+++ b/clang-tools-extra/clangd/unittests/SyncAPI.cpp
@@ -105,11 +105,12 @@ llvm::Expected<RenameResult> runRename(ClangdServer &Server, PathRef File,
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);
}
diff --git a/clang-tools-extra/clangd/unittests/SyncAPI.h b/clang-tools-extra/clangd/unittests/SyncAPI.h
index aa641fee91af..fd0f5dba604d 100644
--- a/clang-tools-extra/clangd/unittests/SyncAPI.h
+++ b/clang-tools-extra/clangd/unittests/SyncAPI.h
@@ -46,6 +46,7 @@ llvm::Expected<RenameResult> runRename(ClangdServer &Server, PathRef File,
llvm::Expected<RenameResult>
runPrepareRename(ClangdServer &Server, PathRef File, Position Pos,
+ llvm::Optional<std::string> NewName,
const clangd::RenameOptions &RenameOpts);
llvm::Expected<tooling::Replacements>
More information about the cfe-commits
mailing list