This revision was automatically updated to reflect the committed changes.
Closed by commit rL361392: [LibTooling] Update Transformer to use RangeSelector
instead of NodePart enum. (authored by ymandel, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.
Changed prior to commit:
https://reviews.llvm.org/D62149?vs=200712&id=200741#toc
Repository:
rL LLVM
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D62149/new/
https://reviews.llvm.org/D62149
Files:
cfe/trunk/include/clang/Tooling/Refactoring/Transformer.h
cfe/trunk/lib/Tooling/Refactoring/Transformer.cpp
cfe/trunk/unittests/Tooling/TransformerTest.cpp
Index: cfe/trunk/lib/Tooling/Refactoring/Transformer.cpp
===================================================================
--- cfe/trunk/lib/Tooling/Refactoring/Transformer.cpp
+++ cfe/trunk/lib/Tooling/Refactoring/Transformer.cpp
@@ -32,11 +32,7 @@
using ast_type_traits::ASTNodeKind;
using ast_type_traits::DynTypedNode;
using llvm::Error;
-using llvm::Expected;
-using llvm::Optional;
using llvm::StringError;
-using llvm::StringRef;
-using llvm::Twine;
using MatchResult = MatchFinder::MatchResult;
@@ -71,91 +67,12 @@
return false;
}
-static llvm::Error invalidArgumentError(Twine Message) {
- return llvm::make_error<StringError>(llvm::errc::invalid_argument, Message);
-}
-
-static llvm::Error typeError(StringRef Id, const ASTNodeKind &Kind,
- Twine Message) {
- return invalidArgumentError(
- Message + " (node id=" + Id + " kind=" + Kind.asStringRef() + ")");
-}
-
-static llvm::Error missingPropertyError(StringRef Id, Twine Description,
- StringRef Property) {
- return invalidArgumentError(Description + " requires property '" + Property +
- "' (node id=" + Id + ")");
-}
-
-static Expected<CharSourceRange>
-getTargetRange(StringRef Target, const DynTypedNode &Node, ASTNodeKind Kind,
- NodePart TargetPart, ASTContext &Context) {
- switch (TargetPart) {
- case NodePart::Node: {
- // For non-expression statements, associate any trailing semicolon with the
- // statement text. However, if the target was intended as an expression (as
- // indicated by its kind) then we do not associate any trailing semicolon
- // with it. We only associate the exact expression text.
- if (Node.get<Stmt>() != nullptr) {
- auto ExprKind = ASTNodeKind::getFromNodeKind<clang::Expr>();
- if (!ExprKind.isBaseOf(Kind))
- return getExtendedRange(Node, tok::TokenKind::semi, Context);
- }
- return CharSourceRange::getTokenRange(Node.getSourceRange());
- }
- case NodePart::Member:
- if (auto *M = Node.get<clang::MemberExpr>())
- return CharSourceRange::getTokenRange(
- M->getMemberNameInfo().getSourceRange());
- return typeError(Target, Node.getNodeKind(),
- "NodePart::Member applied to non-MemberExpr");
- case NodePart::Name:
- if (const auto *D = Node.get<clang::NamedDecl>()) {
- if (!D->getDeclName().isIdentifier())
- return missingPropertyError(Target, "NodePart::Name", "identifier");
- SourceLocation L = D->getLocation();
- auto R = CharSourceRange::getTokenRange(L, L);
- // Verify that the range covers exactly the name.
- // FIXME: extend this code to support cases like `operator +` or
- // `foo<int>` for which this range will be too short. Doing so will
- // require subcasing `NamedDecl`, because it doesn't provide virtual
- // access to the \c DeclarationNameInfo.
- if (getText(R, Context) != D->getName())
- return CharSourceRange();
- return R;
- }
- if (const auto *E = Node.get<clang::DeclRefExpr>()) {
- if (!E->getNameInfo().getName().isIdentifier())
- return missingPropertyError(Target, "NodePart::Name", "identifier");
- SourceLocation L = E->getLocation();
- return CharSourceRange::getTokenRange(L, L);
- }
- if (const auto *I = Node.get<clang::CXXCtorInitializer>()) {
- if (!I->isMemberInitializer() && I->isWritten())
- return missingPropertyError(Target, "NodePart::Name",
- "explicit member initializer");
- SourceLocation L = I->getMemberLocation();
- return CharSourceRange::getTokenRange(L, L);
- }
- return typeError(
- Target, Node.getNodeKind(),
- "NodePart::Name applied to neither DeclRefExpr, NamedDecl nor "
- "CXXCtorInitializer");
- }
- llvm_unreachable("Unexpected case in NodePart type.");
-}
-
Expected<SmallVector<tooling::detail::Transformation, 1>>
tooling::detail::translateEdits(const MatchResult &Result,
llvm::ArrayRef<ASTEdit> Edits) {
- SmallVector<Transformation, 1> Transformations;
- auto &NodesMap = Result.Nodes.getMap();
+ SmallVector<tooling::detail::Transformation, 1> Transformations;
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> Range = getTargetRange(
- Edit.Target, It->second, Edit.Kind, Edit.Part, *Result.Context);
+ Expected<CharSourceRange> Range = Edit.TargetRange(Result);
if (!Range)
return Range.takeError();
if (Range->isInvalid() ||
@@ -164,7 +81,7 @@
auto Replacement = Edit.Replacement(Result);
if (!Replacement)
return Replacement.takeError();
- Transformation T;
+ tooling::detail::Transformation T;
T.Range = *Range;
T.Replacement = std::move(*Replacement);
Transformations.push_back(std::move(T));
@@ -172,6 +89,13 @@
return Transformations;
}
+ASTEdit tooling::change(RangeSelector S, TextGenerator Replacement) {
+ ASTEdit E;
+ E.TargetRange = std::move(S);
+ E.Replacement = std::move(Replacement);
+ return E;
+}
+
RewriteRule tooling::makeRule(DynTypedMatcher M,
SmallVector<ASTEdit, 1> Edits) {
return RewriteRule{
@@ -255,7 +179,7 @@
DynTypedMatcher M = joinCaseMatchers(Rule);
M.setAllowBind(true);
// `tryBind` is guaranteed to succeed, because `AllowBind` was set to true.
- return *M.tryBind(RewriteRule::RootId);
+ return *M.tryBind(RewriteRule::RootID);
}
// Finds the case that was "selected" -- that is, whose matcher triggered the
@@ -275,7 +199,7 @@
llvm_unreachable("No tag found for this rule.");
}
-constexpr llvm::StringLiteral RewriteRule::RootId;
+constexpr llvm::StringLiteral RewriteRule::RootID;
void Transformer::registerMatchers(MatchFinder *MatchFinder) {
MatchFinder->addDynamicMatcher(tooling::detail::buildMatcher(Rule), this);
@@ -287,7 +211,7 @@
// Verify the existence and validity of the AST node that roots this rule.
auto &NodesMap = Result.Nodes.getMap();
- auto Root = NodesMap.find(RewriteRule::RootId);
+ auto Root = NodesMap.find(RewriteRule::RootID);
assert(Root != NodesMap.end() && "Transformation failed: missing root node.");
SourceLocation RootLoc = Result.SourceManager->getExpansionLoc(
Root->second.getSourceRange().getBegin());
Index: cfe/trunk/unittests/Tooling/TransformerTest.cpp
===================================================================
--- cfe/trunk/unittests/Tooling/TransformerTest.cpp
+++ cfe/trunk/unittests/Tooling/TransformerTest.cpp
@@ -7,8 +7,8 @@
//===----------------------------------------------------------------------===//
#include "clang/Tooling/Refactoring/Transformer.h"
-
#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/Tooling/Refactoring/RangeSelector.h"
#include "clang/Tooling/Tooling.h"
#include "llvm/Support/Errc.h"
#include "llvm/Support/Error.h"
@@ -147,7 +147,7 @@
on(expr(hasType(isOrPointsTo(StringType)))
.bind(StringExpr)),
callee(cxxMethodDecl(hasName("c_str")))))),
- change<clang::Expr>("REPLACED"));
+ change(text("REPLACED")));
R.Cases[0].Explanation = text("Use size() method directly on string.");
return R;
}
@@ -183,7 +183,7 @@
hasName("proto::ProtoCommandLineFlag"))))
.bind(Flag)),
unless(callee(cxxMethodDecl(hasName("GetProto"))))),
- change<clang::Expr>(Flag, "EXPR"));
+ change(node(Flag), text("EXPR")));
std::string Input = R"cc(
proto::ProtoCommandLineFlag flag;
@@ -201,9 +201,8 @@
TEST_F(TransformerTest, NodePartNameNamedDecl) {
StringRef Fun = "fun";
- RewriteRule Rule =
- makeRule(functionDecl(hasName("bad")).bind(Fun),
- change<clang::FunctionDecl>(Fun, NodePart::Name, "good"));
+ RewriteRule Rule = makeRule(functionDecl(hasName("bad")).bind(Fun),
+ change(name(Fun), text("good")));
std::string Input = R"cc(
int bad(int x);
@@ -235,7 +234,7 @@
StringRef Ref = "ref";
testRule(makeRule(declRefExpr(to(functionDecl(hasName("bad")))).bind(Ref),
- change<clang::Expr>(Ref, NodePart::Name, "good")),
+ change(name(Ref), text("good"))),
Input, Expected);
}
@@ -253,7 +252,7 @@
StringRef Ref = "ref";
Transformer T(makeRule(declRefExpr(to(functionDecl())).bind(Ref),
- change<clang::Expr>(Ref, NodePart::Name, "good")),
+ change(name(Ref), text("good"))),
consumer());
T.registerMatchers(&MatchFinder);
EXPECT_FALSE(rewrite(Input));
@@ -262,7 +261,7 @@
TEST_F(TransformerTest, NodePartMember) {
StringRef E = "expr";
RewriteRule Rule = makeRule(memberExpr(member(hasName("bad"))).bind(E),
- change<clang::Expr>(E, NodePart::Member, "good"));
+ change(member(E), text("good")));
std::string Input = R"cc(
struct S {
@@ -315,8 +314,7 @@
)cc";
StringRef E = "expr";
- testRule(makeRule(memberExpr().bind(E),
- change<clang::Expr>(E, NodePart::Member, "good")),
+ testRule(makeRule(memberExpr().bind(E), change(member(E), text("good"))),
Input, Expected);
}
@@ -348,7 +346,7 @@
StringRef MemExpr = "member";
testRule(makeRule(memberExpr().bind(MemExpr),
- change<clang::Expr>(MemExpr, NodePart::Member, "good")),
+ change(member(MemExpr), text("good"))),
Input, Expected);
}
@@ -371,8 +369,9 @@
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 */ }")}),
+ {change(node(C), text("true")),
+ change(statement(T), text("{ /* then */ }")),
+ change(statement(E), text("{ /* else */ }"))}),
Input, Expected);
}
@@ -383,7 +382,7 @@
hasName("proto::ProtoCommandLineFlag"))))
.bind(Flag)),
unless(callee(cxxMethodDecl(hasName("GetProto"))))),
- change<clang::Expr>(Flag, "PROTO"));
+ change(node(Flag), text("PROTO")));
std::string Input = R"cc(
proto::ProtoCommandLineFlag flag;
@@ -410,7 +409,7 @@
hasArgument(0, cxxMemberCallExpr(
on(expr().bind(S)),
callee(cxxMethodDecl(hasName("c_str")))))),
- change<clang::Expr>("DISTINCT"));
+ change(text("DISTINCT")));
}
TEST_F(TransformerTest, OrderedRuleRelated) {
@@ -475,7 +474,7 @@
-> llvm::Expected<std::string> {
return llvm::createStringError(llvm::errc::invalid_argument, "ERROR");
};
- Transformer T(makeRule(binaryOperator().bind(O), change<Expr>(O, AlwaysFail)),
+ Transformer T(makeRule(binaryOperator().bind(O), change(node(O), AlwaysFail)),
consumer());
T.registerMatchers(&MatchFinder);
EXPECT_FALSE(rewrite(Input));
@@ -488,10 +487,10 @@
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")}),
- consumer());
+ Transformer T(makeRule(binaryOperator(hasLHS(expr().bind(L))).bind(O),
+ {change(node(O), text("DELETE_OP")),
+ change(node(L), text("DELETE_LHS"))}),
+ consumer());
T.registerMatchers(&MatchFinder);
EXPECT_FALSE(rewrite(Input));
EXPECT_THAT(Changes, IsEmpty());
@@ -503,7 +502,7 @@
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")),
+ Transformer T(makeRule(expr().bind(E), change(node(E), text("DELETE_EXPR"))),
consumer());
T.registerMatchers(&MatchFinder);
// The rewrite process fails because the changes conflict with each other...
@@ -517,7 +516,7 @@
// Syntax error in the function body:
std::string Input = "void errorOccurred() { 3 }";
Transformer T(makeRule(functionDecl(hasName("errorOccurred")),
- change<Decl>("DELETED;")),
+ change(text("DELETED;"))),
consumer());
T.registerMatchers(&MatchFinder);
// The rewrite process itself fails...
Index: cfe/trunk/include/clang/Tooling/Refactoring/Transformer.h
===================================================================
--- cfe/trunk/include/clang/Tooling/Refactoring/Transformer.h
+++ cfe/trunk/include/clang/Tooling/Refactoring/Transformer.h
@@ -19,6 +19,7 @@
#include "clang/ASTMatchers/ASTMatchers.h"
#include "clang/ASTMatchers/ASTMatchersInternal.h"
#include "clang/Tooling/Refactoring/AtomicChange.h"
+#include "clang/Tooling/Refactoring/RangeSelector.h"
#include "llvm/ADT/STLExtras.h"
#include "llvm/ADT/SmallVector.h"
#include "llvm/Support/Error.h"
@@ -30,19 +31,6 @@
namespace clang {
namespace tooling {
-/// Determines the part of the AST node to replace. We support this to work
-/// around the fact that the AST does not differentiate various syntactic
-/// elements into their own nodes, so users can specify them relative to a node,
-/// instead.
-enum class NodePart {
- /// The node itself.
- Node,
- /// Given a \c MemberExpr, selects the member's token.
- Member,
- /// Given a \c NamedDecl or \c CxxCtorInitializer, selects that token of the
- /// relevant name, not including qualifiers.
- Name,
-};
// Note that \p TextGenerator is allowed to fail, e.g. when trying to access a
// matched node that was not bound. Allowing this to fail simplifies error
@@ -76,73 +64,28 @@
// (`RewriteRule::Explanation`) instead. Notes serve the rare cases wherein
// edit-specific diagnostics are required.
//
-// `ASTEdit` should be built using the `change` convenience fucntions. For
+// `ASTEdit` should be built using the `change` convenience functions. For
// example,
// \code
-// change<FunctionDecl>(fun, NodePart::Name, "Frodo")
+// change(name(fun), text("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.")
+// using stencil::cat;
+// change(statement(thenNode), cat("{", thenNode, "}"))
+// change(callArgs(call), cat(x, ",", y))
// \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")
+// change(cat("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;
+ RangeSelector TargetRange;
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. A simple rule
@@ -175,9 +118,9 @@
// We expect RewriteRules will most commonly include only one case.
SmallVector<Case, 1> Cases;
- // Id used as the default target of each match. The node described by the
+ // ID used as the default target of each match. The node described by the
// matcher is should always be bound to this id.
- static constexpr llvm::StringLiteral RootId = "___root___";
+ static constexpr llvm::StringLiteral RootID = "___root___";
};
/// Convenience function for constructing a simple \c RewriteRule.
@@ -235,10 +178,17 @@
// ```
RewriteRule applyFirst(ArrayRef<RewriteRule> Rules);
-// 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));
+/// Replaces a portion of the source text with \p Replacement.
+ASTEdit change(RangeSelector Target, TextGenerator Replacement);
+
+/// Replaces the entirety of a RewriteRule's match with \p Replacement. For
+/// example, to replace a function call, one could write:
+/// \code
+/// makeRule(callExpr(callee(functionDecl(hasName("foo")))),
+/// change(text("bar()")))
+/// \endcode
+inline ASTEdit change(TextGenerator Replacement) {
+ return change(node(RewriteRule::RootID), std::move(Replacement));
}
/// The following three functions are a low-level part of the RewriteRule
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
