[clang-tools-extra] 5a7276b - [clangd] Have suppression comments take precedence over warning-as-error

[Nathan Ridge via cfe-commits]

Author: Nathan Ridge
Date: 2020-05-12T02:29:03-04:00
New Revision: 5a7276b3548589590b81975d5c3e487dd91ac097

URL: https://github.com/llvm/llvm-project/commit/5a7276b3548589590b81975d5c3e487dd91ac097
DIFF: https://github.com/llvm/llvm-project/commit/5a7276b3548589590b81975d5c3e487dd91ac097.diff

LOG: [clangd] Have suppression comments take precedence over warning-as-error

Summary: This matches the clang-tidy behaviour.

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

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

Tags: #clang

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

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 61221aaa1491..e63f105b1b6c 100644
--- a/clang-tools-extra/clangd/ParsedAST.cpp
+++ b/clang-tools-extra/clangd/ParsedAST.cpp
@@ -314,18 +314,12 @@ ParsedAST::build(llvm::StringRef Filename, const ParseInputs &Inputs,
         std::string CheckName = CTContext->getCheckName(Info.getID());
         bool IsClangTidyDiag = !CheckName.empty();
         if (IsClangTidyDiag) {
-          // Check for warning-as-error.
-          // We deliberately let this take precedence over suppression comments
-          // to match clang-tidy's behaviour.
-          if (DiagLevel == DiagnosticsEngine::Warning &&
-              CTContext->treatAsError(CheckName)) {
-            return DiagnosticsEngine::Error;
-          }
-
           // Check for suppression comment. Skip the check for diagnostics not
           // in the main file, because we don't want that function to query the
           // source buffer for preamble files. For the same reason, we ask
           // shouldSuppressDiagnostic to avoid I/O.
+          // We let suppression comments take precedence over warning-as-error
+          // to match clang-tidy's behaviour.
           bool IsInsideMainFile =
               Info.hasSourceManager() &&
               isInsideMainFile(Info.getLocation(), Info.getSourceManager());
@@ -334,6 +328,12 @@ ParsedAST::build(llvm::StringRef Filename, const ParseInputs &Inputs,
                                              /*AllowIO=*/false)) {
             return DiagnosticsEngine::Ignored;
           }
+
+          // Check for warning-as-error.
+          if (DiagLevel == DiagnosticsEngine::Warning &&
+              CTContext->treatAsError(CheckName)) {
+            return DiagnosticsEngine::Error;
+          }
         }
       }
       return DiagLevel;

diff  --git a/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp b/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
index b94de7412ce3..3d838c78a3c5 100644
--- a/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
+++ b/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
@@ -374,23 +374,17 @@ n]] = 10; // error-ok
                   Fix(Source.range(), "ident", "change 'ide\\…' to 'ident'"))));
 }
 
-TEST(DiagnosticTest, ClangTidyWarningAsErrorTrumpsSuppressionComment) {
+TEST(DiagnosticTest, ClangTidySuppressionCommentTrumpsWarningAsError) {
   Annotations Main(R"cpp(
     int main() {
       int i = 3;
-      double f = [[8]] / i;  // NOLINT // error-ok
+      double f = [[8]] / i;  // NOLINT
     }
   )cpp");
   TestTU TU = TestTU::withCode(Main.code());
   TU.ClangTidyChecks = "bugprone-integer-division";
   TU.ClangTidyWarningsAsErrors = "bugprone-integer-division";
-  EXPECT_THAT(
-      TU.build().getDiagnostics(),
-      UnorderedElementsAre(::testing::AllOf(
-          Diag(Main.range(), "result of integer division used in a floating "
-                             "point context; possible loss of precision"),
-          DiagSource(Diag::ClangTidy), DiagName("bugprone-integer-division"),
-          DiagSeverity(DiagnosticsEngine::Error))));
+  EXPECT_THAT(TU.build().getDiagnostics(), UnorderedElementsAre());
 }
 
 TEST(DiagnosticsTest, Preprocessor) {