[clangd] Include textual diagnostic ID as Diagnostic.code.
Reviewers: kadircet
Subscribers: ilya-biryukov, ioeric, MaskRay, jkorous, arphaman, jdoerfert, cfe-commits
Tags: #clang
Differential Revision: https://reviews.llvm.org/D58291
git-svn-id: https://llvm.org/svn/llvm-project/clang-tools-extra/trunk@358575 91177308-0d34-0410-b5e6-96231b3b80d8
diff --git a/clangd/ClangdUnit.cpp b/clangd/ClangdUnit.cpp
index 473edb6..9d0efec 100644
--- a/clangd/ClangdUnit.cpp
+++ b/clangd/ClangdUnit.cpp
@@ -371,11 +371,7 @@
// So just inform the preprocessor of EOF, while keeping everything alive.
Clang->getPreprocessor().EndSourceFile();
- std::vector<Diag> Diags = ASTDiags.take();
- // Populate diagnostic source.
- for (auto &D : Diags)
- D.S =
- !CTContext->getCheckName(D.ID).empty() ? Diag::ClangTidy : Diag::Clang;
+ std::vector<Diag> Diags = ASTDiags.take(CTContext.getPointer());
// Add diagnostics from the preamble, if any.
if (Preamble)
Diags.insert(Diags.begin(), Preamble->Diags.begin(), Preamble->Diags.end());
@@ -544,8 +540,6 @@
vlog("Built preamble of size {0} for file {1}", BuiltPreamble->getSize(),
FileName);
std::vector<Diag> Diags = PreambleDiagnostics.take();
- for (auto &Diag : Diags)
- Diag.S = Diag::Clang;
return std::make_shared<PreambleData>(
std::move(*BuiltPreamble), std::move(Diags),
SerializedDeclsCollector.takeIncludes(), std::move(StatCache),
diff --git a/clangd/Diagnostics.cpp b/clangd/Diagnostics.cpp
index 9f7161b..8a6ceba 100644
--- a/clangd/Diagnostics.cpp
+++ b/clangd/Diagnostics.cpp
@@ -7,9 +7,12 @@
//===----------------------------------------------------------------------===//
#include "Diagnostics.h"
+#include "../clang-tidy/ClangTidyDiagnosticConsumer.h"
#include "Compiler.h"
#include "Logger.h"
#include "SourceCode.h"
+#include "clang/Basic/AllDiagnostics.h"
+#include "clang/Basic/DiagnosticIDs.h"
#include "clang/Basic/SourceManager.h"
#include "clang/Lex/Lexer.h"
#include "llvm/Support/Capacity.h"
@@ -18,9 +21,31 @@
namespace clang {
namespace clangd {
-
namespace {
+const char *getDiagnosticCode(unsigned ID) {
+ switch (ID) {
+#define DIAG(ENUM, CLASS, DEFAULT_MAPPING, DESC, GROPU, SFINAE, NOWERROR, \
+ SHOWINSYSHEADER, CATEGORY) \
+ case clang::diag::ENUM: \
+ return #ENUM;
+#include "clang/Basic/DiagnosticASTKinds.inc"
+#include "clang/Basic/DiagnosticAnalysisKinds.inc"
+#include "clang/Basic/DiagnosticCommentKinds.inc"
+#include "clang/Basic/DiagnosticCommonKinds.inc"
+#include "clang/Basic/DiagnosticDriverKinds.inc"
+#include "clang/Basic/DiagnosticFrontendKinds.inc"
+#include "clang/Basic/DiagnosticLexKinds.inc"
+#include "clang/Basic/DiagnosticParseKinds.inc"
+#include "clang/Basic/DiagnosticRefactoringKinds.inc"
+#include "clang/Basic/DiagnosticSemaKinds.inc"
+#include "clang/Basic/DiagnosticSerializationKinds.inc"
+#undef DIAG
+ default:
+ return nullptr;
+ }
+}
+
bool mentionsMainFile(const Diag &D) {
if (D.InsideMainFile)
return true;
@@ -253,6 +278,18 @@
{
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)
@@ -290,7 +327,25 @@
llvm_unreachable("Unknown diagnostic level!");
}
-std::vector<Diag> StoreDiags::take() { return std::move(Output); }
+std::vector<Diag> StoreDiags::take(const clang::tidy::ClangTidyContext *Tidy) {
+ // Fill in name/source now that we have all the context needed to map them.
+ for (auto &Diag : Output) {
+ if (const char *ClangDiag = getDiagnosticCode(Diag.ID)) {
+ Diag.Name = ClangDiag;
+ Diag.Source = Diag::Clang;
+ continue;
+ }
+ if (Tidy != nullptr) {
+ std::string TidyDiag = Tidy->getCheckName(Diag.ID);
+ if (!TidyDiag.empty()) {
+ Diag.Name = std::move(TidyDiag);
+ Diag.Source = Diag::ClangTidy;
+ continue;
+ }
+ }
+ }
+ return std::move(Output);
+}
void StoreDiags::BeginSourceFile(const LangOptions &Opts,
const Preprocessor *) {
diff --git a/clangd/Diagnostics.h b/clangd/Diagnostics.h
index 9b8d80f..1ff9fcf 100644
--- a/clangd/Diagnostics.h
+++ b/clangd/Diagnostics.h
@@ -20,6 +20,9 @@
#include <string>
namespace clang {
+namespace tidy {
+class ClangTidyContext;
+} // namespace tidy
namespace clangd {
struct ClangdDiagnosticOptions {
@@ -68,14 +71,14 @@
/// A top-level diagnostic that may have Notes and Fixes.
struct Diag : DiagBase {
- // Diagnostic enum ID.
- unsigned ID;
+ unsigned ID; // e.g. member of clang::diag, or clang-tidy assigned ID.
+ std::string Name; // if ID was recognized.
// The source of this diagnostic.
- enum Source {
+ enum {
+ Unknown,
Clang,
ClangTidy,
- };
- Source S = Clang;
+ } Source = Unknown;
/// Elaborate on the problem, usually pointing to a related piece of code.
std::vector<Note> Notes;
/// *Alternative* fixes for this diagnostic, one should be chosen.
@@ -104,7 +107,8 @@
/// the diag itself nor its notes are in the main file).
class StoreDiags : public DiagnosticConsumer {
public:
- std::vector<Diag> take();
+ // The ClangTidyContext populates Source and Name for clang-tidy diagnostics.
+ std::vector<Diag> take(const clang::tidy::ClangTidyContext *Tidy = nullptr);
void BeginSourceFile(const LangOptions &Opts, const Preprocessor *) override;
void EndSourceFile() override;
diff --git a/clangd/Protocol.cpp b/clangd/Protocol.cpp
index 649110f..60c047b 100644
--- a/clangd/Protocol.cpp
+++ b/clangd/Protocol.cpp
@@ -429,6 +429,10 @@
Diag["category"] = *D.category;
if (D.codeActions)
Diag["codeActions"] = D.codeActions;
+ if (!D.code.empty())
+ Diag["code"] = D.code;
+ if (!D.source.empty())
+ Diag["source"] = D.source;
return std::move(Diag);
}
@@ -438,6 +442,8 @@
return false;
O.map("severity", R.severity);
O.map("category", R.category);
+ O.map("code", R.code);
+ O.map("source", R.source);
return true;
}
diff --git a/clangd/Protocol.h b/clangd/Protocol.h
index 0659c93..5e8e7b4 100644
--- a/clangd/Protocol.h
+++ b/clangd/Protocol.h
@@ -592,13 +592,11 @@
int severity = 0;
/// The diagnostic's code. Can be omitted.
- /// Note: Not currently used by clangd
- // std::string code;
+ std::string code;
/// A human-readable string describing the source of this
/// diagnostic, e.g. 'typescript' or 'super lint'.
- /// Note: Not currently used by clangd
- // std::string source;
+ std::string source;
/// The diagnostic's message.
std::string message;
diff --git a/test/clangd/compile-commands-path-in-initialize.test b/test/clangd/compile-commands-path-in-initialize.test
index 87086e3..e070562 100644
--- a/test/clangd/compile-commands-path-in-initialize.test
+++ b/test/clangd/compile-commands-path-in-initialize.test
@@ -21,6 +21,7 @@
# CHECK-NEXT: "params": {
# CHECK-NEXT: "diagnostics": [
# CHECK-NEXT: {
+# CHECK-NEXT: "code": "warn_pragma_message",
# CHECK-NEXT: "message": "MACRO is one",
---
{"jsonrpc":"2.0","id":10000,"method":"shutdown"}
diff --git a/test/clangd/diagnostic-category.test b/test/clangd/diagnostic-category.test
index 08ad581..a6f224d 100644
--- a/test/clangd/diagnostic-category.test
+++ b/test/clangd/diagnostic-category.test
@@ -7,6 +7,7 @@
# CHECK-NEXT: "diagnostics": [
# CHECK-NEXT: {
# CHECK-NEXT: "category": "Semantic Issue",
+# CHECK-NEXT: "code": "err_use_with_wrong_tag",
# CHECK-NEXT: "message": "Use of 'Point' with tag type that does not match previous declaration (fix available)\n\nfoo.c:1:8: note: previous use is here",
# CHECK-NEXT: "range": {
# CHECK-NEXT: "end": {
@@ -18,7 +19,8 @@
# CHECK-NEXT: "line": 0
# CHECK-NEXT: }
# CHECK-NEXT: },
-# CHECK-NEXT: "severity": 1
+# CHECK-NEXT: "severity": 1,
+# CHECK-NEXT: "source": "clang"
# CHECK-NEXT: },
# CHECK-NEXT: {
# CHECK-NEXT: "message": "Previous use is here\n\nfoo.c:1:18: error: use of 'Point' with tag type that does not match previous declaration",
diff --git a/test/clangd/diagnostics.test b/test/clangd/diagnostics.test
index 3f35940..a43198b 100644
--- a/test/clangd/diagnostics.test
+++ b/test/clangd/diagnostics.test
@@ -1,11 +1,12 @@
-# RUN: clangd -lit-test < %s | FileCheck -strict-whitespace %s
+# RUN: clangd -lit-test -clang-tidy-checks=bugprone-sizeof-expression < %s | FileCheck -strict-whitespace %s
{"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{},"trace":"off"}}
---
-{"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"test:///foo.c","languageId":"c","version":1,"text":"void main() {}"}}}
+{"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"test:///foo.c","languageId":"c","version":1,"text":"void main() {\n(void)sizeof(42);\n}"}}}
# CHECK: "method": "textDocument/publishDiagnostics",
# CHECK-NEXT: "params": {
# CHECK-NEXT: "diagnostics": [
# CHECK-NEXT: {
+# CHECK-NEXT: "code": "ext_main_returns_nonint",
# CHECK-NEXT: "message": "Return type of 'main' is not 'int' (fix available)",
# CHECK-NEXT: "range": {
# CHECK-NEXT: "end": {
@@ -17,7 +18,24 @@
# CHECK-NEXT: "line": 0
# CHECK-NEXT: }
# CHECK-NEXT: },
-# CHECK-NEXT: "severity": 2
+# CHECK-NEXT: "severity": 2,
+# CHECK-NEXT: "source": "clang"
+# CHECK-NEXT: },
+# CHECK-NEXT: {
+# CHECK-NEXT: "code": "bugprone-sizeof-expression",
+# CHECK-NEXT: "message": "Suspicious usage of 'sizeof(K)'; did you mean 'K'? [bugprone-sizeof-expression]",
+# CHECK-NEXT: "range": {
+# CHECK-NEXT: "end": {
+# CHECK-NEXT: "character": 12,
+# CHECK-NEXT: "line": 1
+# CHECK-NEXT: },
+# CHECK-NEXT: "start": {
+# CHECK-NEXT: "character": 6,
+# CHECK-NEXT: "line": 1
+# CHECK-NEXT: }
+# CHECK-NEXT: },
+# CHECK-NEXT: "severity": 2,
+# CHECK-NEXT: "source": "clang-tidy"
# CHECK-NEXT: }
# CHECK-NEXT: ],
# CHECK-NEXT: "uri": "file://{{.*}}/foo.c"
diff --git a/test/clangd/did-change-configuration-params.test b/test/clangd/did-change-configuration-params.test
index 3e1f4e3..35dcb55 100644
--- a/test/clangd/did-change-configuration-params.test
+++ b/test/clangd/did-change-configuration-params.test
@@ -24,6 +24,7 @@
# CHECK-NEXT: "params": {
# CHECK-NEXT: "diagnostics": [
# CHECK-NEXT: {
+# CHECK-NEXT: "code": "warn_uninit_var",
# CHECK-NEXT: "message": "Variable 'i' is uninitialized when used here (fix available)",
# CHECK-NEXT: "range": {
# CHECK-NEXT: "end": {
@@ -35,7 +36,8 @@
# CHECK-NEXT: "line": 0
# CHECK-NEXT: }
# CHECK-NEXT: },
-# CHECK-NEXT: "severity": 1
+# CHECK-NEXT: "severity": 1,
+# CHECK-NEXT: "source": "clang"
# CHECK-NEXT: }
# CHECK-NEXT: ],
# CHECK-NEXT: "uri": "file://{{.*}}/foo.c"
diff --git a/test/clangd/execute-command.test b/test/clangd/execute-command.test
index 3c9c45f..db7e2aa 100644
--- a/test/clangd/execute-command.test
+++ b/test/clangd/execute-command.test
@@ -6,6 +6,7 @@
# CHECK-NEXT: "params": {
# CHECK-NEXT: "diagnostics": [
# CHECK-NEXT: {
+# CHECK-NEXT: "code": "warn_condition_is_assignment",
# CHECK-NEXT: "message": "Using the result of an assignment as a condition without parentheses (fixes available)",
# CHECK-NEXT: "range": {
# CHECK-NEXT: "end": {
@@ -17,7 +18,8 @@
# CHECK-NEXT: "line": 0
# CHECK-NEXT: }
# CHECK-NEXT: },
-# CHECK-NEXT: "severity": 2
+# CHECK-NEXT: "severity": 2,
+# CHECK-NEXT: "source": "clang"
# CHECK-NEXT: }
# CHECK-NEXT: ],
# CHECK-NEXT: "uri": "file://{{.*}}/foo.c"
diff --git a/test/clangd/fixits-codeaction.test b/test/clangd/fixits-codeaction.test
index 47d7565..6876f4d 100644
--- a/test/clangd/fixits-codeaction.test
+++ b/test/clangd/fixits-codeaction.test
@@ -6,6 +6,7 @@
# CHECK-NEXT: "params": {
# CHECK-NEXT: "diagnostics": [
# CHECK-NEXT: {
+# CHECK-NEXT: "code": "warn_condition_is_assignment",
# CHECK-NEXT: "message": "Using the result of an assignment as a condition without parentheses (fixes available)",
# CHECK-NEXT: "range": {
# CHECK-NEXT: "end": {
@@ -17,19 +18,21 @@
# CHECK-NEXT: "line": 0
# CHECK-NEXT: }
# CHECK-NEXT: },
-# CHECK-NEXT: "severity": 2
+# CHECK-NEXT: "severity": 2,
+# CHECK-NEXT: "source": "clang"
# 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":0,"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 (fixes available)"}]}}}
+{"jsonrpc":"2.0","id":2,"method":"textDocument/codeAction","params":{"textDocument":{"uri":"test:///foo.c"},"range":{"start":{"line":0,"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 (fixes available)", "code": "warn_condition_is_assignment", "source": "clang"}]}}}
# CHECK: "id": 2,
# CHECK-NEXT: "jsonrpc": "2.0",
# CHECK-NEXT: "result": [
# CHECK-NEXT: {
# CHECK-NEXT: "diagnostics": [
# CHECK-NEXT: {
+# CHECK-NEXT: "code": "warn_condition_is_assignment",
# CHECK-NEXT: "message": "Using the result of an assignment as a condition without parentheses (fixes available)",
# CHECK-NEXT: "range": {
# CHECK-NEXT: "end": {
@@ -41,7 +44,8 @@
# CHECK-NEXT: "line": 0
# CHECK-NEXT: }
# CHECK-NEXT: },
-# CHECK-NEXT: "severity": 2
+# CHECK-NEXT: "severity": 2,
+# CHECK-NEXT: "source": "clang"
# CHECK-NEXT: }
# CHECK-NEXT: ],
# CHECK-NEXT: "edit": {
@@ -82,6 +86,7 @@
# CHECK-NEXT: {
# CHECK-NEXT: "diagnostics": [
# CHECK-NEXT: {
+# CHECK-NEXT: "code": "warn_condition_is_assignment",
# CHECK-NEXT: "message": "Using the result of an assignment as a condition without parentheses (fixes available)",
# CHECK-NEXT: "range": {
# CHECK-NEXT: "end": {
@@ -93,7 +98,8 @@
# CHECK-NEXT: "line": 0
# CHECK-NEXT: }
# CHECK-NEXT: },
-# CHECK-NEXT: "severity": 2
+# CHECK-NEXT: "severity": 2,
+# CHECK-NEXT: "source": "clang"
# CHECK-NEXT: }
# CHECK-NEXT: ],
# CHECK-NEXT: "edit": {
diff --git a/test/clangd/fixits-command.test b/test/clangd/fixits-command.test
index da50832..c24275f 100644
--- a/test/clangd/fixits-command.test
+++ b/test/clangd/fixits-command.test
@@ -6,6 +6,7 @@
# CHECK-NEXT: "params": {
# CHECK-NEXT: "diagnostics": [
# CHECK-NEXT: {
+# CHECK-NEXT: "code": "warn_condition_is_assignment",
# CHECK-NEXT: "message": "Using the result of an assignment as a condition without parentheses (fixes available)",
# CHECK-NEXT: "range": {
# CHECK-NEXT: "end": {
@@ -17,7 +18,8 @@
# CHECK-NEXT: "line": 0
# CHECK-NEXT: }
# CHECK-NEXT: },
-# CHECK-NEXT: "severity": 2
+# CHECK-NEXT: "severity": 2,
+# CHECK-NEXT: "source": "clang"
# CHECK-NEXT: }
# CHECK-NEXT: ],
# CHECK-NEXT: "uri": "file://{{.*}}/foo.c"
diff --git a/test/clangd/fixits-embed-in-diagnostic.test b/test/clangd/fixits-embed-in-diagnostic.test
index 560fca6..304f96e 100644
--- a/test/clangd/fixits-embed-in-diagnostic.test
+++ b/test/clangd/fixits-embed-in-diagnostic.test
@@ -6,6 +6,7 @@
# CHECK-NEXT: "params": {
# CHECK-NEXT: "diagnostics": [
# CHECK-NEXT: {
+# CHECK-NEXT: "code": "err_use_with_wrong_tag",
# CHECK-NEXT: "codeActions": [
# CHECK-NEXT: {
# CHECK-NEXT: "edit": {
@@ -42,7 +43,8 @@
# CHECK-NEXT: "line": 0
# CHECK-NEXT: }
# CHECK-NEXT: },
-# CHECK-NEXT: "severity": 1
+# CHECK-NEXT: "severity": 1,
+# CHECK-NEXT: "source": "clang"
# CHECK-NEXT: },
# CHECK-NEXT: {
# CHECK-NEXT: "message": "Previous use is here\n\nfoo.c:1:18: error: use of 'Point' with tag type that does not match previous declaration",
diff --git a/unittests/clangd/DiagnosticsTests.cpp b/unittests/clangd/DiagnosticsTests.cpp
index 469a672..f406ec4 100644
--- a/unittests/clangd/DiagnosticsTests.cpp
+++ b/unittests/clangd/DiagnosticsTests.cpp
@@ -12,6 +12,7 @@
#include "TestIndex.h"
#include "TestTU.h"
#include "index/MemIndex.h"
+#include "clang/Basic/DiagnosticSema.h"
#include "llvm/Support/ScopedPrinter.h"
#include "gmock/gmock.h"
#include "gtest/gtest.h"
@@ -58,7 +59,8 @@
std::tie(LSPDiag.range, LSPDiag.severity, LSPDiag.message);
}
-MATCHER_P(DiagSource, Source, "") { return arg.S == Source; }
+MATCHER_P(DiagSource, S, "") { return arg.Source == S; }
+MATCHER_P(DiagName, N, "") { return arg.Name == N; }
MATCHER_P(EqualToFix, Fix, "LSP fix " + llvm::to_string(Fix)) {
if (arg.Message != Fix.Message)
@@ -105,6 +107,7 @@
AllOf(Diag(Test.range("typo"),
"use of undeclared identifier 'goo'; did you mean 'foo'?"),
DiagSource(Diag::Clang),
+ DiagName("err_undeclared_var_use_suggest"),
WithFix(
Fix(Test.range("typo"), "foo", "change 'go\\ o' to 'foo'")),
// This is a pretty normal range.
@@ -149,7 +152,7 @@
EXPECT_THAT(TU.build().getDiagnostics(),
ElementsAre(testing::AllOf(
Diag(Test.range(), "'not-found.h' file not found"),
- DiagSource(Diag::Clang))));
+ DiagSource(Diag::Clang), DiagName("err_pp_file_not_found"))));
}
TEST(DiagnosticsTest, ClangTidy) {
@@ -175,6 +178,7 @@
"inclusion of deprecated C++ header 'assert.h'; consider "
"using 'cassert' instead [modernize-deprecated-headers]"),
DiagSource(Diag::ClangTidy),
+ DiagName("modernize-deprecated-headers"),
WithFix(Fix(Test.range("deprecated"), "<cassert>",
"change '\"assert.h\"' to '<cassert>'"))),
Diag(Test.range("doubled"),
@@ -185,6 +189,7 @@
"side effects in the 1st macro argument 'X' are repeated in "
"macro expansion [bugprone-macro-repeated-side-effects]"),
DiagSource(Diag::ClangTidy),
+ DiagName("bugprone-macro-repeated-side-effects"),
WithNote(Diag(Test.range("macrodef"),
"macro 'SQUARE' defined here "
"[bugprone-macro-repeated-side-effects]"))),
@@ -246,6 +251,9 @@
TEST(DiagnosticsTest, ToLSP) {
clangd::Diag D;
+ D.ID = clang::diag::err_enum_class_reference;
+ D.Name = "err_enum_class_reference";
+ D.Source = clangd::Diag::Clang;
D.Message = "something terrible happened";
D.Range = {pos(1, 2), pos(3, 4)};
D.InsideMainFile = true;
@@ -314,6 +322,10 @@
LSPDiags,
ElementsAre(Pair(EqualToLSPDiag(MainLSP), ElementsAre(EqualToFix(F))),
Pair(EqualToLSPDiag(NoteInMainLSP), IsEmpty())));
+ EXPECT_EQ(LSPDiags[0].first.code, "err_enum_class_reference");
+ EXPECT_EQ(LSPDiags[0].first.source, "clang");
+ EXPECT_EQ(LSPDiags[1].first.code, "");
+ EXPECT_EQ(LSPDiags[1].first.source, "");
}
struct SymbolWithHeader {
