[clang-tidy] Escape diagnostic messages before passing to `diag` in Transformer.

Messages generated by Transformer rules may have `%` in them, which
needs to be escaped before being passed to `diag`, which interprets them
specially (and crashes if they are misused).

Differential Revision: https://reviews.llvm.org/D124952
This commit is contained in:
Yitzhak Mandelbaum 2022-05-04 18:47:08 +00:00
parent c1d6dca694
commit 9a8d33dbd8
2 changed files with 50 additions and 1 deletions

View File

@ -27,6 +27,33 @@ static void verifyRule(const RewriteRuleWith<std::string> &Rule) {
" explicitly provide an empty explanation if none is desired");
}
// If a string unintentionally containing '%' is passed as a diagnostic, Clang
// will claim the string is ill-formed and assert-fail. This function escapes
// such strings so they can be safely used in diagnostics.
std::string escapeForDiagnostic(std::string ToEscape) {
// Optimize for the common case that the string does not contain `%` at the
// cost of an extra scan over the string in the slow case.
auto Pos = ToEscape.find('%');
if (Pos == ToEscape.npos)
return ToEscape;
std::string Result;
Result.reserve(ToEscape.size());
// Convert position to a count.
++Pos;
Result.append(ToEscape, 0, Pos);
Result += '%';
for (auto N = ToEscape.size(); Pos < N; ++Pos) {
const char C = ToEscape.at(Pos);
Result += C;
if (C == '%')
Result += '%';
}
return Result;
}
TransformerClangTidyCheck::TransformerClangTidyCheck(StringRef Name,
ClangTidyContext *Context)
: ClangTidyCheck(Name, Context),
@ -99,7 +126,8 @@ void TransformerClangTidyCheck::check(
}
// Associate the diagnostic with the location of the first change.
DiagnosticBuilder Diag = diag((*Edits)[0].Range.getBegin(), *Explanation);
DiagnosticBuilder Diag =
diag((*Edits)[0].Range.getBegin(), escapeForDiagnostic(*Explanation));
for (const auto &T : *Edits)
switch (T.Kind) {
case transformer::EditKind::Range:

View File

@ -90,6 +90,27 @@ TEST(TransformerClangTidyCheckTest, DiagnosticsCorrectlyGenerated) {
EXPECT_EQ(Errors[0].Message.FileOffset, 10U);
}
TEST(TransformerClangTidyCheckTest, DiagnosticMessageEscaped) {
class GiveDiagWithPercentSymbol : public TransformerClangTidyCheck {
public:
GiveDiagWithPercentSymbol(StringRef Name, ClangTidyContext *Context)
: TransformerClangTidyCheck(makeRule(returnStmt(),
noopEdit(node(RootID)),
cat("bad code: x % y % z")),
Name, Context) {}
};
std::string Input = "int somecode() { return 0; }";
std::vector<ClangTidyError> Errors;
EXPECT_EQ(Input,
test::runCheckOnCode<GiveDiagWithPercentSymbol>(Input, &Errors));
ASSERT_EQ(Errors.size(), 1U);
// The message stored in this field shouldn't include escaped percent signs,
// because the diagnostic printer should have _unescaped_ them when processing
// the diagnostic. The only behavior observable/verifiable by the test is that
// the presence of the '%' doesn't crash Clang.
EXPECT_EQ(Errors[0].Message.Message, "bad code: x % y % z");
}
class IntLitCheck : public TransformerClangTidyCheck {
public:
IntLitCheck(StringRef Name, ClangTidyContext *Context)