r358697 - [LibTooling] Extend Transformer to support multiple simultaneous changes.
Yitzhak Mandelbaum via cfe-commits
cfe-commits at lists.llvm.org
Thu Apr 18 10:52:24 PDT 2019
Author: ymandel
Date: Thu Apr 18 10:52:24 2019
New Revision: 358697
URL: http://llvm.org/viewvc/llvm-project?rev=358697&view=rev
Log:
[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
Modified:
cfe/trunk/include/clang/Tooling/Refactoring/Transformer.h
cfe/trunk/lib/Tooling/Refactoring/Transformer.cpp
cfe/trunk/unittests/Tooling/TransformerTest.cpp
Modified: cfe/trunk/include/clang/Tooling/Refactoring/Transformer.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Tooling/Refactoring/Transformer.h?rev=358697&r1=358696&r2=358697&view=diff
==============================================================================
--- cfe/trunk/include/clang/Tooling/Refactoring/Transformer.h (original)
+++ cfe/trunk/include/clang/Tooling/Refactoring/Transformer.h Thu Apr 18 10:52:24 2019
@@ -20,13 +20,13 @@
#include "clang/ASTMatchers/ASTMatchersInternal.h"
#include "clang/Tooling/Refactoring/AtomicChange.h"
#include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/SmallVector.h"
#include "llvm/Support/Error.h"
#include <deque>
#include <functional>
#include <string>
#include <type_traits>
#include <utility>
-#include <vector>
namespace clang {
namespace tooling {
@@ -47,6 +47,98 @@ enum class NodePart {
using TextGenerator =
std::function<std::string(const ast_matchers::MatchFinder::MatchResult &)>;
+/// Wraps a string as a TextGenerator.
+inline TextGenerator text(std::string M) {
+ return [M](const ast_matchers::MatchFinder::MatchResult &) { return M; };
+}
+
+// Description of a source-code edit, expressed in terms of an AST node.
+// Includes: an ID for the (bound) node, a selector for source related to the
+// node, a replacement and, optionally, an explanation for the edit.
+//
+// * Target: the source code impacted by the rule. This identifies an AST node,
+// or part thereof (\c Part), whose source range indicates the extent of the
+// replacement applied by the replacement term. By default, the extent is the
+// node matched by the pattern term (\c NodePart::Node). Target's are typed
+// (\c Kind), which guides the determination of the node extent.
+//
+// * Replacement: a function that produces a replacement string for the target,
+// based on the match result.
+//
+// * Note: (optional) a note specifically for this edit, potentially referencing
+// elements of the match. This will be displayed to the user, where possible;
+// for example, in clang-tidy diagnostics. Use of notes should be rare --
+// explanations of the entire rewrite should be set in the rule
+// (`RewriteRule::Explanation`) instead. Notes serve the rare cases wherein
+// edit-specific diagnostics are required.
+//
+// `ASTEdit` should be built using the `change` convenience fucntions. For
+// example,
+// \code
+// change<FunctionDecl>(fun, NodePart::Name, "Frodo")
+// \endcode
+// Or, if we use Stencil for the TextGenerator:
+// \code
+// change<Stmt>(thenNode, stencil::cat("{", thenNode, "}"))
+// change<Expr>(call, NodePart::Args, stencil::cat(x, ",", y))
+// .note("argument order changed.")
+// \endcode
+// Or, if you are changing the node corresponding to the rule's matcher, you can
+// use the single-argument override of \c change:
+// \code
+// change<Expr>("different_expr")
+// \endcode
+struct ASTEdit {
+ // The (bound) id of the node whose source will be replaced. This id should
+ // never be the empty string.
+ std::string Target;
+ ast_type_traits::ASTNodeKind Kind;
+ NodePart Part;
+ TextGenerator Replacement;
+ TextGenerator Note;
+};
+
+// Convenience functions for creating \c ASTEdits. They all must be explicitly
+// instantiated with the desired AST type. Each overload includes both \c
+// std::string and \c TextGenerator versions.
+
+// FIXME: For overloads taking a \c NodePart, constrain the valid values of \c
+// Part based on the type \c T.
+template <typename T>
+ASTEdit change(StringRef Target, NodePart Part, TextGenerator Replacement) {
+ ASTEdit E;
+ E.Target = Target.str();
+ E.Kind = ast_type_traits::ASTNodeKind::getFromNodeKind<T>();
+ E.Part = Part;
+ E.Replacement = std::move(Replacement);
+ return E;
+}
+
+template <typename T>
+ASTEdit change(StringRef Target, NodePart Part, std::string Replacement) {
+ return change<T>(Target, Part, text(std::move(Replacement)));
+}
+
+/// Variant of \c change for which the NodePart defaults to the whole node.
+template <typename T>
+ASTEdit change(StringRef Target, TextGenerator Replacement) {
+ return change<T>(Target, NodePart::Node, std::move(Replacement));
+}
+
+/// Variant of \c change for which the NodePart defaults to the whole node.
+template <typename T>
+ASTEdit change(StringRef Target, std::string Replacement) {
+ return change<T>(Target, text(std::move(Replacement)));
+}
+
+/// Variant of \c change that selects the node of the entire match.
+template <typename T> ASTEdit change(TextGenerator Replacement);
+
+/// Variant of \c change that selects the node of the entire match.
+template <typename T> ASTEdit change(std::string Replacement) {
+ return change<T>(text(std::move(Replacement)));
+}
+
/// Description of a source-code transformation.
//
// A *rewrite rule* describes a transformation of source code. It has the
@@ -55,19 +147,10 @@ using TextGenerator =
// * Matcher: the pattern term, expressed as clang matchers (with Transformer
// extensions).
//
-// * Target: the source code impacted by the rule. This identifies an AST node,
-// or part thereof (\c TargetPart), whose source range indicates the extent of
-// the replacement applied by the replacement term. By default, the extent is
-// the node matched by the pattern term (\c NodePart::Node). Target's are
-// typed (\c TargetKind), which guides the determination of the node extent
-// and might, in the future, statically constrain the set of eligible
-// NodeParts for a given node.
-//
-// * Replacement: a function that produces a replacement string for the target,
-// based on the match result.
+// * Edits: a set of Edits to the source code, described with ASTEdits.
//
// * Explanation: explanation of the rewrite. This will be displayed to the
-// user, where possible (for example, in clang-tidy fix descriptions).
+// user, where possible; for example, in clang-tidy diagnostics.
//
// Rules have an additional, implicit, component: the parameters. These are
// portions of the pattern which are left unspecified, yet named so that we can
@@ -77,92 +160,37 @@ using TextGenerator =
// AST. However, in all cases, we refer to named portions of the pattern as
// parameters.
//
-// RewriteRule is constructed in a "fluent" style, by creating a builder and
-// chaining setters of individual components.
-// \code
-// RewriteRule MyRule = buildRule(functionDecl(...)).replaceWith(...);
-// \endcode
-//
-// The \c Transformer class should then be used to apply the rewrite rule and
-// obtain the corresponding replacements.
+// The \c Transformer class should be used to apply the rewrite rule and obtain
+// the corresponding replacements.
struct RewriteRule {
// `Matcher` describes the context of this rule. It should always be bound to
- // at least `RootId`. The builder class below takes care of this
- // binding. Here, we bind it to a trivial Matcher to enable the default
- // constructor, since DynTypedMatcher has no default constructor.
- ast_matchers::internal::DynTypedMatcher Matcher = ast_matchers::stmt();
- // The (bound) id of the node whose source will be replaced. This id should
- // never be the empty string.
- std::string Target;
- ast_type_traits::ASTNodeKind TargetKind;
- NodePart TargetPart;
- TextGenerator Replacement;
+ // at least `RootId`.
+ ast_matchers::internal::DynTypedMatcher Matcher;
+ SmallVector<ASTEdit, 1> Edits;
TextGenerator Explanation;
// Id used as the default target of each match. The node described by the
- // matcher is guaranteed to be bound to this id, for all rewrite rules
- // constructed with the builder class.
+ // matcher is should always be bound to this id.
static constexpr llvm::StringLiteral RootId = "___root___";
};
-/// A fluent builder class for \c RewriteRule. See comments on \c RewriteRule.
-class RewriteRuleBuilder {
- RewriteRule Rule;
-
-public:
- RewriteRuleBuilder(ast_matchers::internal::DynTypedMatcher M) {
- M.setAllowBind(true);
- // `tryBind` is guaranteed to succeed, because `AllowBind` was set to true.
- Rule.Matcher = *M.tryBind(RewriteRule::RootId);
- Rule.Target = RewriteRule::RootId;
- Rule.TargetKind = M.getSupportedKind();
- Rule.TargetPart = NodePart::Node;
- }
-
- /// (Implicit) "build" operator to build a RewriteRule from this builder.
- operator RewriteRule() && { return std::move(Rule); }
-
- // Sets the target kind based on a clang AST node type.
- template <typename T> RewriteRuleBuilder as();
-
- template <typename T>
- RewriteRuleBuilder change(llvm::StringRef Target,
- NodePart Part = NodePart::Node);
-
- RewriteRuleBuilder replaceWith(TextGenerator Replacement);
- RewriteRuleBuilder replaceWith(std::string Replacement) {
- return replaceWith(text(std::move(Replacement)));
- }
-
- RewriteRuleBuilder because(TextGenerator Explanation);
- RewriteRuleBuilder because(std::string Explanation) {
- return because(text(std::move(Explanation)));
- }
-
-private:
- // Wraps a string as a TextGenerator.
- static TextGenerator text(std::string M) {
- return [M](const ast_matchers::MatchFinder::MatchResult &) { return M; };
- }
-};
-
-/// Convenience factory functions for starting construction of a \c RewriteRule.
-inline RewriteRuleBuilder buildRule(ast_matchers::internal::DynTypedMatcher M) {
- return RewriteRuleBuilder(std::move(M));
+/// Convenience function for constructing a \c RewriteRule. Takes care of
+/// binding the matcher to RootId.
+RewriteRule makeRule(ast_matchers::internal::DynTypedMatcher M,
+ SmallVector<ASTEdit, 1> Edits);
+
+/// Convenience overload of \c makeRule for common case of only one edit.
+inline RewriteRule makeRule(ast_matchers::internal::DynTypedMatcher M,
+ ASTEdit Edit) {
+ SmallVector<ASTEdit, 1> Edits;
+ Edits.emplace_back(std::move(Edit));
+ return makeRule(std::move(M), std::move(Edits));
}
-template <typename T> RewriteRuleBuilder RewriteRuleBuilder::as() {
- Rule.TargetKind = ast_type_traits::ASTNodeKind::getFromNodeKind<T>();
- return *this;
-}
-
-template <typename T>
-RewriteRuleBuilder RewriteRuleBuilder::change(llvm::StringRef TargetId,
- NodePart Part) {
- Rule.Target = TargetId;
- Rule.TargetKind = ast_type_traits::ASTNodeKind::getFromNodeKind<T>();
- Rule.TargetPart = Part;
- return *this;
+// Define this overload of `change` here because RewriteRule::RootId is not in
+// scope at the declaration point above.
+template <typename T> ASTEdit change(TextGenerator Replacement) {
+ return change<T>(RewriteRule::RootId, NodePart::Node, std::move(Replacement));
}
/// A source "transformation," represented by a character range in the source to
@@ -172,13 +200,22 @@ struct Transformation {
std::string Replacement;
};
-/// Attempts to apply a rule to a match. Returns an empty transformation if the
-/// match is not eligible for rewriting (certain interactions with macros, for
+/// Attempts to translate `Edits`, which are in terms of AST nodes bound in the
+/// match `Result`, into Transformations, which are in terms of the source code
+/// text. This function is a low-level part of the API, provided to support
+/// interpretation of a \c RewriteRule in a tool, like \c Transformer, rather
+/// than direct use by end users.
+///
+/// Returns an empty vector if any of the edits apply to portions of the source
+/// that are ineligible for rewriting (certain interactions with macros, for
/// example). Fails if any invariants are violated relating to bound nodes in
-/// the match.
-Expected<Transformation>
-applyRewriteRule(const RewriteRule &Rule,
- const ast_matchers::MatchFinder::MatchResult &Match);
+/// the match. However, it does not fail in the case of conflicting edits --
+/// conflict handling is left to clients. We recommend use of the \c
+/// AtomicChange or \c Replacements classes for assistance in detecting such
+/// conflicts.
+Expected<SmallVector<Transformation, 1>>
+translateEdits(const ast_matchers::MatchFinder::MatchResult &Result,
+ llvm::ArrayRef<ASTEdit> Edits);
/// Handles the matcher and callback registration for a single rewrite rule, as
/// defined by the arguments of the constructor.
@@ -187,7 +224,9 @@ public:
using ChangeConsumer =
std::function<void(const clang::tooling::AtomicChange &Change)>;
- /// \param Consumer Receives each successful rewrites as an \c AtomicChange.
+ /// \param Consumer Receives each successful rewrite as an \c AtomicChange.
+ /// Note that clients are responsible for handling the case that independent
+ /// \c AtomicChanges conflict with each other.
Transformer(RewriteRule Rule, ChangeConsumer Consumer)
: Rule(std::move(Rule)), Consumer(std::move(Consumer)) {}
Modified: cfe/trunk/lib/Tooling/Refactoring/Transformer.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Tooling/Refactoring/Transformer.cpp?rev=358697&r1=358696&r2=358697&view=diff
==============================================================================
--- cfe/trunk/lib/Tooling/Refactoring/Transformer.cpp (original)
+++ cfe/trunk/lib/Tooling/Refactoring/Transformer.cpp Thu Apr 18 10:52:24 2019
@@ -144,60 +144,78 @@ getTargetRange(StringRef Target, const D
llvm_unreachable("Unexpected case in NodePart type.");
}
-Expected<Transformation>
-tooling::applyRewriteRule(const RewriteRule &Rule,
- const ast_matchers::MatchFinder::MatchResult &Match) {
- if (Match.Context->getDiagnostics().hasErrorOccurred())
- return Transformation();
-
- auto &NodesMap = Match.Nodes.getMap();
- auto It = NodesMap.find(Rule.Target);
- assert (It != NodesMap.end() && "Rule.Target must be bound in the match.");
-
- Expected<CharSourceRange> TargetOrErr =
- getTargetRange(Rule.Target, It->second, Rule.TargetKind, Rule.TargetPart,
- *Match.Context);
- if (auto Err = TargetOrErr.takeError())
- return std::move(Err);
- auto &Target = *TargetOrErr;
- if (Target.isInvalid() ||
- isOriginMacroBody(*Match.SourceManager, Target.getBegin()))
- return Transformation();
-
- return Transformation{Target, Rule.Replacement(Match)};
+Expected<SmallVector<Transformation, 1>>
+tooling::translateEdits(const MatchResult &Result,
+ llvm::ArrayRef<ASTEdit> Edits) {
+ SmallVector<Transformation, 1> Transformations;
+ auto &NodesMap = Result.Nodes.getMap();
+ for (const auto &Edit : Edits) {
+ auto It = NodesMap.find(Edit.Target);
+ assert(It != NodesMap.end() && "Edit target must be bound in the match.");
+
+ Expected<CharSourceRange> RangeOrErr = getTargetRange(
+ Edit.Target, It->second, Edit.Kind, Edit.Part, *Result.Context);
+ if (auto Err = RangeOrErr.takeError())
+ return std::move(Err);
+ Transformation T;
+ T.Range = *RangeOrErr;
+ if (T.Range.isInvalid() ||
+ isOriginMacroBody(*Result.SourceManager, T.Range.getBegin()))
+ return SmallVector<Transformation, 0>();
+ T.Replacement = Edit.Replacement(Result);
+ Transformations.push_back(std::move(T));
+ }
+ return Transformations;
}
-constexpr llvm::StringLiteral RewriteRule::RootId;
-
-RewriteRuleBuilder RewriteRuleBuilder::replaceWith(TextGenerator T) {
- Rule.Replacement = std::move(T);
- return *this;
+RewriteRule tooling::makeRule(ast_matchers::internal::DynTypedMatcher M,
+ SmallVector<ASTEdit, 1> Edits) {
+ M.setAllowBind(true);
+ // `tryBind` is guaranteed to succeed, because `AllowBind` was set to true.
+ return RewriteRule{*M.tryBind(RewriteRule::RootId), std::move(Edits)};
}
-RewriteRuleBuilder RewriteRuleBuilder::because(TextGenerator T) {
- Rule.Explanation = std::move(T);
- return *this;
-}
+constexpr llvm::StringLiteral RewriteRule::RootId;
void Transformer::registerMatchers(MatchFinder *MatchFinder) {
MatchFinder->addDynamicMatcher(Rule.Matcher, this);
}
void Transformer::run(const MatchResult &Result) {
- auto ChangeOrErr = applyRewriteRule(Rule, Result);
- if (auto Err = ChangeOrErr.takeError()) {
- llvm::errs() << "Rewrite failed: " << llvm::toString(std::move(Err))
+ if (Result.Context->getDiagnostics().hasErrorOccurred())
+ return;
+
+ // Verify the existence and validity of the AST node that roots this rule.
+ auto &NodesMap = Result.Nodes.getMap();
+ auto Root = NodesMap.find(RewriteRule::RootId);
+ assert(Root != NodesMap.end() && "Transformation failed: missing root node.");
+ SourceLocation RootLoc = Result.SourceManager->getExpansionLoc(
+ Root->second.getSourceRange().getBegin());
+ assert(RootLoc.isValid() && "Invalid location for Root node of match.");
+
+ auto TransformationsOrErr = translateEdits(Result, Rule.Edits);
+ if (auto Err = TransformationsOrErr.takeError()) {
+ llvm::errs() << "Transformation failed: " << llvm::toString(std::move(Err))
<< "\n";
return;
}
- auto &Change = *ChangeOrErr;
- auto &Range = Change.Range;
- if (Range.isInvalid()) {
+ auto &Transformations = *TransformationsOrErr;
+ if (Transformations.empty()) {
// No rewrite applied (but no error encountered either).
+ RootLoc.print(llvm::errs() << "note: skipping match at loc ",
+ *Result.SourceManager);
+ llvm::errs() << "\n";
return;
}
- AtomicChange AC(*Result.SourceManager, Range.getBegin());
- if (auto Err = AC.replace(*Result.SourceManager, Range, Change.Replacement))
- AC.setError(llvm::toString(std::move(Err)));
+
+ // Convert the result to an AtomicChange.
+ AtomicChange AC(*Result.SourceManager, RootLoc);
+ for (const auto &T : Transformations) {
+ if (auto Err = AC.replace(*Result.SourceManager, T.Range, T.Replacement)) {
+ AC.setError(llvm::toString(std::move(Err)));
+ break;
+ }
+ }
+
Consumer(AC);
}
Modified: cfe/trunk/unittests/Tooling/TransformerTest.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Tooling/TransformerTest.cpp?rev=358697&r1=358696&r2=358697&view=diff
==============================================================================
--- cfe/trunk/unittests/Tooling/TransformerTest.cpp (original)
+++ cfe/trunk/unittests/Tooling/TransformerTest.cpp Thu Apr 18 10:52:24 2019
@@ -13,36 +13,11 @@
#include "gmock/gmock.h"
#include "gtest/gtest.h"
-namespace clang {
-namespace tooling {
-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;
+using namespace clang;
+using namespace tooling;
+using namespace ast_matchers;
+namespace {
constexpr char KHeaderContents[] = R"cc(
struct string {
string(const char*);
@@ -59,6 +34,9 @@ constexpr char KHeaderContents[] = R"cc(
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 @@ protected:
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 @@ TEST_F(TransformerTest, StrlenSizeMacro)
// 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, Flag) {
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 @@ TEST_F(TransformerTest, NodePartNameDecl
)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 @@ TEST_F(TransformerTest, NodePartNameDecl
)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 @@ TEST_F(TransformerTest, NodePartMemberQu
)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 @@ TEST_F(TransformerTest, NodePartMemberMu
)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 @@ TEST_F(TransformerTest, NodePartMemberMu
// 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 @@ TEST_F(TransformerTest, NoTransformation
testRule(ruleStrlenSize(), Input, Input);
}
} // namespace
-} // namespace tooling
-} // namespace clang
More information about the cfe-commits
mailing list