[clangd] Send CodeAction responses to textDocument/codeAction (LSP 3.8)

Summary:
I don't bother mirroring the full capabilities struct, just parse the
bits we care about. I'll send a new patch to use this approach elsewhere too.

Reviewers: kadircet

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

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

llvm-svn: 344617

clang-tools-extra/clangd/ClangdLSPServer.cpp

@@ -103,6 +103,8 @@ void ClangdLSPServer::onInitialize(InitializeParams &Params) {
       Params.capabilities.textDocument.publishDiagnostics.clangdFixSupport;
   DiagOpts.SendDiagnosticCategory =
       Params.capabilities.textDocument.publishDiagnostics.categorySupport;
+  SupportsCodeAction =
+      Params.capabilities.textDocument.codeActionLiteralSupport;
 
   if (Params.capabilities.workspace && Params.capabilities.workspace->symbol &&
       Params.capabilities.workspace->symbol->symbolKind &&
@@ -339,29 +341,53 @@ void ClangdLSPServer::onDocumentSymbol(DocumentSymbolParams &Params) {
       });
 }
 
+static Optional<Command> asCommand(const CodeAction &Action) {
+  Command Cmd;
+  if (Action.command && Action.edit)
+    return llvm::None; // Not representable. (We never emit these anyway).
+  if (Action.command) {
+    Cmd = *Action.command;
+  } else if (Action.edit) {
+    Cmd.command = Command::CLANGD_APPLY_FIX_COMMAND;
+    Cmd.workspaceEdit = *Action.edit;
+  } else {
+    return llvm::None;
+  }
+  Cmd.title = Action.title;
+  if (Action.kind && *Action.kind == CodeAction::QUICKFIX_KIND)
+    Cmd.title = "Apply fix: " + Cmd.title;
+  return Cmd;
+}
+
 void ClangdLSPServer::onCodeAction(CodeActionParams &Params) {
-  // We provide a code action for each diagnostic at the requested location
-  // which has FixIts available.
-  auto Code = DraftMgr.getDraft(Params.textDocument.uri.file());
-  if (!Code)
+  // We provide a code action for Fixes on the specified diagnostics.
+  if (!DraftMgr.getDraft(Params.textDocument.uri.file()))
     return replyError(ErrorCode::InvalidParams,
                       "onCodeAction called for non-added file");
 
-  std::vector<Command> Commands;
+  std::vector<CodeAction> Actions;
   for (Diagnostic &D : Params.context.diagnostics) {
     for (auto &F : getFixes(Params.textDocument.uri.file(), D)) {
-      WorkspaceEdit WE;
-      std::vector<TextEdit> Edits(F.Edits.begin(), F.Edits.end());
-      Commands.emplace_back();
-      Commands.back().title = llvm::formatv("Apply fix: {0}", F.Message);
-      Commands.back().command = ExecuteCommandParams::CLANGD_APPLY_FIX_COMMAND;
-      Commands.back().workspaceEdit.emplace();
-      Commands.back().workspaceEdit->changes = {
-          {Params.textDocument.uri.uri(), std::move(Edits)},
-      };
+      Actions.emplace_back();
+      Actions.back().title = F.Message;
+      Actions.back().kind = CodeAction::QUICKFIX_KIND;
+      Actions.back().diagnostics = {D};
+      Actions.back().edit.emplace();
+      Actions.back().edit->changes.emplace();
+      (*Actions.back().edit->changes)[Params.textDocument.uri.uri()] = {
+          F.Edits.begin(), F.Edits.end()};
     }
   }
-  reply(json::Array(Commands));
+
+  if (SupportsCodeAction)
+    reply(json::Array(Actions));
+  else {
+    std::vector<Command> Commands;
+    for (const auto &Action : Actions)
+      if (auto Command = asCommand(Action))
+        Commands.push_back(std::move(*Command));
+    reply(json::Array(Commands));
+  }
 }
 
 void ClangdLSPServer::onCompletion(TextDocumentPositionParams &Params) {

clang-tools-extra/clangd/ClangdLSPServer.h

@@ -166,6 +166,8 @@ private:
   SymbolKindBitset SupportedSymbolKinds;
   /// The supported completion item kinds of the client.
   CompletionItemKindBitset SupportedCompletionItemKinds;
+  // Whether the client supports CodeAction response objects.
+  bool SupportsCodeAction = false;
 
   // Store of the current versions of the open documents.
   DraftStore DraftMgr;

clang-tools-extra/clangd/Protocol.cpp

@@ -251,6 +251,9 @@ bool fromJSON(const json::Value &Params, TextDocumentClientCapabilities &R) {
     return false;
   O.map("completion", R.completion);
   O.map("publishDiagnostics", R.publishDiagnostics);
+  if (auto *CodeAction = Params.getAsObject()->getObject("codeAction"))
+    if (CodeAction->getObject("codeActionLiteralSupport"))
+      R.codeActionLiteralSupport = true;
   return true;
 }
 
@@ -360,6 +363,17 @@ bool fromJSON(const json::Value &Params, DocumentSymbolParams &R) {
   return O && O.map("textDocument", R.textDocument);
 }
 
+llvm::json::Value toJSON(const Diagnostic &D) {
+  json::Object Diag{
+      {"range", D.range},
+      {"severity", D.severity},
+      {"message", D.message},
+  };
+  // FIXME: this should be used for publishDiagnostics.
+  // FIXME: send category and fixes when appropriate.
+  return std::move(Diag);
+}
+
 bool fromJSON(const json::Value &Params, Diagnostic &R) {
   json::ObjectMapper O(Params);
   if (!O || !O.map("range", R.range) || !O.map("message", R.message))
@@ -448,6 +462,21 @@ json::Value toJSON(const Command &C) {
   return std::move(Cmd);
 }
 
+const llvm::StringLiteral CodeAction::QUICKFIX_KIND = "quickfix";
+
+llvm::json::Value toJSON(const CodeAction &CA) {
+  auto CodeAction = json::Object{{"title", CA.title}};
+  if (CA.kind)
+    CodeAction["kind"] = *CA.kind;
+  if (CA.diagnostics)
+    CodeAction["diagnostics"] = json::Array(*CA.diagnostics);
+  if (CA.edit)
+    CodeAction["edit"] = *CA.edit;
+  if (CA.command)
+    CodeAction["command"] = *CA.command;
+  return std::move(CodeAction);
+}
+
 json::Value toJSON(const WorkspaceEdit &WE) {
   if (!WE.changes)
     return json::Object{};

clang-tools-extra/clangd/Protocol.h

@@ -385,6 +385,10 @@ struct TextDocumentClientCapabilities {
 
   /// Capabilities specific to the 'textDocument/publishDiagnostics'
   PublishDiagnosticsClientCapabilities publishDiagnostics;
+
+  /// Flattened from codeAction.codeActionLiteralSupport.
+  // FIXME: flatten other properties in this way.
+  bool codeActionLiteralSupport = false;
 };
 bool fromJSON(const llvm::json::Value &, TextDocumentClientCapabilities &);
 
@@ -613,6 +617,7 @@ struct Diagnostic {
   /// which the issue was produced, e.g. "Semantic Issue" or "Parse Issue".
   std::string category;
 };
+llvm::json::Value toJSON(const Diagnostic &);
 
 /// A LSP-specific comparator used to find diagnostic in a container like
 /// std:map.
@@ -680,6 +685,32 @@ struct Command : public ExecuteCommandParams {
 };
 llvm::json::Value toJSON(const Command &C);
 
+/// A code action represents a change that can be performed in code, e.g. to fix
+/// a problem or to refactor code.
+///
+/// A CodeAction must set either `edit` and/or a `command`. If both are
+/// supplied, the `edit` is applied first, then the `command` is executed.
+struct CodeAction {
+  /// A short, human-readable, title for this code action.
+  std::string title;
+
+  /// The kind of the code action.
+  /// Used to filter code actions.
+  llvm::Optional<std::string> kind;
+  const static llvm::StringLiteral QUICKFIX_KIND;
+
+  /// The diagnostics that this code action resolves.
+  llvm::Optional<std::vector<Diagnostic>> diagnostics;
+
+  /// The workspace edit this code action performs.
+  llvm::Optional<WorkspaceEdit> edit;
+
+  /// A command this code action executes. If a code action provides an edit
+  /// and a command, first the edit is executed and then the command.
+  llvm::Optional<Command> command;
+};
+llvm::json::Value toJSON(const CodeAction &);
+
 /// Represents information about programming constructs like variables, classes,
 /// interfaces etc.
 struct SymbolInformation {

clang-tools-extra/test/clangd/fixits-codeaction.test

@@ -0,0 +1,126 @@
+# RUN: clangd -lit-test < %s | FileCheck -strict-whitespace %s
+{"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{"textDocument":{"codeAction":{"codeActionLiteralSupport":{}}}},"trace":"off"}}
+---
+{"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"test:///foo.c","languageId":"c","version":1,"text":"int main(int i, char **a) { if (i = 2) {}}"}}}
+#      CHECK:    "method": "textDocument/publishDiagnostics",
+# CHECK-NEXT:  "params": {
+# CHECK-NEXT:    "diagnostics": [
+# CHECK-NEXT:      {
+# CHECK-NEXT:        "message": "Using the result of an assignment as a condition without parentheses",
+# CHECK-NEXT:        "range": {
+# CHECK-NEXT:          "end": {
+# CHECK-NEXT:            "character": 37,
+# CHECK-NEXT:            "line": 0
+# CHECK-NEXT:          },
+# CHECK-NEXT:          "start": {
+# CHECK-NEXT:            "character": 32,
+# CHECK-NEXT:            "line": 0
+# CHECK-NEXT:          }
+# CHECK-NEXT:        },
+# CHECK-NEXT:        "severity": 2
+# CHECK-NEXT:      }
+# CHECK-NEXT:    ],
+# CHECK-NEXT:    "uri": "file://{{.*}}/foo.c"
+# CHECK-NEXT:  }
+---
+{"jsonrpc":"2.0","id":2,"method":"textDocument/codeAction","params":{"textDocument":{"uri":"test:///foo.c"},"range":{"start":{"line":104,"character":13},"end":{"line":0,"character":35}},"context":{"diagnostics":[{"range":{"start": {"line": 0, "character": 32}, "end": {"line": 0, "character": 37}},"severity":2,"message":"Using the result of an assignment as a condition without parentheses"}]}}}
+#      CHECK:  "id": 2,
+# CHECK-NEXT:  "jsonrpc": "2.0",
+# CHECK-NEXT:  "result": [
+# CHECK-NEXT:    {
+# CHECK-NEXT:      "diagnostics": [
+# CHECK-NEXT:        {
+# CHECK-NEXT:          "message": "Using the result of an assignment as a condition without parentheses",
+# CHECK-NEXT:          "range": {
+# CHECK-NEXT:            "end": {
+# CHECK-NEXT:              "character": 37,
+# CHECK-NEXT:              "line": 0
+# CHECK-NEXT:            },
+# CHECK-NEXT:            "start": {
+# CHECK-NEXT:              "character": 32,
+# CHECK-NEXT:              "line": 0
+# CHECK-NEXT:            }
+# CHECK-NEXT:          },
+# CHECK-NEXT:          "severity": 2
+# CHECK-NEXT:        }
+# CHECK-NEXT:      ],
+# CHECK-NEXT:      "edit": {
+# CHECK-NEXT:        "changes": {
+# CHECK-NEXT:          "file://{{.*}}/foo.c": [
+# CHECK-NEXT:            {
+# CHECK-NEXT:              "newText": "(",
+# CHECK-NEXT:              "range": {
+# CHECK-NEXT:                "end": {
+# CHECK-NEXT:                  "character": 32,
+# CHECK-NEXT:                  "line": 0
+# CHECK-NEXT:                },
+# CHECK-NEXT:                "start": {
+# CHECK-NEXT:                  "character": 32,
+# CHECK-NEXT:                  "line": 0
+# CHECK-NEXT:                }
+# CHECK-NEXT:              }
+# CHECK-NEXT:            },
+# CHECK-NEXT:            {
+# CHECK-NEXT:              "newText": ")",
+# CHECK-NEXT:              "range": {
+# CHECK-NEXT:                "end": {
+# CHECK-NEXT:                  "character": 37,
+# CHECK-NEXT:                  "line": 0
+# CHECK-NEXT:                },
+# CHECK-NEXT:                "start": {
+# CHECK-NEXT:                  "character": 37,
+# CHECK-NEXT:                  "line": 0
+# CHECK-NEXT:                }
+# CHECK-NEXT:              }
+# CHECK-NEXT:            }
+# CHECK-NEXT:          ]
+# CHECK-NEXT:        }
+# CHECK-NEXT:      },
+# CHECK-NEXT:      "kind": "quickfix",
+# CHECK-NEXT:      "title": "place parentheses around the assignment to silence this warning"
+# CHECK-NEXT:    },
+# CHECK-NEXT:    {
+# CHECK-NEXT:      "diagnostics": [
+# CHECK-NEXT:        {
+# CHECK-NEXT:          "message": "Using the result of an assignment as a condition without parentheses",
+# CHECK-NEXT:          "range": {
+# CHECK-NEXT:            "end": {
+# CHECK-NEXT:              "character": 37,
+# CHECK-NEXT:              "line": 0
+# CHECK-NEXT:            },
+# CHECK-NEXT:            "start": {
+# CHECK-NEXT:              "character": 32,
+# CHECK-NEXT:              "line": 0
+# CHECK-NEXT:            }
+# CHECK-NEXT:          },
+# CHECK-NEXT:          "severity": 2
+# CHECK-NEXT:        }
+# CHECK-NEXT:      ],
+# CHECK-NEXT:      "edit": {
+# CHECK-NEXT:        "changes": {
+# CHECK-NEXT:          "file://{{.*}}/foo.c": [
+# CHECK-NEXT:            {
+# CHECK-NEXT:              "newText": "==",
+# CHECK-NEXT:              "range": {
+# CHECK-NEXT:                "end": {
+# CHECK-NEXT:                  "character": 35,
+# CHECK-NEXT:                  "line": 0
+# CHECK-NEXT:                },
+# CHECK-NEXT:                "start": {
+# CHECK-NEXT:                  "character": 34,
+# CHECK-NEXT:                  "line": 0
+# CHECK-NEXT:                }
+# CHECK-NEXT:              }
+# CHECK-NEXT:            }
+# CHECK-NEXT:          ]
+# CHECK-NEXT:        }
+# CHECK-NEXT:      },
+# CHECK-NEXT:      "kind": "quickfix",
+# CHECK-NEXT:      "title": "use '==' to turn this assignment into an equality comparison"
+# CHECK-NEXT:    }
+# CHECK-NEXT:  ]
+---
+{"jsonrpc":"2.0","id":4,"method":"shutdown"}
+---
+{"jsonrpc":"2.0","method":"exit"}
+