Index: clangd/ClangdUnit.cpp =================================================================== --- clangd/ClangdUnit.cpp +++ clangd/ClangdUnit.cpp @@ -371,11 +371,7 @@ // So just inform the preprocessor of EOF, while keeping everything alive. Clang->getPreprocessor().EndSourceFile(); - std::vector Diags = ASTDiags.take(); - // Populate diagnostic source. - for (auto &D : Diags) - D.S = - !CTContext->getCheckName(D.ID).empty() ? Diag::ClangTidy : Diag::Clang; + std::vector 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 Diags = PreambleDiagnostics.take(); - for (auto &Diag : Diags) - Diag.S = Diag::Clang; return std::make_shared( std::move(*BuiltPreamble), std::move(Diags), SerializedDeclsCollector.takeIncludes(), std::move(StatCache), Index: clangd/Diagnostics.h =================================================================== --- clangd/Diagnostics.h +++ clangd/Diagnostics.h @@ -20,6 +20,9 @@ #include 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 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 take(); + // The ClangTidyContext populates Source and Name for clang-tidy diagnostics. + std::vector take(const clang::tidy::ClangTidyContext *Tidy = nullptr); void BeginSourceFile(const LangOptions &Opts, const Preprocessor *) override; void EndSourceFile() override; Index: clangd/Diagnostics.cpp =================================================================== --- clangd/Diagnostics.cpp +++ 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 StoreDiags::take() { return std::move(Output); } +std::vector 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 *) { Index: clangd/Protocol.h =================================================================== --- clangd/Protocol.h +++ 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; Index: clangd/Protocol.cpp =================================================================== --- clangd/Protocol.cpp +++ 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; } Index: test/clangd/compile-commands-path-in-initialize.test =================================================================== --- test/clangd/compile-commands-path-in-initialize.test +++ 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"} Index: test/clangd/diagnostic-category.test =================================================================== --- test/clangd/diagnostic-category.test +++ 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", Index: test/clangd/diagnostics.test =================================================================== --- test/clangd/diagnostics.test +++ 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" Index: test/clangd/did-change-configuration-params.test =================================================================== --- test/clangd/did-change-configuration-params.test +++ 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" Index: test/clangd/execute-command.test =================================================================== --- test/clangd/execute-command.test +++ 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" Index: test/clangd/fixits-codeaction.test =================================================================== --- test/clangd/fixits-codeaction.test +++ 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": { Index: test/clangd/fixits-command.test =================================================================== --- test/clangd/fixits-command.test +++ 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" Index: test/clangd/fixits-embed-in-diagnostic.test =================================================================== --- test/clangd/fixits-embed-in-diagnostic.test +++ 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", Index: unittests/clangd/DiagnosticsTests.cpp =================================================================== --- unittests/clangd/DiagnosticsTests.cpp +++ 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"), "", "change '\"assert.h\"' to ''"))), 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 {