[libTooling] Cleanup and reorder `RewriteRule.h`.

[ymandel]

Summary

This patch lifts RootID out of the RewriteRule class so that constructs
(e.g. inline functions) can that refer to the root id don't need to depend on
the RewriteRule class.

With this dependency, the patch is able to collect all ASTEdit helper function
declarations together with the class declaration, before the introduction of the
RewriteRule class. In the process, we also adjust some of the comments.

This patch is essentially a NFC.

[ymandel]

fix typo

[gribozavr2]

clang/include/clang/Tooling/Transformer/RewriteRule.h

147

[ymandel]

fixed typo

clang/include/clang/Tooling/Transformer/RewriteRule.h

struct ASTEdit {
   // Not all transformations will want or need to attach metadata and therefore
   // should not be requierd to do so.
   AnyGenerator Metadata = [](const ast_matchers::MatchFinder::MatchResult &)
       -> llvm::Expected<llvm::Any> {
     return llvm::Expected<llvm::Any>(llvm::Any());
   };
 };
 
+/// Generates a single (specified) edit.
+EditGenerator edit(ASTEdit E);
+
 /// Lifts a list of `ASTEdit`s into an `EditGenerator`.
 ///
 /// The `EditGenerator` will return 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) and it will fail if any invariants
 /// are violated relating to bound nodes in 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.
 EditGenerator editList(llvm::SmallVector<ASTEdit, 1> Edits);
-/// Convenience form of `editList` for a single edit.
-EditGenerator edit(ASTEdit);
 
-/// Convenience generator for a no-op edit generator.
+/// Generates no edits.
 inline EditGenerator noEdits() { return editList({}); }
 
-/// Convenience version of `ifBound` specialized to `ASTEdit`.
+/// Version of `ifBound` specialized to `ASTEdit`.
 inline EditGenerator ifBound(std::string ID, ASTEdit TrueEdit,
                              ASTEdit FalseEdit) {
   return ifBound(std::move(ID), edit(std::move(TrueEdit)),
                  edit(std::move(FalseEdit)));
 }
 
-/// Convenience version of `ifBound` that has no "False" branch. If the node is
-/// not bound, then no edits are produced.
+/// Version of `ifBound` that has no "False" branch. If the node is not bound,
+/// then no edits are produced.
 inline EditGenerator ifBound(std::string ID, ASTEdit TrueEdit) {
   return ifBound(std::move(ID), edit(std::move(TrueEdit)), noEdits());
 }
 
 /// Flattens a list of generators into a single generator whose elements are the
 /// concatenation of the results of the argument generators.
 EditGenerator flattenVector(SmallVector<EditGenerator, 2> Generators);
 
 namespace detail {
-/// Convenience function to construct an \c EditGenerator. Overloaded for common
+/// Helper function to construct an \c EditGenerator. Overloaded for common
 /// cases so that user doesn't need to specify which factory function to
 /// use. This pattern gives benefits similar to implicit constructors, while
 /// maintaing a higher degree of explicitness.
 inline EditGenerator injectEdits(ASTEdit E) { return edit(std::move(E)); }
 inline EditGenerator injectEdits(EditGenerator G) { return G; }
 } // namespace detail
 
 template <typename... Ts> EditGenerator flatten(Ts &&...Edits) {
   return flattenVector({detail::injectEdits(std::forward<Ts>(Edits))...});
 }
 
