diff --git a/clang-tools-extra/clangd/Config.h b/clang-tools-extra/clangd/Config.h --- a/clang-tools-extra/clangd/Config.h +++ b/clang-tools-extra/clangd/Config.h @@ -28,6 +28,7 @@ #include "llvm/ADT/FunctionExtras.h" #include "llvm/ADT/Optional.h" #include "llvm/ADT/StringMap.h" +#include "llvm/ADT/StringSet.h" #include #include @@ -77,6 +78,12 @@ llvm::Optional External; } Index; + /// Controls warnings and errors when parsing code. + struct { + bool SuppressAll = false; + llvm::StringSet<> Suppress; + } Diagnostics; + /// Style of the codebase. struct { // Namespaces that should always be fully qualified, meaning no "using" diff --git a/clang-tools-extra/clangd/ConfigCompile.cpp b/clang-tools-extra/clangd/ConfigCompile.cpp --- a/clang-tools-extra/clangd/ConfigCompile.cpp +++ b/clang-tools-extra/clangd/ConfigCompile.cpp @@ -27,6 +27,7 @@ #include "Config.h" #include "ConfigFragment.h" #include "ConfigProvider.h" +#include "Diagnostics.h" #include "Features.inc" #include "TidyProvider.h" #include "support/Logger.h" @@ -187,6 +188,7 @@ compile(std::move(F.If)); compile(std::move(F.CompileFlags)); compile(std::move(F.Index)); + compile(std::move(F.Diagnostics)); compile(std::move(F.ClangTidy)); } @@ -328,6 +330,27 @@ }); } + void compile(Fragment::DiagnosticsBlock &&F) { + std::vector Normalized; + for (const auto &Suppressed : F.Suppress) { + if (*Suppressed == "*") { + Out.Apply.push_back([&](const Params &, Config &C) { + C.Diagnostics.SuppressAll = true; + C.Diagnostics.Suppress.clear(); + }); + return; + } + Normalized.push_back(normalizeSuppressedCode(*Suppressed)); + } + if (!Normalized.empty()) + Out.Apply.push_back([Normalized](const Params &, Config &C) { + if (C.Diagnostics.SuppressAll) + return; + for (llvm::StringRef N : Normalized) + C.Diagnostics.Suppress.insert(N); + }); + } + void compile(Fragment::StyleBlock &&F) { if (!F.FullyQualifiedNamespaces.empty()) { std::vector FullyQualifiedNamespaces; diff --git a/clang-tools-extra/clangd/ConfigFragment.h b/clang-tools-extra/clangd/ConfigFragment.h --- a/clang-tools-extra/clangd/ConfigFragment.h +++ b/clang-tools-extra/clangd/ConfigFragment.h @@ -181,6 +181,24 @@ }; IndexBlock Index; + /// Controls behavior of diagnostics (errors and warnings). + struct DiagnosticsBlock { + /// Diagnostic codes that should be suppressed. + /// + /// Valid values are: + /// - *, to disable all diagnostics + /// - diagnostic codes exposed by clangd (e.g unknown_type, -Wunused-result) + /// - clang internal diagnostic codes (e.g. err_unknown_type) + /// - warning categories (e.g. unused-result) + /// - clang-tidy check names (e.g. bugprone-narrowing-conversions) + /// + /// This is a simple filter. Diagnostics can be controlled in other ways + /// (e.g. by disabling a clang-tidy check, or the -Wunused compile flag). + /// This often has other advantages, such as skipping some analysis. + std::vector> Suppress; + }; + DiagnosticsBlock Diagnostics; + // Describes the style of the codebase, beyond formatting. struct StyleBlock { // Namespaces that should always be fully qualified, meaning no "using" @@ -195,6 +213,7 @@ /// /// The settings are merged with any settings found in .clang-tidy /// configiration files with these ones taking precedence. + // FIXME: move this to Diagnostics.Tidy. struct ClangTidyBlock { std::vector> Add; /// List of checks to disable. diff --git a/clang-tools-extra/clangd/Diagnostics.h b/clang-tools-extra/clangd/Diagnostics.h --- a/clang-tools-extra/clangd/Diagnostics.h +++ b/clang-tools-extra/clangd/Diagnostics.h @@ -159,6 +159,15 @@ bool LastPrimaryDiagnosticWasSuppressed = false; }; +/// Determine whether a (non-clang-tidy) diagnostic is suppressed by config. +bool isBuiltinDiagnosticSuppressed(unsigned ID, + const llvm::StringSet<> &Suppressed); +/// Take a user-specified diagnostic code, and convert it to a normalized form +/// stored in the config and consumed by isBuiltinDiagnosticsSuppressed. +/// +/// (This strips err_ and -W prefix so we can match with or without them.) +llvm::StringRef normalizeSuppressedCode(llvm::StringRef); + } // namespace clangd } // namespace clang diff --git a/clang-tools-extra/clangd/Diagnostics.cpp b/clang-tools-extra/clangd/Diagnostics.cpp --- a/clang-tools-extra/clangd/Diagnostics.cpp +++ b/clang-tools-extra/clangd/Diagnostics.cpp @@ -801,5 +801,23 @@ Output.push_back(std::move(*LastDiag)); } +bool isBuiltinDiagnosticSuppressed(unsigned ID, + const llvm::StringSet<> &Suppress) { + if (const char *CodePtr = getDiagnosticCode(ID)) { + if (Suppress.contains(normalizeSuppressedCode(CodePtr))) + return true; + } + StringRef Warning = DiagnosticIDs::getWarningOptionForDiag(ID); + if (!Warning.empty() && Suppress.contains(Warning)) + return true; + return false; +} + +llvm::StringRef normalizeSuppressedCode(llvm::StringRef Code) { + Code.consume_front("err_"); + Code.consume_front("-W"); + return Code; +} + } // namespace clangd } // namespace clang diff --git a/clang-tools-extra/clangd/ParsedAST.cpp b/clang-tools-extra/clangd/ParsedAST.cpp --- a/clang-tools-extra/clangd/ParsedAST.cpp +++ b/clang-tools-extra/clangd/ParsedAST.cpp @@ -12,6 +12,7 @@ #include "../clang-tidy/ClangTidyModuleRegistry.h" #include "AST.h" #include "Compiler.h" +#include "Config.h" #include "Diagnostics.h" #include "Headers.h" #include "IncludeFixer.h" @@ -315,12 +316,18 @@ Check->registerMatchers(&CTFinder); } - if (!CTChecks.empty()) { - ASTDiags.setLevelAdjuster([&CTContext](DiagnosticsEngine::Level DiagLevel, - const clang::Diagnostic &Info) { + ASTDiags.setLevelAdjuster([&, &Cfg(Config::current())]( + DiagnosticsEngine::Level DiagLevel, + const clang::Diagnostic &Info) { + if (Cfg.Diagnostics.SuppressAll || + isBuiltinDiagnosticSuppressed(Info.getID(), Cfg.Diagnostics.Suppress)) + return DiagnosticsEngine::Ignored; + if (!CTChecks.empty()) { std::string CheckName = CTContext->getCheckName(Info.getID()); bool IsClangTidyDiag = !CheckName.empty(); if (IsClangTidyDiag) { + if (Cfg.Diagnostics.Suppress.contains(CheckName)) + return DiagnosticsEngine::Ignored; // 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 @@ -342,9 +349,9 @@ return DiagnosticsEngine::Error; } } - return DiagLevel; - }); - } + } + return DiagLevel; + }); } // Add IncludeFixer which can recover diagnostics caused by missing includes diff --git a/clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp b/clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp --- a/clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp +++ b/clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp @@ -11,6 +11,7 @@ #include "ConfigTesting.h" #include "Features.inc" #include "TestFS.h" +#include "clang/Basic/DiagnosticSema.h" #include "llvm/ADT/None.h" #include "llvm/ADT/Optional.h" #include "llvm/ADT/StringRef.h" @@ -30,6 +31,7 @@ using ::testing::IsEmpty; using ::testing::SizeIs; using ::testing::StartsWith; +using ::testing::UnorderedElementsAre; class ConfigCompileTests : public ::testing::Test { protected: @@ -183,6 +185,39 @@ } } +TEST_F(ConfigCompileTests, DiagnosticSuppression) { + Frag.Diagnostics.Suppress.emplace_back("bugprone-use-after-move"); + Frag.Diagnostics.Suppress.emplace_back("unreachable-code"); + Frag.Diagnostics.Suppress.emplace_back("-Wunused-variable"); + Frag.Diagnostics.Suppress.emplace_back("typecheck_bool_condition"); + Frag.Diagnostics.Suppress.emplace_back("err_unexpected_friend"); + Frag.Diagnostics.Suppress.emplace_back("warn_alloca"); + EXPECT_TRUE(compileAndApply()); + EXPECT_THAT(Conf.Diagnostics.Suppress.keys(), + UnorderedElementsAre("bugprone-use-after-move", + "unreachable-code", "unused-variable", + "typecheck_bool_condition", + "unexpected_friend", "warn_alloca")); + EXPECT_TRUE(isBuiltinDiagnosticSuppressed(diag::warn_unreachable, + Conf.Diagnostics.Suppress)); + // Subcategory not respected/suppressed. + EXPECT_FALSE(isBuiltinDiagnosticSuppressed(diag::warn_unreachable_break, + Conf.Diagnostics.Suppress)); + EXPECT_TRUE(isBuiltinDiagnosticSuppressed(diag::warn_unused_variable, + Conf.Diagnostics.Suppress)); + EXPECT_TRUE(isBuiltinDiagnosticSuppressed(diag::err_typecheck_bool_condition, + Conf.Diagnostics.Suppress)); + EXPECT_TRUE(isBuiltinDiagnosticSuppressed(diag::err_unexpected_friend, + Conf.Diagnostics.Suppress)); + EXPECT_TRUE(isBuiltinDiagnosticSuppressed(diag::warn_alloca, + Conf.Diagnostics.Suppress)); + + Frag.Diagnostics.Suppress.emplace_back("*"); + EXPECT_TRUE(compileAndApply()); + EXPECT_TRUE(Conf.Diagnostics.SuppressAll); + EXPECT_THAT(Conf.Diagnostics.Suppress, IsEmpty()); +} + TEST_F(ConfigCompileTests, Tidy) { Frag.ClangTidy.Add.emplace_back("bugprone-use-after-move"); Frag.ClangTidy.Add.emplace_back("llvm-*"); diff --git a/clang-tools-extra/clangd/unittests/ConfigYAMLTests.cpp b/clang-tools-extra/clangd/unittests/ConfigYAMLTests.cpp --- a/clang-tools-extra/clangd/unittests/ConfigYAMLTests.cpp +++ b/clang-tools-extra/clangd/unittests/ConfigYAMLTests.cpp @@ -98,7 +98,7 @@ YAML.range()); } -TEST(ParseYAML, Diagnostics) { +TEST(ParseYAML, ConfigDiagnostics) { CapturedDiags Diags; Annotations YAML(R"yaml( If: diff --git a/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp b/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp --- a/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp +++ b/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp @@ -7,6 +7,7 @@ //===----------------------------------------------------------------------===// #include "Annotations.h" +#include "Config.h" #include "Diagnostics.h" #include "ParsedAST.h" #include "Protocol.h" @@ -16,6 +17,7 @@ #include "TestTU.h" #include "TidyProvider.h" #include "index/MemIndex.h" +#include "support/Context.h" #include "support/Path.h" #include "clang/Basic/Diagnostic.h" #include "clang/Basic/DiagnosticSema.h" @@ -371,6 +373,28 @@ DiagSource(Diag::ClangTidy), DiagName("modernize-loop-convert")))); } +TEST(DiagnosticTest, RespectsDiagnosticConfig) { + Annotations Main(R"cpp( + // error-ok + void x() { + [[unknown]](); + $ret[[return]] 42; + } + )cpp"); + auto TU = TestTU::withCode(Main.code()); + EXPECT_THAT( + TU.build().getDiagnostics(), + ElementsAre(Diag(Main.range(), "use of undeclared identifier 'unknown'"), + Diag(Main.range("ret"), + "void function 'x' should not return a value"))); + Config Cfg; + Cfg.Diagnostics.Suppress.insert("return-type"); + WithContextValue WithCfg(Config::Key, std::move(Cfg)); + EXPECT_THAT(TU.build().getDiagnostics(), + ElementsAre(Diag(Main.range(), + "use of undeclared identifier 'unknown'"))); +} + TEST(DiagnosticTest, ClangTidySuppressionComment) { Annotations Main(R"cpp( int main() {