[PATCH] D103387: [clangd] Fix feature modules to drop diagnostics

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet created this revision.
kadircet added a reviewer: sammccall.
Herald added subscribers: usaxena95, arphaman.
kadircet requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov.
Herald added a project: clang-tools-extra.

Ignored diagnostics were only checked after level adjusters and assumed
it would stay the same for the rest. But it can also be modified by
FeatureModules.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D103387

Files:
  clang-tools-extra/clangd/Diagnostics.cpp
  clang-tools-extra/clangd/unittests/FeatureModulesTests.cpp


Index: clang-tools-extra/clangd/unittests/FeatureModulesTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/FeatureModulesTests.cpp
+++ clang-tools-extra/clangd/unittests/FeatureModulesTests.cpp
@@ -6,6 +6,7 @@
 //
 //===----------------------------------------------------------------------===//
 
+#include "Annotations.h"
 #include "FeatureModule.h"
 #include "Selection.h"
 #include "TestTU.h"
@@ -53,6 +54,37 @@
   EXPECT_EQ(Actual->get()->id(), TweakID);
 }
 
+TEST(FeatureModulesTest, SuppressDiags) {
+  struct DiagModifierModule final : public FeatureModule {
+    struct Listener : public FeatureModule::ASTListener {
+      void sawDiagnostic(const clang::Diagnostic &Info,
+                         clangd::Diag &Diag) override {
+        Diag.Severity = DiagnosticsEngine::Ignored;
+      }
+    };
+    std::unique_ptr<ASTListener> astListeners() override {
+      return std::make_unique<Listener>();
+    };
+  };
+  FeatureModuleSet FMS;
+  FMS.add(std::make_unique<DiagModifierModule>());
+
+  Annotations Code("[[test]]; /* error-ok */");
+  TestTU TU;
+  TU.Code = Code.code().str();
+
+  {
+    auto AST = TU.build();
+    EXPECT_THAT(*AST.getDiagnostics(), testing::Not(testing::IsEmpty()));
+  }
+
+  TU.FeatureModules = &FMS;
+  {
+    auto AST = TU.build();
+    EXPECT_THAT(*AST.getDiagnostics(), testing::IsEmpty());
+  }
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clang-tools-extra/clangd/Diagnostics.cpp
===================================================================
--- clang-tools-extra/clangd/Diagnostics.cpp
+++ clang-tools-extra/clangd/Diagnostics.cpp
@@ -727,29 +727,29 @@
     // Handle the new main diagnostic.
     flushLastDiag();
 
-    if (Adjuster) {
+    // FIXME: Merge with feature modules.
+    if (Adjuster)
       DiagLevel = Adjuster(DiagLevel, Info);
-      if (DiagLevel == DiagnosticsEngine::Ignored) {
-        LastPrimaryDiagnosticWasSuppressed = true;
-        return;
-      }
-    }
-    LastPrimaryDiagnosticWasSuppressed = false;
 
     LastDiag = Diag();
     FillDiagBase(*LastDiag);
     LastDiagLoc.emplace(Info.getLocation(), Info.getSourceManager());
     LastDiagOriginallyError = OriginallyError;
+    LastPrimaryDiagnosticWasSuppressed = false;
 
+    if (DiagCB)
+      DiagCB(Info, *LastDiag);
+    if (LastDiag->Severity == DiagnosticsEngine::Ignored) {
+      LastPrimaryDiagnosticWasSuppressed = true;
+      return;
+    }
     if (!Info.getFixItHints().empty())
       AddFix(true /* try to invent a message instead of repeating the diag */);
     if (Fixer) {
-      auto ExtraFixes = Fixer(DiagLevel, Info);
+      auto ExtraFixes = Fixer(LastDiag->Severity, Info);
       LastDiag->Fixes.insert(LastDiag->Fixes.end(), ExtraFixes.begin(),
                              ExtraFixes.end());
     }
-    if (DiagCB)
-      DiagCB(Info, *LastDiag);
   } else {
     // Handle a note to an existing diagnostic.
 
@@ -781,7 +781,7 @@
 }
 
 void StoreDiags::flushLastDiag() {
-  if (!LastDiag)
+  if (!LastDiag || LastPrimaryDiagnosticWasSuppressed)
     return;
   auto Finish = llvm::make_scope_exit([&, NDiags(Output.size())] {
     if (Output.size() == NDiags) // No new diag emitted.

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits