[clangd] Allow diagnostics to be suppressed with configuration

[sammccall]

Summary

This has been specifically requested:

https://github.com/clangd/vscode-clangd/issues/114

and various issues can be addressed with this as a workaround, e.g.:

https://github.com/clangd/clangd/issues/662

[hokein]

clang-tools-extra/clangd/Diagnostics.cpp

808

nit: use normalizeSuppressedCode(Code)?

[ArcsinX]

clang-tools-extra/clangd/ParsedAST.cpp

319

This line breaks GCC 5.4 build with the following log:

llvm-project/clang-tools-extra/clangd/ParsedAST.cpp:319:55: error: binding ‘const clang::clangd::Config’ to reference of type ‘clang::clangd::Config&’ discards qualifiers
     ASTDiags.setLevelAdjuster([&, &Cfg(Config::current())](
                                                       ^

Unsure is it a compiler bug or not. Moving Cfg out of the capture list or using a cast to Config & solves the problem.

[sammccall]

clang-tools-extra/clangd/ParsedAST.cpp

319

Thanks! Appears to be https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66735

Landed the workaround as 12de8e1399fecf691639ba430b3824acb1311e70

clang-tools-extra/clangd/Config.h

 
 #ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_CONFIG_H
 #define LLVM_CLANG_TOOLS_EXTRA_CLANGD_CONFIG_H
 
 #include "support/Context.h"
 #include "llvm/ADT/FunctionExtras.h"
 #include "llvm/ADT/Optional.h"
 #include "llvm/ADT/StringMap.h"
+#include "llvm/ADT/StringSet.h"
 #include <string>
 #include <vector>
 
 namespace clang {
 namespace clangd {
 
 /// Settings that express user/project preferences and control clangd behavior.
 ///
struct Config {
   };
   /// Controls background-index behavior.
   struct {
     /// Whether this TU should be indexed.
     BackgroundPolicy Background = BackgroundPolicy::Build;
     llvm::Optional<ExternalIndexSpec> 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"
     // declarations, always spell out the whole name (with or without leading
     // ::). All nested namespaces are affected as well.
     std::vector<std::string> FullyQualifiedNamespaces;
   } Style;
 

clang-tools-extra/clangd/ConfigCompile.cpp

 //  - problems can be reported in stage 1, in stage 2 we must silently recover
 //
 //===----------------------------------------------------------------------===//
 
 #include "CompileCommands.h"
 #include "Config.h"
 #include "ConfigFragment.h"
 #include "ConfigProvider.h"
+#include "Diagnostics.h"
 #include "Features.inc"
 #include "TidyProvider.h"
 #include "support/Logger.h"
 #include "support/Trace.h"
 #include "llvm/ADT/None.h"
 #include "llvm/ADT/Optional.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallString.h"
void compile(Fragment &&F) {
     if (!F.Source.Directory.empty()) {
       FragmentDirectory = llvm::sys::path::convert_to_slash(F.Source.Directory);
       if (FragmentDirectory.back() != '/')
         FragmentDirectory += '/';
     }
     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));
   }
 
   void compile(Fragment::IfBlock &&F) {
     if (F.HasUnrecognizedCondition)
       Out.Conditions.push_back([&](const Params &) { return false; });
 
     auto PathMatch = std::make_unique<std::vector<llvm::Regex>>();
Out.Apply.push_back([Spec(std::move(Spec))](const Params &P, Config &C) {
       C.Index.External = Spec;
       // Disable background indexing for the files under the mountpoint.
       // Note that this will overwrite statements in any previous fragments
       // (including the current one).
       C.Index.Background = Config::BackgroundPolicy::Skip;
     });
   }
 
+  void compile(Fragment::DiagnosticsBlock &&F) {
+    std::vector<llvm::StringRef> 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<std::string> FullyQualifiedNamespaces;
       for (auto &N : F.FullyQualifiedNamespaces) {
         // Normalize the data by dropping both leading and trailing ::
         StringRef Namespace(*N);
         Namespace.consume_front("::");
         Namespace.consume_back("::");

clang-tools-extra/clangd/ConfigFragment.h

struct ExternalBlock {
       /// associated with the config fragment. Absolute in case of user config
       /// and relative otherwise. Should always use forward-slashes.
       llvm::Optional<Located<std::string>> MountPoint;
     };
     llvm::Optional<Located<ExternalBlock>> External;
   };
   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<Located<std::string>> Suppress;
+  };
+  DiagnosticsBlock Diagnostics;
+
   // Describes the style of the codebase, beyond formatting.
   struct StyleBlock {
     // Namespaces that should always be fully qualified, meaning no "using"
     // declarations, always spell out the whole name (with or without leading
     // ::). All nested namespaces are affected as well.
     // Affects availability of the AddUsing tweak.
     std::vector<Located<std::string>> FullyQualifiedNamespaces;
   };
   StyleBlock Style;
 
   /// Controls how clang-tidy will run over the code base.
   ///
   /// 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<Located<std::string>> Add;
     /// List of checks to disable.
     /// Takes precedence over Add. To enable all llvm checks except include
     /// order:
     ///   Add: llvm-*
     ///   Remove: llvm-include-onder
     std::vector<Located<std::string>> Remove;

clang-tools-extra/clangd/Diagnostics.h

private:
   llvm::Optional<FullSourceLoc> LastDiagLoc; // Valid only when LastDiag is set.
   bool LastDiagOriginallyError = false;      // Valid only when LastDiag is set.
   SourceManager *OrigSrcMgr = nullptr;
 
   llvm::DenseSet<std::pair<unsigned, unsigned>> IncludedErrorLocations;
   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
 
 #endif // LLVM_CLANG_TOOLS_EXTRA_CLANGD_DIAGNOSTICS_H

clang-tools-extra/clangd/Diagnostics.cpp

if (tryMoveToMainFile(*LastDiag, *LastDiagLoc)) {
         return;
     }
   }
   if (!mentionsMainFile(*LastDiag))
     return;
   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

clang-tools-extra/clangd/ParsedAST.cpp

 //===--- ParsedAST.cpp -------------------------------------------*- C++-*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include "ParsedAST.h"
 #include "../clang-tidy/ClangTidyCheck.h"
 #include "../clang-tidy/ClangTidyDiagnosticConsumer.h"
 #include "../clang-tidy/ClangTidyModuleRegistry.h"
 #include "AST.h"
 #include "Compiler.h"
+#include "Config.h"
 #include "Diagnostics.h"
 #include "Headers.h"
 #include "IncludeFixer.h"
 #include "Preamble.h"
 #include "SourceCode.h"
 #include "TidyProvider.h"
 #include "index/CanonicalIncludes.h"
 #include "index/Index.h"
llvm::erase_if(CTChecks, [&](const auto &Check) {
       return !Check->isLanguageVersionSupported(CTContext->getLangOpts());
     });
     Preprocessor *PP = &Clang->getPreprocessor();
     for (const auto &Check : CTChecks) {
       Check->registerPPCallbacks(Clang->getSourceManager(), PP, PP);
       Check->registerMatchers(&CTFinder);
     }
 
-    if (!CTChecks.empty()) {
-      ASTDiags.setLevelAdjuster([&CTContext](DiagnosticsEngine::Level DiagLevel,
-                                             const clang::Diagnostic &Info) {
+    ASTDiags.setLevelAdjuster([&, &Cfg(Config::current())](
+                                  DiagnosticsEngine::Level DiagLevel,
+      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
           // 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());
           if (IsInsideMainFile &&
               tidy::shouldSuppressDiagnostic(DiagLevel, Info, *CTContext,
                                              /*AllowIO=*/false)) {
             return DiagnosticsEngine::Ignored;
           }
 
           // Check for warning-as-error.
           if (DiagLevel == DiagnosticsEngine::Warning &&
               CTContext->treatAsError(CheckName)) {
             return DiagnosticsEngine::Error;
           }
         }
-        return DiagLevel;
-      });
-    }
-  }
+      }
 
   // Add IncludeFixer which can recover diagnostics caused by missing includes
   // (e.g. incomplete type) and attach include insertion fixes to diagnostics.
   llvm::Optional<IncludeFixer> FixIncludes;
   auto BuildDir = VFS->getCurrentWorkingDirectory();
   if (Inputs.Index && !BuildDir.getError()) {
     auto Style = getFormatStyleForFile(Filename, Inputs.Contents, *Inputs.TFS);
     auto Inserter = std::make_shared<IncludeInserter>(

clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp

 //===-- ConfigCompileTests.cpp --------------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include "Config.h"
 #include "ConfigFragment.h"
 #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"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/SourceMgr.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
 #include <string>
 
 namespace clang {
 namespace clangd {
 namespace config {
 namespace {
 using ::testing::AllOf;
 using ::testing::Contains;
 using ::testing::ElementsAre;
 using ::testing::IsEmpty;
 using ::testing::SizeIs;
 using ::testing::StartsWith;
+using ::testing::UnorderedElementsAre;
 
 class ConfigCompileTests : public ::testing::Test {
 protected:
   CapturedDiags Diags;
   Config Conf;
   Fragment Frag;
   Params Parm;
 
for (const auto &Case : Cases) {
     Frag = {};
     Frag.If.PathExclude.emplace_back(Case.PathSpec);
     Frag.Source.Directory = Case.Directory;
     EXPECT_NE(compileAndApply(), Case.ShouldMatch);
     ASSERT_THAT(Diags.Diagnostics, IsEmpty());
   }
 }
 
+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-*");
   Frag.ClangTidy.Remove.emplace_back("llvm-include-order");
   Frag.ClangTidy.Remove.emplace_back("readability-*");
   Frag.ClangTidy.CheckOptions.emplace_back(
       std::make_pair(std::string("StrictMode"), std::string("true")));
   Frag.ClangTidy.CheckOptions.emplace_back(std::make_pair(

clang-tools-extra/clangd/unittests/ConfigYAMLTests.cpp

If:
   EXPECT_THAT(Diags.Diagnostics, IsEmpty());
   ASSERT_EQ(Results.size(), 1u);
   ASSERT_NE(Results.front().Source.Manager, nullptr);
   EXPECT_EQ(toRange(Results.front().If.PathMatch.front().Range,
                     *Results.front().Source.Manager),
             YAML.range());
 }
 
-TEST(ParseYAML, Diagnostics) {
+TEST(ParseYAML, ConfigDiagnostics) {
   CapturedDiags Diags;
   Annotations YAML(R"yaml(
 If:
   $unknown[[UnknownCondition]]: "foo"
 CompileFlags:
   Add: 'first'
 ---
 CompileFlags: {$unexpected^

clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp

 //===--- DiagnosticsTests.cpp ------------------------------------*- C++-*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include "Annotations.h"
+#include "Config.h"
 #include "Diagnostics.h"
 #include "ParsedAST.h"
 #include "Protocol.h"
 #include "SourceCode.h"
 #include "TestFS.h"
 #include "TestIndex.h"
 #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"
 #include "llvm/Support/ScopedPrinter.h"
 #include "llvm/Support/TargetSelect.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
 #include <algorithm>
TEST(DiagnosticTest, NoMultipleDiagnosticInFlight) {
   TU.ClangTidyProvider = addTidyChecks("modernize-loop-convert");
   EXPECT_THAT(
       TU.build().getDiagnostics(),
       UnorderedElementsAre(::testing::AllOf(
           Diag(Main.range(), "use range-based for loop instead"),
           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() {
       int i = 3;
       double d = 8 / i;  // NOLINT
       // NOLINTNEXTLINE
       double e = 8 / i;
       #define BAD 8 / i