Skip to content

Commit 0e2cb54

Browse files
committed
fixup! [clang-tidy] Improved --verify-config when using literal style in config file
Partially rewrote verifyChecks() to realign the checks parsing logic. Instead of manually parsing the string of globs, verifyCHecks() now uses a GlobList which is responsible to parse the string and return a list of regexes. This ensures that any changes made to the checks parsing logic has to be made at only one place rather than two.
1 parent f015496 commit 0e2cb54

File tree

3 files changed

+34
-48
lines changed

3 files changed

+34
-48
lines changed

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

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -19,12 +19,17 @@ static bool consumeNegativeIndicator(StringRef &GlobList) {
1919
return GlobList.consume_front("-");
2020
}
2121

22-
// Converts first glob from the comma-separated list of globs to Regex and
23-
// removes it and the trailing comma from the GlobList.
24-
static llvm::Regex consumeGlob(StringRef &GlobList) {
22+
// Extract the first glob from the comma-separated list of globs
23+
// removes it and the trailing comma from the GlobList and
24+
// returns the extracted glob.
25+
static llvm::StringRef extractNextGlob(StringRef &GlobList) {
2526
StringRef UntrimmedGlob = GlobList.substr(0, GlobList.find_first_of(",\n"));
2627
StringRef Glob = UntrimmedGlob.trim();
2728
GlobList = GlobList.substr(UntrimmedGlob.size() + 1);
29+
return Glob;
30+
}
31+
32+
static llvm::Regex createRegexFromGlob(StringRef &Glob) {
2833
SmallString<128> RegexText("^");
2934
StringRef MetaChars("()^$|*+?.[]\\{}");
3035
for (char C : Glob) {
@@ -43,7 +48,8 @@ GlobList::GlobList(StringRef Globs, bool KeepNegativeGlobs /* =true */) {
4348
do {
4449
GlobListItem Item;
4550
Item.IsPositive = !consumeNegativeIndicator(Globs);
46-
Item.Regex = consumeGlob(Globs);
51+
Item.Text = extractNextGlob(Globs);
52+
Item.Regex = createRegexFromGlob(Item.Text);
4753
if (Item.IsPositive || KeepNegativeGlobs)
4854
Items.push_back(std::move(Item));
4955
} while (!Globs.empty());

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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,8 +44,12 @@ class GlobList {
4444
struct GlobListItem {
4545
bool IsPositive;
4646
llvm::Regex Regex;
47+
llvm::StringRef Text;
4748
};
4849
SmallVector<GlobListItem, 0> Items;
50+
51+
public:
52+
const SmallVectorImpl<GlobListItem> &getItems() const { return Items; };
4953
};
5054

5155
/// A \p GlobList that caches search results, so that search is performed only

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

Lines changed: 20 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -454,55 +454,31 @@ static constexpr StringLiteral VerifyConfigWarningEnd = " [-verify-config]\n";
454454

455455
static bool verifyChecks(const StringSet<> &AllChecks, StringRef CheckGlob,
456456
StringRef Source) {
457-
llvm::StringRef Cur = CheckGlob;
458-
llvm::StringRef Rest = CheckGlob;
457+
GlobList Globs(CheckGlob);
459458
bool AnyInvalid = false;
460-
while (!Cur.empty() || !Rest.empty()) {
461-
Cur = Rest.substr(0, Rest.find_first_of(",\n"));
462-
Rest = Rest.substr(Cur.size() + 1);
463-
Cur = Cur.trim();
464-
465-
if (Cur.empty())
466-
continue;
467-
Cur.consume_front("-");
468-
if (Cur.starts_with("clang-diagnostic"))
459+
for (const auto &Item : Globs.getItems()) {
460+
const llvm::Regex &Reg = Item.Regex;
461+
const llvm::StringRef Text = Item.Text;
462+
if (Text.starts_with("clang-diagnostic"))
469463
continue;
470-
if (Cur.contains('*')) {
471-
SmallString<128> RegexText("^");
472-
StringRef MetaChars("()^$|*+?.[]\\{}");
473-
for (char C : Cur) {
474-
if (C == '*')
475-
RegexText.push_back('.');
476-
else if (MetaChars.contains(C))
477-
RegexText.push_back('\\');
478-
RegexText.push_back(C);
479-
}
480-
RegexText.push_back('$');
481-
llvm::Regex Glob(RegexText);
482-
std::string Error;
483-
if (!Glob.isValid(Error)) {
484-
AnyInvalid = true;
485-
llvm::WithColor::error(llvm::errs(), Source)
486-
<< "building check glob '" << Cur << "' " << Error << "'\n";
487-
continue;
488-
}
489-
if (llvm::none_of(AllChecks.keys(),
490-
[&Glob](StringRef S) { return Glob.match(S); })) {
491-
AnyInvalid = true;
464+
if (llvm::none_of(AllChecks.keys(), [&Reg](StringRef S) {
465+
llvm::errs() << S << '\n';
466+
return Reg.match(S);
467+
})) {
468+
AnyInvalid = true;
469+
if (Item.Text.contains('*'))
492470
llvm::WithColor::warning(llvm::errs(), Source)
493-
<< "check glob '" << Cur << "' doesn't match any known check"
471+
<< "check glob '" << Text << "' doesn't match any known check"
494472
<< VerifyConfigWarningEnd;
473+
else {
474+
llvm::raw_ostream &Output =
475+
llvm::WithColor::warning(llvm::errs(), Source)
476+
<< "unknown check '" << Text << '\'';
477+
llvm::StringRef Closest = closest(Text, AllChecks);
478+
if (!Closest.empty())
479+
Output << "; did you mean '" << Closest << '\'';
480+
Output << VerifyConfigWarningEnd;
495481
}
496-
} else {
497-
if (AllChecks.contains(Cur))
498-
continue;
499-
AnyInvalid = true;
500-
llvm::raw_ostream &Output = llvm::WithColor::warning(llvm::errs(), Source)
501-
<< "unknown check '" << Cur << '\'';
502-
llvm::StringRef Closest = closest(Cur, AllChecks);
503-
if (!Closest.empty())
504-
Output << "; did you mean '" << Closest << '\'';
505-
Output << VerifyConfigWarningEnd;
506482
}
507483
}
508484
return AnyInvalid;

0 commit comments

Comments
 (0)