From cea9f054327be2eb83093f0202a7814b904f1076 Mon Sep 17 00:00:00 2001 From: Sam McCall Date: Thu, 11 Feb 2021 16:32:09 +0100 Subject: [PATCH] [clangd] Move command handlers into a map in ClangdLSPServer. NFC Differential Revision: https://reviews.llvm.org/D96507 --- clang-tools-extra/clangd/ClangdLSPServer.cpp | 184 ++++++++++++++------------- clang-tools-extra/clangd/ClangdLSPServer.h | 26 +++- clang-tools-extra/clangd/Protocol.cpp | 34 +++-- clang-tools-extra/clangd/Protocol.h | 21 +-- 4 files changed, 140 insertions(+), 125 deletions(-) diff --git a/clang-tools-extra/clangd/ClangdLSPServer.cpp b/clang-tools-extra/clangd/ClangdLSPServer.cpp index 4263edb..2aca82e 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 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 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 Reply) { - auto ApplyEdit = [this](WorkspaceEdit WE, std::string SuccessMessage, - decltype(Reply) Reply) { - ApplyWorkspaceEditParams Edit; - Edit.edit = std::move(WE); - call( - "workspace/applyEdit", std::move(Edit), - [Reply = std::move(Reply), SuccessMessage = std::move(SuccessMessage)]( - llvm::Expected 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 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( + auto It = CommandHandlers.find(Params.command); + if (It == CommandHandlers.end()) { + return Reply(llvm::make_error( llvm::formatv("Unsupported command \"{0}\".", Params.command).str(), ErrorCode::InvalidParams)); } + It->second(Params.argument, std::move(Reply)); +} + +void ClangdLSPServer::onCommandApplyEdit(const WorkspaceEdit &WE, + Callback 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 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 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 Reply) { + ApplyWorkspaceEditParams Edit; + Edit.edit = std::move(WE); + call( + "workspace/applyEdit", std::move(Edit), + [Reply = std::move(Reply), SuccessMessage = std::move(Success)]( + llvm::Expected 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 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 d8863a7..ef99acc 100644 --- a/clang-tools-extra/clangd/ClangdLSPServer.h +++ b/clang-tools-extra/clangd/ClangdLSPServer.h @@ -126,7 +126,6 @@ private: void onDocumentHighlight(const TextDocumentPositionParams &, Callback>); void onFileEvent(const DidChangeWatchedFilesParams &); - void onCommand(const ExecuteCommandParams &, Callback); void onWorkspaceSymbol(const WorkspaceSymbolParams &, Callback>); void onPrepareRename(const TextDocumentPositionParams &, @@ -160,6 +159,18 @@ private: /// hierarchy. void onMemoryUsage(Callback); + llvm::StringMap)>> + CommandHandlers; + void onCommand(const ExecuteCommandParams &, Callback); + + /// Implement commands. + void onCommandApplyEdit(const WorkspaceEdit &, Callback); + void onCommandApplyTweak(const TweakArgs &, Callback); + + void applyEdit(WorkspaceEdit WE, llvm::json::Value Success, + Callback Reply); + std::vector getFixes(StringRef File, const clangd::Diagnostic &D); /// Checks if completion request should be ignored. We need this due to the @@ -263,6 +274,19 @@ private: Params.value = std::move(Value); notify("$/progress", Params); } + template + void bindCommand(llvm::StringLiteral Method, + void (ClangdLSPServer::*Handler)(const Param &, + Callback)) { + CommandHandlers[Method] = [Method, Handler, + this](llvm::json::Value RawParams, + Callback Reply) { + auto P = parse(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 a1bf136..74e6b8c 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 59dc4f2..922e701 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 different 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; - llvm::Optional 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); -- 2.7.4