[clang-tools-extra] r345119 - [clangd] Embed fixes as CodeAction, instead of clangd_fixes. Clean up serialization.
Sam McCall via cfe-commits
cfe-commits at lists.llvm.org
Wed Oct 24 00:59:38 PDT 2018
Author: sammccall
Date: Wed Oct 24 00:59:38 2018
New Revision: 345119
URL: http://llvm.org/viewvc/llvm-project?rev=345119&view=rev
Log:
[clangd] Embed fixes as CodeAction, instead of clangd_fixes. Clean up serialization.
Summary:
CodeAction provides us with a standard way of representing fixes inline, so
use it, replacing our existing ad-hoc extension.
After this, it's easy to serialize diagnostics using the structured
toJSON/Protocol.h mechanism rather than assembling JSON ad-hoc.
Reviewers: hokein, arphaman
Subscribers: ilya-biryukov, ioeric, MaskRay, jkorous, kadircet, cfe-commits
Differential Revision: https://reviews.llvm.org/D53391
Modified:
clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
clang-tools-extra/trunk/clangd/Diagnostics.cpp
clang-tools-extra/trunk/clangd/Diagnostics.h
clang-tools-extra/trunk/clangd/Protocol.cpp
clang-tools-extra/trunk/clangd/Protocol.h
clang-tools-extra/trunk/test/clangd/fixits-embed-in-diagnostic.test
clang-tools-extra/trunk/unittests/clangd/ClangdUnitTests.cpp
Modified: clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp?rev=345119&r1=345118&r2=345119&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp (original)
+++ clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp Wed Oct 24 00:59:38 2018
@@ -549,14 +549,8 @@ void ClangdLSPServer::onCodeAction(const
std::vector<CodeAction> Actions;
for (const Diagnostic &D : Params.context.diagnostics) {
for (auto &F : getFixes(Params.textDocument.uri.file(), D)) {
- Actions.emplace_back();
- Actions.back().title = F.Message;
- Actions.back().kind = CodeAction::QUICKFIX_KIND;
+ Actions.push_back(toCodeAction(F, Params.textDocument.uri));
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()};
}
}
@@ -724,36 +718,16 @@ std::vector<Fix> ClangdLSPServer::getFix
void ClangdLSPServer::onDiagnosticsReady(PathRef File,
std::vector<Diag> Diagnostics) {
- json::Array DiagnosticsJSON;
-
+ URIForFile URI(File);
+ std::vector<Diagnostic> LSPDiagnostics;
DiagnosticToReplacementMap LocalFixIts; // Temporary storage
for (auto &Diag : Diagnostics) {
- toLSPDiags(Diag, [&](clangd::Diagnostic Diag, ArrayRef<Fix> Fixes) {
- json::Object LSPDiag({
- {"range", Diag.range},
- {"severity", Diag.severity},
- {"message", Diag.message},
- });
- // LSP extension: embed the fixes in the diagnostic.
- if (DiagOpts.EmbedFixesInDiagnostics && !Fixes.empty()) {
- json::Array ClangdFixes;
- for (const auto &Fix : Fixes) {
- WorkspaceEdit WE;
- URIForFile URI{File};
- WE.changes = {{URI.uri(), std::vector<TextEdit>(Fix.Edits.begin(),
- Fix.Edits.end())}};
- ClangdFixes.push_back(
- json::Object{{"edit", toJSON(WE)}, {"title", Fix.Message}});
- }
- LSPDiag["clangd_fixes"] = std::move(ClangdFixes);
- }
- if (DiagOpts.SendDiagnosticCategory && !Diag.category.empty())
- LSPDiag["category"] = Diag.category;
- DiagnosticsJSON.push_back(std::move(LSPDiag));
-
- auto &FixItsForDiagnostic = LocalFixIts[Diag];
- llvm::copy(Fixes, std::back_inserter(FixItsForDiagnostic));
- });
+ toLSPDiags(Diag, URI, DiagOpts,
+ [&](clangd::Diagnostic Diag, ArrayRef<Fix> Fixes) {
+ auto &FixItsForDiagnostic = LocalFixIts[Diag];
+ llvm::copy(Fixes, std::back_inserter(FixItsForDiagnostic));
+ LSPDiagnostics.push_back(std::move(Diag));
+ });
}
// Cache FixIts
@@ -766,8 +740,8 @@ void ClangdLSPServer::onDiagnosticsReady
// Publish diagnostics.
notify("textDocument/publishDiagnostics",
json::Object{
- {"uri", URIForFile{File}},
- {"diagnostics", std::move(DiagnosticsJSON)},
+ {"uri", URI},
+ {"diagnostics", std::move(LSPDiagnostics)},
});
}
Modified: clang-tools-extra/trunk/clangd/Diagnostics.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/Diagnostics.cpp?rev=345119&r1=345118&r2=345119&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/Diagnostics.cpp (original)
+++ clang-tools-extra/trunk/clangd/Diagnostics.cpp Wed Oct 24 00:59:38 2018
@@ -227,19 +227,37 @@ raw_ostream &operator<<(raw_ostream &OS,
return OS;
}
-void toLSPDiags(const Diag &D,
- function_ref<void(clangd::Diagnostic, ArrayRef<Fix>)> OutFn) {
+CodeAction toCodeAction(const Fix &F, const URIForFile &File) {
+ CodeAction Action;
+ Action.title = F.Message;
+ Action.kind = CodeAction::QUICKFIX_KIND;
+ Action.edit.emplace();
+ Action.edit->changes.emplace();
+ (*Action.edit->changes)[File.uri()] = {F.Edits.begin(), F.Edits.end()};
+ return Action;
+}
+
+void toLSPDiags(
+ const Diag &D, const URIForFile &File, const ClangdDiagnosticOptions &Opts,
+ function_ref<void(clangd::Diagnostic, ArrayRef<Fix>)> OutFn) {
auto FillBasicFields = [](const DiagBase &D) -> clangd::Diagnostic {
clangd::Diagnostic Res;
Res.range = D.Range;
Res.severity = getSeverity(D.Severity);
- Res.category = D.Category;
return Res;
};
{
clangd::Diagnostic Main = FillBasicFields(D);
Main.message = mainMessage(D);
+ if (Opts.EmbedFixesInDiagnostics) {
+ Main.codeActions.emplace();
+ for (const auto &Fix : D.Fixes)
+ Main.codeActions->push_back(toCodeAction(Fix, File));
+ }
+ if (Opts.SendDiagnosticCategory && !D.Category.empty())
+ Main.category = D.Category;
+
OutFn(std::move(Main), D.Fixes);
}
Modified: clang-tools-extra/trunk/clangd/Diagnostics.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/Diagnostics.h?rev=345119&r1=345118&r2=345119&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/Diagnostics.h (original)
+++ clang-tools-extra/trunk/clangd/Diagnostics.h Wed Oct 24 00:59:38 2018
@@ -78,9 +78,12 @@ llvm::raw_ostream &operator<<(llvm::raw_
/// file do not have a corresponding LSP diagnostic, but can still be included
/// as part of their main diagnostic's message.
void toLSPDiags(
- const Diag &D,
+ const Diag &D, const URIForFile &File, const ClangdDiagnosticOptions &Opts,
llvm::function_ref<void(clangd::Diagnostic, llvm::ArrayRef<Fix>)> OutFn);
+/// Convert from Fix to LSP CodeAction.
+CodeAction toCodeAction(const Fix &D, const URIForFile &File);
+
/// Convert from clang diagnostic level to LSP severity.
int getSeverity(DiagnosticsEngine::Level L);
Modified: clang-tools-extra/trunk/clangd/Protocol.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/Protocol.cpp?rev=345119&r1=345118&r2=345119&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/Protocol.cpp (original)
+++ clang-tools-extra/trunk/clangd/Protocol.cpp Wed Oct 24 00:59:38 2018
@@ -210,8 +210,8 @@ bool fromJSON(const json::Value &Params,
if (auto *Diagnostics = TextDocument->getObject("publishDiagnostics")) {
if (auto CategorySupport = Diagnostics->getBoolean("categorySupport"))
R.DiagnosticCategory = *CategorySupport;
- if (auto ClangdFixSupport = Diagnostics->getBoolean("clangdFixSupport"))
- R.DiagnosticFixes = *ClangdFixSupport;
+ if (auto CodeActions = Diagnostics->getBoolean("codeActionsInline"))
+ R.DiagnosticFixes = *CodeActions;
}
if (auto *Completion = TextDocument->getObject("completion")) {
if (auto *Item = Completion->getObject("completionItem")) {
@@ -348,8 +348,10 @@ json::Value toJSON(const Diagnostic &D)
{"severity", D.severity},
{"message", D.message},
};
- // FIXME: this should be used for publishDiagnostics.
- // FIXME: send category and fixes when appropriate.
+ if (D.category)
+ Diag["category"] = *D.category;
+ if (D.codeActions)
+ Diag["codeActions"] = D.codeActions;
return std::move(Diag);
}
@@ -358,6 +360,7 @@ bool fromJSON(const json::Value &Params,
if (!O || !O.map("range", R.range) || !O.map("message", R.message))
return false;
O.map("severity", R.severity);
+ O.map("category", R.category);
return true;
}
Modified: clang-tools-extra/trunk/clangd/Protocol.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/Protocol.h?rev=345119&r1=345118&r2=345119&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/Protocol.h (original)
+++ clang-tools-extra/trunk/clangd/Protocol.h Wed Oct 24 00:59:38 2018
@@ -323,9 +323,8 @@ struct ClientCapabilities {
/// workspace.symbol.symbolKind.valueSet
llvm::Optional<SymbolKindBitset> WorkspaceSymbolKinds;
- /// Whether the client accepts diagnostics with fixes attached using the
- /// "clangd_fixes" extension.
- /// textDocument.publishDiagnostics.clangdFixSupport
+ /// Whether the client accepts diagnostics with codeActions attached inline.
+ /// textDocument.publishDiagnostics.codeActionsInline.
bool DiagnosticFixes = false;
/// Whether the client accepts diagnostics with category attached to it
@@ -536,6 +535,7 @@ struct DocumentSymbolParams {
};
bool fromJSON(const llvm::json::Value &, DocumentSymbolParams &);
+struct CodeAction;
struct Diagnostic {
/// The range at which the message applies.
Range range;
@@ -560,7 +560,12 @@ struct Diagnostic {
/// An LSP extension that's used to send the name of the category over to the
/// client. The category typically describes the compilation stage during
/// which the issue was produced, e.g. "Semantic Issue" or "Parse Issue".
- std::string category;
+ llvm::Optional<std::string> category;
+
+ /// Clangd extension: code actions related to this diagnostic.
+ /// Only with capability textDocument.publishDiagnostics.codeActionsInline.
+ /// (These actions can also be obtained using textDocument/codeAction).
+ llvm::Optional<std::vector<CodeAction>> codeActions;
};
llvm::json::Value toJSON(const Diagnostic &);
Modified: clang-tools-extra/trunk/test/clangd/fixits-embed-in-diagnostic.test
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clangd/fixits-embed-in-diagnostic.test?rev=345119&r1=345118&r2=345119&view=diff
==============================================================================
--- clang-tools-extra/trunk/test/clangd/fixits-embed-in-diagnostic.test (original)
+++ clang-tools-extra/trunk/test/clangd/fixits-embed-in-diagnostic.test Wed Oct 24 00:59:38 2018
@@ -1,12 +1,12 @@
# RUN: clangd -lit-test < %s | FileCheck -strict-whitespace %s
-{"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{"textDocument":{"publishDiagnostics":{"clangdFixSupport":true}}},"trace":"off"}}
+{"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{"textDocument":{"publishDiagnostics":{"codeActionsInline":true}}},"trace":"off"}}
---
{"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"test:///foo.c","languageId":"c","version":1,"text":"struct Point {}; union Point p;"}}}
-# CHECK: "method": "textDocument/publishDiagnostics",
-# CHECK-NEXT: "params": {
-# CHECK-NEXT: "diagnostics": [
+# CHECK: "method": "textDocument/publishDiagnostics",
+# CHECK-NEXT: "params": {
+# CHECK-NEXT: "diagnostics": [
# CHECK-NEXT: {
-# CHECK-NEXT: "clangd_fixes": [
+# CHECK-NEXT: "codeActions": [
# CHECK-NEXT: {
# CHECK-NEXT: "edit": {
# CHECK-NEXT: "changes": {
@@ -27,6 +27,7 @@
# CHECK-NEXT: ]
# CHECK-NEXT: }
# CHECK-NEXT: },
+# CHECK-NEXT: "kind": "quickfix",
# CHECK-NEXT: "title": "change 'union' to 'struct'"
# CHECK-NEXT: }
# CHECK-NEXT: ],
Modified: clang-tools-extra/trunk/unittests/clangd/ClangdUnitTests.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/ClangdUnitTests.cpp?rev=345119&r1=345118&r2=345119&view=diff
==============================================================================
--- clang-tools-extra/trunk/unittests/clangd/ClangdUnitTests.cpp (original)
+++ clang-tools-extra/trunk/unittests/clangd/ClangdUnitTests.cpp Wed Oct 24 00:59:38 2018
@@ -198,10 +198,13 @@ main.cpp:2:3: error: something terrible
// Transform dianostics and check the results.
std::vector<std::pair<clangd::Diagnostic, std::vector<clangd::Fix>>> LSPDiags;
- toLSPDiags(D, [&](clangd::Diagnostic LSPDiag, ArrayRef<clangd::Fix> Fixes) {
- LSPDiags.push_back({std::move(LSPDiag),
- std::vector<clangd::Fix>(Fixes.begin(), Fixes.end())});
- });
+ toLSPDiags(
+ D, URIForFile("/path/to/foo/bar/main.cpp"), ClangdDiagnosticOptions(),
+ [&](clangd::Diagnostic LSPDiag, ArrayRef<clangd::Fix> Fixes) {
+ LSPDiags.push_back(
+ {std::move(LSPDiag),
+ std::vector<clangd::Fix>(Fixes.begin(), Fixes.end())});
+ });
EXPECT_THAT(
LSPDiags,
More information about the cfe-commits
mailing list