[clang-tidy] Improved --verify-config when using literal style in config file

[felix642]

Specifying checks using the literal style (|) in the clang-tidy config file is currently supported but was not implemented for the --verify-config options. This means that clang-tidy would work properly but, using the --verify-config option would raise an error due to some checks not being parsed properly.

Fixes #53737

CC @SimplyDanny

[llvmbot]

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

@llvm/pr-subscribers-clang-tidy

Author: Félix-Antoine Constantin (felix642)

Changes

Specifying checks using the literal style (|) in the clang-tidy config file is currently supported but was not implemented for the --verify-config options. This means that clang-tidy would work properly but, using the --verify-config option would raise an error due to some checks not being parsed properly.

Fixes #53737

CC @SimplyDanny

---

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

3 Files Affected:

• (modified) clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp (+6-3)
• (modified) clang-tools-extra/docs/ReleaseNotes.rst (+3)
• (modified) clang-tools-extra/test/clang-tidy/infrastructure/verify-config.cpp (+12)

diff --git a/clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp b/clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
index 9f3d6b6db6cbca..b68a6215b383a6 100644
--- a/clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
+++ b/clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
@@ -454,11 +454,14 @@ static constexpr StringLiteral VerifyConfigWarningEnd = " [-verify-config]\n";
 
 static bool verifyChecks(const StringSet<> &AllChecks, StringRef CheckGlob,
                          StringRef Source) {
-  llvm::StringRef Cur, Rest;
+  llvm::StringRef Cur = CheckGlob;
+  llvm::StringRef Rest = CheckGlob;
   bool AnyInvalid = false;
-  for (std::tie(Cur, Rest) = CheckGlob.split(',');
-       !(Cur.empty() && Rest.empty()); std::tie(Cur, Rest) = Rest.split(',')) {
+  while (!Cur.empty() || !Rest.empty()) {
+    Cur = Rest.substr(0, Rest.find_first_of(",\n"));
+    Rest = Rest.substr(Cur.size() + 1);
     Cur = Cur.trim();
+
     if (Cur.empty())
       continue;
     Cur.consume_front("-");
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 44680f79de6f54..a699aa9aadd908 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -251,6 +251,9 @@ Miscellaneous
   option is specified. Now ``clang-apply-replacements`` applies formatting only with
   the option.
 
+- Fixed ``--verify-check`` option not properly parsing checks when using the 
+  literal operator in the ``.clang-tidy`` config
+
 Improvements to include-fixer
 -----------------------------
 
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 421f8641281acb..3659285986482a 100644
--- a/clang-tools-extra/test/clang-tidy/infrastructure/verify-config.cpp
+++ b/clang-tools-extra/test/clang-tidy/infrastructure/verify-config.cpp
@@ -18,3 +18,15 @@
 // 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]
+
+// RUN: echo -e 'Checks: |\n bugprone-argument-comment\n bugprone-assert-side-effect,\n bugprone-bool-pointer-implicit-conversion\n readability-use-anyof*' > %T/MyClangTidyConfig
+// RUN: clang-tidy -verify-config \
+// RUN: --config-file=%T/MyClangTidyConfig | FileCheck %s -check-prefix=CHECK-VERIFY-BLOCK-OK
+// CHECK-VERIFY-BLOCK-OK: No config errors detected.
+
+// RUN: echo -e 'Checks: |\n bugprone-arguments-*\n bugprone-assert-side-effects\n bugprone-bool-pointer-implicit-conversion' > %T/MyClangTidyConfigBad
+// RUN: not clang-tidy -verify-config \
+// RUN: --config-file=%T/MyClangTidyConfigBad 2>&1 | FileCheck %s -check-prefix=CHECK-VERIFY-BLOCK-BAD
+// CHECK-VERIFY-BLOCK-BAD: command-line option '-config': warning: check glob 'bugprone-arguments-*' doesn't match any known check [-verify-config]
+// CHECK-VERIFY-BLOCK-BAD: command-line option '-config': warning: unknown check 'bugprone-assert-side-effects'; did you mean 'bugprone-assert-side-effect' [-verify-config]
+

[SimplyDanny]

Seems like the parsing logic got duplicated. That eventually caused the misbehavior in verification. Alignment would be preferred.

From my point of view, the fix is okay for now though.

[github-actions]

✅ With the latest revision this PR passed the Python code formatter.

[github-actions]

✅ With the latest revision this PR passed the C/C++ code formatter.

[felix642]

Thank you for the review @SimplyDanny, as per your suggestion, I've rewrote part of my fix to unify the parsing logic rather than updating the duplicated code. The method now uses the GlobList which already handles correctly the regexes generation from the list of checks.

[PiotrZSL]

LGTM