[clang-tools-extra] r358675 - [clangd] Support relatedInformation in diagnostics.
Sam McCall via cfe-commits
cfe-commits at lists.llvm.org
Thu Apr 18 08:17:07 PDT 2019
Author: sammccall
Date: Thu Apr 18 08:17:07 2019
New Revision: 358675
URL: http://llvm.org/viewvc/llvm-project?rev=358675&view=rev
Log:
[clangd] Support relatedInformation in diagnostics.
Summary: We already have the structure internally, we just need to expose it.
Reviewers: ilya-biryukov
Subscribers: ioeric, MaskRay, jkorous, arphaman, kadircet, cfe-commits
Tags: #clang
Differential Revision: https://reviews.llvm.org/D60267
Added:
clang-tools-extra/trunk/test/clangd/diagnostics-notes.test
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/unittests/clangd/DiagnosticsTests.cpp
Modified: clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp?rev=358675&r1=358674&r2=358675&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp (original)
+++ clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp Thu Apr 18 08:17:07 2019
@@ -348,6 +348,8 @@ void ClangdLSPServer::onInitialize(const
CCOpts.EnableSnippets = Params.capabilities.CompletionSnippets;
DiagOpts.EmbedFixesInDiagnostics = Params.capabilities.DiagnosticFixes;
DiagOpts.SendDiagnosticCategory = Params.capabilities.DiagnosticCategory;
+ DiagOpts.EmitRelatedLocations =
+ Params.capabilities.DiagnosticRelatedInformation;
if (Params.capabilities.WorkspaceSymbolKinds)
SupportedSymbolKinds |= *Params.capabilities.WorkspaceSymbolKinds;
if (Params.capabilities.CompletionItemKinds)
Modified: clang-tools-extra/trunk/clangd/Diagnostics.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/Diagnostics.cpp?rev=358675&r1=358674&r2=358675&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/Diagnostics.cpp (original)
+++ clang-tools-extra/trunk/clangd/Diagnostics.cpp Thu Apr 18 08:17:07 2019
@@ -10,6 +10,7 @@
#include "../clang-tidy/ClangTidyDiagnosticConsumer.h"
#include "Compiler.h"
#include "Logger.h"
+#include "Protocol.h"
#include "SourceCode.h"
#include "clang/Basic/AllDiagnostics.h"
#include "clang/Basic/DiagnosticIDs.h"
@@ -175,9 +176,7 @@ std::string capitalize(std::string Messa
}
/// Returns a message sent to LSP for the main diagnostic in \p D.
-/// The message includes all the notes with their corresponding locations.
-/// However, notes with fix-its are excluded as those usually only contain a
-/// fix-it message and just add noise if included in the message for diagnostic.
+/// This message may include notes, if they're not emited in some other way.
/// Example output:
///
/// no matching function for call to 'foo'
@@ -186,29 +185,34 @@ std::string capitalize(std::string Messa
///
/// dir1/dir2/dir3/../../dir4/header.h:12:23
/// note: candidate function not viable: requires 3 arguments
-std::string mainMessage(const Diag &D, bool DisplayFixesCount) {
+std::string mainMessage(const Diag &D, const ClangdDiagnosticOptions &Opts) {
std::string Result;
llvm::raw_string_ostream OS(Result);
OS << D.Message;
- if (DisplayFixesCount && !D.Fixes.empty())
+ if (Opts.DisplayFixesCount && !D.Fixes.empty())
OS << " (" << (D.Fixes.size() > 1 ? "fixes" : "fix") << " available)";
- for (auto &Note : D.Notes) {
- OS << "\n\n";
- printDiag(OS, Note);
- }
+ // If notes aren't emitted as structured info, add them to the message.
+ if (!Opts.EmitRelatedLocations)
+ for (auto &Note : D.Notes) {
+ OS << "\n\n";
+ printDiag(OS, Note);
+ }
OS.flush();
return capitalize(std::move(Result));
}
/// Returns a message sent to LSP for the note of the main diagnostic.
-/// The message includes the main diagnostic to provide the necessary context
-/// for the user to understand the note.
-std::string noteMessage(const Diag &Main, const DiagBase &Note) {
+std::string noteMessage(const Diag &Main, const DiagBase &Note,
+ const ClangdDiagnosticOptions &Opts) {
std::string Result;
llvm::raw_string_ostream OS(Result);
OS << Note.Message;
- OS << "\n\n";
- printDiag(OS, Main);
+ // If the client doesn't support structured links between the note and the
+ // original diagnostic, then emit the main diagnostic to give context.
+ if (!Opts.EmitRelatedLocations) {
+ OS << "\n\n";
+ printDiag(OS, Main);
+ }
OS.flush();
return capitalize(std::move(Result));
}
@@ -275,39 +279,54 @@ void toLSPDiags(
return Res;
};
- {
- clangd::Diagnostic Main = FillBasicFields(D);
- Main.message = mainMessage(D, Opts.DisplayFixesCount);
- if (!D.Name.empty())
- Main.code = D.Name;
- switch (D.Source) {
- case Diag::Clang:
- Main.source = "clang";
- break;
- case Diag::ClangTidy:
- Main.source = "clang-tidy";
- break;
- case Diag::Unknown:
- break;
- }
- if (Opts.EmbedFixesInDiagnostics) {
- Main.codeActions.emplace();
- for (const auto &Fix : D.Fixes)
- Main.codeActions->push_back(toCodeAction(Fix, File));
+ clangd::Diagnostic Main = FillBasicFields(D);
+ Main.code = D.Name;
+ switch (D.Source) {
+ case Diag::Clang:
+ Main.source = "clang";
+ break;
+ case Diag::ClangTidy:
+ Main.source = "clang-tidy";
+ break;
+ case Diag::Unknown:
+ break;
+ }
+ 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;
+
+ Main.message = mainMessage(D, Opts);
+ if (Opts.EmitRelatedLocations) {
+ Main.relatedInformation.emplace();
+ for (auto &Note : D.Notes) {
+ if (!Note.AbsFile) {
+ vlog("Dropping note from unknown file: {0}", Note);
+ continue;
+ }
+ DiagnosticRelatedInformation RelInfo;
+ RelInfo.location.range = Note.Range;
+ RelInfo.location.uri =
+ URIForFile::canonicalize(*Note.AbsFile, File.file());
+ RelInfo.message = noteMessage(D, Note, Opts);
+ Main.relatedInformation->push_back(std::move(RelInfo));
}
- if (Opts.SendDiagnosticCategory && !D.Category.empty())
- Main.category = D.Category;
-
- OutFn(std::move(Main), D.Fixes);
}
+ OutFn(std::move(Main), D.Fixes);
- for (auto &Note : D.Notes) {
- if (!Note.InsideMainFile)
- continue;
- clangd::Diagnostic Res = FillBasicFields(Note);
- Res.message = noteMessage(D, Note);
- OutFn(std::move(Res), llvm::ArrayRef<Fix>());
- }
+ // If we didn't emit the notes as relatedLocations, emit separate diagnostics
+ // so the user can find the locations easily.
+ if (!Opts.EmitRelatedLocations)
+ for (auto &Note : D.Notes) {
+ if (!Note.InsideMainFile)
+ continue;
+ clangd::Diagnostic Res = FillBasicFields(Note);
+ Res.message = noteMessage(D, Note, Opts);
+ OutFn(std::move(Res), llvm::ArrayRef<Fix>());
+ }
}
int getSeverity(DiagnosticsEngine::Level L) {
@@ -396,6 +415,9 @@ void StoreDiags::HandleDiagnostic(Diagno
D.Message = Message.str();
D.InsideMainFile = InsideMainFile;
D.File = Info.getSourceManager().getFilename(Info.getLocation());
+ auto &SM = Info.getSourceManager();
+ D.AbsFile = getCanonicalPath(
+ SM.getFileEntryForID(SM.getFileID(Info.getLocation())), SM);
D.Severity = DiagLevel;
D.Category = DiagnosticIDs::getCategoryNameFromID(
DiagnosticIDs::getCategoryNumberForDiag(Info.getID()))
Modified: clang-tools-extra/trunk/clangd/Diagnostics.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/Diagnostics.h?rev=358675&r1=358674&r2=358675&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/Diagnostics.h (original)
+++ clang-tools-extra/trunk/clangd/Diagnostics.h Thu Apr 18 08:17:07 2019
@@ -30,6 +30,11 @@ struct ClangdDiagnosticOptions {
/// diagnostics that are sent to the client.
bool EmbedFixesInDiagnostics = false;
+ /// If true, Clangd uses the relatedInformation field to include other
+ /// locations (in particular attached notes).
+ /// Otherwise, these are flattened into the diagnostic message.
+ bool EmitRelatedLocations = false;
+
/// If true, Clangd uses an LSP extension to send the diagnostic's
/// category to the client. The category typically describes the compilation
/// stage during which the issue was produced, e.g. "Semantic Issue" or "Parse
@@ -47,6 +52,9 @@ struct DiagBase {
// Intended to be used only in error messages.
// May be relative, absolute or even artifically constructed.
std::string File;
+ // Absolute path to containing file, if available.
+ llvm::Optional<std::string> AbsFile;
+
clangd::Range Range;
DiagnosticsEngine::Level Severity = DiagnosticsEngine::Note;
std::string Category;
Modified: clang-tools-extra/trunk/clangd/Protocol.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/Protocol.cpp?rev=358675&r1=358674&r2=358675&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/Protocol.cpp (original)
+++ clang-tools-extra/trunk/clangd/Protocol.cpp Thu Apr 18 08:17:07 2019
@@ -277,6 +277,8 @@ bool fromJSON(const llvm::json::Value &P
R.DiagnosticCategory = *CategorySupport;
if (auto CodeActions = Diagnostics->getBoolean("codeActionsInline"))
R.DiagnosticFixes = *CodeActions;
+ if (auto RelatedInfo = Diagnostics->getBoolean("relatedInformation"))
+ R.DiagnosticRelatedInformation = *RelatedInfo;
}
if (auto *Completion = TextDocument->getObject("completion")) {
if (auto *Item = Completion->getObject("completionItem")) {
@@ -419,6 +421,13 @@ bool fromJSON(const llvm::json::Value &P
return O && O.map("textDocument", R.textDocument);
}
+llvm::json::Value toJSON(const DiagnosticRelatedInformation &DRI) {
+ return llvm::json::Object{
+ {"location", DRI.location},
+ {"message", DRI.message},
+ };
+}
+
llvm::json::Value toJSON(const Diagnostic &D) {
llvm::json::Object Diag{
{"range", D.range},
@@ -433,6 +442,8 @@ llvm::json::Value toJSON(const Diagnosti
Diag["code"] = D.code;
if (!D.source.empty())
Diag["source"] = D.source;
+ if (D.relatedInformation)
+ Diag["relatedInformation"] = *D.relatedInformation;
return std::move(Diag);
}
Modified: clang-tools-extra/trunk/clangd/Protocol.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/Protocol.h?rev=358675&r1=358674&r2=358675&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/Protocol.h (original)
+++ clang-tools-extra/trunk/clangd/Protocol.h Thu Apr 18 08:17:07 2019
@@ -366,6 +366,10 @@ struct ClientCapabilities {
/// textDocument.publishDiagnostics.codeActionsInline.
bool DiagnosticFixes = false;
+ /// Whether the client accepts diagnostics with related locations.
+ /// textDocument.publishDiagnostics.relatedInformation.
+ bool DiagnosticRelatedInformation = false;
+
/// Whether the client accepts diagnostics with category attached to it
/// using the "category" extension.
/// textDocument.publishDiagnostics.categorySupport
@@ -582,6 +586,18 @@ struct DocumentSymbolParams {
};
bool fromJSON(const llvm::json::Value &, DocumentSymbolParams &);
+
+/// Represents a related message and source code location for a diagnostic.
+/// This should be used to point to code locations that cause or related to a
+/// diagnostics, e.g when duplicating a symbol in a scope.
+struct DiagnosticRelatedInformation {
+ /// The location of this related diagnostic information.
+ Location location;
+ /// The message of this related diagnostic information.
+ std::string message;
+};
+llvm::json::Value toJSON(const DiagnosticRelatedInformation &);
+
struct CodeAction;
struct Diagnostic {
/// The range at which the message applies.
@@ -601,6 +617,10 @@ struct Diagnostic {
/// The diagnostic's message.
std::string message;
+ /// An array of related diagnostic information, e.g. when symbol-names within
+ /// a scope collide all definitions can be marked via this property.
+ llvm::Optional<std::vector<DiagnosticRelatedInformation>> relatedInformation;
+
/// The diagnostic's category. Can be omitted.
/// 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
Added: clang-tools-extra/trunk/test/clangd/diagnostics-notes.test
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clangd/diagnostics-notes.test?rev=358675&view=auto
==============================================================================
--- clang-tools-extra/trunk/test/clangd/diagnostics-notes.test (added)
+++ clang-tools-extra/trunk/test/clangd/diagnostics-notes.test Thu Apr 18 08:17:07 2019
@@ -0,0 +1,48 @@
+# RUN: clangd -lit-test < %s | FileCheck -strict-whitespace %s
+{"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{"textDocument":{"publishDiagnostics":{"relatedInformation":true}}},"trace":"off"}}
+---
+{"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"test:///foo.cc","languageId":"cpp","version":1,"text":"int x;\nint x;"}}}
+# CHECK: "method": "textDocument/publishDiagnostics",
+# CHECK-NEXT: "params": {
+# CHECK-NEXT: "diagnostics": [
+# CHECK-NEXT: {
+# CHECK-NEXT: "code": "redefinition",
+# CHECK-NEXT: "message": "Redefinition of 'x'",
+# CHECK-NEXT: "range": {
+# CHECK-NEXT: "end": {
+# CHECK-NEXT: "character": 5,
+# CHECK-NEXT: "line": 1
+# CHECK-NEXT: },
+# CHECK-NEXT: "start": {
+# CHECK-NEXT: "character": 4,
+# CHECK-NEXT: "line": 1
+# CHECK-NEXT: }
+# CHECK-NEXT: },
+# CHECK-NEXT: "relatedInformation": [
+# CHECK-NEXT: {
+# CHECK-NEXT: "location": {
+# CHECK-NEXT: "range": {
+# CHECK-NEXT: "end": {
+# CHECK-NEXT: "character": 5,
+# CHECK-NEXT: "line": 0
+# CHECK-NEXT: },
+# CHECK-NEXT: "start": {
+# CHECK-NEXT: "character": 4,
+# CHECK-NEXT: "line": 0
+# CHECK-NEXT: }
+# CHECK-NEXT: },
+# CHECK-NEXT: "uri": "{{.*}}foo.cc"
+# CHECK-NEXT: },
+# CHECK-NEXT: "message": "Previous definition is here"
+# CHECK-NEXT: }
+# CHECK-NEXT: ],
+# CHECK-NEXT: "severity": 1,
+# CHECK-NEXT: "source": "clang"
+# CHECK-NEXT: }
+# CHECK-NEXT: ],
+# CHECK-NEXT: "uri": "file://{{.*}}/foo.cc"
+# CHECK-NEXT: }
+---
+{"jsonrpc":"2.0","id":5,"method":"shutdown"}
+---
+{"jsonrpc":"2.0","method":"exit"}
Modified: clang-tools-extra/trunk/unittests/clangd/DiagnosticsTests.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/DiagnosticsTests.cpp?rev=358675&r1=358674&r2=358675&view=diff
==============================================================================
--- clang-tools-extra/trunk/unittests/clangd/DiagnosticsTests.cpp (original)
+++ clang-tools-extra/trunk/unittests/clangd/DiagnosticsTests.cpp Thu Apr 18 08:17:07 2019
@@ -8,10 +8,14 @@
#include "Annotations.h"
#include "ClangdUnit.h"
+#include "Diagnostics.h"
+#include "Protocol.h"
#include "SourceCode.h"
#include "TestIndex.h"
+#include "TestFS.h"
#include "TestTU.h"
#include "index/MemIndex.h"
+#include "clang/Basic/Diagnostic.h"
#include "clang/Basic/DiagnosticSema.h"
#include "llvm/Support/ScopedPrinter.h"
#include "gmock/gmock.h"
@@ -55,8 +59,12 @@ MATCHER_P3(Fix, Range, Replacement, Mess
MATCHER_P(EqualToLSPDiag, LSPDiag,
"LSP diagnostic " + llvm::to_string(LSPDiag)) {
- return std::tie(arg.range, arg.severity, arg.message) ==
- std::tie(LSPDiag.range, LSPDiag.severity, LSPDiag.message);
+ if (toJSON(arg) != toJSON(LSPDiag)) {
+ *result_listener << llvm::formatv("expected:\n{0:2}\ngot\n{1:2}",
+ toJSON(LSPDiag), toJSON(arg)).str();
+ return false;
+ }
+ return true;
}
MATCHER_P(DiagSource, S, "") { return arg.Source == S; }
@@ -248,6 +256,11 @@ TEST(DiagnosticsTest, NoFixItInMacro) {
}
TEST(DiagnosticsTest, ToLSP) {
+ URIForFile MainFile =
+ URIForFile::canonicalize(testPath("foo/bar/main.cpp"), "");
+ URIForFile HeaderFile =
+ URIForFile::canonicalize(testPath("foo/bar/header.h"), "");
+
clangd::Diag D;
D.ID = clang::diag::err_enum_class_reference;
D.Name = "enum_class_reference";
@@ -257,6 +270,7 @@ TEST(DiagnosticsTest, ToLSP) {
D.InsideMainFile = true;
D.Severity = DiagnosticsEngine::Error;
D.File = "foo/bar/main.cpp";
+ D.AbsFile = MainFile.file();
clangd::Note NoteInMain;
NoteInMain.Message = "declared somewhere in the main file";
@@ -264,6 +278,8 @@ TEST(DiagnosticsTest, ToLSP) {
NoteInMain.Severity = DiagnosticsEngine::Remark;
NoteInMain.File = "../foo/bar/main.cpp";
NoteInMain.InsideMainFile = true;
+ NoteInMain.AbsFile = MainFile.file();
+
D.Notes.push_back(NoteInMain);
clangd::Note NoteInHeader;
@@ -272,44 +288,37 @@ TEST(DiagnosticsTest, ToLSP) {
NoteInHeader.Severity = DiagnosticsEngine::Note;
NoteInHeader.File = "../foo/baz/header.h";
NoteInHeader.InsideMainFile = false;
+ NoteInHeader.AbsFile = HeaderFile.file();
D.Notes.push_back(NoteInHeader);
clangd::Fix F;
F.Message = "do something";
D.Fixes.push_back(F);
- auto MatchingLSP = [](const DiagBase &D, StringRef Message) {
- clangd::Diagnostic Res;
- Res.range = D.Range;
- Res.severity = getSeverity(D.Severity);
- Res.message = Message;
- return Res;
- };
-
// Diagnostics should turn into these:
- clangd::Diagnostic MainLSP =
- MatchingLSP(D, R"(Something terrible happened (fix available)
+ clangd::Diagnostic MainLSP;
+ MainLSP.range = D.Range;
+ MainLSP.severity = getSeverity(DiagnosticsEngine::Error);
+ MainLSP.code = "enum_class_reference";
+ MainLSP.source = "clang";
+ MainLSP.message = R"(Something terrible happened (fix available)
main.cpp:6:7: remark: declared somewhere in the main file
../foo/baz/header.h:10:11:
-note: declared somewhere in the header file)");
+note: declared somewhere in the header file)";
- clangd::Diagnostic NoteInMainLSP =
- MatchingLSP(NoteInMain, R"(Declared somewhere in the main file
+ clangd::Diagnostic NoteInMainLSP;
+ NoteInMainLSP.range = NoteInMain.Range;
+ NoteInMainLSP.severity = getSeverity(DiagnosticsEngine::Remark);
+ NoteInMainLSP.message = R"(Declared somewhere in the main file
-main.cpp:2:3: error: something terrible happened)");
+main.cpp:2:3: error: something terrible happened)";
- // Transform dianostics and check the results.
+ ClangdDiagnosticOptions Opts;
+ // Transform diagnostics and check the results.
std::vector<std::pair<clangd::Diagnostic, std::vector<clangd::Fix>>> LSPDiags;
- toLSPDiags(D,
-#ifdef _WIN32
- URIForFile::canonicalize("c:\\path\\to\\foo\\bar\\main.cpp",
- /*TUPath=*/""),
-#else
- URIForFile::canonicalize("/path/to/foo/bar/main.cpp", /*TUPath=*/""),
-#endif
- ClangdDiagnosticOptions(),
+ toLSPDiags(D, MainFile, Opts,
[&](clangd::Diagnostic LSPDiag, ArrayRef<clangd::Fix> Fixes) {
LSPDiags.push_back(
{std::move(LSPDiag),
@@ -324,6 +333,30 @@ main.cpp:2:3: error: something terrible
EXPECT_EQ(LSPDiags[0].first.source, "clang");
EXPECT_EQ(LSPDiags[1].first.code, "");
EXPECT_EQ(LSPDiags[1].first.source, "");
+
+ // Same thing, but don't flatten notes into the main list.
+ LSPDiags.clear();
+ Opts.EmitRelatedLocations = true;
+ toLSPDiags(D, MainFile, Opts,
+ [&](clangd::Diagnostic LSPDiag, ArrayRef<clangd::Fix> Fixes) {
+ LSPDiags.push_back(
+ {std::move(LSPDiag),
+ std::vector<clangd::Fix>(Fixes.begin(), Fixes.end())});
+ });
+ MainLSP.message = "Something terrible happened (fix available)";
+ DiagnosticRelatedInformation NoteInMainDRI;
+ NoteInMainDRI.message = "Declared somewhere in the main file";
+ NoteInMainDRI.location.range = NoteInMain.Range;
+ NoteInMainDRI.location.uri = MainFile;
+ MainLSP.relatedInformation = {NoteInMainDRI};
+ DiagnosticRelatedInformation NoteInHeaderDRI;
+ NoteInHeaderDRI.message = "Declared somewhere in the header file";
+ NoteInHeaderDRI.location.range = NoteInHeader.Range;
+ NoteInHeaderDRI.location.uri = HeaderFile;
+ MainLSP.relatedInformation = {NoteInMainDRI, NoteInHeaderDRI};
+ EXPECT_THAT(
+ LSPDiags,
+ ElementsAre(Pair(EqualToLSPDiag(MainLSP), ElementsAre(EqualToFix(F)))));
}
struct SymbolWithHeader {
More information about the cfe-commits
mailing list