Skip to content

Commit 1844054

Browse files
tJenerymand
authored andcommitted
[libTooling] Generalize string explanation as templated metadata
Change RewriteRule from holding an `Explanation` to being able to generate arbitrary metadata. Where TransformerClangTidyCheck was interested in a string description for the diagnostic, other tools may be interested in richer metadata at a higher level of abstraction than at the edit level (which is currently available as ASTEdit::Metadata). Reviewed By: ymandel Differential Revision: https://reviews.llvm.org/D120360
1 parent 0f05200 commit 1844054

File tree

11 files changed

+479
-133
lines changed

11 files changed

+479
-133
lines changed

clang-tools-extra/clang-tidy/abseil/CleanupCtadCheck.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ namespace clang {
2323
namespace tidy {
2424
namespace abseil {
2525

26-
RewriteRule CleanupCtadCheckImpl() {
26+
RewriteRuleWith<std::string> CleanupCtadCheckImpl() {
2727
auto warning_message = cat("prefer absl::Cleanup's class template argument "
2828
"deduction pattern in C++17 and higher");
2929

clang-tools-extra/clang-tidy/abseil/StringFindStrContainsCheck.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ using ::clang::transformer::cat;
3030
using ::clang::transformer::changeTo;
3131
using ::clang::transformer::makeRule;
3232
using ::clang::transformer::node;
33-
using ::clang::transformer::RewriteRule;
33+
using ::clang::transformer::RewriteRuleWith;
3434

3535
AST_MATCHER(Type, isCharType) { return Node.isCharType(); }
3636

@@ -39,7 +39,7 @@ static const char DefaultStringLikeClasses[] = "::std::basic_string;"
3939
"::absl::string_view";
4040
static const char DefaultAbseilStringsMatchHeader[] = "absl/strings/match.h";
4141

42-
static transformer::RewriteRule
42+
static transformer::RewriteRuleWith<std::string>
4343
makeRewriteRule(const std::vector<std::string> &StringLikeClassNames,
4444
StringRef AbseilStringsMatchHeader) {
4545
auto StringLikeClass = cxxRecordDecl(hasAnyName(SmallVector<StringRef, 4>(
@@ -62,7 +62,7 @@ makeRewriteRule(const std::vector<std::string> &StringLikeClassNames,
6262
hasArgument(1, cxxDefaultArgExpr())),
6363
onImplicitObjectArgument(expr().bind("string_being_searched")));
6464

65-
RewriteRule Rule = applyFirst(
65+
RewriteRuleWith<std::string> Rule = applyFirst(
6666
{makeRule(
6767
binaryOperator(hasOperatorName("=="),
6868
hasOperands(ignoringParenImpCasts(StringNpos),

clang-tools-extra/clang-tidy/bugprone/StringviewNullptrCheck.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ AST_MATCHER(clang::VarDecl, isDirectInitialization) {
3737

3838
} // namespace
3939

40-
RewriteRule StringviewNullptrCheckImpl() {
40+
RewriteRuleWith<std::string> StringviewNullptrCheckImpl() {
4141
auto construction_warning =
4242
cat("constructing basic_string_view from null is undefined; replace with "
4343
"the default constructor");

clang-tools-extra/clang-tidy/utils/TransformerClangTidyCheck.cpp

Lines changed: 17 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -13,16 +13,16 @@
1313
namespace clang {
1414
namespace tidy {
1515
namespace utils {
16-
using transformer::RewriteRule;
16+
using transformer::RewriteRuleWith;
1717

1818
#ifndef NDEBUG
19-
static bool hasExplanation(const RewriteRule::Case &C) {
20-
return C.Explanation != nullptr;
19+
static bool hasGenerator(const transformer::Generator<std::string> &G) {
20+
return G != nullptr;
2121
}
2222
#endif
2323

24-
static void verifyRule(const RewriteRule &Rule) {
25-
assert(llvm::all_of(Rule.Cases, hasExplanation) &&
24+
static void verifyRule(const RewriteRuleWith<std::string> &Rule) {
25+
assert(llvm::all_of(Rule.Metadata, hasGenerator) &&
2626
"clang-tidy checks must have an explanation by default;"
2727
" explicitly provide an empty explanation if none is desired");
2828
}
@@ -39,23 +39,24 @@ TransformerClangTidyCheck::TransformerClangTidyCheck(StringRef Name,
3939
// we would be accessing `getLangOpts` and `Options` before the underlying
4040
// `ClangTidyCheck` instance was properly initialized.
4141
TransformerClangTidyCheck::TransformerClangTidyCheck(
42-
std::function<Optional<RewriteRule>(const LangOptions &,
43-
const OptionsView &)>
42+
std::function<Optional<RewriteRuleWith<std::string>>(const LangOptions &,
43+
const OptionsView &)>
4444
MakeRule,
4545
StringRef Name, ClangTidyContext *Context)
4646
: TransformerClangTidyCheck(Name, Context) {
47-
if (Optional<RewriteRule> R = MakeRule(getLangOpts(), Options))
47+
if (Optional<RewriteRuleWith<std::string>> R =
48+
MakeRule(getLangOpts(), Options))
4849
setRule(std::move(*R));
4950
}
5051

51-
TransformerClangTidyCheck::TransformerClangTidyCheck(RewriteRule R,
52-
StringRef Name,
53-
ClangTidyContext *Context)
52+
TransformerClangTidyCheck::TransformerClangTidyCheck(
53+
RewriteRuleWith<std::string> R, StringRef Name, ClangTidyContext *Context)
5454
: TransformerClangTidyCheck(Name, Context) {
5555
setRule(std::move(R));
5656
}
5757

58-
void TransformerClangTidyCheck::setRule(transformer::RewriteRule R) {
58+
void TransformerClangTidyCheck::setRule(
59+
transformer::RewriteRuleWith<std::string> R) {
5960
verifyRule(R);
6061
Rule = std::move(R);
6162
}
@@ -77,8 +78,9 @@ void TransformerClangTidyCheck::check(
7778
if (Result.Context->getDiagnostics().hasErrorOccurred())
7879
return;
7980

80-
RewriteRule::Case Case = transformer::detail::findSelectedCase(Result, Rule);
81-
Expected<SmallVector<transformer::Edit, 1>> Edits = Case.Edits(Result);
81+
size_t I = transformer::detail::findSelectedCase(Result, Rule);
82+
Expected<SmallVector<transformer::Edit, 1>> Edits =
83+
Rule.Cases[I].Edits(Result);
8284
if (!Edits) {
8385
llvm::errs() << "Rewrite failed: " << llvm::toString(Edits.takeError())
8486
<< "\n";
@@ -89,7 +91,7 @@ void TransformerClangTidyCheck::check(
8991
if (Edits->empty())
9092
return;
9193

92-
Expected<std::string> Explanation = Case.Explanation->eval(Result);
94+
Expected<std::string> Explanation = Rule.Metadata[I]->eval(Result);
9395
if (!Explanation) {
9496
llvm::errs() << "Error in explanation: "
9597
<< llvm::toString(Explanation.takeError()) << "\n";

clang-tools-extra/clang-tidy/utils/TransformerClangTidyCheck.h

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -48,15 +48,16 @@ class TransformerClangTidyCheck : public ClangTidyCheck {
4848
/// cases where the options disable the check.
4949
///
5050
/// See \c setRule for constraints on the rule.
51-
TransformerClangTidyCheck(std::function<Optional<transformer::RewriteRule>(
52-
const LangOptions &, const OptionsView &)>
53-
MakeRule,
54-
StringRef Name, ClangTidyContext *Context);
51+
TransformerClangTidyCheck(
52+
std::function<Optional<transformer::RewriteRuleWith<std::string>>(
53+
const LangOptions &, const OptionsView &)>
54+
MakeRule,
55+
StringRef Name, ClangTidyContext *Context);
5556

5657
/// Convenience overload of the constructor when the rule doesn't have any
5758
/// dependencies.
58-
TransformerClangTidyCheck(transformer::RewriteRule R, StringRef Name,
59-
ClangTidyContext *Context);
59+
TransformerClangTidyCheck(transformer::RewriteRuleWith<std::string> R,
60+
StringRef Name, ClangTidyContext *Context);
6061

6162
void registerPPCallbacks(const SourceManager &SM, Preprocessor *PP,
6263
Preprocessor *ModuleExpanderPP) override;
@@ -74,10 +75,10 @@ class TransformerClangTidyCheck : public ClangTidyCheck {
7475
/// is a bug. If no explanation is desired, indicate that explicitly (for
7576
/// example, by passing `text("no explanation")` to `makeRule` as the
7677
/// `Explanation` argument).
77-
void setRule(transformer::RewriteRule R);
78+
void setRule(transformer::RewriteRuleWith<std::string> R);
7879

7980
private:
80-
transformer::RewriteRule Rule;
81+
transformer::RewriteRuleWith<std::string> Rule;
8182
IncludeInserter Inserter;
8283
};
8384

clang-tools-extra/unittests/clang-tidy/TransformerClangTidyCheckTest.cpp

Lines changed: 16 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -28,14 +28,14 @@ using transformer::IncludeFormat;
2828
using transformer::makeRule;
2929
using transformer::node;
3030
using transformer::noopEdit;
31-
using transformer::RewriteRule;
31+
using transformer::RewriteRuleWith;
3232
using transformer::RootID;
3333
using transformer::statement;
3434

3535
// Invert the code of an if-statement, while maintaining its semantics.
36-
RewriteRule invertIf() {
36+
RewriteRuleWith<std::string> invertIf() {
3737
StringRef C = "C", T = "T", E = "E";
38-
RewriteRule Rule = makeRule(
38+
RewriteRuleWith<std::string> Rule = makeRule(
3939
ifStmt(hasCondition(expr().bind(C)), hasThen(stmt().bind(T)),
4040
hasElse(stmt().bind(E))),
4141
change(statement(RootID), cat("if(!(", node(std::string(C)), ")) ",
@@ -140,8 +140,9 @@ TEST(TransformerClangTidyCheckTest, TwoMatchesInMacroExpansion) {
140140
}
141141

142142
// A trivial rewrite-rule generator that requires Objective-C code.
143-
Optional<RewriteRule> needsObjC(const LangOptions &LangOpts,
144-
const ClangTidyCheck::OptionsView &Options) {
143+
Optional<RewriteRuleWith<std::string>>
144+
needsObjC(const LangOptions &LangOpts,
145+
const ClangTidyCheck::OptionsView &Options) {
145146
if (!LangOpts.ObjC)
146147
return None;
147148
return makeRule(clang::ast_matchers::functionDecl(),
@@ -165,8 +166,9 @@ TEST(TransformerClangTidyCheckTest, DisableByLang) {
165166
}
166167

167168
// A trivial rewrite rule generator that checks config options.
168-
Optional<RewriteRule> noSkip(const LangOptions &LangOpts,
169-
const ClangTidyCheck::OptionsView &Options) {
169+
Optional<RewriteRuleWith<std::string>>
170+
noSkip(const LangOptions &LangOpts,
171+
const ClangTidyCheck::OptionsView &Options) {
170172
if (Options.get("Skip", "false") == "true")
171173
return None;
172174
return makeRule(clang::ast_matchers::functionDecl(),
@@ -194,10 +196,11 @@ TEST(TransformerClangTidyCheckTest, DisableByConfig) {
194196
Input, nullptr, "input.cc", None, Options));
195197
}
196198

197-
RewriteRule replaceCall(IncludeFormat Format) {
199+
RewriteRuleWith<std::string> replaceCall(IncludeFormat Format) {
198200
using namespace ::clang::ast_matchers;
199-
RewriteRule Rule = makeRule(callExpr(callee(functionDecl(hasName("f")))),
200-
change(cat("other()")), cat("no message"));
201+
RewriteRuleWith<std::string> Rule =
202+
makeRule(callExpr(callee(functionDecl(hasName("f")))),
203+
change(cat("other()")), cat("no message"));
201204
addInclude(Rule, "clang/OtherLib.h", Format);
202205
return Rule;
203206
}
@@ -243,10 +246,10 @@ TEST(TransformerClangTidyCheckTest, AddIncludeAngled) {
243246
}
244247

245248
class IncludeOrderCheck : public TransformerClangTidyCheck {
246-
static RewriteRule rule() {
249+
static RewriteRuleWith<std::string> rule() {
247250
using namespace ::clang::ast_matchers;
248-
RewriteRule Rule = transformer::makeRule(integerLiteral(), change(cat("5")),
249-
cat("no message"));
251+
RewriteRuleWith<std::string> Rule = transformer::makeRule(
252+
integerLiteral(), change(cat("5")), cat("no message"));
250253
addInclude(Rule, "bar.h", IncludeFormat::Quoted);
251254
return Rule;
252255
}

0 commit comments

Comments
 (0)