[clangd] Move command handlers into a map in ClangdLSPServer. NFC

[kadircet]

thanks, lgtm!

clang-tools-extra/clangd/ClangdLSPServer.cpp

589

nit: llvm::append_range(Commands, CommandHandlers.keys())

651

nit: std::move(Commands)

clang-tools-extra/clangd/Protocol.h

916–917

can we also move these into ClangdLSPServer now ?

[thakis]

Looks like this doesn't build on Windows: http://45.33.8.238/win/33161/step_4.txt (clang-cl + msvc stdlib)

clang-tools-extra/clangd/ClangdLSPServer.h

private:
   void onGoToImplementation(const TextDocumentPositionParams &,
                             Callback<std::vector<Location>>);
   void onReference(const ReferenceParams &, Callback<std::vector<Location>>);
   void onSwitchSourceHeader(const TextDocumentIdentifier &,
                             Callback<llvm::Optional<URIForFile>>);
   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 &,
                        Callback<llvm::Optional<Range>>);
   void onRename(const RenameParams &, Callback<WorkspaceEdit>);
   void onHover(const TextDocumentPositionParams &,
                Callback<llvm::Optional<Hover>>);
   void onTypeHierarchy(const TypeHierarchyParams &,
void onDocumentLink(const DocumentLinkParams &,
                       Callback<std::vector<DocumentLink>>);
   void onSemanticTokens(const SemanticTokensParams &, Callback<SemanticTokens>);
   void onSemanticTokensDelta(const SemanticTokensDeltaParams &,
                              Callback<SemanticTokensOrDelta>);
   /// This is a clangd extension. Provides a json tree representing memory usage
   /// 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
   /// limitation of the LSP. Per LSP, a client sends requests for all "trigger
   /// character" we specify, but for '>' and ':' we need to check they actually
   /// produce '->' and '::', respectively.
   bool shouldRunCompletion(const CompletionParams &Params) const;
 
void callRaw(StringRef Method, llvm::json::Value Params,
                Callback<llvm::json::Value> CB);
   void notify(StringRef Method, llvm::json::Value Params);
   template <typename T> void progress(const llvm::json::Value &Token, T Value) {
     ProgressParams<T> Params;
     Params.token = Token;
     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.
   ClangdDiagnosticOptions DiagOpts;
   /// The supported kinds of the client.
   SymbolKindBitset SupportedSymbolKinds;
   /// The supported completion item kinds of the client.
   CompletionItemKindBitset SupportedCompletionItemKinds;

clang-tools-extra/clangd/ClangdLSPServer.cpp

 #include <memory>
 #include <mutex>
 #include <string>
 #include <vector>
 
 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,
                                    "method_name");
 
 // LSP defines file versions as numbers that increase.
 // ClangdServer treats them as opaque and therefore uses strings instead.
 std::string encodeVersion(int64_t LSPVersion) {
   return llvm::to_string(LSPVersion);
 }
 llvm::Optional<int64_t> decodeVersion(llvm::StringRef Encoded) {
   int64_t Result;
   if (llvm::to_integer(Encoded, Result, 10))
     return Result;
   if (!Encoded.empty()) // Empty can be e.g. diagnostics on close.
     elog("unexpected non-numeric version {0}", 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,
                         Range Selection) {
   CodeAction CA;
   CA.title = T.Title;
   CA.kind = T.Kind.str();
   // This tweak may have an expensive second stage, we only run it if the user
   // actually chooses it in the UI. We reply with a command that would run the
   // corresponding tweak.
   // FIXME: for some tweaks, computing the edits is cheap and we could send them
   //        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;
 }
 
 void adjustSymbolKinds(llvm::MutableArrayRef<DocumentSymbol> Syms,
                        SymbolKindBitset Kinds) {
   for (auto &S : Syms) {
     S.kind = adjustKindToCapability(S.kind, Kinds);
     adjustSymbolKinds(S.children, Kinds);
void ClangdLSPServer::onInitialize(const InitializeParams &Params,
   // via textDocument.codeAction.codeActionLiteralSupport.
   llvm::json::Value CodeActionProvider = true;
   if (Params.capabilities.CodeActionStructure)
     CodeActionProvider = llvm::json::Object{
         {"codeActionKinds",
          {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"},
                            {"version", getClangToolFullVersion("clangd")}}},
        {"capabilities",
         llvm::json::Object{
             {"textDocumentSync",
              llvm::json::Object{
llvm::json::Object Result{
             {"hoverProvider", true},
             {"renameProvider", std::move(RenameProvider)},
             {"selectionRangeProvider", true},
             {"documentSymbolProvider", true},
             {"workspaceSymbolProvider", true},
             {"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
              llvm::json::Object{{"automaticReload", true}}},
             {"callHierarchyProvider", true},
         }}}};
   if (Opts.Encoding)
     Result["offsetEncoding"] = *Opts.Encoding;
void ClangdLSPServer::onFileEvent(const DidChangeWatchedFilesParams &Params) {
   // FIXME: observe config files, immediately expire time-based caches, reparse:
   //  - compile_commands.json and compile_flags.txt
   //  - .clang_format and .clang-tidy
   //  - .clangd and clangd/config.yaml
 }
 
 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);
-        });
-  };
+  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));
 
-  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"));
+void ClangdLSPServer::onCommandApplyEdit(const WorkspaceEdit &WE,
+                                         Callback<llvm::json::Value> Reply) {
+  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());
 
-    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());
+  auto Action = [this, Reply = std::move(Reply), File = Args.file,
+                 Code = std::move(*Code)](
 
-      assert(R->ShowMessage ||
-             (!R->ApplyEdits.empty() && "tweak has no effect"));
+    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>(
-        llvm::formatv("Unsupported command \"{0}\".", Params.command).str(),
-        ErrorCode::InvalidParams));
-  }
+    return applyEdit(std::move(WE), "Tweak applied.", std::move(Reply));
+  Server->applyTweak(Args.file.file(), Args.selection, Args.tweakID,
+
+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(
     const WorkspaceSymbolParams &Params,
     Callback<std::vector<SymbolInformation>> Reply) {
   Server->workspaceSymbols(
       Params.query, Opts.CodeComplete.Limit,
       [Reply = std::move(Reply),

 
 static llvm::Optional<Command> asCommand(const CodeAction &Action) {
   Command Cmd;
   if (Action.command && Action.edit)
     return None; // Not representable. (We never emit these anyway).
   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;
   }
   Cmd.title = Action.title;
   if (Action.kind && *Action.kind == CodeAction::QUICKFIX_KIND)
     Cmd.title = "Apply fix: " + Cmd.title;
   return Cmd;
 }
ClangdLSPServer::ClangdLSPServer(class Transport &Transp,
   MsgHandler->bind("callHierarchy/outgoingCalls", &ClangdLSPServer::onCallHierarchyOutgoingCalls);
   MsgHandler->bind("textDocument/selectionRange", &ClangdLSPServer::onSelectionRange);
   MsgHandler->bind("textDocument/documentLink", &ClangdLSPServer::onDocumentLink);
   MsgHandler->bind("textDocument/semanticTokens/full", &ClangdLSPServer::onSemanticTokens);
   MsgHandler->bind("textDocument/semanticTokens/full/delta", &ClangdLSPServer::onSemanticTokensDelta);
   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
 }
 
 ClangdLSPServer::~ClangdLSPServer() {
   IsBeingDestroyed = true;
   // Explicitly destroy ClangdServer first, blocking on threads it owns.
   // This ensures they don't access any other members.
   Server.reset();

clang-tools-extra/clangd/Protocol.h

struct TweakArgs {
   /// A selection provided by the client on a textDocument/codeAction request.
   Range selection;
   /// ID of the tweak that should be executed. Corresponds to Tweak::id().
   std::string tweakID;
 };
 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> 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);
 
 struct Command : public ExecuteCommandParams {
   std::string title;
 };
 llvm::json::Value toJSON(const Command &C);

clang-tools-extra/clangd/Protocol.cpp

 }
 
 bool fromJSON(const llvm::json::Value &Params, WorkspaceEdit &R,
               llvm::json::Path P) {
   llvm::json::ObjectMapper O(Params, P);
   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));
-  }
-  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.
+  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 (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) {
   llvm::json::Object O{
       {"name", P.name},
       {"kind", static_cast<int>(P.kind)},
       {"location", P.location},
       {"containerName", P.containerName},

 bool fromJSON(const llvm::json::Value &Params, WorkspaceSymbolParams &R,
               llvm::json::Path P) {
   llvm::json::ObjectMapper O(Params, P);
   return O && O.map("query", R.query);
 }
 
 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);
 }
 
 const llvm::StringLiteral CodeAction::QUICKFIX_KIND = "quickfix";
 const llvm::StringLiteral CodeAction::REFACTOR_KIND = "refactor";
 const llvm::StringLiteral CodeAction::INFO_KIND = "info";
 
 llvm::json::Value toJSON(const CodeAction &CA) {