hokein updated this revision to Diff 189285.
hokein marked 6 inline comments as done.
hokein added a comment.

Address comments.


Repository:
  rCTE Clang Tools Extra

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D58600/new/

https://reviews.llvm.org/D58600

Files:
  clangd/ClangdUnit.cpp
  clangd/Diagnostics.cpp
  clangd/Diagnostics.h
  clangd/Protocol.cpp
  clangd/Protocol.h
  test/clangd/diagnostic-category.test
  test/clangd/diagnostics.test
  test/clangd/did-change-configuration-params.test
  test/clangd/execute-command.test
  test/clangd/fixits-codeaction.test
  test/clangd/fixits-command.test
  test/clangd/fixits-embed-in-diagnostic.test
  unittests/clangd/DiagnosticsTests.cpp

Index: unittests/clangd/DiagnosticsTests.cpp
===================================================================
--- unittests/clangd/DiagnosticsTests.cpp
+++ unittests/clangd/DiagnosticsTests.cpp
@@ -58,6 +58,8 @@
          std::tie(LSPDiag.range, LSPDiag.severity, LSPDiag.message);
 }
 
+MATCHER_P(DiagSource, Source, "") { return arg.S == Source; }
+
 MATCHER_P(EqualToFix, Fix, "LSP fix " + llvm::to_string(Fix)) {
   if (arg.Message != Fix.Message)
     return false;
@@ -102,6 +104,7 @@
           // This range spans lines.
           AllOf(Diag(Test.range("typo"),
                      "use of undeclared identifier 'goo'; did you mean 'foo'?"),
+                DiagSource(Diag::Clang),
                 WithFix(
                     Fix(Test.range("typo"), "foo", "change 'go\\ o' to 'foo'")),
                 // This is a pretty normal range.
@@ -137,6 +140,18 @@
           WithFix(Fix(Test.range(), "int", "change return type to 'int'")))));
 }
 
+TEST(DiagnosticsTest, DiagnosticPreamble) {
+  Annotations Test(R"cpp(
+    #include $[["not-found.h"]]
+  )cpp");
+
+  auto TU = TestTU::withCode(Test.code());
+  EXPECT_THAT(TU.build().getDiagnostics(),
+              ElementsAre(testing::AllOf(
+                  Diag(Test.range(), "'not-found.h' file not found"),
+                  DiagSource(Diag::Clang))));
+}
+
 TEST(DiagnosticsTest, ClangTidy) {
   Annotations Test(R"cpp(
     #include $deprecated[["assert.h"]]
@@ -159,6 +174,7 @@
           AllOf(Diag(Test.range("deprecated"),
                      "inclusion of deprecated C++ header 'assert.h'; consider "
                      "using 'cassert' instead [modernize-deprecated-headers]"),
+                DiagSource(Diag::ClangTidy),
                 WithFix(Fix(Test.range("deprecated"), "<cassert>",
                             "change '\"assert.h\"' to '<cassert>'"))),
           Diag(Test.range("doubled"),
@@ -168,6 +184,7 @@
               Diag(Test.range("macroarg"),
                    "side effects in the 1st macro argument 'X' are repeated in "
                    "macro expansion [bugprone-macro-repeated-side-effects]"),
+              DiagSource(Diag::ClangTidy),
               WithNote(Diag(Test.range("macrodef"),
                             "macro 'SQUARE' defined here "
                             "[bugprone-macro-repeated-side-effects]"))),
Index: test/clangd/fixits-embed-in-diagnostic.test
===================================================================
--- test/clangd/fixits-embed-in-diagnostic.test
+++ test/clangd/fixits-embed-in-diagnostic.test
@@ -42,7 +42,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/fixits-command.test
===================================================================
--- test/clangd/fixits-command.test
+++ test/clangd/fixits-command.test
@@ -17,7 +17,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
@@ -17,7 +17,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/execute-command.test
===================================================================
--- test/clangd/execute-command.test
+++ test/clangd/execute-command.test
@@ -17,7 +17,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/did-change-configuration-params.test
===================================================================
--- test/clangd/did-change-configuration-params.test
+++ test/clangd/did-change-configuration-params.test
@@ -35,7 +35,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/diagnostics.test
===================================================================
--- test/clangd/diagnostics.test
+++ test/clangd/diagnostics.test
@@ -17,7 +17,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/diagnostic-category.test
===================================================================
--- test/clangd/diagnostic-category.test
+++ test/clangd/diagnostic-category.test
@@ -18,7 +18,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: clangd/Protocol.h
===================================================================
--- clangd/Protocol.h
+++ clangd/Protocol.h
@@ -571,8 +571,7 @@
 
   /// 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
@@ -368,6 +368,8 @@
     Diag["category"] = *D.category;
   if (D.codeActions)
     Diag["codeActions"] = D.codeActions;
+  if (!D.source.empty())
+    Diag["source"] = D.source;
   return std::move(Diag);
 }
 
@@ -377,6 +379,7 @@
     return false;
   O.map("severity", R.severity);
   O.map("category", R.category);
+  O.map("source", R.source);
   return true;
 }
 
Index: clangd/Diagnostics.h
===================================================================
--- clangd/Diagnostics.h
+++ clangd/Diagnostics.h
@@ -68,6 +68,14 @@
 
 /// A top-level diagnostic that may have Notes and Fixes.
 struct Diag : DiagBase {
+  // Diagnostic enum ID.
+  unsigned ID;
+  // The source of this diagnostic.
+  enum Source {
+    Clang,
+    ClangTidy,
+  };
+  Source S = Clang;
   /// 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.
Index: clangd/Diagnostics.cpp
===================================================================
--- clangd/Diagnostics.cpp
+++ clangd/Diagnostics.cpp
@@ -109,6 +109,16 @@
   llvm_unreachable("unhandled DiagnosticsEngine::Level");
 }
 
+llvm::StringRef diagSourceToString(Diag::Source S) {
+  switch (S) {
+  case Diag::Clang:
+    return "clang";
+  case Diag::ClangTidy:
+    return "clang-tidy";
+  }
+  llvm_unreachable("unhandled Diag::Source");
+}
+
 /// Prints a single diagnostic in a clang-like manner, the output includes
 /// location, severity and error message. An example of the output message is:
 ///
@@ -253,6 +263,7 @@
   {
     clangd::Diagnostic Main = FillBasicFields(D);
     Main.message = mainMessage(D, Opts.DisplayFixesCount);
+    Main.source = diagSourceToString(D.S);
     if (Opts.EmbedFixesInDiagnostics) {
       Main.codeActions.emplace();
       for (const auto &Fix : D.Fixes)
@@ -377,6 +388,7 @@
     flushLastDiag();
 
     LastDiag = Diag();
+    LastDiag->ID = Info.getID();
     FillDiagBase(*LastDiag);
 
     if (!Info.getFixItHints().empty())
Index: clangd/ClangdUnit.cpp
===================================================================
--- clangd/ClangdUnit.cpp
+++ clangd/ClangdUnit.cpp
@@ -372,6 +372,10 @@
   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;
   // Add diagnostics from the preamble, if any.
   if (Preamble)
     Diags.insert(Diags.begin(), Preamble->Diags.begin(), Preamble->Diags.end());
@@ -539,8 +543,11 @@
   if (BuiltPreamble) {
     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), PreambleDiagnostics.take(),
+        std::move(*BuiltPreamble), std::move(Diags),
         SerializedDeclsCollector.takeIncludes(), std::move(StatCache),
         SerializedDeclsCollector.takeCanonicalIncludes());
   } else {
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to