Author: maskray Date: Sat May 18 21:19:14 2019 New Revision: 361113 URL: http://llvm.org/viewvc/llvm-project?rev=361113&view=rev Log: [clangd] Respect WarningsAsErrors configuration for clang-tidy
This completes the fix for https://bugs.llvm.org/show_bug.cgi?id=41218. Reviewed By: sammccall Patch by Nathan Ridge! Differential Revision: https://reviews.llvm.org/D61841 Modified: clang-tools-extra/trunk/clangd/ClangdUnit.cpp clang-tools-extra/trunk/clangd/Diagnostics.cpp clang-tools-extra/trunk/clangd/Diagnostics.h clang-tools-extra/trunk/clangd/unittests/DiagnosticsTests.cpp clang-tools-extra/trunk/clangd/unittests/TestTU.cpp clang-tools-extra/trunk/clangd/unittests/TestTU.h Modified: clang-tools-extra/trunk/clangd/ClangdUnit.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdUnit.cpp?rev=361113&r1=361112&r2=361113&view=diff ============================================================================== --- clang-tools-extra/trunk/clangd/ClangdUnit.cpp (original) +++ clang-tools-extra/trunk/clangd/ClangdUnit.cpp Sat May 18 21:19:14 2019 @@ -332,14 +332,22 @@ ParsedAST::build(std::unique_ptr<Compile 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 @@ ParsedAST::build(std::unique_ptr<Compile 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) { Modified: clang-tools-extra/trunk/clangd/Diagnostics.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/Diagnostics.cpp?rev=361113&r1=361112&r2=361113&view=diff ============================================================================== --- clang-tools-extra/trunk/clangd/Diagnostics.cpp (original) +++ clang-tools-extra/trunk/clangd/Diagnostics.cpp Sat May 18 21:19:14 2019 @@ -514,9 +514,12 @@ void StoreDiags::HandleDiagnostic(Diagno // 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; Modified: clang-tools-extra/trunk/clangd/Diagnostics.h URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/Diagnostics.h?rev=361113&r1=361112&r2=361113&view=diff ============================================================================== --- clang-tools-extra/trunk/clangd/Diagnostics.h (original) +++ clang-tools-extra/trunk/clangd/Diagnostics.h Sat May 18 21:19:14 2019 @@ -128,20 +128,20 @@ public: using DiagFixer = std::function<std::vector<Fix>(DiagnosticsEngine::Level, const clang::Diagnostic &)>; - using DiagFilter = - std::function<bool(DiagnosticsEngine::Level, const clang::Diagnostic &)>; + using LevelAdjuster = std::function<DiagnosticsEngine::Level( + DiagnosticsEngine::Level, const clang::Diagnostic &)>; /// 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<Diag> Output; llvm::Optional<LangOptions> LangOpts; llvm::Optional<Diag> LastDiag; Modified: clang-tools-extra/trunk/clangd/unittests/DiagnosticsTests.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/unittests/DiagnosticsTests.cpp?rev=361113&r1=361112&r2=361113&view=diff ============================================================================== --- clang-tools-extra/trunk/clangd/unittests/DiagnosticsTests.cpp (original) +++ clang-tools-extra/trunk/clangd/unittests/DiagnosticsTests.cpp Sat May 18 21:19:14 2019 @@ -73,6 +73,7 @@ MATCHER_P(EqualToLSPDiag, LSPDiag, 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 @@ TEST(DiagnosticTest, ClangTidySuppressio 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: Modified: clang-tools-extra/trunk/clangd/unittests/TestTU.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/unittests/TestTU.cpp?rev=361113&r1=361112&r2=361113&view=diff ============================================================================== --- clang-tools-extra/trunk/clangd/unittests/TestTU.cpp (original) +++ clang-tools-extra/trunk/clangd/unittests/TestTU.cpp Sat May 18 21:19:14 2019 @@ -48,6 +48,7 @@ ParsedAST TestTU::build() const { 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; Modified: clang-tools-extra/trunk/clangd/unittests/TestTU.h URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/unittests/TestTU.h?rev=361113&r1=361112&r2=361113&view=diff ============================================================================== --- clang-tools-extra/trunk/clangd/unittests/TestTU.h (original) +++ clang-tools-extra/trunk/clangd/unittests/TestTU.h Sat May 18 21:19:14 2019 @@ -57,6 +57,7 @@ struct TestTU { std::vector<const char *> ExtraArgs; llvm::Optional<std::string> ClangTidyChecks; + llvm::Optional<std::string> ClangTidyWarningsAsErrors; // Index to use when building AST. const SymbolIndex *ExternalIndex = nullptr; _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits