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
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...