[clang-tools-extra] f94ed6f - [clang-tidy] Improved --verify-config when using literal style in config file (#85591)
via cfe-commits
cfe-commits at lists.llvm.org
Mon Apr 22 11:05:36 PDT 2024
Author: Félix-Antoine Constantin
Date: 2024-04-22T20:05:32+02:00
New Revision: f94ed6f7977305db8fa6b48a85c9db2b8cc4d3b3
URL: https://github.com/llvm/llvm-project/commit/f94ed6f7977305db8fa6b48a85c9db2b8cc4d3b3
DIFF: https://github.com/llvm/llvm-project/commit/f94ed6f7977305db8fa6b48a85c9db2b8cc4d3b3.diff
LOG: [clang-tidy] Improved --verify-config when using literal style in config file (#85591)
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
Added:
Modified:
clang-tools-extra/clang-tidy/GlobList.cpp
clang-tools-extra/clang-tidy/GlobList.h
clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/test/clang-tidy/infrastructure/verify-config.cpp
Removed:
################################################################################
diff --git a/clang-tools-extra/clang-tidy/GlobList.cpp b/clang-tools-extra/clang-tidy/GlobList.cpp
index dfe3f7c505b174..8f09ee075bbd6e 100644
--- a/clang-tools-extra/clang-tidy/GlobList.cpp
+++ b/clang-tools-extra/clang-tidy/GlobList.cpp
@@ -19,12 +19,17 @@ static bool consumeNegativeIndicator(StringRef &GlobList) {
return GlobList.consume_front("-");
}
-// Converts first glob from the comma-separated list of globs to Regex and
-// removes it and the trailing comma from the GlobList.
-static llvm::Regex consumeGlob(StringRef &GlobList) {
+// Extracts the first glob from the comma-separated list of globs,
+// removes it and the trailing comma from the GlobList and
+// returns the extracted glob.
+static llvm::StringRef extractNextGlob(StringRef &GlobList) {
StringRef UntrimmedGlob = GlobList.substr(0, GlobList.find_first_of(",\n"));
StringRef Glob = UntrimmedGlob.trim();
GlobList = GlobList.substr(UntrimmedGlob.size() + 1);
+ return Glob;
+}
+
+static llvm::Regex createRegexFromGlob(StringRef &Glob) {
SmallString<128> RegexText("^");
StringRef MetaChars("()^$|*+?.[]\\{}");
for (char C : Glob) {
@@ -43,7 +48,8 @@ GlobList::GlobList(StringRef Globs, bool KeepNegativeGlobs /* =true */) {
do {
GlobListItem Item;
Item.IsPositive = !consumeNegativeIndicator(Globs);
- Item.Regex = consumeGlob(Globs);
+ Item.Text = extractNextGlob(Globs);
+ Item.Regex = createRegexFromGlob(Item.Text);
if (Item.IsPositive || KeepNegativeGlobs)
Items.push_back(std::move(Item));
} while (!Globs.empty());
diff --git a/clang-tools-extra/clang-tidy/GlobList.h b/clang-tools-extra/clang-tidy/GlobList.h
index 44af182e43b002..4317928270adff 100644
--- a/clang-tools-extra/clang-tidy/GlobList.h
+++ b/clang-tools-extra/clang-tidy/GlobList.h
@@ -44,8 +44,12 @@ class GlobList {
struct GlobListItem {
bool IsPositive;
llvm::Regex Regex;
+ llvm::StringRef Text;
};
SmallVector<GlobListItem, 0> Items;
+
+public:
+ const SmallVectorImpl<GlobListItem> &getItems() const { return Items; };
};
/// A \p GlobList that caches search results, so that search is performed only
diff --git a/clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp b/clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
index 9f3d6b6db6cbca..f82f4417141d3d 100644
--- a/clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
+++ b/clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
@@ -454,52 +454,27 @@ static constexpr StringLiteral VerifyConfigWarningEnd = " [-verify-config]\n";
static bool verifyChecks(const StringSet<> &AllChecks, StringRef CheckGlob,
StringRef Source) {
- llvm::StringRef Cur, Rest;
+ GlobList Globs(CheckGlob);
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())
+ for (const auto &Item : Globs.getItems()) {
+ if (Item.Text.starts_with("clang-diagnostic"))
continue;
- Cur.consume_front("-");
- if (Cur.starts_with("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;
+ if (llvm::none_of(AllChecks.keys(),
+ [&Item](StringRef S) { return Item.Regex.match(S); })) {
+ AnyInvalid = true;
+ if (Item.Text.contains('*'))
llvm::WithColor::warning(llvm::errs(), Source)
- << "check glob '" << Cur << "' doesn't match any known check"
+ << "check glob '" << Item.Text << "' doesn't match any known check"
<< VerifyConfigWarningEnd;
+ else {
+ llvm::raw_ostream &Output =
+ llvm::WithColor::warning(llvm::errs(), Source)
+ << "unknown check '" << Item.Text << '\'';
+ llvm::StringRef Closest = closest(Item.Text, AllChecks);
+ if (!Closest.empty())
+ Output << "; did you mean '" << Closest << '\'';
+ Output << 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;
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 9ef1d38d3c4560..f3f9a81f9a8e82 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -103,6 +103,9 @@ Improvements to clang-tidy
- Improved :program:`check_clang_tidy.py` script. Added argument `-export-fixes`
to aid in clang-tidy and test development.
+- Fixed ``--verify-config`` option not properly parsing checks when using the
+ literal operator in the ``.clang-tidy`` config.
+
New 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 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]
+
More information about the cfe-commits
mailing list