[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

llvm-svn: 358575

Author: sam-mccall

clang-tools-extra/clangd/ClangdUnit.cpp

@@ -371,11 +371,7 @@ ParsedAST::build(std::unique_ptr<CompilerInvocation> CI,
   // 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 @@ buildPreamble(PathRef FileName, CompilerInvocation &CI,
     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),

clang-tools-extra/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 @@ void toLSPDiags(
   {
     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 @@ int getSeverity(DiagnosticsEngine::Level L) {
   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 *) {

clang-tools-extra/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 @@ struct Note : DiagBase {};
 
 /// 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 @@ int getSeverity(DiagnosticsEngine::Level L);
 /// 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;

clang-tools-extra/clangd/Protocol.cpp

@@ -429,6 +429,10 @@ llvm::json::Value toJSON(const Diagnostic &D) {
     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 @@ bool fromJSON(const llvm::json::Value &Params, Diagnostic &R) {
     return false;
   O.map("severity", R.severity);
   O.map("category", R.category);
+  O.map("code", R.code);
+  O.map("source", R.source);
   return true;
 }
 

clang-tools-extra/clangd/Protocol.h

@@ -592,13 +592,11 @@ struct Diagnostic {
   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;

clang-tools-extra/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"}

clang-tools-extra/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",

clang-tools-extra/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"

clang-tools-extra/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"

clang-tools-extra/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"