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

[njames93]

Summary

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.

[carlosgalvezp]

This sounds like great functionality, surely saving a lot of headaches! Any reason why we wouldn't want this active by default? I'd personally even go one step further and make it hard errors - warnings are easy to miss and ignore - but I can see how it can be problematic for some people.

[njames93]

In D127446#3572491, @carlosgalvezp wrote:

This sounds like great functionality, surely saving a lot of headaches! Any reason why we wouldn't want this active by default? I'd personally even go one step further and make it hard errors - warnings are easy to miss and ignore - but I can see how it can be problematic for some people.

I choose not to as I'd imagine it could would result in duplicated warnings when batch processing using run_clang_tidy over a large project.
Also if there's any edge cases, maybe with the static analyser, I wouldn't want those diagnostics triggering by default with no way to silence them.
I wouldn't be opposed to the possibility of making on by default after its had some time in the wild.

[aaron.ballman]

Thanks for this, it seems like an interesting feature!

clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp

263

436

Building check glob instead?

450

Unknown check instead?

573

Unknown check option instead?

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

12

It's unfortunate that warning: appears in the middle of the diagnostic as opposed to at the start. I wonder if this can be reworked to say: warning: Unknown check 'whatever'; did you mean 'whatever'? [-verify-config] or something?

Also, no test coverage for the error case and for the unknown check case where there's no closest match worth talking about.

[njames93]

clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp

263

:)

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

12

Warning appearing in the middle is kind of by design as when clang emits diagnostics, the source location is the first thing emitted, then the type(warning|error). In this case command-line option '-config' is the "source location"

Also, no test coverage for the error case.

I'm not 100% on that, there is no checking when the check glob is actually built and I haven't figured out how to break it.

[njames93]

Address reviewer comments.

[aaron.ballman]

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

12

then the type(warning|error). In this case command-line option '-config' is the "source location"

Ah, thanks for explaining your logic. FWIW, Clang uses a different style for that (<command line>): https://godbolt.org/z/f1rdsbjqe Perhaps we should follow suit? (In fact, we could use the diagnostic engine for this instead of emitting to the error stream ourselves?)

[njames93]

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

12

The issue is, we don't have the actual source information right now to print better error messages.

[aaron.ballman]

LGTM!

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

12

After some off-list discussion (thanks @njames93!) I'm retracting my concerns here for the moment because there are larger issues with the diagnostics engine to be solved first.

[njames93]

Before this lands @sammccall @kadircet Do you have any concerns about this API for getting the options not building a static instance here?

clang-tools-extra/clang-tidy/ClangTidy.h

 //===--- ClangTidy.h - clang-tidy -------------------------------*- 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
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CLANGTIDY_H
 #define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CLANGTIDY_H
 
 #include "ClangTidyDiagnosticConsumer.h"
 #include "ClangTidyOptions.h"
+#include "llvm/ADT/StringSet.h"
 #include <memory>
 #include <vector>
 
 namespace llvm {
 class raw_ostream;
 } // namespace llvm
 
 namespace clang {
private:
   std::unique_ptr<ClangTidyCheckFactories> CheckFactories;
 };
 
 /// Fills the list of check names that are enabled when the provided
 /// filters are applied.
 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
 /// effective options from all created checks. The returned set of options
 /// includes default check-specific options for all keys not overridden by \p
 /// Options.
 ClangTidyOptions::OptionMap
 getCheckOptions(const ClangTidyOptions &Options,

clang-tools-extra/clang-tidy/ClangTidy.cpp

for (const auto &Error : Errors) {
     tooling::Diagnostic Diag = Error;
     TUD.Diagnostics.insert(TUD.Diagnostics.end(), Diag);
   }
 
   yaml::Output YAML(OS);
   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

clang-tools-extra/clang-tidy/ClangTidyCheck.cpp

 ClangTidyCheck::OptionsView::OptionsView(
     StringRef CheckName, const ClangTidyOptions::OptionMap &CheckOptions,
     ClangTidyContext *Context)
     : NamePrefix((CheckName + ".").str()), CheckOptions(CheckOptions),
       Context(Context) {}
 
 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);
   return None;
 }
 
 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())
     return IterGlobal;
   if (IterGlobal == Options.end())
     return IterLocal;
   if (IterLocal->getValue().Priority >= IterGlobal->getValue().Priority)
     return IterLocal;
   return IterGlobal;
 }
 
 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;
 }
 
 static Optional<bool> getAsBool(StringRef Value,
                                 const llvm::Twine &LookupName) {
 
if (llvm::Optional<StringRef> ValueOr = get(LocalName)) {
     diagnoseBadBooleanOption(NamePrefix + LocalName, *ValueOr);
   }
   return None;
 }
 
 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;
     diagnoseBadBooleanOption(Iter->getKey(), Iter->getValue().Value);
   }
   return None;
 }
 
