From 20841d41e762293b319b1a69b9f3cf72052fd9ee Mon Sep 17 00:00:00 2001 From: Sam McCall Date: Tue, 16 Oct 2018 16:29:41 +0000 Subject: [PATCH] [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 | 56 ++++++--- clang-tools-extra/clangd/ClangdLSPServer.h | 2 + clang-tools-extra/clangd/Protocol.cpp | 29 +++++ clang-tools-extra/clangd/Protocol.h | 31 +++++ .../test/clangd/fixits-codeaction.test | 126 +++++++++++++++++++++ .../clangd/{fixits.test => fixits-command.test} | 0 6 files changed, 229 insertions(+), 15 deletions(-) create mode 100644 clang-tools-extra/test/clangd/fixits-codeaction.test rename clang-tools-extra/test/clangd/{fixits.test => fixits-command.test} (100%) diff --git a/clang-tools-extra/clangd/ClangdLSPServer.cpp b/clang-tools-extra/clangd/ClangdLSPServer.cpp index 021c287..b186306 100644 --- a/clang-tools-extra/clangd/ClangdLSPServer.cpp +++ b/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 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 Commands; + std::vector Actions; for (Diagnostic &D : Params.context.diagnostics) { for (auto &F : getFixes(Params.textDocument.uri.file(), D)) { - WorkspaceEdit WE; - std::vector 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 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) { diff --git a/clang-tools-extra/clangd/ClangdLSPServer.h b/clang-tools-extra/clangd/ClangdLSPServer.h index f77b24b..6fb5248 100644 --- a/clang-tools-extra/clangd/ClangdLSPServer.h +++ b/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; diff --git a/clang-tools-extra/clangd/Protocol.cpp b/clang-tools-extra/clangd/Protocol.cpp index ced7cf2..daab132 100644 --- a/clang-tools-extra/clangd/Protocol.cpp +++ b/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{}; diff --git a/clang-tools-extra/clangd/Protocol.h b/clang-tools-extra/clangd/Protocol.h index 51eb707..7026d47 100644 --- a/clang-tools-extra/clangd/Protocol.h +++ b/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 kind; + const static llvm::StringLiteral QUICKFIX_KIND; + + /// The diagnostics that this code action resolves. + llvm::Optional> diagnostics; + + /// The workspace edit this code action performs. + llvm::Optional 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; +}; +llvm::json::Value toJSON(const CodeAction &); + /// Represents information about programming constructs like variables, classes, /// interfaces etc. struct SymbolInformation { diff --git a/clang-tools-extra/test/clangd/fixits-codeaction.test b/clang-tools-extra/test/clangd/fixits-codeaction.test new file mode 100644 index 0000000..97dd4ff --- /dev/null +++ b/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"} + diff --git a/clang-tools-extra/test/clangd/fixits.test b/clang-tools-extra/test/clangd/fixits-command.test similarity index 100% rename from clang-tools-extra/test/clangd/fixits.test rename to clang-tools-extra/test/clangd/fixits-command.test -- 2.7.4