[clang-tools-extra] 5ca68d5 - [clang-tidy] Add `-verify-config` command line argument

Nathan James via cfe-commits cfe-commits at lists.llvm.org
Thu Jun 23 11:23:18 PDT 2022


Author: Nathan James
Date: 2022-06-23T19:23:09+01:00
New Revision: 5ca68d5845c04421eef9659119ba2c12d2a56803

URL: https://github.com/llvm/llvm-project/commit/5ca68d5845c04421eef9659119ba2c12d2a56803
DIFF: https://github.com/llvm/llvm-project/commit/5ca68d5845c04421eef9659119ba2c12d2a56803.diff

LOG: [clang-tidy] Add `-verify-config` command line argument

Adds a `-verify-config` command line argument, that when specified will verify the Checks and CheckOptions fields in the config files:
 - A warning will be raised for any check that doesn't correspond to a registered check, a suggestion will also be emitted for close misses.
 - A warning will be raised for any check glob(containing *) that doesn't match any registered check.
 - A warning will be raised for any CheckOption that isn't read by any registered check, a suggestion will also be emitted for close misses.

This can be useful if debuging why a certain check isn't enabled, or the options are being handled as you expect them to be.

Reviewed By: aaron.ballman

Differential Revision: https://reviews.llvm.org/D127446

Added: 
    clang-tools-extra/test/clang-tidy/infrastructure/verify-config.cpp

Modified: 
    clang-tools-extra/clang-tidy/ClangTidy.cpp
    clang-tools-extra/clang-tidy/ClangTidy.h
    clang-tools-extra/clang-tidy/ClangTidyCheck.cpp
    clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h
    clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
    clang-tools-extra/docs/ReleaseNotes.rst
    clang-tools-extra/docs/clang-tidy/index.rst

Removed: 
    


################################################################################
diff  --git a/clang-tools-extra/clang-tidy/ClangTidy.cpp b/clang-tools-extra/clang-tidy/ClangTidy.cpp
index f7ffcdbc692b8..4007363505ed6 100644
--- a/clang-tools-extra/clang-tidy/ClangTidy.cpp
+++ b/clang-tools-extra/clang-tidy/ClangTidy.cpp
@@ -623,5 +623,40 @@ void exportReplacements(const llvm::StringRef MainFilePath,
   YAML << TUD;
 }
 
+NamesAndOptions
+getAllChecksAndOptions(bool AllowEnablingAnalyzerAlphaCheckers) {
+  NamesAndOptions Result;
+  ClangTidyOptions Opts;
+  Opts.Checks = "*";
+  clang::tidy::ClangTidyContext Context(
+      std::make_unique<DefaultOptionsProvider>(ClangTidyGlobalOptions(), Opts),
+      AllowEnablingAnalyzerAlphaCheckers);
+  ClangTidyCheckFactories Factories;
+  for (const ClangTidyModuleRegistry::entry &Module :
+       ClangTidyModuleRegistry::entries()) {
+    Module.instantiate()->addCheckFactories(Factories);
+  }
+
+  for (const auto &Factory : Factories)
+    Result.Names.insert(Factory.getKey());
+
+#if CLANG_TIDY_ENABLE_STATIC_ANALYZER
+  SmallString<64> Buffer(AnalyzerCheckNamePrefix);
+  size_t DefSize = Buffer.size();
+  for (const auto &AnalyzerCheck : AnalyzerOptions::getRegisteredCheckers(
+           AllowEnablingAnalyzerAlphaCheckers)) {
+    Buffer.truncate(DefSize);
+    Buffer.append(AnalyzerCheck);
+    Result.Names.insert(Buffer);
+  }
+#endif // CLANG_TIDY_ENABLE_STATIC_ANALYZER
+
+  Context.setOptionsCollector(&Result.Options);
+  for (const auto &Factory : Factories) {
+    Factory.getValue()(Factory.getKey(), &Context);
+  }
+
+  return Result;
+}
 } // namespace tidy
 } // namespace clang

diff  --git a/clang-tools-extra/clang-tidy/ClangTidy.h b/clang-tools-extra/clang-tidy/ClangTidy.h
index 507d1ce6e572d..51d9e226c7946 100644
--- a/clang-tools-extra/clang-tidy/ClangTidy.h
+++ b/clang-tools-extra/clang-tidy/ClangTidy.h
@@ -11,6 +11,7 @@
 
 #include "ClangTidyDiagnosticConsumer.h"
 #include "ClangTidyOptions.h"
+#include "llvm/ADT/StringSet.h"
 #include <memory>
 #include <vector>
 