void ClangTidyCheck::OptionsView::store<bool>(
     ClangTidyOptions::OptionMap &Options, StringRef LocalName,
     bool Value) const {
   store(Options, LocalName, Value ? StringRef("true") : StringRef("false"));
 }
 
 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;
 
   StringRef Value = Iter->getValue().Value;
   StringRef Closest;
   unsigned EditDistance = 3;
   for (const auto &NameAndEnum : Mapping) {

clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h

 #define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CLANGTIDYDIAGNOSTICCONSUMER_H
 
 #include "ClangTidyOptions.h"
 #include "ClangTidyProfiling.h"
 #include "NoLintDirectiveHandler.h"
 #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 {
 
 class ASTContext;
 class SourceManager;
 
 namespace tidy {
DiagLevelAndFormatString getDiagLevelAndFormatString(unsigned DiagnosticID,
                                                        SourceLocation Loc) {
     return DiagLevelAndFormatString(
         static_cast<DiagnosticIDs::Level>(
             DiagEngine->getDiagnosticLevel(DiagnosticID, Loc)),
         std::string(
             DiagEngine->getDiagnosticIDs()->getDescription(DiagnosticID)));
   }
 
+  void setOptionsCollector(llvm::StringSet<> *Collector) {
+    OptionsCollector = Collector;
+  }
+  llvm::StringSet<> *getOptionsCollector() const { return OptionsCollector; }
+
 private:
   // Writes to Stats.
   friend class ClangTidyDiagnosticConsumer;
 
   DiagnosticsEngine *DiagEngine;
   std::unique_ptr<ClangTidyOptionsProvider> OptionsProvider;
 
   std::string CurrentFile;
private:
   bool Profile;
   std::string ProfilePrefix;
 
   bool AllowEnablingAnalyzerAlphaCheckers;
 
   bool SelfContainedDiags;
 
   NoLintDirectiveHandler NoLintHandler;
+  llvm::StringSet<> *OptionsCollector = nullptr;
 };
 
 /// Gets the Fix attached to \p Diagnostic.
 /// If there isn't a Fix attached to the diagnostic and \p AnyFix is true, Check
 /// to see if exactly one note has a Fix and return it. Otherwise return
 /// nullptr.
 const llvm::StringMap<tooling::Replacements> *
 getFixIt(const tooling::Diagnostic &Diagnostic, bool AnyFix);

clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp

 ///
 //===----------------------------------------------------------------------===//
 
 #include "ClangTidyMain.h"
 #include "../ClangTidy.h"
 #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"
 #include "llvm/Support/Signals.h"
 #include "llvm/Support/TargetSelect.h"
 #include "llvm/Support/WithColor.h"
 
 using namespace clang::tooling;

 Use colors in diagnostics. If not set, colors
 will be used if the terminal connected to
 standard output supports colors.
 This option overrides the 'UseColor' option in
 .clang-tidy file, if any.
 )"),
                               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 recognised.
