maxjhandsome
/
llvm-project
forked from OSchip/llvm-project
1
0
Fork 0
Code Issues Pull Requests Packages Releases Wiki Activity

[clangd] Add a callback mechanism for handling responses from client. 

Summary:
The callback will be invoked in clangd when we receive a reply from the client.

This is a prerequisite of implementing a generic mechanism for chainable
refactorings (e.g. extract variable and rename), this would allow server to
trigger a new request to the LSP client after receiving a reply from the client.

Reviewers: sammccall

Subscribers: ilya-biryukov, MaskRay, jkorous, arphaman, kadircet, cfe-commits

Tags: #clang

Differential Revision: https://reviews.llvm.org/D65387

llvm-svn: 367845
This commit is contained in:
Haojian Wu 2019-08-05 12:48:09 +00:00
parent e4bb2b181f
commit f25163498b
6 changed files with 201 additions and 29 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

136
clang-tools-extra/clangd/ClangdLSPServer.cpp
View File

@ -146,11 +146,39 @@ public:
bool onReply(llvm::json::Value ID,
llvm::Expected<llvm::json::Value> Result) override {
WithContext HandlerContext(handlerContext());
// We ignore replies, just log them.
if (Result)
Callback<llvm::json::Value> ReplyHandler = nullptr;
if (auto IntID = ID.getAsInteger()) {
std::lock_guard<std::mutex> Mutex(CallMutex);
// Find a corresponding callback for the request ID;
for (size_t Index = 0; Index < ReplyCallbacks.size(); ++Index) {
if (ReplyCallbacks[Index].first == *IntID) {
ReplyHandler = std::move(ReplyCallbacks[Index].second);
ReplyCallbacks.erase(ReplyCallbacks.begin() +
Index); // remove the entry
break;
}
}
}
if (!ReplyHandler) {
// No callback being found, use a default log callback.
ReplyHandler = [&ID](llvm::Expected<llvm::json::Value> Result) {
elog("received a reply with ID {0}, but there was no such call", ID);
if (!Result)
llvm::consumeError(Result.takeError());
};
}
// Log and run the reply handler.
if (Result) {
log("<-- reply({0})", ID);
else
log("<-- reply({0}) error: {1}", ID, llvm::toString(Result.takeError()));
ReplyHandler(std::move(Result));
} else {
auto Err = Result.takeError();
log("<-- reply({0}) error: {1}", ID, Err);
ReplyHandler(std::move(Err));
}
return true;
}
@ -171,6 +199,35 @@ public:
};
}
// Bind a reply callback to a request. The callback will be invoked when
// clangd receives the reply from the LSP client.
// Return a call id of the request.
llvm::json::Value bindReply(Callback<llvm::json::Value> Reply) {
llvm::Optional<std::pair<int, Callback<llvm::json::Value>>> OldestCB;
int ID;
{
std::lock_guard<std::mutex> Mutex(CallMutex);
ID = NextCallID++;
ReplyCallbacks.emplace_back(ID, std::move(Reply));
// If the queue overflows, we assume that the client didn't reply the
// oldest request, and run the corresponding callback which replies an
// error to the client.
if (ReplyCallbacks.size() > MaxReplayCallbacks) {
elog("more than {0} outstanding LSP calls, forgetting about {1}",
MaxReplayCallbacks, ReplyCallbacks.front().first);
OldestCB = std::move(ReplyCallbacks.front());
ReplyCallbacks.pop_front();
}
}
if (OldestCB)
OldestCB->second(llvm::createStringError(
llvm::inconvertibleErrorCode(),
llvm::formatv("failed to receive a client reply for request ({0})",
OldestCB->first)));
return ID;
}
// Bind an LSP method name to a notification.
template <typename Param>
void bind(const char *Method,
@ -220,7 +277,13 @@ private:
ReplyOnce &operator=(const ReplyOnce &) = delete;
~ReplyOnce() {
if (Server && !Replied) {
// There's one legitimate reason to never reply to a request: clangd's
// request handler send a call to the client (e.g. applyEdit) and the
// client never replied. In this case, the ReplyOnce is owned by
// ClangdLSPServer's reply callback table and is destroyed along with the
// server. We don't attempt to send a reply in this case, there's little
// to be gained from doing so.
if (Server && !Server->IsBeingDestroyed && !Replied) {
elog("No reply to message {0}({1})", Method, ID);
assert(false && "must reply to all calls!");
(*this)(llvm::make_error<LSPError>("server failed to reply",
@ -255,6 +318,16 @@ private:
llvm::StringMap<std::function<void(llvm::json::Value)>> Notifications;
llvm::StringMap<std::function<void(llvm::json::Value, ReplyOnce)>> Calls;
// The maximum number of callbacks held in clangd.
//
// We bound the maximum size to the pending map to prevent memory leakage
// for cases where LSP clients don't reply for the request.
static constexpr int MaxReplayCallbacks = 100;
mutable std::mutex CallMutex;
int NextCallID = 0; /* GUARDED_BY(CallMutex) */
std::deque<std::pair</*RequestID*/ int,
/*ReplyHandler*/ Callback<llvm::json::Value>>>
ReplyCallbacks; /* GUARDED_BY(CallMutex) */
// Method calls may be cancelled by ID, so keep track of their state.
// This needs a mutex: handlers may finish on a different thread, and that's
@ -308,12 +381,13 @@ private:
ClangdLSPServer &Server;
};
constexpr int ClangdLSPServer::MessageHandler::MaxReplayCallbacks;
// call(), notify(), and reply() wrap the Transport, adding logging and locking.
void ClangdLSPServer::call(llvm::StringRef Method, llvm::json::Value Params) {
auto ID = NextCallID++;
void ClangdLSPServer::callRaw(StringRef Method, llvm::json::Value Params,
Callback<llvm::json::Value> CB) {
auto ID = MsgHandler->bindReply(std::move(CB));
log("--> {0}({1})", Method, ID);
// We currently don't handle responses, so no need to store ID anywhere.
std::lock_guard<std::mutex> Lock(TranspWriter);
Transp.call(Method, std::move(Params), ID);
}
@ -496,13 +570,28 @@ void ClangdLSPServer::onFileEvent(const DidChangeWatchedFilesParams &Params) {
void ClangdLSPServer::onCommand(const ExecuteCommandParams &Params,
Callback<llvm::json::Value> Reply) {
auto ApplyEdit = [this](WorkspaceEdit WE) {
auto ApplyEdit = [this](WorkspaceEdit WE,
Callback<ApplyWorkspaceEditResponse> Reply) {
ApplyWorkspaceEditParams Edit;
Edit.edit = std::move(WE);
// Ideally, we would wait for the response and if there is no error, we
// would reply success/failure to the original RPC.
call("workspace/applyEdit", Edit);
call("workspace/applyEdit", std::move(Edit), std::move(Reply));
};
// FIXME: this lambda is tangled and confusing, refactor it.
auto ReplyAfterApplyingEdit =
[](decltype(Reply) Reply, std::string SuccessMessage,
llvm::Expected<ApplyWorkspaceEditResponse> Response) {
if (!Response)
return Reply(Response.takeError());
if (!Response->applied) {
std::string Reason = Response->failureReason
? *Response->failureReason
: "unknown reason";
return Reply(llvm::createStringError(
llvm::inconvertibleErrorCode(),
("edits were not applied: " + Reason).c_str()));
}
return Reply(SuccessMessage);
};
if (Params.command == ExecuteCommandParams::CLANGD_APPLY_FIX_COMMAND &&
Params.workspaceEdit) {
// The flow for "apply-fix" :
@ -511,11 +600,11 @@ void ClangdLSPServer::onCommand(const ExecuteCommandParams &Params,
// 3. We send code actions, with the fixit embedded as context
// 4. The user selects the fixit, the editor asks us to apply it
// 5. We unwrap the changes and send them back to the editor
// 6. The editor applies the changes (applyEdit), and sends us a reply (but
// we ignore it)
Reply("Fix applied.");
ApplyEdit(*Params.workspaceEdit);
// 6. The editor applies the changes (applyEdit), and sends us a reply
// 7. We unwrap the reply and send a reply to the editor.
ApplyEdit(*Params.workspaceEdit,
Bind(ReplyAfterApplyingEdit, std::move(Reply),
std::string("Fix applied.")));
} else if (Params.command == ExecuteCommandParams::CLANGD_APPLY_TWEAK &&
Params.tweakArgs) {
auto Code = DraftMgr.getDraft(Params.tweakArgs->file.file());
@ -524,9 +613,9 @@ void ClangdLSPServer::onCommand(const ExecuteCommandParams &Params,
llvm::inconvertibleErrorCode(),
"trying to apply a code action for a non-added file"));
auto Action = [this, ApplyEdit](decltype(Reply) Reply, URIForFile File,
std::string Code,
llvm::Expected<Tweak::Effect> R) {
auto Action = [this, ApplyEdit, ReplyAfterApplyingEdit](
decltype(Reply) Reply, URIForFile File, std::string Code,
llvm::Expected<Tweak::Effect> R) {
if (!R)
return Reply(R.takeError());
@ -534,15 +623,16 @@ void ClangdLSPServer::onCommand(const ExecuteCommandParams &Params,
WorkspaceEdit WE;
WE.changes.emplace();
(*WE.changes)[File.uri()] = replacementsToEdits(Code, *R->ApplyEdit);
ApplyEdit(std::move(WE));
ApplyEdit(std::move(WE), Bind(ReplyAfterApplyingEdit, std::move(Reply),
std::string("Tweak applied.")));
}
if (R->ShowMessage) {
ShowMessageParams Msg;
Msg.message = *R->ShowMessage;
Msg.type = MessageType::Info;
notify("window/showMessage", Msg);
Reply("Tweak applied.");
}
Reply("Tweak applied.");
};
Server->applyTweak(Params.tweakArgs->file.file(),
Params.tweakArgs->selection, Params.tweakArgs->tweakID,
@ -1051,7 +1141,7 @@ ClangdLSPServer::ClangdLSPServer(
// clang-format on
}
ClangdLSPServer::~ClangdLSPServer() = default;
ClangdLSPServer::~ClangdLSPServer() { IsBeingDestroyed = true; }
bool ClangdLSPServer::run() {
// Run the Language Server loop.

27
clang-tools-extra/clangd/ClangdLSPServer.h
View File

@ -133,6 +133,9 @@ private:
/// Language Server client.
bool ShutdownRequestReceived = false;
/// Used to indicate the ClangdLSPServer is being destroyed.
std::atomic<bool> IsBeingDestroyed = {false};
std::mutex FixItsMutex;
typedef std::map<clangd::Diagnostic, std::vector<Fix>, LSPDiagnosticCompare>
DiagnosticToReplacementMap;
@ -146,9 +149,29 @@ private:
clangd::Transport &Transp;
class MessageHandler;
std::unique_ptr<MessageHandler> MsgHandler;
std::atomic<int> NextCallID = {0};
std::mutex TranspWriter;
void call(StringRef Method, llvm::json::Value Params);
template <typename Response>
void call(StringRef Method, llvm::json::Value Params, Callback<Response> CB) {
// Wrap the callback with LSP conversion and error-handling.
auto HandleReply = [](decltype(CB) CB,
llvm::Expected<llvm::json::Value> RawResponse) {
Response Rsp;
if (!RawResponse) {
CB(RawResponse.takeError());
} else if (fromJSON(*RawResponse, Rsp)) {
CB(std::move(Rsp));
} else {
elog("Failed to decode {0} response", *RawResponse);
CB(llvm::make_error<LSPError>("failed to decode reponse",
ErrorCode::InvalidParams));
}
};
callRaw(Method, std::move(Params),
Bind(std::move(HandleReply), std::move(CB)));
}
void callRaw(StringRef Method, llvm::json::Value Params,
Callback<llvm::json::Value> CB);
void notify(StringRef Method, llvm::json::Value Params);
const FileSystemProvider &FSProvider;

9
clang-tools-extra/clangd/Protocol.cpp
View File

@ -670,6 +670,15 @@ llvm::json::Value toJSON(const ApplyWorkspaceEditParams &Params) {
return llvm::json::Object{{"edit", Params.edit}};
}
bool fromJSON(const llvm::json::Value &Response,
ApplyWorkspaceEditResponse &R) {
llvm::json::ObjectMapper O(Response);
if (!O || !O.map("applied", R.applied))
return false;
O.map("failureReason", R.failureReason);
return true;
}
bool fromJSON(const llvm::json::Value &Params, TextDocumentPositionParams &R) {
llvm::json::ObjectMapper O(Params);
return O && O.map("textDocument", R.textDocument) &&

6
clang-tools-extra/clangd/Protocol.h
View File

@ -873,6 +873,12 @@ struct ApplyWorkspaceEditParams {
};
llvm::json::Value toJSON(const ApplyWorkspaceEditParams &);
struct ApplyWorkspaceEditResponse {
bool applied = true;
llvm::Optional<std::string> failureReason;
};
bool fromJSON(const llvm::json::Value &, ApplyWorkspaceEditResponse &);
struct TextDocumentPositionParams {
/// The text document.
TextDocumentIdentifier textDocument;

9
clang-tools-extra/clangd/test/fixits-command.test
View File

@ -165,10 +165,6 @@
# CHECK-NEXT: ]
---
{"jsonrpc":"2.0","id":4,"method":"workspace/executeCommand","params":{"command":"clangd.applyFix","arguments":[{"changes":{"test:///foo.c":[{"range":{"start":{"line":0,"character":32},"end":{"line":0,"character":32}},"newText":"("},{"range":{"start":{"line":0,"character":37},"end":{"line":0,"character":37}},"newText":")"}]}}]}}
# CHECK: "id": 4,
# CHECK-NEXT: "jsonrpc": "2.0",
# CHECK-NEXT: "result": "Fix applied."
#
# CHECK: "id": 0,
# CHECK-NEXT: "jsonrpc": "2.0",
# CHECK-NEXT: "method": "workspace/applyEdit",
@ -207,6 +203,11 @@
# CHECK-NEXT: }
# CHECK-NEXT: }
---
{"jsonrpc":"2.0","id":0,"result":{"applied":true}}
# CHECK: "id": 4,
# CHECK-NEXT: "jsonrpc": "2.0",
# CHECK-NEXT: "result": "Fix applied."
---
{"jsonrpc":"2.0","id":4,"method":"shutdown"}
---
{"jsonrpc":"2.0","method":"exit"}

43
clang-tools-extra/clangd/test/request-reply.test Normal file
View File

@ -0,0 +1,43 @@
# RUN: clangd -log=verbose -lit-test < %s | FileCheck -strict-whitespace %s
{"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{},"trace":"off"}}
---
{"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"test:///main.cpp","languageId":"cpp","version":1,"text":"auto i = 0;"}}}
---
{"jsonrpc":"2.0","id":4,"method":"workspace/executeCommand","params":{"command":"clangd.applyTweak","arguments":[{"file":"file:///clangd-test/main.cpp","selection":{"end":{"character":4,"line":0},"start":{"character":0,"line":0}},"tweakID":"ExpandAutoType"}]}}
# CHECK: "id": 0,
# CHECK: "method": "workspace/applyEdit",
# CHECK: "newText": "int",
# CHECK-NEXT: "range": {
# CHECK-NEXT: "end": {
# CHECK-NEXT: "character": 4,
# CHECK-NEXT: "line": 0
# CHECK-NEXT: },
# CHECK-NEXT: "start": {
# CHECK-NEXT: "character": 0,
# CHECK-NEXT: "line": 0
# CHECK-NEXT: }
# CHECK-NEXT: }
---
{"jsonrpc":"2.0","id":0,"result":{"applied":false}}
# CHECK: "error": {
# CHECK-NEXT: "code": -32001,
# CHECK-NEXT: "message": "edits were not applied: unknown reason"
# CHECK-NEXT: },
# CHECK-NEXT: "id": 4,
---
{"jsonrpc":"2.0","id":5,"method":"workspace/executeCommand","params":{"command":"clangd.applyTweak","arguments":[{"file":"file:///clangd-test/main.cpp","selection":{"end":{"character":4,"line":0},"start":{"character":0,"line":0}},"tweakID":"ExpandAutoType"}]}}
# CHECK: "id": 1,
# CHECK: "method": "workspace/applyEdit",
---
{"jsonrpc":"2.0","id":1,"result":{"applied":true}}
# CHECK: "id": 5,
# CHECK-NEXT: "jsonrpc": "2.0",
# CHECK-NEXT: "result": "Tweak applied."
---
# a reply with an invalid id.
{"jsonrpc":"2.0","id":"invalid","result":{"applied":true}}
# clangd doesn't reply, just emits an elog.
---
{"jsonrpc":"2.0","id":6,"method":"shutdown"}
---
{"jsonrpc":"2.0","method":"exit"}