Index: clang-tools-extra/trunk/clangd/ClangdUnit.cpp =================================================================== --- clang-tools-extra/trunk/clangd/ClangdUnit.cpp +++ clang-tools-extra/trunk/clangd/ClangdUnit.cpp @@ -332,14 +332,22 @@ CTContext->setASTContext(&Clang->getASTContext()); CTContext->setCurrentFile(MainInput.getFile()); CTFactories.createChecks(CTContext.getPointer(), CTChecks); - ASTDiags.suppressDiagnostics([&CTContext]( - DiagnosticsEngine::Level DiagLevel, - const clang::Diagnostic &Info) { + ASTDiags.setLevelAdjuster([&CTContext](DiagnosticsEngine::Level DiagLevel, + const clang::Diagnostic &Info) { if (CTContext) { - bool IsClangTidyDiag = !CTContext->getCheckName(Info.getID()).empty(); + std::string CheckName = CTContext->getCheckName(Info.getID()); + bool IsClangTidyDiag = !CheckName.empty(); if (IsClangTidyDiag) { - // Skip the ShouldSuppressDiagnostic check for diagnostics not in - // the main file, because we don't want that function to query the + // 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 not to follow macro expansions, since // those might take us into a preamble file as well. @@ -350,11 +358,11 @@ if (IsInsideMainFile && tidy::ShouldSuppressDiagnostic( DiagLevel, Info, *CTContext, /* CheckMacroExpansion = */ false)) { - return true; + return DiagnosticsEngine::Ignored; } } } - return false; + return DiagLevel; }); Preprocessor *PP = &Clang->getPreprocessor(); for (const auto &Check : CTChecks) { Index: clang-tools-extra/trunk/clangd/Diagnostics.h =================================================================== --- clang-tools-extra/trunk/clangd/Diagnostics.h +++ clang-tools-extra/trunk/clangd/Diagnostics.h @@ -128,20 +128,20 @@ using DiagFixer = std::function(DiagnosticsEngine::Level, const clang::Diagnostic &)>; - using DiagFilter = - std::function; + using LevelAdjuster = std::function; /// If set, possibly adds fixes for diagnostics using \p Fixer. void contributeFixes(DiagFixer Fixer) { this->Fixer = Fixer; } - /// If set, ignore diagnostics for which \p SuppressionFilter returns true. - void suppressDiagnostics(DiagFilter SuppressionFilter) { - this->SuppressionFilter = SuppressionFilter; - } + /// If set, this allows the client of this class to adjust the level of + /// diagnostics, such as promoting warnings to errors, or ignoring + /// diagnostics. + void setLevelAdjuster(LevelAdjuster Adjuster) { this->Adjuster = Adjuster; } private: void flushLastDiag(); DiagFixer Fixer = nullptr; - DiagFilter SuppressionFilter = nullptr; + LevelAdjuster Adjuster = nullptr; std::vector Output; llvm::Optional LangOpts; llvm::Optional LastDiag; Index: clang-tools-extra/trunk/clangd/Diagnostics.cpp =================================================================== --- clang-tools-extra/trunk/clangd/Diagnostics.cpp +++ clang-tools-extra/trunk/clangd/Diagnostics.cpp @@ -514,9 +514,12 @@ // Handle the new main diagnostic. flushLastDiag(); - if (SuppressionFilter && SuppressionFilter(DiagLevel, Info)) { - LastPrimaryDiagnosticWasSuppressed = true; - return; + if (Adjuster) { + DiagLevel = Adjuster(DiagLevel, Info); + if (DiagLevel == DiagnosticsEngine::Ignored) { + LastPrimaryDiagnosticWasSuppressed = true; + return; + } } LastPrimaryDiagnosticWasSuppressed = false; Index: clang-tools-extra/trunk/clangd/unittests/DiagnosticsTests.cpp =================================================================== --- clang-tools-extra/trunk/clangd/unittests/DiagnosticsTests.cpp +++ clang-tools-extra/trunk/clangd/unittests/DiagnosticsTests.cpp @@ -73,6 +73,7 @@ MATCHER_P(DiagSource, S, "") { return arg.Source == S; } MATCHER_P(DiagName, N, "") { return arg.Name == N; } +MATCHER_P(DiagSeverity, S, "") { return arg.Severity == S; } MATCHER_P(EqualToFix, Fix, "LSP fix " + llvm::to_string(Fix)) { if (arg.Message != Fix.Message) @@ -227,6 +228,44 @@ DiagSource(Diag::ClangTidy), DiagName("bugprone-integer-division")))); } +TEST(DiagnosticTest, ClangTidyWarningAsError) { + Annotations Main(R"cpp( + int main() { + int i = 3; + double f = [[8]] / i; + } + )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)))); +} + +TEST(DiagnosticTest, ClangTidyWarningAsErrorTrumpsSuppressionComment) { + Annotations Main(R"cpp( + int main() { + int i = 3; + 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)))); +} + TEST(DiagnosticsTest, Preprocessor) { // This looks like a preamble, but there's an #else in the middle! // Check that: Index: clang-tools-extra/trunk/clangd/unittests/TestTU.h =================================================================== --- clang-tools-extra/trunk/clangd/unittests/TestTU.h +++ clang-tools-extra/trunk/clangd/unittests/TestTU.h @@ -57,6 +57,7 @@ std::vector ExtraArgs; llvm::Optional ClangTidyChecks; + llvm::Optional ClangTidyWarningsAsErrors; // Index to use when building AST. const SymbolIndex *ExternalIndex = nullptr; Index: clang-tools-extra/trunk/clangd/unittests/TestTU.cpp =================================================================== --- clang-tools-extra/trunk/clangd/unittests/TestTU.cpp +++ clang-tools-extra/trunk/clangd/unittests/TestTU.cpp @@ -48,6 +48,7 @@ Inputs.FS = buildTestFS(Files); Inputs.Opts = ParseOptions(); Inputs.Opts.ClangTidyOpts.Checks = ClangTidyChecks; + Inputs.Opts.ClangTidyOpts.WarningsAsErrors = ClangTidyWarningsAsErrors; Inputs.Index = ExternalIndex; if (Inputs.Index) Inputs.Opts.SuggestMissingIncludes = true;