+// Every rewrite rule is triggered by a match against some AST node.
+// Transformer guarantees that this ID is bound to the triggering node whenever
+// a rewrite rule is applied.
+extern const char RootID[];
+
+/// Replaces a portion of the source text with \p Replacement.
+ASTEdit changeTo(RangeSelector Target, TextGenerator Replacement);
+/// DEPRECATED: use \c changeTo.
+inline ASTEdit change(RangeSelector Target, TextGenerator Replacement) {
+  return changeTo(std::move(Target), std::move(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")))),
+///            changeTo(cat("bar()")))
+/// \endcode
+inline ASTEdit changeTo(TextGenerator Replacement) {
+  return changeTo(node(RootID), std::move(Replacement));
+}
+/// DEPRECATED: use \c changeTo.
+inline ASTEdit change(TextGenerator Replacement) {
+  return changeTo(std::move(Replacement));
+}
+
+/// Inserts \p Replacement before \p S, leaving the source selected by \S
+/// unchanged.
+inline ASTEdit insertBefore(RangeSelector S, TextGenerator Replacement) {
+  return changeTo(before(std::move(S)), std::move(Replacement));
+}
+
+/// Inserts \p Replacement after \p S, leaving the source selected by \S
+/// unchanged.
+inline ASTEdit insertAfter(RangeSelector S, TextGenerator Replacement) {
+  return changeTo(after(std::move(S)), std::move(Replacement));
+}
+
+/// Removes the source selected by \p S.
+ASTEdit remove(RangeSelector S);
+
+// FIXME: If `Metadata` returns an `llvm::Expected<T>` the `AnyGenerator` will
+// construct an `llvm::Expected<llvm::Any>` where no error is present but the
+// `llvm::Any` holds the error. This is unlikely but potentially surprising.
+// Perhaps the `llvm::Expected` should be unwrapped, or perhaps this should be a
+// compile-time error. No solution here is perfect.
+//
+// Note: This function template accepts any type callable with a MatchResult
+// rather than a `std::function` because the return-type needs to be deduced. If
+// it accepted a `std::function<R(MatchResult)>`, lambdas or other callable
+// types would not be able to deduce `R`, and users would be forced to specify
+// explicitly the type they intended to return by wrapping the lambda at the
+// call-site.
+template <typename Callable>
+inline ASTEdit withMetadata(ASTEdit Edit, Callable Metadata) {
+  Edit.Metadata =
+      [Gen = std::move(Metadata)](
+          const ast_matchers::MatchFinder::MatchResult &R) -> llvm::Any {
+    return Gen(R);
+  };
+
+  return Edit;
+}
+
+/// Assuming that the inner range is enclosed by the outer range, creates
+/// precision edits to remove the parts of the outer range that are not included
+/// in the inner range.
+inline EditGenerator shrinkTo(RangeSelector outer, RangeSelector inner) {
+  return editList({remove(enclose(before(outer), before(inner))),
+                   remove(enclose(after(inner), after(outer)))});
+}
+
 /// Format of the path in an include directive -- angle brackets or quotes.
 enum class IncludeFormat {
   Quoted,
   Angled,
 };
 
 /// Description of a source-code transformation.
 //

 //
 // The \c Transformer class can be used to apply the rewrite rule and obtain the
 // corresponding replacements.
 struct RewriteRule {
   struct Case {
     ast_matchers::internal::DynTypedMatcher Matcher;
     EditGenerator Edits;
     TextGenerator Explanation;
-    // Include paths to add to the file affected by this case.  These are
-    // bundled with the `Case`, rather than the `RewriteRule`, because each case
-    // might have different associated changes to the includes.
+    /// Include paths to add to the file affected by this case.  These are
+    /// bundled with the `Case`, rather than the `RewriteRule`, because each
+    /// case might have different associated changes to the includes.
     std::vector<std::pair<std::string, IncludeFormat>> AddedIncludes;
   };
   // 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
-  // matcher is should always be bound to this id.
-  static constexpr llvm::StringLiteral RootID = "___root___";
+  /// DEPRECATED: use `::clang::transformer::RootID` instead.
+  static const llvm::StringRef RootID;
 };
 
-/// Convenience function for constructing a simple \c RewriteRule.
+/// Constructs a simple \c RewriteRule.
 RewriteRule makeRule(ast_matchers::internal::DynTypedMatcher M,
                      EditGenerator Edits, TextGenerator Explanation = nullptr);
 
-/// Convenience function for constructing a \c RewriteRule from multiple
-/// `ASTEdit`s.
+/// Constructs a \c RewriteRule from multiple `ASTEdit`s.
 inline RewriteRule makeRule(ast_matchers::internal::DynTypedMatcher M,
                             llvm::SmallVector<ASTEdit, 1> Edits,
                             TextGenerator Explanation = nullptr) {
   return makeRule(std::move(M), editList(std::move(Edits)),
                   std::move(Explanation));
 }
 
-/// Convenience overload of \c makeRule for common case of only one edit.
+/// Overload of \c makeRule for common case of only one edit.
 inline RewriteRule makeRule(ast_matchers::internal::DynTypedMatcher M,
                             ASTEdit Edit,
                             TextGenerator Explanation = nullptr) {
   return makeRule(std::move(M), edit(std::move(Edit)), std::move(Explanation));
 }
 
 /// For every case in Rule, adds an include directive for the given header. The
 /// common use is assumed to be a rule with only one case. For example, to

 //     callExpr(callee(eq_fun), callExpr(hasArgument(1, method_call)));
 //
 // RewriteRule R = applyFirst({makeRule(two_calls, two_calls_action),
 //                             makeRule(left_call, left_call_action),
 //                             makeRule(right_call, right_call_action)});
 // ```
 RewriteRule applyFirst(ArrayRef<RewriteRule> Rules);
 
-/// Replaces a portion of the source text with \p Replacement.
-ASTEdit changeTo(RangeSelector Target, TextGenerator Replacement);
-/// DEPRECATED: use \c changeTo.
-inline ASTEdit change(RangeSelector Target, TextGenerator Replacement) {
-  return changeTo(std::move(Target), std::move(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")))),
-///            changeTo(cat("bar()")))
-/// \endcode
-inline ASTEdit changeTo(TextGenerator Replacement) {
-  return changeTo(node(std::string(RewriteRule::RootID)),
-                  std::move(Replacement));
-}
-/// DEPRECATED: use \c changeTo.
-inline ASTEdit change(TextGenerator Replacement) {
-  return changeTo(std::move(Replacement));
-}
-
-/// Inserts \p Replacement before \p S, leaving the source selected by \S
-/// unchanged.
-inline ASTEdit insertBefore(RangeSelector S, TextGenerator Replacement) {
-  return changeTo(before(std::move(S)), std::move(Replacement));
-}
-
-/// Inserts \p Replacement after \p S, leaving the source selected by \S
-/// unchanged.
-inline ASTEdit insertAfter(RangeSelector S, TextGenerator Replacement) {
-  return changeTo(after(std::move(S)), std::move(Replacement));
-}
-
-/// Removes the source selected by \p S.
-ASTEdit remove(RangeSelector S);
-
-// FIXME: If `Metadata` returns an `llvm::Expected<T>` the `AnyGenerator` will
-// construct an `llvm::Expected<llvm::Any>` where no error is present but the
-// `llvm::Any` holds the error. This is unlikely but potentially surprising.
-// Perhaps the `llvm::Expected` should be unwrapped, or perhaps this should be a
-// compile-time error. No solution here is perfect.
-//
-// Note: This function template accepts any type callable with a MatchResult
-// rather than a `std::function` because the return-type needs to be deduced. If
-// it accepted a `std::function<R(MatchResult)>`, lambdas or other callable types
-// would not be able to deduce `R`, and users would be forced to specify
-// explicitly the type they intended to return by wrapping the lambda at the
-// call-site.
-template <typename Callable>
-inline ASTEdit withMetadata(ASTEdit Edit, Callable Metadata) {
-  Edit.Metadata =
-      [Gen = std::move(Metadata)](
-          const ast_matchers::MatchFinder::MatchResult &R) -> llvm::Any {
-    return Gen(R);
-  };
-
-  return Edit;
-}
-
-/// Assuming that the inner range is enclosed by the outer range, creates
-/// precision edits to remove the parts of the outer range that are not included
-/// in the inner range.
-inline EditGenerator shrinkTo(RangeSelector outer, RangeSelector inner) {
-  return editList({remove(enclose(before(outer), before(inner))),
-                   remove(enclose(after(inner), after(outer)))});
-}
-
 /// Applies `Rule` to all descendants of the node bound to `NodeId`. `Rule` can
 /// refer to nodes bound by the calling rule. `Rule` is not applied to the node
 /// itself.
 ///
 /// For example,
 /// ```
 /// auto InlineX =
 ///     makeRule(declRefExpr(to(varDecl(hasName("x")))), changeTo(cat("3")));

clang/lib/Tooling/Transformer/RewriteRule.cpp

 using namespace clang;
 using namespace transformer;
 
 using ast_matchers::MatchFinder;
 using ast_matchers::internal::DynTypedMatcher;
 
 using MatchResult = MatchFinder::MatchResult;
 
+const char transformer::RootID[] = "___root___";
+
 static Expected<SmallVector<transformer::Edit, 1>>
 translateEdits(const MatchResult &Result, ArrayRef<ASTEdit> ASTEdits) {
   SmallVector<transformer::Edit, 1> Edits;
   for (const auto &E : ASTEdits) {
     Expected<CharSourceRange> Range = E.TargetRange(Result);
     if (!Range)
       return Range.takeError();
     llvm::Optional<CharSourceRange> EditRange =
transformer::detail::buildMatchers(const RewriteRule &Rule) {
   // beginners, who are (largely) the target audience of Transformer.
   std::vector<DynTypedMatcher> Matchers;
   for (const auto &Bucket : Buckets) {
     DynTypedMatcher M = DynTypedMatcher::constructVariadic(
         DynTypedMatcher::VO_AnyOf, Bucket.first,
         taggedMatchers("Tag", Bucket.second, TK_IgnoreUnlessSpelledInSource));
     M.setAllowBind(true);
     // `tryBind` is guaranteed to succeed, because `AllowBind` was set to true.
-    Matchers.push_back(
-        M.tryBind(RewriteRule::RootID)->withTraversalKind(TK_AsIs));
+    Matchers.push_back(M.tryBind(RootID)->withTraversalKind(TK_AsIs));
   }
   return Matchers;
 }
 
 DynTypedMatcher transformer::detail::buildMatcher(const RewriteRule &Rule) {
   std::vector<DynTypedMatcher> Ms = buildMatchers(Rule);
   assert(Ms.size() == 1 && "Cases must have compatible matchers.");
   return Ms[0];
 }
 
 SourceLocation transformer::detail::getRuleMatchLoc(const MatchResult &Result) {
   auto &NodesMap = Result.Nodes.getMap();
-  auto Root = NodesMap.find(RewriteRule::RootID);
+  auto Root = NodesMap.find(RootID);
   assert(Root != NodesMap.end() && "Transformation failed: missing root node.");
   llvm::Optional<CharSourceRange> RootRange = tooling::getRangeForEdit(
       CharSourceRange::getTokenRange(Root->second.getSourceRange()),
       *Result.Context);
   if (RootRange)
     return RootRange->getBegin();
   // The match doesn't have a coherent range, so fall back to the expansion
   // location as the "beginning" of the match.
transformer::detail::findSelectedCase(const MatchResult &Result,
   for (size_t i = 0, N = Rule.Cases.size(); i < N; ++i) {
     std::string Tag = ("Tag" + Twine(i)).str();
     if (NodesMap.find(Tag) != NodesMap.end())
       return Rule.Cases[i];
   }
   llvm_unreachable("No tag found for this rule.");
 }
 
-constexpr llvm::StringLiteral RewriteRule::RootID;
+const llvm::StringRef RewriteRule::RootID = ::clang::transformer::RootID;
 
 TextGenerator tooling::text(std::string M) {
   return std::make_shared<SimpleTextGenerator>(std::move(M));
 }