[PATCH] D92874: [clangd] Validate clang-tidy Checks in clangd config.

[Nathan James via Phabricator via cfe-commits]

njames93 updated this revision to Diff 311926.
njames93 added a comment.

Address some comments


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D92874/new/

https://reviews.llvm.org/D92874

Files:
  clang-tools-extra/clangd/ConfigCompile.cpp
  clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp


Index: clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
+++ clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
@@ -200,6 +200,24 @@
   EXPECT_EQ(Conf.ClangTidy.CheckOptions.lookup("StrictMode"), "true");
   EXPECT_EQ(Conf.ClangTidy.CheckOptions.lookup("example-check.ExampleOption"),
             "0");
+  EXPECT_THAT(Diags.Diagnostics, IsEmpty());
+}
+
+TEST_F(ConfigCompileTests, TidyBadChecks) {
+  Frag.ClangTidy.Add.emplace_back("unknown-check");
+  Frag.ClangTidy.Remove.emplace_back("*");
+  Frag.ClangTidy.Remove.emplace_back("llvm-includeorder");
+  EXPECT_TRUE(compileAndApply());
+  // Ensure bad checks are stripped from the glob.
+  EXPECT_EQ(Conf.ClangTidy.Checks, "-*");
+  EXPECT_THAT(
+      Diags.Diagnostics,
+      ElementsAre(
+          AllOf(DiagMessage("clang-tidy check 'unknown-check' was not found"),
+                DiagKind(llvm::SourceMgr::DK_Warning)),
+          AllOf(
+              DiagMessage("clang-tidy check 'llvm-includeorder' was not found"),
+              DiagKind(llvm::SourceMgr::DK_Warning))));
 }
 
 TEST_F(ConfigCompileTests, ExternalBlockWarnOnMultipleSource) {
Index: clang-tools-extra/clangd/ConfigCompile.cpp
===================================================================
--- clang-tools-extra/clangd/ConfigCompile.cpp
+++ clang-tools-extra/clangd/ConfigCompile.cpp
@@ -23,6 +23,7 @@
 //
 //===----------------------------------------------------------------------===//
 
+#include "../clang-tidy/ClangTidyModuleRegistry.h"
 #include "CompileCommands.h"
 #include "Config.h"
 #include "ConfigFragment.h"
@@ -35,7 +36,9 @@
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/StringRef.h"
+#include "llvm/ADT/StringSet.h"
 #include "llvm/ADT/StringSwitch.h"
+#include "llvm/Support/Allocator.h"
 #include "llvm/Support/Error.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/Format.h"
@@ -47,7 +50,31 @@
 #include <string>
 
 namespace clang {
+namespace tidy {
+/// Returns if a clang tidy \p Check has been registered.
+static bool isRegisteredCheck(StringRef Check) {
+  assert(!Check.empty());
+  assert(!Check.contains('*') && "isRegisteredCheck doesn't support globs");
+  assert(Check.trim().size() == Check.size() &&
+         "Check has trailing or leading whitespace");
+  assert(Check.front() != '-');
+
+  static const llvm::StringSet<llvm::BumpPtrAllocator> AllChecks = [] {
+    llvm::StringSet<llvm::BumpPtrAllocator> Result;
+    ClangTidyCheckFactories Factories;
+    for (ClangTidyModuleRegistry::entry E : ClangTidyModuleRegistry::entries())
+      E.instantiate()->addCheckFactories(Factories);
+    for (const auto &Factory : Factories)
+      Result.insert(Factory.getKey());
+    return Result;
+  }();
+
+  return AllChecks.contains(Check);
+}
+
+} // namespace tidy
 namespace clangd {
+
 namespace config {
 namespace {
 
@@ -349,13 +376,19 @@
 
   void appendTidyCheckSpec(std::string &CurSpec,
                            const Located<std::string> &Arg, bool IsPositive) {
-    StringRef Str = *Arg;
+    StringRef Str = StringRef(*Arg).trim();
     // Don't support negating here, its handled if the item is in the Add or
     // Remove list.
     if (Str.startswith("-") || Str.contains(',')) {
       diag(Error, "Invalid clang-tidy check name", Arg.Range);
       return;
     }
+    if (!Str.contains('*') && !tidy::isRegisteredCheck(Str)) {
+      diag(Warning,
+           llvm::formatv("clang-tidy check '{0}' was not found", Str).str(),
+           Arg.Range);
+      return;
+    }
     CurSpec += ',';
     if (!IsPositive)
       CurSpec += '-';

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits