[LibTooling] Extend Transformer to support multiple simultaneous changes.
Summary: This revision allows users to specify independent changes to multiple (related) sections of the input. Previously, only a single section of input could be selected for replacement.
Reviewers: ilya-biryukov
Reviewed By: ilya-biryukov
Subscribers: jfb, cfe-commits
Tags: #clang
Differential Revision: https://reviews.llvm.org/D60408
llvm-svn: 358697
diff --git a/clang/unittests/Tooling/TransformerTest.cpp b/clang/unittests/Tooling/TransformerTest.cpp
index a25ce32..b35c195 100644
--- a/clang/unittests/Tooling/TransformerTest.cpp
+++ b/clang/unittests/Tooling/TransformerTest.cpp
@@ -13,36 +13,11 @@
#include "gmock/gmock.h"
#include "gtest/gtest.h"
-namespace clang {
-namespace tooling {
+using namespace clang;
+using namespace tooling;
+using namespace ast_matchers;
+
namespace {
-using ast_matchers::anyOf;
-using ast_matchers::argumentCountIs;
-using ast_matchers::callee;
-using ast_matchers::callExpr;
-using ast_matchers::cxxMemberCallExpr;
-using ast_matchers::cxxMethodDecl;
-using ast_matchers::cxxRecordDecl;
-using ast_matchers::declRefExpr;
-using ast_matchers::expr;
-using ast_matchers::functionDecl;
-using ast_matchers::hasAnyName;
-using ast_matchers::hasArgument;
-using ast_matchers::hasDeclaration;
-using ast_matchers::hasElse;
-using ast_matchers::hasName;
-using ast_matchers::hasType;
-using ast_matchers::ifStmt;
-using ast_matchers::member;
-using ast_matchers::memberExpr;
-using ast_matchers::namedDecl;
-using ast_matchers::on;
-using ast_matchers::pointsTo;
-using ast_matchers::to;
-using ast_matchers::unless;
-
-using llvm::StringRef;
-
constexpr char KHeaderContents[] = R"cc(
struct string {
string(const char*);
@@ -59,6 +34,9 @@
PCFProto& GetProto();
};
} // namespace proto
+ class Logger {};
+ void operator<<(Logger& l, string msg);
+ Logger& log(int level);
)cc";
static ast_matchers::internal::Matcher<clang::QualType>
@@ -141,18 +119,15 @@
static RewriteRule ruleStrlenSize() {
StringRef StringExpr = "strexpr";
auto StringType = namedDecl(hasAnyName("::basic_string", "::string"));
- return buildRule(
- callExpr(
- callee(functionDecl(hasName("strlen"))),
- hasArgument(0, cxxMemberCallExpr(
- on(expr(hasType(isOrPointsTo(StringType)))
- .bind(StringExpr)),
- callee(cxxMethodDecl(hasName("c_str")))))))
- // Specify the intended type explicitly, because the matcher "type" of
- // `callExpr()` is `Stmt`, not `Expr`.
- .as<clang::Expr>()
- .replaceWith("REPLACED")
- .because("Use size() method directly on string.");
+ auto R = makeRule(
+ callExpr(callee(functionDecl(hasName("strlen"))),
+ hasArgument(0, cxxMemberCallExpr(
+ on(expr(hasType(isOrPointsTo(StringType)))
+ .bind(StringExpr)),
+ callee(cxxMethodDecl(hasName("c_str")))))),
+ change<clang::Expr>("REPLACED"));
+ R.Explanation = text("Use size() method directly on string.");
+ return R;
}
TEST_F(TransformerTest, StrlenSize) {
@@ -181,15 +156,12 @@
// Tests replacing an expression.
TEST_F(TransformerTest, Flag) {
StringRef Flag = "flag";
- RewriteRule Rule =
- buildRule(
- cxxMemberCallExpr(on(expr(hasType(cxxRecordDecl(hasName(
- "proto::ProtoCommandLineFlag"))))
- .bind(Flag)),
- unless(callee(cxxMethodDecl(hasName("GetProto"))))))
- .change<clang::Expr>(Flag)
- .replaceWith("EXPR")
- .because("Use GetProto() to access proto fields.");
+ RewriteRule Rule = makeRule(
+ cxxMemberCallExpr(on(expr(hasType(cxxRecordDecl(
+ hasName("proto::ProtoCommandLineFlag"))))
+ .bind(Flag)),
+ unless(callee(cxxMethodDecl(hasName("GetProto"))))),
+ change<clang::Expr>(Flag, "EXPR"));
std::string Input = R"cc(
proto::ProtoCommandLineFlag flag;
@@ -207,9 +179,9 @@
TEST_F(TransformerTest, NodePartNameNamedDecl) {
StringRef Fun = "fun";
- RewriteRule Rule = buildRule(functionDecl(hasName("bad")).bind(Fun))
- .change<clang::FunctionDecl>(Fun, NodePart::Name)
- .replaceWith("good");
+ RewriteRule Rule =
+ makeRule(functionDecl(hasName("bad")).bind(Fun),
+ change<clang::FunctionDecl>(Fun, NodePart::Name, "good"));
std::string Input = R"cc(
int bad(int x);
@@ -240,9 +212,8 @@
)cc";
StringRef Ref = "ref";
- testRule(buildRule(declRefExpr(to(functionDecl(hasName("bad")))).bind(Ref))
- .change<clang::Expr>(Ref, NodePart::Name)
- .replaceWith("good"),
+ testRule(makeRule(declRefExpr(to(functionDecl(hasName("bad")))).bind(Ref),
+ change<clang::Expr>(Ref, NodePart::Name, "good")),
Input, Expected);
}
@@ -259,17 +230,15 @@
)cc";
StringRef Ref = "ref";
- testRule(buildRule(declRefExpr(to(functionDecl())).bind(Ref))
- .change<clang::Expr>(Ref, NodePart::Name)
- .replaceWith("good"),
+ testRule(makeRule(declRefExpr(to(functionDecl())).bind(Ref),
+ change<clang::Expr>(Ref, NodePart::Name, "good")),
Input, Input);
}
TEST_F(TransformerTest, NodePartMember) {
StringRef E = "expr";
- RewriteRule Rule = buildRule(memberExpr(member(hasName("bad"))).bind(E))
- .change<clang::Expr>(E, NodePart::Member)
- .replaceWith("good");
+ RewriteRule Rule = makeRule(memberExpr(member(hasName("bad"))).bind(E),
+ change<clang::Expr>(E, NodePart::Member, "good"));
std::string Input = R"cc(
struct S {
@@ -322,9 +291,8 @@
)cc";
StringRef E = "expr";
- testRule(buildRule(memberExpr().bind(E))
- .change<clang::Expr>(E, NodePart::Member)
- .replaceWith("good"),
+ testRule(makeRule(memberExpr().bind(E),
+ change<clang::Expr>(E, NodePart::Member, "good")),
Input, Expected);
}
@@ -355,9 +323,32 @@
)cc";
StringRef MemExpr = "member";
- testRule(buildRule(memberExpr().bind(MemExpr))
- .change<clang::Expr>(MemExpr, NodePart::Member)
- .replaceWith("good"),
+ testRule(makeRule(memberExpr().bind(MemExpr),
+ change<clang::Expr>(MemExpr, NodePart::Member, "good")),
+ Input, Expected);
+}
+
+TEST_F(TransformerTest, MultiChange) {
+ std::string Input = R"cc(
+ void foo() {
+ if (10 > 1.0)
+ log(1) << "oh no!";
+ else
+ log(0) << "ok";
+ }
+ )cc";
+ std::string Expected = R"(
+ void foo() {
+ if (true) { /* then */ }
+ else { /* else */ }
+ }
+ )";
+
+ StringRef C = "C", T = "T", E = "E";
+ testRule(makeRule(ifStmt(hasCondition(expr().bind(C)),
+ hasThen(stmt().bind(T)), hasElse(stmt().bind(E))),
+ {change<Expr>(C, "true"), change<Stmt>(T, "{ /* then */ }"),
+ change<Stmt>(E, "{ /* else */ }")}),
Input, Expected);
}
@@ -365,6 +356,52 @@
// Negative tests (where we expect no transformation to occur).
//
+// Tests for a conflict in edits from a single match for a rule.
+TEST_F(TransformerTest, OverlappingEditsInRule) {
+ std::string Input = "int conflictOneRule() { return 3 + 7; }";
+ // Try to change the whole binary-operator expression AND one its operands:
+ StringRef O = "O", L = "L";
+ Transformer T(
+ makeRule(binaryOperator(hasLHS(expr().bind(L))).bind(O),
+ {change<Expr>(O, "DELETE_OP"), change<Expr>(L, "DELETE_LHS")}),
+ [this](const AtomicChange &C) { Changes.push_back(C); });
+ T.registerMatchers(&MatchFinder);
+ // The rewrite process fails...
+ EXPECT_TRUE(rewrite(Input));
+ // ... but one AtomicChange was consumed:
+ ASSERT_EQ(Changes.size(), 1);
+ EXPECT_TRUE(Changes[0].hasError());
+}
+
+// Tests for a conflict in edits across multiple matches (of the same rule).
+TEST_F(TransformerTest, OverlappingEditsMultipleMatches) {
+ std::string Input = "int conflictOneRule() { return -7; }";
+ // Try to change the whole binary-operator expression AND one its operands:
+ StringRef E = "E";
+ Transformer T(makeRule(expr().bind(E), change<Expr>(E, "DELETE_EXPR")),
+ [this](const AtomicChange &C) { Changes.push_back(C); });
+ T.registerMatchers(&MatchFinder);
+ // The rewrite process fails because the changes conflict with each other...
+ EXPECT_FALSE(rewrite(Input));
+ // ... but all changes are (individually) fine:
+ ASSERT_EQ(Changes.size(), 2);
+ EXPECT_FALSE(Changes[0].hasError());
+ EXPECT_FALSE(Changes[1].hasError());
+}
+
+TEST_F(TransformerTest, ErrorOccurredMatchSkipped) {
+ // Syntax error in the function body:
+ std::string Input = "void errorOccurred() { 3 }";
+ Transformer T(
+ makeRule(functionDecl(hasName("errorOccurred")), change<Decl>("DELETED;")),
+ [this](const AtomicChange &C) { Changes.push_back(C); });
+ T.registerMatchers(&MatchFinder);
+ // The rewrite process itself fails...
+ EXPECT_FALSE(rewrite(Input));
+ // ... and no changes are produced in the process.
+ EXPECT_THAT(Changes, ::testing::IsEmpty());
+}
+
TEST_F(TransformerTest, NoTransformationInMacro) {
std::string Input = R"cc(
#define MACRO(str) strlen((str).c_str())
@@ -385,5 +422,3 @@
testRule(ruleStrlenSize(), Input, Input);
}
} // namespace
-} // namespace tooling
-} // namespace clang