[clang-tools-extra] cea9f05 - [clangd] Move command handlers into a map in ClangdLSPServer. NFC
Sam McCall via cfe-commits
cfe-commits at lists.llvm.org
Fri Feb 12 06:57:50 PST 2021
Author: Sam McCall
Date: 2021-02-12T15:57:43+01:00
New Revision: cea9f054327be2eb83093f0202a7814b904f1076
URL: https://github.com/llvm/llvm-project/commit/cea9f054327be2eb83093f0202a7814b904f1076
DIFF: https://github.com/llvm/llvm-project/commit/cea9f054327be2eb83093f0202a7814b904f1076.diff
LOG: [clangd] Move command handlers into a map in ClangdLSPServer. NFC
Differential Revision: https://reviews.llvm.org/D96507
Added:
Modified:
clang-tools-extra/clangd/ClangdLSPServer.cpp
clang-tools-extra/clangd/ClangdLSPServer.h
clang-tools-extra/clangd/Protocol.cpp
clang-tools-extra/clangd/Protocol.h
Removed:
################################################################################
diff --git a/clang-tools-extra/clangd/ClangdLSPServer.cpp b/clang-tools-extra/clangd/ClangdLSPServer.cpp
index 4263edb8cd6a..2aca82ee09d5 100644
--- a/clang-tools-extra/clangd/ClangdLSPServer.cpp
+++ b/clang-tools-extra/clangd/ClangdLSPServer.cpp
@@ -51,7 +51,6 @@
namespace clang {
namespace clangd {
namespace {
-
// Tracks end-to-end latency of high level lsp calls. Measurements are in
// seconds.
constexpr trace::Metric LSPLatency("lsp_latency", trace::Metric::Distribution,
@@ -71,6 +70,9 @@ llvm::Optional<int64_t> decodeVersion(llvm::StringRef Encoded) {
return llvm::None;
}
+const llvm::StringLiteral APPLY_FIX_COMMAND = "clangd.applyFix";
+const llvm::StringLiteral APPLY_TWEAK_COMMAND = "clangd.applyTweak";
+
/// Transforms a tweak into a code action that would apply it if executed.
/// EXPECTS: T.prepare() was called and returned true.
CodeAction toCodeAction(const ClangdServer::TweakRef &T, const URIForFile &File,
@@ -85,11 +87,12 @@ CodeAction toCodeAction(const ClangdServer::TweakRef &T, const URIForFile &File,
// directly.
CA.command.emplace();
CA.command->title = T.Title;
- CA.command->command = std::string(Command::CLANGD_APPLY_TWEAK);
- CA.command->tweakArgs.emplace();
- CA.command->tweakArgs->file = File;
- CA.command->tweakArgs->tweakID = T.ID;
- CA.command->tweakArgs->selection = Selection;
+ CA.command->command = std::string(APPLY_TWEAK_COMMAND);
+ TweakArgs Args;
+ Args.file = File;
+ Args.tweakID = T.ID;
+ Args.selection = Selection;
+ CA.command->argument = std::move(Args);
return CA;
}
@@ -582,6 +585,10 @@ void ClangdLSPServer::onInitialize(const InitializeParams &Params,
{CodeAction::QUICKFIX_KIND, CodeAction::REFACTOR_KIND,
CodeAction::INFO_KIND}}};
+ std::vector<llvm::StringRef> Commands;
+ llvm::append_range(Commands, CommandHandlers.keys());
+ llvm::sort(Commands);
+
llvm::json::Object Result{
{{"serverInfo",
llvm::json::Object{{"name", "clangd"},
@@ -641,11 +648,7 @@ void ClangdLSPServer::onInitialize(const InitializeParams &Params,
{"referencesProvider", true},
{"astProvider", true}, // clangd extension
{"executeCommandProvider",
- llvm::json::Object{
- {"commands",
- {ExecuteCommandParams::CLANGD_APPLY_FIX_COMMAND,
- ExecuteCommandParams::CLANGD_APPLY_TWEAK}},
- }},
+ llvm::json::Object{{"commands", Commands}}},
{"typeHierarchyProvider", true},
{"memoryUsageProvider", true}, // clangd extension
{"compilationDatabase", // clangd extension
@@ -730,85 +733,86 @@ void ClangdLSPServer::onFileEvent(const DidChangeWatchedFilesParams &Params) {
void ClangdLSPServer::onCommand(const ExecuteCommandParams &Params,
Callback<llvm::json::Value> Reply) {
- auto ApplyEdit = [this](WorkspaceEdit WE, std::string SuccessMessage,
- decltype(Reply) Reply) {
- ApplyWorkspaceEditParams Edit;
- Edit.edit = std::move(WE);
- call<ApplyWorkspaceEditResponse>(
- "workspace/applyEdit", std::move(Edit),
- [Reply = std::move(Reply), SuccessMessage = std::move(SuccessMessage)](
- llvm::Expected<ApplyWorkspaceEditResponse> Response) mutable {
- if (!Response)
- return Reply(Response.takeError());
- if (!Response->applied) {
- std::string Reason = Response->failureReason
- ? *Response->failureReason
- : "unknown reason";
- return Reply(error("edits were not applied: {0}", Reason));
- }
- return Reply(SuccessMessage);
- });
- };
-
- if (Params.command == ExecuteCommandParams::CLANGD_APPLY_FIX_COMMAND &&
- Params.workspaceEdit) {
- // The flow for "apply-fix" :
- // 1. We publish a diagnostic, including fixits
- // 2. The user clicks on the diagnostic, the editor asks us for code actions
- // 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
- // 7. We unwrap the reply and send a reply to the editor.
- ApplyEdit(*Params.workspaceEdit, "Fix applied.", std::move(Reply));
- } else if (Params.command == ExecuteCommandParams::CLANGD_APPLY_TWEAK &&
- Params.tweakArgs) {
- auto Code = DraftMgr.getDraft(Params.tweakArgs->file.file());
- if (!Code)
- return Reply(error("trying to apply a code action for a non-added file"));
-
- auto Action = [this, ApplyEdit, Reply = std::move(Reply),
- File = Params.tweakArgs->file, Code = std::move(*Code)](
- llvm::Expected<Tweak::Effect> R) mutable {
- if (!R)
- return Reply(R.takeError());
-
- assert(R->ShowMessage ||
- (!R->ApplyEdits.empty() && "tweak has no effect"));
-
- if (R->ShowMessage) {
- ShowMessageParams Msg;
- Msg.message = *R->ShowMessage;
- Msg.type = MessageType::Info;
- notify("window/showMessage", Msg);
- }
- // When no edit is specified, make sure we Reply().
- if (R->ApplyEdits.empty())
- return Reply("Tweak applied.");
-
- if (auto Err = validateEdits(DraftMgr, R->ApplyEdits))
- return Reply(std::move(Err));
-
- WorkspaceEdit WE;
- WE.changes.emplace();
- for (const auto &It : R->ApplyEdits) {
- (*WE.changes)[URI::createFile(It.first()).toString()] =
- It.second.asTextEdits();
- }
- // ApplyEdit will take care of calling Reply().
- return ApplyEdit(std::move(WE), "Tweak applied.", std::move(Reply));
- };
- Server->applyTweak(Params.tweakArgs->file.file(),
- Params.tweakArgs->selection, Params.tweakArgs->tweakID,
- std::move(Action));
- } else {
- // We should not get here because ExecuteCommandParams would not have
- // parsed in the first place and this handler should not be called. But if
- // more commands are added, this will be here has a safe guard.
- Reply(llvm::make_error<LSPError>(
+ auto It = CommandHandlers.find(Params.command);
+ if (It == CommandHandlers.end()) {
+ return Reply(llvm::make_error<LSPError>(
llvm::formatv("Unsupported command \"{0}\".", Params.command).str(),
ErrorCode::InvalidParams));
}
+ It->second(Params.argument, std::move(Reply));
+}
+
+void ClangdLSPServer::onCommandApplyEdit(const WorkspaceEdit &WE,
+ Callback<llvm::json::Value> Reply) {
+ // The flow for "apply-fix" :
+ // 1. We publish a diagnostic, including fixits
+ // 2. The user clicks on the diagnostic, the editor asks us for code actions
+ // 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
+ // 7. We unwrap the reply and send a reply to the editor.
+ applyEdit(WE, "Fix applied.", std::move(Reply));
+}
+
+void ClangdLSPServer::onCommandApplyTweak(const TweakArgs &Args,
+ Callback<llvm::json::Value> Reply) {
+ auto Code = DraftMgr.getDraft(Args.file.file());
+ if (!Code)
+ return Reply(error("trying to apply a code action for a non-added file"));
+
+ auto Action = [this, Reply = std::move(Reply), File = Args.file,
+ Code = std::move(*Code)](
+ llvm::Expected<Tweak::Effect> R) mutable {
+ if (!R)
+ return Reply(R.takeError());
+
+ assert(R->ShowMessage || (!R->ApplyEdits.empty() && "tweak has no effect"));
+
+ if (R->ShowMessage) {
+ ShowMessageParams Msg;
+ Msg.message = *R->ShowMessage;
+ Msg.type = MessageType::Info;
+ notify("window/showMessage", Msg);
+ }
+ // When no edit is specified, make sure we Reply().
+ if (R->ApplyEdits.empty())
+ return Reply("Tweak applied.");
+
+ if (auto Err = validateEdits(DraftMgr, R->ApplyEdits))
+ return Reply(std::move(Err));
+
+ WorkspaceEdit WE;
+ WE.changes.emplace();
+ for (const auto &It : R->ApplyEdits) {
+ (*WE.changes)[URI::createFile(It.first()).toString()] =
+ It.second.asTextEdits();
+ }
+ // ApplyEdit will take care of calling Reply().
+ return applyEdit(std::move(WE), "Tweak applied.", std::move(Reply));
+ };
+ Server->applyTweak(Args.file.file(), Args.selection, Args.tweakID,
+ std::move(Action));
+}
+
+void ClangdLSPServer::applyEdit(WorkspaceEdit WE, llvm::json::Value Success,
+ Callback<llvm::json::Value> Reply) {
+ ApplyWorkspaceEditParams Edit;
+ Edit.edit = std::move(WE);
+ call<ApplyWorkspaceEditResponse>(
+ "workspace/applyEdit", std::move(Edit),
+ [Reply = std::move(Reply), SuccessMessage = std::move(Success)](
+ llvm::Expected<ApplyWorkspaceEditResponse> Response) mutable {
+ if (!Response)
+ return Reply(Response.takeError());
+ if (!Response->applied) {
+ std::string Reason = Response->failureReason
+ ? *Response->failureReason
+ : "unknown reason";
+ return Reply(error("edits were not applied: {0}", Reason));
+ }
+ return Reply(SuccessMessage);
+ });
}
void ClangdLSPServer::onWorkspaceSymbol(
@@ -998,8 +1002,8 @@ static llvm::Optional<Command> asCommand(const CodeAction &Action) {
if (Action.command) {
Cmd = *Action.command;
} else if (Action.edit) {
- Cmd.command = std::string(Command::CLANGD_APPLY_FIX_COMMAND);
- Cmd.workspaceEdit = *Action.edit;
+ Cmd.command = std::string(APPLY_FIX_COMMAND);
+ Cmd.argument = *Action.edit;
} else {
return None;
}
@@ -1530,6 +1534,8 @@ ClangdLSPServer::ClangdLSPServer(class Transport &Transp,
MsgHandler->bind("$/memoryUsage", &ClangdLSPServer::onMemoryUsage);
if (Opts.FoldingRanges)
MsgHandler->bind("textDocument/foldingRange", &ClangdLSPServer::onFoldingRange);
+ bindCommand(APPLY_FIX_COMMAND, &ClangdLSPServer::onCommandApplyEdit);
+ bindCommand(APPLY_TWEAK_COMMAND, &ClangdLSPServer::onCommandApplyTweak);
// clang-format on
}
diff --git a/clang-tools-extra/clangd/ClangdLSPServer.h b/clang-tools-extra/clangd/ClangdLSPServer.h
index d8863a7c67e8..ef99acc02866 100644
--- a/clang-tools-extra/clangd/ClangdLSPServer.h
+++ b/clang-tools-extra/clangd/ClangdLSPServer.h
@@ -126,7 +126,6 @@ class ClangdLSPServer : private ClangdServer::Callbacks {
void onDocumentHighlight(const TextDocumentPositionParams &,
Callback<std::vector<DocumentHighlight>>);
void onFileEvent(const DidChangeWatchedFilesParams &);
- void onCommand(const ExecuteCommandParams &, Callback<llvm::json::Value>);
void onWorkspaceSymbol(const WorkspaceSymbolParams &,
Callback<std::vector<SymbolInformation>>);
void onPrepareRename(const TextDocumentPositionParams &,
@@ -160,6 +159,18 @@ class ClangdLSPServer : private ClangdServer::Callbacks {
/// hierarchy.
void onMemoryUsage(Callback<MemoryTree>);
+ llvm::StringMap<llvm::unique_function<void(const llvm::json::Value &,
+ Callback<llvm::json::Value>)>>
+ CommandHandlers;
+ void onCommand(const ExecuteCommandParams &, Callback<llvm::json::Value>);
+
+ /// Implement commands.
+ void onCommandApplyEdit(const WorkspaceEdit &, Callback<llvm::json::Value>);
+ void onCommandApplyTweak(const TweakArgs &, Callback<llvm::json::Value>);
+
+ void applyEdit(WorkspaceEdit WE, llvm::json::Value Success,
+ Callback<llvm::json::Value> Reply);
+
std::vector<Fix> getFixes(StringRef File, const clangd::Diagnostic &D);
/// Checks if completion request should be ignored. We need this due to the
@@ -263,6 +274,19 @@ class ClangdLSPServer : private ClangdServer::Callbacks {
Params.value = std::move(Value);
notify("$/progress", Params);
}
+ template <typename Param, typename Result>
+ void bindCommand(llvm::StringLiteral Method,
+ void (ClangdLSPServer::*Handler)(const Param &,
+ Callback<Result>)) {
+ CommandHandlers[Method] = [Method, Handler,
+ this](llvm::json::Value RawParams,
+ Callback<Result> Reply) {
+ auto P = parse<Param>(RawParams, Method, "command");
+ if (!P)
+ return Reply(P.takeError());
+ (this->*Handler)(*P, std::move(Reply));
+ };
+ }
const ThreadsafeFS &TFS;
/// Options used for diagnostics.
diff --git a/clang-tools-extra/clangd/Protocol.cpp b/clang-tools-extra/clangd/Protocol.cpp
index a1bf136ff6b0..74e6b8c72a1c 100644
--- a/clang-tools-extra/clangd/Protocol.cpp
+++ b/clang-tools-extra/clangd/Protocol.cpp
@@ -655,27 +655,27 @@ bool fromJSON(const llvm::json::Value &Params, WorkspaceEdit &R,
return O && O.map("changes", R.changes);
}
-const llvm::StringLiteral ExecuteCommandParams::CLANGD_APPLY_FIX_COMMAND =
- "clangd.applyFix";
-const llvm::StringLiteral ExecuteCommandParams::CLANGD_APPLY_TWEAK =
- "clangd.applyTweak";
-
bool fromJSON(const llvm::json::Value &Params, ExecuteCommandParams &R,
llvm::json::Path P) {
llvm::json::ObjectMapper O(Params, P);
if (!O || !O.map("command", R.command))
return false;
- const auto *Args = Params.getAsObject()->getArray("arguments");
- if (R.command == ExecuteCommandParams::CLANGD_APPLY_FIX_COMMAND) {
- return Args && Args->size() == 1 &&
- fromJSON(Args->front(), R.workspaceEdit,
- P.field("arguments").index(0));
+ const auto *Args = Params.getAsObject()->get("arguments");
+ if (!Args)
+ return true; // Missing args is ok, argument is null.
+ const auto *ArgsArray = Args->getAsArray();
+ if (!ArgsArray) {
+ P.field("arguments").report("expected array");
+ return false;
}
- if (R.command == ExecuteCommandParams::CLANGD_APPLY_TWEAK)
- return Args && Args->size() == 1 &&
- fromJSON(Args->front(), R.tweakArgs, P.field("arguments").index(0));
- return false; // Unrecognized command.
+ if (ArgsArray->size() > 1) {
+ P.field("arguments").report("Command should have 0 or 1 argument");
+ return false;
+ } else if (ArgsArray->size() == 1) {
+ R.argument = ArgsArray->front();
+ }
+ return true;
}
llvm::json::Value toJSON(const SymbolInformation &P) {
@@ -743,10 +743,8 @@ bool fromJSON(const llvm::json::Value &Params, WorkspaceSymbolParams &R,
llvm::json::Value toJSON(const Command &C) {
auto Cmd = llvm::json::Object{{"title", C.title}, {"command", C.command}};
- if (C.workspaceEdit)
- Cmd["arguments"] = {*C.workspaceEdit};
- if (C.tweakArgs)
- Cmd["arguments"] = {*C.tweakArgs};
+ if (!C.argument.getAsNull())
+ Cmd["arguments"] = llvm::json::Array{C.argument};
return std::move(Cmd);
}
diff --git a/clang-tools-extra/clangd/Protocol.h b/clang-tools-extra/clangd/Protocol.h
index 59dc4f209b34..922e701d77ef 100644
--- a/clang-tools-extra/clangd/Protocol.h
+++ b/clang-tools-extra/clangd/Protocol.h
@@ -913,26 +913,13 @@ struct TweakArgs {
bool fromJSON(const llvm::json::Value &, TweakArgs &, llvm::json::Path);
llvm::json::Value toJSON(const TweakArgs &A);
-/// Exact commands are not specified in the protocol so we define the
-/// ones supported by Clangd here. The protocol specifies the command arguments
-/// to be "any[]" but to make this safer and more manageable, each command we
-/// handle maps to a certain llvm::Optional of some struct to contain its
-/// arguments. Different commands could reuse the same llvm::Optional as
-/// arguments but a command that needs
diff erent arguments would simply add a
-/// new llvm::Optional and not use any other ones. In practice this means only
-/// one argument type will be parsed and set.
struct ExecuteCommandParams {
- // Command to apply fix-its. Uses WorkspaceEdit as argument.
- const static llvm::StringLiteral CLANGD_APPLY_FIX_COMMAND;
- // Command to apply the code action. Uses TweakArgs as argument.
- const static llvm::StringLiteral CLANGD_APPLY_TWEAK;
-
- /// The command identifier, e.g. CLANGD_APPLY_FIX_COMMAND
+ /// The identifier of the actual command handler.
std::string command;
- // Arguments
- llvm::Optional<WorkspaceEdit> workspaceEdit;
- llvm::Optional<TweakArgs> tweakArgs;
+ // This is `arguments?: []any` in LSP.
+ // All clangd's commands accept a single argument (or none => null).
+ llvm::json::Value argument = nullptr;
};
bool fromJSON(const llvm::json::Value &, ExecuteCommandParams &,
llvm::json::Path);
More information about the cfe-commits
mailing list