[clang-tools-extra] 66a2e3a - [clangd] Send EOF before resetting diagnostics consumer

[Kadir Cetinkaya via cfe-commits]

Author: Kadir Cetinkaya
Date: 2020-08-13T18:32:59+02:00
New Revision: 66a2e3a525645ad8d356ef4f5b752bfcae3c27b7

URL: https://github.com/llvm/llvm-project/commit/66a2e3a525645ad8d356ef4f5b752bfcae3c27b7
DIFF: https://github.com/llvm/llvm-project/commit/66a2e3a525645ad8d356ef4f5b752bfcae3c27b7.diff

LOG: [clangd] Send EOF before resetting diagnostics consumer

Summary:
Some clang-tidy checkers, e.g. llvm-include-order can emit diagnostics
at this callback (as mentioned in the comments).

Clangd was resetting diag consumer to IgnoreDiags before sending EOF, hence we
were unable to emit diagnostics for such checkers.

This patch changes the order of that reset and preprocosser event to make sure
we emit that diag.

Fixes https://github.com/clangd/clangd/issues/314.

Reviewers: sammccall

Subscribers: ilya-biryukov, MaskRay, jkorous, arphaman, usaxena95, cfe-commits

Tags: #clang

Differential Revision: https://reviews.llvm.org/D83178

Added: 
    

Modified: 
    clang-tools-extra/clangd/ParsedAST.cpp
    clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp

Removed: 
    


################################################################################
diff  --git a/clang-tools-extra/clangd/ParsedAST.cpp b/clang-tools-extra/clangd/ParsedAST.cpp
index 35e7fd72c3c5..23d86221e74f 100644
--- a/clang-tools-extra/clangd/ParsedAST.cpp
+++ b/clang-tools-extra/clangd/ParsedAST.cpp
@@ -424,15 +424,15 @@ ParsedAST::build(llvm::StringRef Filename, const ParseInputs &Inputs,
     CTFinder.matchAST(Clang->getASTContext());
   }
 
+  // XXX: This is messy: clang-tidy checks flush some diagnostics at EOF.
+  // However Action->EndSourceFile() would destroy the ASTContext!
+  // So just inform the preprocessor of EOF, while keeping everything alive.
+  Clang->getPreprocessor().EndSourceFile();
   // UnitDiagsConsumer is local, we can not store it in CompilerInstance that
   // has a longer lifetime.
   Clang->getDiagnostics().setClient(new IgnoreDiagnostics);
   // CompilerInstance won't run this callback, do it directly.
   ASTDiags.EndSourceFile();
-  // XXX: This is messy: clang-tidy checks flush some diagnostics at EOF.
-  // However Action->EndSourceFile() would destroy the ASTContext!
-  // So just inform the preprocessor of EOF, while keeping everything alive.
-  Clang->getPreprocessor().EndSourceFile();
 
   std::vector<Diag> Diags = CompilerInvocationDiags;
   // Add diagnostics from the preamble, if any.

diff  --git a/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp b/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
index e8757079c675..6974131562b3 100644
--- a/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
+++ b/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
@@ -29,6 +29,8 @@ namespace clangd {
 namespace {
 
 using ::testing::_;
+using ::testing::AllOf;
+using ::testing::Contains;
 using ::testing::ElementsAre;
 using ::testing::Field;
 using ::testing::IsEmpty;
@@ -278,6 +280,23 @@ TEST(DiagnosticsTest, ClangTidy) {
                   "use a trailing return type for this function")))));
 }
 
+TEST(DiagnosticsTest, ClangTidyEOF) {
+  // clang-format off
+  Annotations Test(R"cpp(
+  [[#]]include <b.h>
+  #include "a.h")cpp");
+  // clang-format on
+  auto TU = TestTU::withCode(Test.code());
+  TU.ExtraArgs = {"-isystem."};
+  TU.AdditionalFiles["a.h"] = TU.AdditionalFiles["b.h"] = "";
+  TU.ClangTidyChecks = "-*, llvm-include-order";
+  EXPECT_THAT(
+      TU.build().getDiagnostics(),
+      Contains(AllOf(Diag(Test.range(), "#includes are not sorted properly"),
+                     DiagSource(Diag::ClangTidy),
+                     DiagName("llvm-include-order"))));
+}
+
 TEST(DiagnosticTest, TemplatesInHeaders) {
   // Diagnostics from templates defined in headers are placed at the expansion.
   Annotations Main(R"cpp(