@@ -57,6 +58,14 @@ class ClangTidyASTConsumerFactory {
 std::vector<std::string> getCheckNames(const ClangTidyOptions &Options,
                                        bool AllowEnablingAnalyzerAlphaCheckers);
 
+struct NamesAndOptions {
+  llvm::StringSet<> Names;
+  llvm::StringSet<> Options;
+};
+
+NamesAndOptions
+getAllChecksAndOptions(bool AllowEnablingAnalyzerAlphaCheckers = true);
+
 /// Returns the effective check-specific options.
 ///
 /// The method configures ClangTidy with the specified \p Options and collects

diff  --git a/clang-tools-extra/clang-tidy/ClangTidyCheck.cpp b/clang-tools-extra/clang-tidy/ClangTidyCheck.cpp
index dc38b4e190cc1..83aef0a40c9a1 100644
--- a/clang-tools-extra/clang-tidy/ClangTidyCheck.cpp
+++ b/clang-tools-extra/clang-tidy/ClangTidyCheck.cpp
@@ -53,6 +53,8 @@ ClangTidyCheck::OptionsView::OptionsView(
 
 llvm::Optional<StringRef>
 ClangTidyCheck::OptionsView::get(StringRef LocalName) const {
+  if (Context->getOptionsCollector())
+    Context->getOptionsCollector()->insert((NamePrefix + LocalName).str());
   const auto &Iter = CheckOptions.find((NamePrefix + LocalName).str());
   if (Iter != CheckOptions.end())
     return StringRef(Iter->getValue().Value);
@@ -60,8 +62,13 @@ ClangTidyCheck::OptionsView::get(StringRef LocalName) const {
 }
 
 static ClangTidyOptions::OptionMap::const_iterator
-findPriorityOption(const ClangTidyOptions::OptionMap &Options, StringRef NamePrefix,
-          StringRef LocalName) {
+findPriorityOption(const ClangTidyOptions::OptionMap &Options,
+                   StringRef NamePrefix, StringRef LocalName,
+                   llvm::StringSet<> *Collector) {
+  if (Collector) {
+    Collector->insert((NamePrefix + LocalName).str());
+    Collector->insert(LocalName);
+  }
   auto IterLocal = Options.find((NamePrefix + LocalName).str());
   auto IterGlobal = Options.find(LocalName);
   if (IterLocal == Options.end())
@@ -75,7 +82,8 @@ findPriorityOption(const ClangTidyOptions::OptionMap &Options, StringRef NamePre
 
 llvm::Optional<StringRef>
 ClangTidyCheck::OptionsView::getLocalOrGlobal(StringRef LocalName) const {
-  auto Iter = findPriorityOption(CheckOptions, NamePrefix, LocalName);
+  auto Iter = findPriorityOption(CheckOptions, NamePrefix, LocalName,
+                                 Context->getOptionsCollector());
   if (Iter != CheckOptions.end())
     return StringRef(Iter->getValue().Value);
   return None;
@@ -108,7 +116,8 @@ ClangTidyCheck::OptionsView::get<bool>(StringRef LocalName) const {
 template <>
 llvm::Optional<bool>
 ClangTidyCheck::OptionsView::getLocalOrGlobal<bool>(StringRef LocalName) const {
-  auto Iter = findPriorityOption(CheckOptions, NamePrefix, LocalName);
+  auto Iter = findPriorityOption(CheckOptions, NamePrefix, LocalName,
+                                 Context->getOptionsCollector());
   if (Iter != CheckOptions.end()) {
     if (auto Result = getAsBool(Iter->getValue().Value, Iter->getKey()))
       return Result;
@@ -139,8 +148,11 @@ void ClangTidyCheck::OptionsView::store<bool>(
 llvm::Optional<int64_t> ClangTidyCheck::OptionsView::getEnumInt(
     StringRef LocalName, ArrayRef<NameAndValue> Mapping, bool CheckGlobal,
     bool IgnoreCase) const {
+  if (!CheckGlobal && Context->getOptionsCollector())
+    Context->getOptionsCollector()->insert((NamePrefix + LocalName).str());
   auto Iter = CheckGlobal
-                  ? findPriorityOption(CheckOptions, NamePrefix, LocalName)
+                  ? findPriorityOption(CheckOptions, NamePrefix, LocalName,
+                                       Context->getOptionsCollector())
                   : CheckOptions.find((NamePrefix + LocalName).str());
   if (Iter == CheckOptions.end())
     return None;

diff  --git a/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h b/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h
index d9424234fcf8d..261e4f7ac8626 100644
--- a/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h
+++ b/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h
@@ -15,6 +15,7 @@
 #include "clang/Basic/Diagnostic.h"
 #include "clang/Tooling/Core/Diagnostic.h"
 #include "llvm/ADT/DenseMap.h"
+#include "llvm/ADT/StringSet.h"
 #include "llvm/Support/Regex.h"
 
 namespace clang {
@@ -201,6 +202,11 @@ class ClangTidyContext {
             DiagEngine->getDiagnosticIDs()->getDescription(DiagnosticID)));
   }
 
+  void setOptionsCollector(llvm::StringSet<> *Collector) {
+    OptionsCollector = Collector;
+  }
+  llvm::StringSet<> *getOptionsCollector() const { return OptionsCollector; }
+
 private:
   // Writes to Stats.
   friend class ClangTidyDiagnosticConsumer;
@@ -230,6 +236,7 @@ class ClangTidyContext {
   bool SelfContainedDiags;
 
   NoLintDirectiveHandler NoLintHandler;
+  llvm::StringSet<> *OptionsCollector = nullptr;
 };
 
 /// Gets the Fix attached to \p Diagnostic.

diff  --git a/clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp b/clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
index 1b0010bdd62a2..564f47d864eb5 100644
--- a/clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
+++ b/clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
@@ -19,6 +19,7 @@
 #include "../ClangTidyForceLinker.h"
 #include "../GlobList.h"
 #include "clang/Tooling/CommonOptionsParser.h"
+#include "llvm/ADT/StringSet.h"
 #include "llvm/Support/InitLLVM.h"
 #include "llvm/Support/PluginLoader.h"
 #include "llvm/Support/Process.h"
@@ -257,6 +258,12 @@ This option overrides the 'UseColor' option in
 )"),
                               cl::init(false), cl::cat(ClangTidyCategory));
 
+static cl::opt<bool> VerifyConfig("verify-config", cl::desc(R"(
+Check the config files to ensure each check and
+option is recognized.
+)"),
+                                  cl::init(false), cl::cat(ClangTidyCategory));
+
 namespace clang {
 namespace tidy {
 
@@ -385,6 +392,74 @@ getVfsFromFile(const std::string &OverlayFile,
   return FS;
 }
 
+static StringRef closest(StringRef Value, const StringSet<> &Allowed) {
+  unsigned MaxEdit = 5U;
+  StringRef Closest;
+  for (auto Item : Allowed.keys()) {
+    unsigned Cur = Value.edit_distance_insensitive(Item, true, MaxEdit);
+    if (Cur < MaxEdit) {
+      Closest = Item;
+      MaxEdit = Cur;
+    }
+  }
+  return Closest;
+}
+
+static constexpr StringLiteral VerifyConfigWarningEnd = " [-verify-config]\n";
+
+static bool verifyChecks(const StringSet<> &AllChecks, StringRef CheckGlob,
+                         StringRef Source) {
+  llvm::StringRef Cur, Rest;
+  bool AnyInvalid = false;
+  for (std::tie(Cur, Rest) = CheckGlob.split(',');
+       !(Cur.empty() && Rest.empty()); std::tie(Cur, Rest) = Rest.split(',')) {
+    Cur = Cur.trim();
+    if (Cur.empty())
+      continue;
+    Cur.consume_front("-");
+    if (Cur.startswith("clang-diagnostic"))
+      continue;
+    if (Cur.contains('*')) {
+      SmallString<128> RegexText("^");
+      StringRef MetaChars("()^$|*+?.[]\\{}");
+      for (char C : Cur) {
+        if (C == '*')
+          RegexText.push_back('.');
+        else if (MetaChars.contains(C))
+          RegexText.push_back('\\');
+        RegexText.push_back(C);
+      }
+      RegexText.push_back('$');
+      llvm::Regex Glob(RegexText);
+      std::string Error;
+      if (!Glob.isValid(Error)) {
+        AnyInvalid = true;
+        llvm::WithColor::error(llvm::errs(), Source)
+            << "building check glob '" << Cur << "' " << Error << "'\n";
+        continue;
+      }
+      if (llvm::none_of(AllChecks.keys(),
+                        [&Glob](StringRef S) { return Glob.match(S); })) {
+        AnyInvalid = true;
+        llvm::WithColor::warning(llvm::errs(), Source)
+            << "check glob '" << Cur << "' doesn't match any known check"
+            << VerifyConfigWarningEnd;
+      }
+    } else {
+      if (AllChecks.contains(Cur))
+        continue;
+      AnyInvalid = true;
+      llvm::raw_ostream &Output = llvm::WithColor::warning(llvm::errs(), Source)
+                                  << "unknown check '" << Cur << '\'';
+      llvm::StringRef Closest = closest(Cur, AllChecks);
+      if (!Closest.empty())
+        Output << "; did you mean '" << Closest << '\'';
+      Output << VerifyConfigWarningEnd;
+    }
+  }
+  return AnyInvalid;
+}
+
 int clangTidyMain(int argc, const char **argv) {
   llvm::InitLLVM X(argc, argv);
 
@@ -478,6 +553,38 @@ int clangTidyMain(int argc, const char **argv) {
     return 0;
   }
 
+  if (VerifyConfig) {
+    std::vector<ClangTidyOptionsProvider::OptionsSource> RawOptions =
+        OptionsProvider->getRawOptions(FileName);
+    NamesAndOptions Valid =
+        getAllChecksAndOptions(AllowEnablingAnalyzerAlphaCheckers);
+    bool AnyInvalid = false;
+    for (const std::pair<ClangTidyOptions, std::string> &OptionWithSource :
+         RawOptions) {
+      const ClangTidyOptions &Opts = OptionWithSource.first;
+      if (Opts.Checks)
+        AnyInvalid |=
+            verifyChecks(Valid.Names, *Opts.Checks, OptionWithSource.second);
+
+      for (auto Key : Opts.CheckOptions.keys()) {
+        if (Valid.Options.contains(Key))
+          continue;
+        AnyInvalid = true;
+        auto &Output =
+            llvm::WithColor::warning(llvm::errs(), OptionWithSource.second)
+            << "unknown check option '" << Key << '\'';
+        llvm::StringRef Closest = closest(Key, Valid.Options);
+        if (!Closest.empty())
+          Output << "; did you mean '" << Closest << '\'';
+        Output << VerifyConfigWarningEnd;
+      }
+    }
+    if (AnyInvalid)
+      return 1;
+    llvm::outs() << "No config errors detected.\n";
+    return 0;
+  }
+
   if (EnabledChecks.empty()) {
     llvm::errs() << "Error: no checks enabled.\n";
     llvm::cl::PrintHelpMessage(/*Hidden=*/false, /*Categorized=*/true);

diff  --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index b1a5d110a9217..7ffa6bcc83065 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -110,6 +110,9 @@ Improvements to clang-tidy
   from suppressing diagnostics associated with macro arguments. This fixes
   `Issue 55134 <https://github.com/llvm/llvm-project/issues/55134>`_.
 
+- Added an option -verify-config which will check the config file to ensure each
+  `Checks` and `CheckOptions` entries are recognised.
+
 New checks
 ^^^^^^^^^^
 

diff  --git a/clang-tools-extra/docs/clang-tidy/index.rst b/clang-tools-extra/docs/clang-tidy/index.rst
index c76259e02f8d4..23d1769097a68 100644
--- a/clang-tools-extra/docs/clang-tidy/index.rst
+++ b/clang-tools-extra/docs/clang-tidy/index.rst
@@ -244,6 +244,9 @@ An overview of all the command-line options:
                                     standard output supports colors.
                                     This option overrides the 'UseColor' option in
                                     .clang-tidy file, if any.
+    --verify-config                -
+                                     Check the config files to ensure each check and
+                                     option is recognized.
     --vfsoverlay=<filename>        -
                                      Overlay the virtual filesystem described by file
                                      over the real file system.

diff  --git a/clang-tools-extra/test/clang-tidy/infrastructure/verify-config.cpp b/clang-tools-extra/test/clang-tidy/infrastructure/verify-config.cpp
new file mode 100644
index 0000000000000..edd6a9ee362df
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/infrastructure/verify-config.cpp
@@ -0,0 +1,17 @@
+// RUN: clang-tidy -verify-config --config='' | FileCheck %s -check-prefix=CHECK-VERIFY-OK
+// CHECK-VERIFY-OK: No config errors detected.
+
+// RUN: not clang-tidy -verify-config \
+// RUN: --checks='-*,bad*glob,llvm*,llvm-includeorder,my-made-up-check' --config='{Checks: "readability-else-after-ret", \
+// RUN: CheckOptions: [{key: "IgnoreMacros", value: "true"}, \
+// RUN:                {key: "StriceMode", value: "true"}, \
+// RUN:                {key: modernize-lop-convert.UseCxx20ReverseRanges, value: true} \
+// RUN:               ]}' 2>&1 | FileCheck %s \
+// RUN: -check-prefix=CHECK-VERIFY -implicit-check-not='{{warning|error}}:'
+
+// CHECK-VERIFY-DAG: command-line option '-config': warning: unknown check 'readability-else-after-ret'; did you mean 'readability-else-after-return' [-verify-config]
+// CHECK-VERIFY-DAG: command-line option '-config': warning: unknown check option 'modernize-lop-convert.UseCxx20ReverseRanges'; did you mean 'modernize-loop-convert.UseCxx20ReverseRanges' [-verify-config]
+// CHECK-VERIFY-DAG: command-line option '-config': warning: unknown check option 'StriceMode'; did you mean 'StrictMode' [-verify-config]
+// CHECK-VERIFY: command-line option '-checks': warning: check glob 'bad*glob' doesn't match any known check [-verify-config]
+// CHECK-VERIFY: command-line option '-checks': warning: unknown check 'llvm-includeorder'; did you mean 'llvm-include-order' [-verify-config]
+// CHECK-VERIFY: command-line option '-checks': warning: unknown check 'my-made-up-check' [-verify-config]


        


More information about the cfe-commits mailing list