+)"),
+                                  cl::init(false), cl::cat(ClangTidyCategory));
+
 namespace clang {
 namespace tidy {
 
 static void printStats(const ClangTidyStats &Stats) {
   if (Stats.errorsIgnored()) {
     llvm::errs() << "Suppressed " << Stats.errorsIgnored() << " warnings (";
     StringRef Separator = "";
     if (Stats.ErrorsIgnoredNonUserCode) {
getVfsFromFile(const std::string &OverlayFile,
   if (!FS) {
     llvm::errs() << "Error: invalid virtual filesystem overlay file '"
                  << OverlayFile << "'.\n";
     return nullptr;
   }
   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 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 CheckGlob '" << 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\n";
+      }
+    } 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 << "\n";
+      else
+        Output << "; did you mean '" << Closest << "'\n";
+    }
+  }
+  return AnyInvalid;
+}
+
 int clangTidyMain(int argc, const char **argv) {
   llvm::InitLLVM X(argc, argv);
 
   // Enable help for -load option, if plugins are enabled.
   if (cl::Option *LoadOpt = cl::getRegisteredOptions().lookup("load"))
     LoadOpt->addCategory(ClangTidyCategory);
 
   llvm::Expected<CommonOptionsParser> OptionsParser =
if (DumpConfig) {
     EffectiveOptions.CheckOptions =
         getCheckOptions(EffectiveOptions, AllowEnablingAnalyzerAlphaCheckers);
     llvm::outs() << configurationAsText(ClangTidyOptions::getDefaults().merge(
                         EffectiveOptions, 0))
                  << "\n";
     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 << "\n";
+        else
+          Output << "; did you mean '" << Closest << "'\n";
+      }
+    }
+    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);
     return 1;
   }
 
   if (PathList.empty()) {
     llvm::errs() << "Error: no input files specified.\n";

clang-tools-extra/docs/ReleaseNotes.rst

- Clang-tidy now consideres newlines as separators of single elements in the `Checks` section in
   this list from each other, newline characters now also work as separators in the parsed YAML. That
   means it is advised to use YAML's block style initiated by the pipe character `|` for the `Checks`
   section in order to benefit from the easier syntax that works without commas.
 
 - Fixed a regression introduced in clang-tidy 14.0.0, which prevented NOLINTs
   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
 ^^^^^^^^^^
 
 - New :doc:`bugprone-shared-ptr-array-mismatch <clang-tidy/checks/bugprone-shared-ptr-array-mismatch>` check.
 
   Finds initializations of C++ shared pointers to non-array type that are initialized with an array.
 
 - New :doc:`bugprone-unchecked-optional-access

clang-tools-extra/docs/clang-tidy/index.rst

--store-check-profile=<prefix> -
                                      these per-TU profiles are instead stored as JSON.
     --system-headers               - Display the errors from system headers.
     --use-color                    - 
                                     Use colors in diagnostics. If not set, colors
                                     will be used if the terminal connected to
                                     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 recognised.
     --vfsoverlay=<filename>        -
                                      Overlay the virtual filesystem described by file
                                      over the real file system.
     --warnings-as-errors=<string>  -
                                      Upgrades warnings to errors. Same format as
                                      '-checks'.
                                      This option's value is appended to the value of
                                      the 'WarningsAsErrors' option in .clang-tidy

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

+// 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' --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'
+// CHECK-VERIFY-DAG: command-line option '-config': warning: Unknown Check Option 'modernize-lop-convert.UseCxx20ReverseRanges'; did you mean 'modernize-loop-convert.UseCxx20ReverseRanges'
+// CHECK-VERIFY-DAG: command-line option '-config': warning: Unknown Check Option 'StriceMode'; did you mean 'StrictMode'
+// CHECK-VERIFY: command-line option '-checks': warning: Check glob 'bad*glob' doesn't match any known check
+// CHECK-VERIFY: command-line option '-checks': warning: Unknown Check 'llvm-includeorder'; did you mean 'llvm-include-order'