[clang-tools-extra] [clang-tidy] fix verify config for custom checks (PR #192260)

[via cfe-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-clang-tools-extra

Author: Totto (Totto16)

<details>
<summary>Changes</summary>

I noticed, that if I ran `clang-tidy --experimental-custom-checks --verify-config` it still couldn't find `custom-*` checks

Without `--experimental-custom-checks` it is expected, that custom checks are not found, but with it, it was strange.

So I debugged this issue in the source code and found the issue in `clang::tidy::getAllChecksAndOptions`

It sets the `ClangTidyOptions` it uses up, by setting `Checks = "*";`, but it never sets the actual custom checks from the config, so the resolved checks don't include custom checks.
By passing in the merged `CustomChecks` from the `FileName` in `ClangTidyMain.cpp` it works as expected.

I added 2 unit tests for the expected behavior (not passing `--experimental-custom-checks` ignores them, passing it recognizes them).

I split the path into 3 commits, for an easier review, but the goal would be to squash them all together.

---
Full diff: https://github.com/llvm/llvm-project/pull/192260.diff


4 Files Affected:

- (modified) clang-tools-extra/clang-tidy/ClangTidy.cpp (+5-2) 
- (modified) clang-tools-extra/clang-tidy/ClangTidy.h (+3-2) 
- (modified) clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp (+16-1) 
- (modified) clang-tools-extra/test/clang-tidy/infrastructure/verify-config.cpp (+8) 


``````````diff
diff --git a/clang-tools-extra/clang-tidy/ClangTidy.cpp b/clang-tools-extra/clang-tidy/ClangTidy.cpp
index 05c8fd02fe86a..5f3c383a8eeb4 100644
--- a/clang-tools-extra/clang-tidy/ClangTidy.cpp
+++ b/clang-tools-extra/clang-tidy/ClangTidy.cpp
@@ -703,11 +703,14 @@ void exportReplacements(const StringRef MainFilePath,
   YAML << TUD;
 }
 
-ChecksAndOptions getAllChecksAndOptions(bool AllowEnablingAnalyzerAlphaCheckers,
-                                        bool ExperimentalCustomChecks) {
+ChecksAndOptions getAllChecksAndOptions(
+    bool AllowEnablingAnalyzerAlphaCheckers, bool ExperimentalCustomChecks,
+    std::optional<ClangTidyOptions::CustomCheckValueList> &&AllCustomChecks) {
   ChecksAndOptions Result;
   ClangTidyOptions Opts;
   Opts.Checks = "*";
+  Opts.CustomChecks = std::move(AllCustomChecks);
+
   ClangTidyContext Context(
       std::make_unique<DefaultOptionsProvider>(ClangTidyGlobalOptions(), Opts),
       AllowEnablingAnalyzerAlphaCheckers, false, ExperimentalCustomChecks);
diff --git a/clang-tools-extra/clang-tidy/ClangTidy.h b/clang-tools-extra/clang-tidy/ClangTidy.h
index 5fac25bd3bbca..77b20ce6df3dd 100644
--- a/clang-tools-extra/clang-tidy/ClangTidy.h
+++ b/clang-tools-extra/clang-tidy/ClangTidy.h
@@ -64,8 +64,9 @@ struct ChecksAndOptions {
   llvm::StringSet<> Options;
 };
 
-ChecksAndOptions getAllChecksAndOptions(bool AllowEnablingAnalyzerAlphaCheckers,
-                                        bool ExperimentalCustomChecks);
+ChecksAndOptions getAllChecksAndOptions(
+    bool AllowEnablingAnalyzerAlphaCheckers, bool ExperimentalCustomChecks,
+    std::optional<ClangTidyOptions::CustomCheckValueList> &&AllCustomChecks);
 
 /// Returns the effective check-specific options.
 ///
diff --git a/clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp b/clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
index f61e2f40ed03b..a88cf4ae8ba7f 100644
--- a/clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
+++ b/clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
@@ -700,8 +700,23 @@ int clangTidyMain(int argc, const char **argv) {
   if (VerifyConfig) {
     const std::vector<ClangTidyOptionsProvider::OptionsSource> RawOptions =
         OptionsProvider->getRawOptions(FileName);
+
+    ClangTidyOptions AllCustomChecksOptions;
+    {
+      unsigned Priority = 0;
+      for (const auto &Source : RawOptions) {
+        ClangTidyOptions PartialOptions;
+        PartialOptions.CustomChecks = Source.first.CustomChecks;
+        AllCustomChecksOptions.mergeWith(PartialOptions, ++Priority);
+      }
+    }
+
+    std::optional<ClangTidyOptions::CustomCheckValueList> AllCustomChecks =
+        AllCustomChecksOptions.CustomChecks;
+
     const ChecksAndOptions Valid = getAllChecksAndOptions(
-        AllowEnablingAnalyzerAlphaCheckers, ExperimentalCustomChecks);
+        AllowEnablingAnalyzerAlphaCheckers, ExperimentalCustomChecks,
+        std::move(AllCustomChecks));
     bool AnyInvalid = false;
     for (const auto &[Opts, Source] : RawOptions) {
       if (Opts.Checks)
diff --git a/clang-tools-extra/test/clang-tidy/infrastructure/verify-config.cpp b/clang-tools-extra/test/clang-tidy/infrastructure/verify-config.cpp
index ed3febee5940e..a94b52a95ec2a 100644
--- a/clang-tools-extra/test/clang-tidy/infrastructure/verify-config.cpp
+++ b/clang-tools-extra/test/clang-tidy/infrastructure/verify-config.cpp
@@ -36,3 +36,11 @@
 // RUN: echo -e 'Checks: "-*,clang-analyzer-optin.cplusplus.UninitializedObject"\nCheckOptions:\n clang-analyzer-optin.cplusplus.UninitializedObject.Pedantic: true' > %t.MyClangTidyConfigCSABad
 // RUN: not clang-tidy --verify-config --config-file=%t.MyClangTidyConfigCSABad 2>&1 | FileCheck %s -check-prefix=CHECK-VERIFY-CSA-BAD -implicit-check-not='{{warnings|error}}'
 // CHECK-VERIFY-CSA-BAD: command-line option '-config': warning: unknown check option 'clang-analyzer-optin.cplusplus.UninitializedObject.Pedantic'; did you mean 'clang-analyzer-optin.cplusplus.UninitializedObject:Pedantic' [-verify-config]
+
+// RUN: echo -e 'Checks: "-*,custom-no-auto-usage-c"\nCustomChecks:\n - Name: no-auto-usage-c\n   Query: match varDecl(hasType(autoType())).bind("decl")\n   Diagnostic:\n      - BindName: decl\n        Message: Don''t use auto in C\n        Level: Warning' > %t.MyClangTidyConfigCustomChecksBad
+// RUN: not clang-tidy --verify-config --config-file=%t.MyClangTidyConfigCustomChecksBad 2>&1 | FileCheck %s -check-prefix=CHECK-VERIFY-CUSTOM-CHECK-BAD -implicit-check-not='{{warnings|error}}'
+// CHECK-VERIFY-CUSTOM-CHECK-BAD: command-line option '-config': warning: unknown check 'custom-no-auto-usage-c' [-verify-config]
+
+// RUN: echo -e 'Checks: "-*,custom-no-auto-usage-c"\nCustomChecks:\n - Name: no-auto-usage-c\n   Query: match varDecl(hasType(autoType())).bind("decl")\n   Diagnostic:\n      - BindName: decl\n        Message: Don''t use auto in C\n        Level: Warning' > %t.MyClangTidyConfigCustomChecksOk
+// RUN: clang-tidy --experimental-custom-checks --verify-config --config-file=%t.MyClangTidyConfigCustomChecksOk 2>&1 | FileCheck %s -check-prefix=CHECK-VERIFY-CUSTOM-CHECK-OK -implicit-check-not='{{warnings|error}}'
+// CHECK-VERIFY-CUSTOM-CHECK-OK: No config errors detected.

``````````

</details>


https://github.com/llvm/llvm-project/pull/192260