Skip to content

Commit f429ab6

Browse files
committed
[clangd] Implement "prepareRename"
Summary: - "prepareRename" request is added in LSP v3.12.0 - also update the vscode-client dependency to pick-up the rename bug fix[1] [1]: microsoft/vscode-languageserver-node#447 Reviewers: sammccall Subscribers: ilya-biryukov, MaskRay, jkorous, arphaman, kadircet, cfe-commits Tags: #clang Differential Revision: https://reviews.llvm.org/D63126 llvm-svn: 366873
1 parent 5ecb880 commit f429ab6

File tree

8 files changed

+98
-15
lines changed

8 files changed

+98
-15
lines changed

clang-tools-extra/clangd/ClangdLSPServer.cpp

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -382,6 +382,14 @@ void ClangdLSPServer::onInitialize(const InitializeParams &Params,
382382
SupportFileStatus = Params.initializationOptions.FileStatus;
383383
HoverContentFormat = Params.capabilities.HoverContentFormat;
384384
SupportsOffsetsInSignatureHelp = Params.capabilities.OffsetsInSignatureHelp;
385+
386+
// Per LSP, renameProvider can be either boolean or RenameOptions.
387+
// RenameOptions will be specified if the client states it supports prepare.
388+
llvm::json::Value RenameProvider =
389+
llvm::json::Object{{"prepareProvider", true}};
390+
if (!Params.capabilities.RenamePrepareSupport) // Only boolean allowed per LSP
391+
RenameProvider = true;
392+
385393
llvm::json::Object Result{
386394
{{"capabilities",
387395
llvm::json::Object{
@@ -409,7 +417,7 @@ void ClangdLSPServer::onInitialize(const InitializeParams &Params,
409417
{"definitionProvider", true},
410418
{"documentHighlightProvider", true},
411419
{"hoverProvider", true},
412-
{"renameProvider", true},
420+
{"renameProvider", std::move(RenameProvider)},
413421
{"documentSymbolProvider", true},
414422
{"workspaceSymbolProvider", true},
415423
{"referencesProvider", true},
@@ -568,6 +576,12 @@ void ClangdLSPServer::onWorkspaceSymbol(
568576
std::move(Reply)));
569577
}
570578

579+
void ClangdLSPServer::onPrepareRename(const TextDocumentPositionParams &Params,
580+
Callback<llvm::Optional<Range>> Reply) {
581+
Server->prepareRename(Params.textDocument.uri.file(), Params.position,
582+
std::move(Reply));
583+
}
584+
571585
void ClangdLSPServer::onRename(const RenameParams &Params,
572586
Callback<WorkspaceEdit> Reply) {
573587
Path File = Params.textDocument.uri.file();
@@ -1015,6 +1029,7 @@ ClangdLSPServer::ClangdLSPServer(
10151029
MsgHandler->bind("textDocument/declaration", &ClangdLSPServer::onGoToDeclaration);
10161030
MsgHandler->bind("textDocument/references", &ClangdLSPServer::onReference);
10171031
MsgHandler->bind("textDocument/switchSourceHeader", &ClangdLSPServer::onSwitchSourceHeader);
1032+
MsgHandler->bind("textDocument/prepareRename", &ClangdLSPServer::onPrepareRename);
10181033
MsgHandler->bind("textDocument/rename", &ClangdLSPServer::onRename);
10191034
MsgHandler->bind("textDocument/hover", &ClangdLSPServer::onHover);
10201035
MsgHandler->bind("textDocument/documentSymbol", &ClangdLSPServer::onDocumentSymbol);

clang-tools-extra/clangd/ClangdLSPServer.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,8 @@ class ClangdLSPServer : private DiagnosticsConsumer {
9595
void onCommand(const ExecuteCommandParams &, Callback<llvm::json::Value>);
9696
void onWorkspaceSymbol(const WorkspaceSymbolParams &,
9797
Callback<std::vector<SymbolInformation>>);
98+
void onPrepareRename(const TextDocumentPositionParams &,
99+
Callback<llvm::Optional<Range>>);
98100
void onRename(const RenameParams &, Callback<WorkspaceEdit>);
99101
void onHover(const TextDocumentPositionParams &,
100102
Callback<llvm::Optional<Hover>>);

clang-tools-extra/clangd/ClangdServer.cpp

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -292,6 +292,35 @@ ClangdServer::formatOnType(llvm::StringRef Code, PathRef File, Position Pos,
292292
return Result;
293293
}
294294

295+
void ClangdServer::prepareRename(PathRef File, Position Pos,
296+
Callback<llvm::Optional<Range>> CB) {
297+
auto Action = [Pos, this](Path File, Callback<llvm::Optional<Range>> CB,
298+
llvm::Expected<InputsAndAST> InpAST) {
299+
if (!InpAST)
300+
return CB(InpAST.takeError());
301+
auto &AST = InpAST->AST;
302+
// Performing the rename isn't substantially more expensive than doing an
303+
// AST-based check, so we just rename and throw away the results. We may
304+
// have to revisit this when we support cross-file rename.
305+
auto Changes = renameWithinFile(AST, File, Pos, "dummy", Index);
306+
if (!Changes) {
307+
// LSP says to return null on failure, but that will result in a generic
308+
// failure message. If we send an LSP error response, clients can surface
309+
// the message to users (VSCode does).
310+
return CB(Changes.takeError());
311+
}
312+
SourceLocation Loc = getBeginningOfIdentifier(
313+
AST, Pos, AST.getSourceManager().getMainFileID());
314+
if (auto Range = getTokenRange(AST.getSourceManager(),
315+
AST.getASTContext().getLangOpts(), Loc))
316+
return CB(*Range);
317+
// Return null if the "rename" is not valid at the position.
318+
CB(llvm::None);
319+
};
320+
WorkScheduler.runWithAST("PrepareRename", File,
321+
Bind(Action, File.str(), std::move(CB)));
322+
}
323+
295324
void ClangdServer::rename(PathRef File, Position Pos, llvm::StringRef NewName,
296325
bool WantFormat, Callback<std::vector<TextEdit>> CB) {
297326
auto Action = [Pos, WantFormat, this](Path File, std::string NewName,

clang-tools-extra/clangd/ClangdServer.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -241,6 +241,10 @@ class ClangdServer {
241241
PathRef File, Position Pos,
242242
StringRef TriggerText);
243243

244+
/// Test the validity of a rename operation.
245+
void prepareRename(PathRef File, Position Pos,
246+
Callback<llvm::Optional<Range>> CB);
247+
244248
/// Rename all occurrences of the symbol at the \p Pos in \p File to
245249
/// \p NewName.
246250
/// If WantFormat is false, the final TextEdit will be not formatted,

clang-tools-extra/clangd/Protocol.cpp

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -331,6 +331,10 @@ bool fromJSON(const llvm::json::Value &Params, ClientCapabilities &R) {
331331
}
332332
}
333333
}
334+
if (auto *Rename = TextDocument->getObject("rename")) {
335+
if (auto RenameSupport = Rename->getBoolean("prepareSupport"))
336+
R.RenamePrepareSupport = *RenameSupport;
337+
}
334338
}
335339
if (auto *Workspace = O->getObject("workspace")) {
336340
if (auto *Symbol = Workspace->getObject("symbol")) {

clang-tools-extra/clangd/Protocol.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -422,6 +422,10 @@ struct ClientCapabilities {
422422
/// The content format that should be used for Hover requests.
423423
/// textDocument.hover.contentEncoding
424424
MarkupKind HoverContentFormat = MarkupKind::PlainText;
425+
426+
/// The client supports testing for validity of rename operations
427+
/// before execution.
428+
bool RenamePrepareSupport = false;
425429
};
426430
bool fromJSON(const llvm::json::Value &, ClientCapabilities &);
427431

clang-tools-extra/clangd/clients/clangd-vscode/package.json

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
"publisher": "llvm-vs-code-extensions",
77
"homepage": "https://clang.llvm.org/extra/clangd.html",
88
"engines": {
9-
"vscode": "^1.30.0"
9+
"vscode": "^1.36.0"
1010
},
1111
"categories": [
1212
"Programming Languages",
@@ -35,8 +35,8 @@
3535
"test": "node ./node_modules/vscode/bin/test"
3636
},
3737
"dependencies": {
38-
"vscode-languageclient": "^5.2.0",
39-
"vscode-languageserver": "^5.2.0"
38+
"vscode-languageclient": "^5.3.0-next.6",
39+
"vscode-languageserver": "^5.3.0-next.6"
4040
},
4141
"devDependencies": {
4242
"@types/mocha": "^2.2.32",

clang-tools-extra/clangd/test/rename.test

Lines changed: 36 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,45 @@
11
# RUN: clangd -lit-test < %s | FileCheck -strict-whitespace %s
2-
{"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{},"trace":"off"}}
2+
{"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{"textDocument": {"rename": {"dynamicRegistration": true, "prepareSupport": true}}},"trace":"off"}}
3+
# CHECK: "renameProvider": {
4+
# CHECK-NEXT: "prepareProvider": true
5+
# CHECK-NEXT: },
36
---
47
{"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"test:///foo.cpp","languageId":"cpp","version":1,"text":"int foo;"}}}
58
---
6-
{"jsonrpc":"2.0","id":1,"method":"textDocument/rename","params":{"textDocument":{"uri":"test:///foo.cpp"},"position":{"line":0,"character":5},"newName":"bar"}}
9+
{"jsonrpc":"2.0","id":1,"method":"textDocument/prepareRename","params":{"textDocument":{"uri":"test:///foo.cpp"},"position":{"line":0,"character":5}}}
710
# CHECK: "id": 1,
811
# CHECK-NEXT: "jsonrpc": "2.0",
912
# CHECK-NEXT: "result": {
13+
# CHECK-NEXT: "end": {
14+
# CHECK-NEXT: "character": 7,
15+
# CHECK-NEXT: "line": 0
16+
# CHECK-NEXT: },
17+
# CHECK-NEXT: "start": {
18+
# CHECK-NEXT: "character": 4,
19+
# CHECK-NEXT: "line": 0
20+
# CHECK-NEXT: }
21+
# CHECK-NEXT: }
22+
---
23+
{"jsonrpc":"2.0","id":2,"method":"textDocument/prepareRename","params":{"textDocument":{"uri":"test:///foo.cpp"},"position":{"line":0,"character":2}}}
24+
# CHECK: "error": {
25+
# CHECK-NEXT: "code": -32001,
26+
# CHECK-NEXT: "message": "there is no symbol at the given location"
27+
# CHECK-NEXT: },
28+
# CHECK-NEXT: "id": 2,
29+
# CHECK-NEXT: "jsonrpc": "2.0"
30+
---
31+
{"jsonrpc":"2.0","id":4,"method":"textDocument/rename","params":{"textDocument":{"uri":"test:///foo.cpp"},"position":{"line":0,"character":2},"newName":"bar"}}
32+
# CHECK: "error": {
33+
# CHECK-NEXT: "code": -32001,
34+
# CHECK-NEXT: "message": "there is no symbol at the given location"
35+
# CHECK-NEXT: },
36+
# CHECK-NEXT: "id": 4,
37+
# CHECK-NEXT: "jsonrpc": "2.0"
38+
---
39+
{"jsonrpc":"2.0","id":3,"method":"textDocument/rename","params":{"textDocument":{"uri":"test:///foo.cpp"},"position":{"line":0,"character":5},"newName":"bar"}}
40+
# CHECK: "id": 3,
41+
# CHECK-NEXT: "jsonrpc": "2.0",
42+
# CHECK-NEXT: "result": {
1043
# CHECK-NEXT: "changes": {
1144
# CHECK-NEXT: "file://{{.*}}/foo.cpp": [
1245
# CHECK-NEXT: {
@@ -26,14 +59,6 @@
2659
# CHECK-NEXT: }
2760
# CHECK-NEXT: }
2861
---
29-
{"jsonrpc":"2.0","id":2,"method":"textDocument/rename","params":{"textDocument":{"uri":"test:///foo.cpp"},"position":{"line":0,"character":2},"newName":"bar"}}
30-
# CHECK: "error": {
31-
# CHECK-NEXT: "code": -32001,
32-
# CHECK-NEXT: "message": "there is no symbol at the given location"
33-
# CHECK-NEXT: },
34-
# CHECK-NEXT: "id": 2,
35-
# CHECK-NEXT: "jsonrpc": "2.0"
36-
---
37-
{"jsonrpc":"2.0","id":3,"method":"shutdown"}
62+
{"jsonrpc":"2.0","id":5,"method":"shutdown"}
3863
---
3964
{"jsonrpc":"2.0","method":"exit"}

0 commit comments

Comments
 (0)