[libTooling] Generalize string explanation as templated metadata

[li.zhe.hua]

Summary

Change RewriteRule from holding an Explanation to being able to generate
arbitrary metadata. Where TransformerClangTidyCheck was interested in a string
description for the diagnostic, other tools may be interested in richer metadata
at a higher level of abstraction than at the edit level (which is currently
available as ASTEdit::Metadata).

[li.zhe.hua]

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

70

Placeholder name; suggestions welcome.

[ymandel]

High level question: why use Any rather than templating?

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

70

hmm. How about MetadataGenerator? Less general, but to the point.

[li.zhe.hua]

Update alias

[li.zhe.hua]

In D120360#3341118, @ymandel wrote:

High level question: why use Any rather than templating?

(Aside: Not sure how to "respond" to comments... just quote in a new comment?)

So, I had a version of this, but I ended up bifurcating the type into RewriteRule (with no metadata) and RewriteRuleWith<T> (with T metadata). I wasn't sure if the cost of having two types here was worth it. For interfaces that didn't care about the metadata generation, there wasn't an easy way to accept a RewriteRule-like parameter.

Finally, there was a question of migration. All uses of RewriteRule with TransformerClangTidyCheck would need to switch over to RewriteRuleWith<std::string>, and I wasn't sure how pervasive that might be. I'm not set on Any, but splitting the type does seem rather costly.

[li.zhe.hua]

As discussed offline, switch to using a template instead of Any.

[ymandel]

Looks really good!

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

64–66

(and move down to after Generator def.)

286–291

Please comment.

329

Maybe add an adaptor for edit generator like we do in Stencil::cat. That will allow us to collapse the 6 overloads (2 * 3) into 2 (for makerule) + 3 (for the adaptor). Only saves 1 but I think it will clearer and more flexible for the future.

526–527

please update

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

22

in RewriteRule.h, we use "detail". Maybe do the same here?

48

I'm afraid the use of the name Consumer (here and below) might be confused with the Consumer argument. What about just Impl or something similar?

122

why won't we have the same SFINAE here to ensure ConsumerFn is invocable?

[li.zhe.hua]

Fixes from comments

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

329

So, I ended up needing to add an overload for std::initializer_list as well, as it was somewhat common for people to do something like

makeRule(matchers(), {doThing1(), doThing2()})

which would have gone to the llvm::SmallVector<ASTEdit, 1> overload.

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

48

I went with NoMetadataImpl (to be more descriptive than "plain") and WithMetadataImpl.

122

I can't figure out how to implement the SFINAE without instantiating Result<void> (which is invalid because T Metadata is illegal then). My current attempt is

template <
    typename T, typename ConsumerFn,
    std::enable_if_t<
        std::conjunction<
            std::negation<std::is_same<T, void>>,
            llvm::is_invocable<ConsumerFn, llvm::Expected<Result<T>>>>::value,
        int> = 0>
explicit Transformer(transformer::RewriteRuleWith<T> Rule,
                     ConsumerFn Consumer);

I suppose I could provide a specialization of Result<void> that is valid? I guess I'll go with that, and just document why it exists?

[li.zhe.hua]

Fix use-after-move in asserts

[asoffer]

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

103–108

Given that we're simply passing this off to the NoMetadataImpl which accepts a std::function, I don't see any reason to accept a generic parameter via sfinae. We could just accept the std::function and move it. Then the implicit conversion from whatever is passed in to std::function will do the heavy sfinae lifting.

122

Accepting the std:function directly should simplify this and avoid the need for the Result<void> specialization.

[ymandel]

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

122

Seems fine, but I'll admit to being out of my depth. Seemed nice to have, but not necessary.

[li.zhe.hua]

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

103–108

So, I ran into this during D119745, where one of the Windows CI builds doesn't have the fix for DR 2132 backported, so overloading on different std::function doesn't work. (To be clear, this using std::function and doing the whole overloading thing works fine in clang locally.)

Thoughts?

[li.zhe.hua]

Try removing all the SFINAE

[li.zhe.hua]

Update comments

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

103–108

Nevermind! I just tried and it works. I suspect this is because the non-template constructor and the template constructor are not ambiguous, as the non-template is tried first, and if it doesn't match, it goes to the template.

[li.zhe.hua]

Fix issue with "explicit specialization in non-namespace scope"

clang-tools-extra/clang-tidy/abseil/CleanupCtadCheck.cpp

 
 using namespace ::clang::ast_matchers;
 using namespace ::clang::transformer;
 
 namespace clang {
 namespace tidy {
 namespace abseil {
 
-RewriteRule CleanupCtadCheckImpl() {
+RewriteRuleWith<std::string> CleanupCtadCheckImpl() {
   auto warning_message = cat("prefer absl::Cleanup's class template argument "
                              "deduction pattern in C++17 and higher");
 
   return makeRule(
       declStmt(hasSingleDecl(varDecl(
           hasType(autoType()), hasTypeLoc(typeLoc().bind("auto_type_loc")),
           hasInitializer(hasDescendant(
               callExpr(callee(functionDecl(hasName("absl::MakeCleanup"))),

clang-tools-extra/clang-tidy/abseil/StringFindStrContainsCheck.cpp

 namespace abseil {
 
 using ::clang::transformer::addInclude;
 using ::clang::transformer::applyFirst;
 using ::clang::transformer::cat;
 using ::clang::transformer::changeTo;
 using ::clang::transformer::makeRule;
 using ::clang::transformer::node;
-using ::clang::transformer::RewriteRule;
+using ::clang::transformer::RewriteRuleWith;
 
 AST_MATCHER(Type, isCharType) { return Node.isCharType(); }
 
 static const char DefaultStringLikeClasses[] = "::std::basic_string;"
                                                "::std::basic_string_view;"
                                                "::absl::string_view";
 static const char DefaultAbseilStringsMatchHeader[] = "absl/strings/match.h";
 
-static transformer::RewriteRule
+static transformer::RewriteRuleWith<std::string>
 makeRewriteRule(const std::vector<std::string> &StringLikeClassNames,
                 StringRef AbseilStringsMatchHeader) {
   auto StringLikeClass = cxxRecordDecl(hasAnyName(SmallVector<StringRef, 4>(
       StringLikeClassNames.begin(), StringLikeClassNames.end())));
   auto StringType =
       hasUnqualifiedDesugaredType(recordType(hasDeclaration(StringLikeClass)));
   auto CharStarType =
       hasUnqualifiedDesugaredType(pointerType(pointee(isAnyCharacter())));
   auto CharType = hasUnqualifiedDesugaredType(isCharType());
   auto StringNpos = declRefExpr(
       to(varDecl(hasName("npos"), hasDeclContext(StringLikeClass))));
   auto StringFind = cxxMemberCallExpr(
       callee(cxxMethodDecl(
           hasName("find"), parameterCountIs(2),
           hasParameter(
               0, parmVarDecl(anyOf(hasType(StringType), hasType(CharStarType),
                                    hasType(CharType)))))),
       on(hasType(StringType)), hasArgument(0, expr().bind("parameter_to_find")),
       anyOf(hasArgument(1, integerLiteral(equals(0))),
             hasArgument(1, cxxDefaultArgExpr())),
       onImplicitObjectArgument(expr().bind("string_being_searched")));
 
-  RewriteRule Rule = applyFirst(
+  RewriteRuleWith<std::string> Rule = applyFirst(
       {makeRule(
            binaryOperator(hasOperatorName("=="),
                           hasOperands(ignoringParenImpCasts(StringNpos),
                                       ignoringParenImpCasts(StringFind))),
            {changeTo(cat("!absl::StrContains(", node("string_being_searched"),
                          ", ", node("parameter_to_find"), ")")),
             addInclude(AbseilStringsMatchHeader)},
            cat("use !absl::StrContains instead of find() == npos")),

clang-tools-extra/clang-tidy/bugprone/StringviewNullptrCheck.cpp

 }
 
 AST_MATCHER(clang::VarDecl, isDirectInitialization) {
   return Node.getInitStyle() != clang::VarDecl::InitializationStyle::CInit;
 }
 
 } // namespace
 
-RewriteRule StringviewNullptrCheckImpl() {
+RewriteRuleWith<std::string> StringviewNullptrCheckImpl() {
   auto construction_warning =
       cat("constructing basic_string_view from null is undefined; replace with "
           "the default constructor");
   auto static_cast_warning =
       cat("casting to basic_string_view from null is undefined; replace with "
           "the empty string");
   auto argument_construction_warning =
       cat("passing null as basic_string_view is undefined; replace with the "

clang-tools-extra/clang-tidy/utils/TransformerClangTidyCheck.h

public:
   /// DEPRECATED: prefer the two argument constructor in conjunction with
   /// \c setRule.
   ///
   /// \p MakeRule generates the rewrite rule to be used by the check, based on
   /// the given language and clang-tidy options. It can return \c None to handle
   /// cases where the options disable the check.
   ///
   /// See \c setRule for constraints on the rule.
-  TransformerClangTidyCheck(std::function<Optional<transformer::RewriteRule>(
-                                const LangOptions &, const OptionsView &)>
-                                MakeRule,
-                            StringRef Name, ClangTidyContext *Context);
+  TransformerClangTidyCheck(
+      std::function<Optional<transformer::RewriteRuleWith<std::string>>(
 
   /// Convenience overload of the constructor when the rule doesn't have any
   /// dependencies.
-  TransformerClangTidyCheck(transformer::RewriteRule R, StringRef Name,
-                            ClangTidyContext *Context);
+  TransformerClangTidyCheck(transformer::RewriteRuleWith<std::string> R,
+                            StringRef Name, ClangTidyContext *Context);
 
   void registerPPCallbacks(const SourceManager &SM, Preprocessor *PP,
                            Preprocessor *ModuleExpanderPP) override;
   void registerMatchers(ast_matchers::MatchFinder *Finder) final;
   void check(const ast_matchers::MatchFinder::MatchResult &Result) final;
 
   /// Derived classes that override this function should call this method from
   /// the overridden method.
   void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
 
   /// Set the rule that this check implements.  All cases in the rule must have
   /// a non-null \c Explanation, even though \c Explanation is optional for
   /// RewriteRule in general. Because the primary purpose of clang-tidy checks
   /// is to provide users with diagnostics, we assume that a missing explanation
   /// is a bug.  If no explanation is desired, indicate that explicitly (for
   /// example, by passing `text("no explanation")` to `makeRule` as the
   /// `Explanation` argument).
-  void setRule(transformer::RewriteRule R);
+  void setRule(transformer::RewriteRuleWith<std::string> R);
 
 private:
-  transformer::RewriteRule Rule;
+  transformer::RewriteRuleWith<std::string> Rule;
   IncludeInserter Inserter;
 };
 
 } // namespace utils
 } // namespace tidy
 } // namespace clang
 
 #endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_TRANSFORMER_CLANG_TIDY_CHECK_H

clang-tools-extra/clang-tidy/utils/TransformerClangTidyCheck.cpp

 //===---------- TransformerClangTidyCheck.cpp - clang-tidy ----------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include "TransformerClangTidyCheck.h"
 #include "clang/Lex/Preprocessor.h"
 #include "llvm/ADT/STLExtras.h"
 
 namespace clang {
 namespace tidy {
 namespace utils {
-using transformer::RewriteRule;
+using transformer::RewriteRuleWith;
 
 #ifndef NDEBUG
-static bool hasExplanation(const RewriteRule::Case &C) {
-  return C.Explanation != nullptr;
+static bool hasGenerator(const transformer::Generator<std::string> &G) {
+  return G != nullptr;
 }
 #endif
 
-static void verifyRule(const RewriteRule &Rule) {
-  assert(llvm::all_of(Rule.Cases, hasExplanation) &&
+static void verifyRule(const RewriteRuleWith<std::string> &Rule) {
+  assert(llvm::all_of(Rule.Metadata, hasGenerator) &&
          "clang-tidy checks must have an explanation by default;"
          " explicitly provide an empty explanation if none is desired");
 }
 
 TransformerClangTidyCheck::TransformerClangTidyCheck(StringRef Name,
                                                      ClangTidyContext *Context)
     : ClangTidyCheck(Name, Context),
       Inserter(
           Options.getLocalOrGlobal("IncludeStyle", IncludeSorter::IS_LLVM)) {}
 
 // This constructor cannot dispatch to the simpler one (below), because, in
 // order to get meaningful results from `getLangOpts` and `Options`, we need the
 // `ClangTidyCheck()` constructor to have been called. If we were to dispatch,
 // we would be accessing `getLangOpts` and `Options` before the underlying
 // `ClangTidyCheck` instance was properly initialized.
 TransformerClangTidyCheck::TransformerClangTidyCheck(
-    std::function<Optional<RewriteRule>(const LangOptions &,
+    std::function<Optional<RewriteRuleWith<std::string>>(const LangOptions &,
                                                          const OptionsView &)>
         MakeRule,
     StringRef Name, ClangTidyContext *Context)
     : TransformerClangTidyCheck(Name, Context) {
-  if (Optional<RewriteRule> R = MakeRule(getLangOpts(), Options))
+  if (Optional<RewriteRuleWith<std::string>> R =
+          MakeRule(getLangOpts(), Options))
     setRule(std::move(*R));
 }
 
-TransformerClangTidyCheck::TransformerClangTidyCheck(RewriteRule R,
-                                                     StringRef Name,
-                                                     ClangTidyContext *Context)
+TransformerClangTidyCheck::TransformerClangTidyCheck(
+    RewriteRuleWith<std::string> R, StringRef Name, ClangTidyContext *Context)
     : TransformerClangTidyCheck(Name, Context) {
   setRule(std::move(R));
 }
 
-void TransformerClangTidyCheck::setRule(transformer::RewriteRule R) {
+void TransformerClangTidyCheck::setRule(
+    transformer::RewriteRuleWith<std::string> R) {
   verifyRule(R);
   Rule = std::move(R);
 }
 
 void TransformerClangTidyCheck::registerPPCallbacks(
     const SourceManager &SM, Preprocessor *PP, Preprocessor *ModuleExpanderPP) {
   Inserter.registerPreprocessor(PP);
 }
 
 void TransformerClangTidyCheck::registerMatchers(
     ast_matchers::MatchFinder *Finder) {
   if (!Rule.Cases.empty())
     for (auto &Matcher : transformer::detail::buildMatchers(Rule))
       Finder->addDynamicMatcher(Matcher, this);
 }
 
 void TransformerClangTidyCheck::check(
     const ast_matchers::MatchFinder::MatchResult &Result) {
   if (Result.Context->getDiagnostics().hasErrorOccurred())
     return;
 
-  RewriteRule::Case Case = transformer::detail::findSelectedCase(Result, Rule);
-  Expected<SmallVector<transformer::Edit, 1>> Edits = Case.Edits(Result);
+  size_t I = transformer::detail::findSelectedCase(Result, Rule);
+  Expected<SmallVector<transformer::Edit, 1>> Edits =
+      Rule.Cases[I].Edits(Result);
   if (!Edits) {
     llvm::errs() << "Rewrite failed: " << llvm::toString(Edits.takeError())
                  << "\n";
     return;
   }
 
   // No rewrite applied, but no error encountered either.
   if (Edits->empty())
     return;
 
-  Expected<std::string> Explanation = Case.Explanation->eval(Result);
+  Expected<std::string> Explanation = Rule.Metadata[I]->eval(Result);
   if (!Explanation) {
     llvm::errs() << "Error in explanation: "
                  << llvm::toString(Explanation.takeError()) << "\n";
     return;
   }
 
   // Associate the diagnostic with the location of the first change.
   DiagnosticBuilder Diag = diag((*Edits)[0].Range.getBegin(), *Explanation);

clang-tools-extra/unittests/clang-tidy/TransformerClangTidyCheckTest.cpp

 using namespace ::clang::ast_matchers;
 
 using transformer::cat;
 using transformer::change;
 using transformer::IncludeFormat;
 using transformer::makeRule;
 using transformer::node;
 using transformer::noopEdit;
-using transformer::RewriteRule;
+using transformer::RewriteRuleWith;
 using transformer::RootID;
 using transformer::statement;
 
 // Invert the code of an if-statement, while maintaining its semantics.
-RewriteRule invertIf() {
+RewriteRuleWith<std::string> invertIf() {
   StringRef C = "C", T = "T", E = "E";
-  RewriteRule Rule = makeRule(
+  RewriteRuleWith<std::string> Rule = makeRule(
       ifStmt(hasCondition(expr().bind(C)), hasThen(stmt().bind(T)),
              hasElse(stmt().bind(E))),
       change(statement(RootID), cat("if(!(", node(std::string(C)), ")) ",
                                     statement(std::string(E)), " else ",
                                     statement(std::string(T)))),
       cat("negate condition and reverse `then` and `else` branches"));
   return Rule;
 }

 #define M(a,b) (1 + a) * (1 + b)
     int f() { return M(RIGHT, RIGHT); }
   )cc";
 
   EXPECT_EQ(Expected, test::runCheckOnCode<BinOpCheck>(Input));
 }
 
 // A trivial rewrite-rule generator that requires Objective-C code.
-Optional<RewriteRule> needsObjC(const LangOptions &LangOpts,
-                                const ClangTidyCheck::OptionsView &Options) {
+Optional<RewriteRuleWith<std::string>>
+needsObjC(const LangOptions &LangOpts,
   if (!LangOpts.ObjC)
     return None;
   return makeRule(clang::ast_matchers::functionDecl(),
                   change(cat("void changed() {}")), cat("no message"));
 }
 
 class NeedsObjCCheck : public TransformerClangTidyCheck {
 public:
   NeedsObjCCheck(StringRef Name, ClangTidyContext *Context)
       : TransformerClangTidyCheck(needsObjC, Name, Context) {}
 };
 
 // Verify that the check only rewrites the code when the input is Objective-C.
 TEST(TransformerClangTidyCheckTest, DisableByLang) {
   const std::string Input = "void log() {}";
   EXPECT_EQ(Input,
             test::runCheckOnCode<NeedsObjCCheck>(Input, nullptr, "input.cc"));
 
   EXPECT_EQ("void changed() {}",
             test::runCheckOnCode<NeedsObjCCheck>(Input, nullptr, "input.mm"));
 }
 
 // A trivial rewrite rule generator that checks config options.
-Optional<RewriteRule> noSkip(const LangOptions &LangOpts,
-                             const ClangTidyCheck::OptionsView &Options) {
+Optional<RewriteRuleWith<std::string>>
+noSkip(const LangOptions &LangOpts,
   if (Options.get("Skip", "false") == "true")
     return None;
   return makeRule(clang::ast_matchers::functionDecl(),
                   changeTo(cat("void nothing();")), cat("no message"));
 }
 
 class ConfigurableCheck : public TransformerClangTidyCheck {
 public:
TEST(TransformerClangTidyCheckTest, DisableByConfig) {
   EXPECT_EQ(Input, test::runCheckOnCode<ConfigurableCheck>(
                        Input, nullptr, "input.cc", None, Options));
 
   Options.CheckOptions["test-check-0.Skip"] = "false";
   EXPECT_EQ(Expected, test::runCheckOnCode<ConfigurableCheck>(
                           Input, nullptr, "input.cc", None, Options));
 }
 
-RewriteRule replaceCall(IncludeFormat Format) {
+RewriteRuleWith<std::string> replaceCall(IncludeFormat Format) {
   using namespace ::clang::ast_matchers;
-  RewriteRule Rule = makeRule(callExpr(callee(functionDecl(hasName("f")))),
-                              change(cat("other()")), cat("no message"));
+  RewriteRuleWith<std::string> Rule =
+      makeRule(callExpr(callee(functionDecl(hasName("f")))),
   addInclude(Rule, "clang/OtherLib.h", Format);
   return Rule;
 }
 
 template <IncludeFormat Format>
 class IncludeCheck : public TransformerClangTidyCheck {
 public:
   IncludeCheck(StringRef Name, ClangTidyContext *Context)
std::string Expected = R"cc(#include <clang/OtherLib.h>
     int h(int x) { return other(); }
   )cc";
 
   EXPECT_EQ(Expected,
             test::runCheckOnCode<IncludeCheck<IncludeFormat::Angled>>(Input));
 }
 
 class IncludeOrderCheck : public TransformerClangTidyCheck {
-  static RewriteRule rule() {
+  static RewriteRuleWith<std::string> rule() {
     using namespace ::clang::ast_matchers;
-    RewriteRule Rule = transformer::makeRule(integerLiteral(), change(cat("5")),
-                                             cat("no message"));
+    RewriteRuleWith<std::string> Rule = transformer::makeRule(
+        integerLiteral(), change(cat("5")), cat("no message"));
     addInclude(Rule, "bar.h", IncludeFormat::Quoted);
     return Rule;
   }
 
 public:
   IncludeOrderCheck(StringRef Name, ClangTidyContext *Context)
       : TransformerClangTidyCheck(rule(), Name, Context) {}
 };

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

 };
 
 /// Maps a match result to a list of concrete edits (with possible
 /// failure). This type is a building block of rewrite rules, but users will
 /// generally work in terms of `ASTEdit`s (below) rather than directly in terms
 /// of `EditGenerator`.
 using EditGenerator = MatchConsumer<llvm::SmallVector<Edit, 1>>;
 
-using TextGenerator = std::shared_ptr<MatchComputation<std::string>>;
+template <typename T> using Generator = std::shared_ptr<MatchComputation<T>>;
+
+using TextGenerator = Generator<std::string>;
 
 using AnyGenerator = MatchConsumer<llvm::Any>;
 
 // 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.

 // A *rewrite rule* describes a transformation of source code. A simple rule
 // contains each of the following components:
 //
 // * Matcher: the pattern term, expressed as clang matchers (with Transformer
 //   extensions).
 //
 // * 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 diagnostics.
-//
 // However, rules can also consist of (sub)rules, where the first that matches
-// is applied and the rest are ignored.  So, the above components are gathered
-// as a `Case` and a rule is a list of cases.
+// is applied and the rest are ignored.  So, the above components together form
+// a logical "case" and a rule is a sequence of cases.
 //
 // Rule cases have an additional, implicit, component: the parameters. These are
 // portions of the pattern which are left unspecified, yet bound in the pattern
 // so that we can reference them in the edits.
 //
 // The \c Transformer class can be used to apply the rewrite rule and obtain the
 // corresponding replacements.
-struct RewriteRule {
+struct RewriteRuleBase {
   struct Case {
     ast_matchers::internal::DynTypedMatcher Matcher;
     EditGenerator Edits;
-    TextGenerator Explanation;
   };
   // We expect RewriteRules will most commonly include only one case.
   SmallVector<Case, 1> Cases;
+};
 
-  /// DEPRECATED: use `::clang::transformer::RootID` instead.
-  static const llvm::StringRef RootID;
+/// A source-code transformation with accompanying metadata.
+///
+/// When a case of the rule matches, the \c Transformer invokes the
+/// corresponding metadata generator and provides it alongside the edits.
+template <typename MetadataT> struct RewriteRuleWith : RewriteRuleBase {
+  SmallVector<Generator<MetadataT>, 1> Metadata;
 };
 
-/// Constructs a simple \c RewriteRule.
+template <> struct RewriteRuleWith<void> : RewriteRuleBase {};
+
+using RewriteRule = RewriteRuleWith<void>;
+
+namespace detail {
+
 RewriteRule makeRule(ast_matchers::internal::DynTypedMatcher M,
-                     EditGenerator Edits, TextGenerator Explanation = nullptr);
+                     EditGenerator Edits);
 
-/// 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));
+template <typename MetadataT>
+RewriteRuleWith<MetadataT> makeRule(ast_matchers::internal::DynTypedMatcher M,
+                                    EditGenerator Edits,
+                                    Generator<MetadataT> Metadata) {
+  RewriteRuleWith<MetadataT> R;
+  R.Cases = {{std::move(M), std::move(Edits)}};
+  R.Metadata = {std::move(Metadata)};
+  return R;
 }
 
-/// 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));
+inline EditGenerator makeEditGenerator(EditGenerator Edits) { return Edits; }
+EditGenerator makeEditGenerator(llvm::SmallVector<ASTEdit, 1> Edits);
+EditGenerator makeEditGenerator(ASTEdit Edit);
+
+} // namespace detail
+
+/// Constructs a simple \c RewriteRule. \c Edits can be an \c EditGenerator,
+/// multiple \c ASTEdits, or a single \c ASTEdit.
+/// @{
+template <int &..., typename EditsT>
+RewriteRule makeRule(ast_matchers::internal::DynTypedMatcher M,
+                     EditsT &&Edits) {
+  return detail::makeRule(
+      std::move(M), detail::makeEditGenerator(std::forward<EditsT>(Edits)));
 }
 
+RewriteRule makeRule(ast_matchers::internal::DynTypedMatcher M,
+                     std::initializer_list<ASTEdit> Edits);
+/// @}
+
+/// Overloads of \c makeRule that also generate metadata when matching.
+/// @{
+template <typename MetadataT, int &..., typename EditsT>
+RewriteRuleWith<MetadataT> makeRule(ast_matchers::internal::DynTypedMatcher M,
+                                    EditsT &&Edits,
+                                    Generator<MetadataT> Metadata) {
+  return detail::makeRule(
+      std::move(M), detail::makeEditGenerator(std::forward<EditsT>(Edits)),
+      std::move(Metadata));
+}
+
+template <typename MetadataT>
+RewriteRuleWith<MetadataT> makeRule(ast_matchers::internal::DynTypedMatcher M,
+                                    std::initializer_list<ASTEdit> Edits,
+                                    Generator<MetadataT> Metadata) {
+  return detail::makeRule(std::move(M),
+                          detail::makeEditGenerator(std::move(Edits)),
+                          std::move(Metadata));
+}
+/// @}
+
 /// 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
 /// replace a function call and add headers corresponding to the new code, one
 /// could write:
 /// \code
 ///   auto R = makeRule(callExpr(callee(functionDecl(hasName("foo")))),
 ///            changeTo(cat("bar()")));
 ///   addInclude(R, "path/to/bar_header.h");
 ///   addInclude(R, "vector", IncludeFormat::Angled);
 /// \endcode
-void addInclude(RewriteRule &Rule, llvm::StringRef Header,
+void addInclude(RewriteRuleBase &Rule, llvm::StringRef Header,
                 IncludeFormat Format = IncludeFormat::Quoted);
 
 /// Applies the first rule whose pattern matches; other rules are ignored.  If
 /// the matchers are independent then order doesn't matter. In that case,
 /// `applyFirst` is simply joining the set of rules into one.
 //
 // `applyFirst` is like an `anyOf` matcher with an edit action attached to each
 // of its cases. Anywhere you'd use `anyOf(m1.bind("id1"), m2.bind("id2"))` and

 //     callExpr(callee(eq_fun), callExpr(hasArgument(0, method_call)));
 // auto right_call =
 //     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);
+/// @{
+template <typename MetadataT>
+RewriteRuleWith<MetadataT>
+applyFirst(ArrayRef<RewriteRuleWith<MetadataT>> Rules) {
+  RewriteRuleWith<MetadataT> R;
+  for (auto &Rule : Rules) {
+    assert(Rule.Cases.size() == Rule.Metadata.size() &&
+           "mis-match in case and metadata array size");
+    R.Cases.append(Rule.Cases.begin(), Rule.Cases.end());
+    R.Metadata.append(Rule.Metadata.begin(), Rule.Metadata.end());
+  }
+  return R;
+}
+
+template <>
+RewriteRuleWith<void> applyFirst(ArrayRef<RewriteRuleWith<void>> Rules);
+
+template <typename MetadataT>
+RewriteRuleWith<MetadataT>
+applyFirst(const std::vector<RewriteRuleWith<MetadataT>> &Rules) {
+  return applyFirst(llvm::makeArrayRef(Rules));
+}
+
+template <typename MetadataT>
+RewriteRuleWith<MetadataT>
+applyFirst(std::initializer_list<RewriteRuleWith<MetadataT>> Rules) {
+  return applyFirst(llvm::makeArrayRef(Rules.begin(), Rules.end()));
+}
+/// @}
+
+/// Converts a \c RewriteRuleWith<T> to a \c RewriteRule by stripping off the
+/// metadata generators.
+template <int &..., typename MetadataT>
+std::enable_if_t<!std::is_same<MetadataT, void>::value, RewriteRule>
+stripMetadata(RewriteRuleWith<MetadataT> Rule) {
+  RewriteRule R;
+  R.Cases = std::move(Rule.Cases);
+  return R;
+}
 
 /// 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 =

 rewriteDescendants(const DynTypedNode &Node, RewriteRule Rule,
                    const ast_matchers::MatchFinder::MatchResult &Result);
 /// @}
 
 /// Builds a single matcher for the rule, covering all of the rule's cases.
 /// Only supports Rules whose cases' matchers share the same base "kind"
 /// (`Stmt`, `Decl`, etc.)  Deprecated: use `buildMatchers` instead, which
 /// supports mixing matchers of different kinds.
-ast_matchers::internal::DynTypedMatcher buildMatcher(const RewriteRule &Rule);
+ast_matchers::internal::DynTypedMatcher
+buildMatcher(const RewriteRuleBase &Rule);
 
 /// Builds a set of matchers that cover the rule.
 ///
 /// One matcher is built for each distinct node matcher base kind: Stmt, Decl,
 /// etc. Node-matchers for `QualType` and `Type` are not permitted, since such
 /// nodes carry no source location information and are therefore not relevant
 /// for rewriting. If any such matchers are included, will return an empty
 /// vector.
 std::vector<ast_matchers::internal::DynTypedMatcher>
-buildMatchers(const RewriteRule &Rule);
+buildMatchers(const RewriteRuleBase &Rule);
 
 /// Gets the beginning location of the source matched by a rewrite rule. If the
 /// match occurs within a macro expansion, returns the beginning of the
 /// expansion point. `Result` must come from the matching of a rewrite rule.
 SourceLocation
 getRuleMatchLoc(const ast_matchers::MatchFinder::MatchResult &Result);
 
-/// Returns the \c Case of \c Rule that was selected in the match result.
-/// Assumes a matcher built with \c buildMatcher.
-const RewriteRule::Case &
-findSelectedCase(const ast_matchers::MatchFinder::MatchResult &Result,
-                 const RewriteRule &Rule);
+/// Returns the index of the \c Case of \c Rule that was selected in the match
+/// result. Assumes a matcher built with \c buildMatcher.
+size_t findSelectedCase(const ast_matchers::MatchFinder::MatchResult &Result,
+                        const RewriteRuleBase &Rule);
 } // namespace detail
 } // namespace transformer
 } // namespace clang
 
 #endif // LLVM_CLANG_TOOLING_TRANSFORMER_REWRITERULE_H

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

 #include "clang/Tooling/Refactoring/AtomicChange.h"
 #include "clang/Tooling/Transformer/RewriteRule.h"
 #include "llvm/Support/Error.h"
 #include <functional>
 #include <utility>
 
 namespace clang {
 namespace tooling {
+
+namespace detail {
+/// Implementation details of \c Transformer with type erasure around
+/// \c RewriteRule and \c RewriteRule<T> as well as the corresponding consumers.
+class TransformerImpl {
+public:
+  virtual ~TransformerImpl() = default;
+
+  void onMatch(const ast_matchers::MatchFinder::MatchResult &Result);
+
+  virtual std::vector<ast_matchers::internal::DynTypedMatcher>
+  buildMatchers() const = 0;
+
+protected:
+  /// Converts a set of \c Edit into a \c AtomicChange per file modified.
+  /// Returns an error if the edits fail to compose, e.g. overlapping edits.
+  static llvm::Expected<llvm::SmallVector<AtomicChange, 1>>
+  convertToAtomicChanges(const llvm::SmallVectorImpl<transformer::Edit> &Edits,
+                         const ast_matchers::MatchFinder::MatchResult &Result);
+
+private:
+  virtual void
+  onMatchImpl(const ast_matchers::MatchFinder::MatchResult &Result) = 0;
+};
+
+/// Implementation for when no metadata is generated as a part of the
+/// \c RewriteRule.
+class NoMetadataImpl final : public TransformerImpl {
+  transformer::RewriteRule Rule;
+  std::function<void(Expected<llvm::MutableArrayRef<AtomicChange>>)> Consumer;
+
+public:
+  explicit NoMetadataImpl(
+      transformer::RewriteRule R,
+      std::function<void(Expected<llvm::MutableArrayRef<AtomicChange>>)>
+          Consumer)
+      : Rule(std::move(R)), Consumer(std::move(Consumer)) {
+    assert(llvm::all_of(Rule.Cases,
+                        [](const transformer::RewriteRule::Case &Case) {
+                          return Case.Edits;
+                        }) &&
+           "edit generator must be provided for each rule");
+  }
+
+private:
+  void onMatchImpl(const ast_matchers::MatchFinder::MatchResult &Result) final;
+  std::vector<ast_matchers::internal::DynTypedMatcher>
+  buildMatchers() const final {
+    return transformer::detail::buildMatchers(Rule);
+  }
+};
+} // namespace detail
+
 /// Handles the matcher and callback registration for a single `RewriteRule`, as
 /// defined by the arguments of the constructor.
 class Transformer : public ast_matchers::MatchFinder::MatchCallback {
 public:
   /// Provides the set of changes to the consumer.  The callback is free to move
   /// or destructively consume the changes as needed.
   ///
   /// We use \c MutableArrayRef as an abstraction to provide decoupling, and we
   /// expect the majority of consumers to copy or move the individual values
   /// into a separate data structure.
   using ChangeSetConsumer = std::function<void(
       Expected<llvm::MutableArrayRef<AtomicChange>> Changes)>;
 
-  /// \param Consumer Receives all rewrites for a single match, or an error.
+  template <typename T> struct Result {
+    llvm::MutableArrayRef<AtomicChange> Changes;
+    T Metadata;
+  };
+
+  // Specialization provided only to simplify SFINAE on the constructor; not
+  // intended for use.
+  template <> struct Result<void> {
+    llvm::MutableArrayRef<AtomicChange> Changes;
+  };
+
+  /// \param Consumer receives all rewrites for a single match, or an error.
   /// Will not necessarily be called for each match; for example, if the rule
   /// generates no edits but does not fail.  Note that clients are responsible
   /// for handling the case that independent \c AtomicChanges conflict with each
   /// other.
-  explicit Transformer(transformer::RewriteRule Rule,
-                       ChangeSetConsumer Consumer)
-      : Rule(std::move(Rule)), Consumer(std::move(Consumer)) {
-    assert(this->Consumer && "Consumer is empty");
-  }
+  template <typename ConsumerFn,
+            std::enable_if_t<
+                llvm::is_invocable<
+                    ConsumerFn,
+                    llvm::Expected<llvm::MutableArrayRef<AtomicChange>>>::value,
+                int> = 0>
+  explicit Transformer(transformer::RewriteRuleWith<void> Rule,
+                       ConsumerFn Consumer)
+      : Impl(std::make_unique<detail::NoMetadataImpl>(std::move(Rule),
+                                                      std::move(Consumer))) {}
+
+  /// \param Consumer receives all rewrites and the associated metadata for a
+  /// single match, or an error. Will always be called for each match, even if
+  /// the rule generates no edits.  Note that clients are responsible for
+  /// handling the case that independent \c AtomicChanges conflict with each
+  /// other.
+  template <
+      typename T, typename ConsumerFn,
+      std::enable_if_t<
+          llvm::conjunction<
+              llvm::negation<std::is_same<T, void>>,
+              llvm::is_invocable<ConsumerFn, llvm::Expected<Result<T>>>>::value,
+          int> = 0>
+  explicit Transformer(transformer::RewriteRuleWith<T> Rule,
+                       ConsumerFn Consumer);
 
   /// N.B. Passes `this` pointer to `MatchFinder`.  So, this object should not
   /// be moved after this call.
   void registerMatchers(ast_matchers::MatchFinder *MatchFinder);
 
   /// Not called directly by users -- called by the framework, via base class
   /// pointer.
   void run(const ast_matchers::MatchFinder::MatchResult &Result) override;
 
 private:
-  transformer::RewriteRule Rule;
-  /// Receives sets of successful rewrites as an
-  /// \c llvm::ArrayRef<AtomicChange>.
-  ChangeSetConsumer Consumer;
+  std::unique_ptr<detail::TransformerImpl> Impl;
+};
+
+namespace detail {
+/// Implementation when metadata is generated as a part of the rewrite. This
+/// happens when we have a \c RewriteRuleWith<T>.
+template <typename T> class WithMetadataImpl final : public TransformerImpl {
+  transformer::RewriteRuleWith<T> Rule;
+  std::function<void(llvm::Expected<Transformer::Result<T>>)> Consumer;
+
+public:
+  explicit WithMetadataImpl(
+      transformer::RewriteRuleWith<T> R,
+      std::function<void(llvm::Expected<Transformer::Result<T>>)> Consumer)
+      : Rule(std::move(R)), Consumer(std::move(Consumer)) {
+    assert(llvm::all_of(Rule.Cases,
+                        [](const transformer::RewriteRuleBase::Case &Case)
+                            -> bool { return !!Case.Edits; }) &&
+           "edit generator must be provided for each rule");
+    assert(llvm::all_of(Rule.Metadata,
+                        [](const typename transformer::Generator<T> &Metadata)
+                            -> bool { return !!Metadata; }) &&
+           "metadata generator must be provided for each rule");
+  }
+
+private:
+  void onMatchImpl(const ast_matchers::MatchFinder::MatchResult &Result) final {
+    size_t I = transformer::detail::findSelectedCase(Result, Rule);
+    auto Transformations = Rule.Cases[I].Edits(Result);
+    if (!Transformations) {
+      Consumer(Transformations.takeError());
+      return;
+    }
+
+    llvm::SmallVector<AtomicChange, 1> Changes;
+    if (!Transformations->empty()) {
+      auto C = convertToAtomicChanges(*Transformations, Result);
+      if (C) {
+        Changes = std::move(*C);
+      } else {
+        Consumer(C.takeError());
+        return;
+      }
+    }
+
+    auto Metadata = Rule.Metadata[I]->eval(Result);
+    if (!Metadata) {
+      Consumer(Metadata.takeError());
+      return;
+    }
+
+    Consumer(Transformer::Result<T>{
+        llvm::MutableArrayRef<AtomicChange>(Changes), std::move(*Metadata)});
+  }
+
+  std::vector<ast_matchers::internal::DynTypedMatcher>
+  buildMatchers() const final {
+    return transformer::detail::buildMatchers(Rule);
+  }
 };
+} // namespace detail
+
+template <
+    typename T, typename ConsumerFn,
+    std::enable_if_t<
+        llvm::conjunction<
+            llvm::negation<std::is_same<T, void>>,
+            llvm::is_invocable<ConsumerFn,
+                               llvm::Expected<Transformer::Result<T>>>>::value,
+        int>>
+Transformer::Transformer(transformer::RewriteRuleWith<T> Rule,
+                         ConsumerFn Consumer)
+    : Impl(std::make_unique<detail::WithMetadataImpl<T>>(std::move(Rule),
+                                                         std::move(Consumer))) {
+}
+
 } // namespace tooling
 } // namespace clang
 
 #endif // LLVM_CLANG_TOOLING_TRANSFORMER_TRANSFORMER_H_

clang/lib/Tooling/Transformer/RewriteRule.cpp

ASTEdit transformer::addInclude(RangeSelector Target, StringRef Header,
                                 IncludeFormat Format) {
   ASTEdit E;
   E.Kind = EditKind::AddInclude;
   E.TargetRange = Target;
   E.Replacement = makeText(formatHeaderPath(Header, Format));
   return E;
 }
 
-RewriteRule transformer::makeRule(DynTypedMatcher M, EditGenerator Edits,
-                                  TextGenerator Explanation) {
-  return RewriteRule{{RewriteRule::Case{std::move(M), std::move(Edits),
-                                        std::move(Explanation)}}};
+EditGenerator
+transformer::detail::makeEditGenerator(llvm::SmallVector<ASTEdit, 1> Edits) {
+  return editList(std::move(Edits));
+}
+
+EditGenerator transformer::detail::makeEditGenerator(ASTEdit Edit) {
+  return edit(std::move(Edit));
+}
+
+RewriteRule transformer::detail::makeRule(DynTypedMatcher M,
+                                          EditGenerator Edits) {
+  RewriteRule R;
+  R.Cases = {{std::move(M), std::move(Edits)}};
+  return R;
+}
+
+RewriteRule transformer::makeRule(ast_matchers::internal::DynTypedMatcher M,
+                                  std::initializer_list<ASTEdit> Edits) {
+  return detail::makeRule(std::move(M),
+                          detail::makeEditGenerator(std::move(Edits)));
 }
 
 namespace {
 
 /// Unconditionally binds the given node set before trying `InnerMatcher` and
 /// keeps the bound nodes on a successful match.
 template <typename T>
 class BindingsMatcher : public ast_matchers::internal::MatcherInterface<T> {
void registerMatchers(const ast_matchers::BoundNodes &Nodes,
                         MatchFinder *MF) {
     for (auto &Matcher : transformer::detail::buildMatchers(Rule))
       MF->addMatcher(forEachDescendantDynamically<T>(Nodes, Matcher), this);
   }
 
   void run(const MatchFinder::MatchResult &Result) override {
     if (!Edits)
       return;
-    transformer::RewriteRule::Case Case =
-        transformer::detail::findSelectedCase(Result, Rule);
-    auto Transformations = Case.Edits(Result);
+    size_t I = transformer::detail::findSelectedCase(Result, Rule);
+    auto Transformations = Rule.Cases[I].Edits(Result);
     if (!Transformations) {
       Edits = Transformations.takeError();
       return;
     }
     Edits->append(Transformations->begin(), Transformations->end());
   }
 
   RewriteRule Rule;
return [NodeId = std::move(NodeId),
     auto It = NodesMap.find(NodeId);
     if (It == NodesMap.end())
       return llvm::make_error<llvm::StringError>(llvm::errc::invalid_argument,
                                                  "ID not bound: " + NodeId);
     return detail::rewriteDescendants(It->second, std::move(Rule), Result);
   };
 }
 
-void transformer::addInclude(RewriteRule &Rule, StringRef Header,
+void transformer::addInclude(RewriteRuleBase &Rule, StringRef Header,
                              IncludeFormat Format) {
   for (auto &Case : Rule.Cases)
     Case.Edits = flatten(std::move(Case.Edits), addInclude(Header, Format));
 }
 
 #ifndef NDEBUG
 // Filters for supported matcher kinds. FIXME: Explicitly list the allowed kinds
 // (all node matcher types except for `QualType` and `Type`), rather than just
Matchers.push_back(!M.getTraversalKind()
                            : std::move(M));
   }
   return Matchers;
 }
 
 // Simply gathers the contents of the various rules into a single rule. The
 // actual work to combine these into an ordered choice is deferred to matcher
 // registration.
-RewriteRule transformer::applyFirst(ArrayRef<RewriteRule> Rules) {
+template <>
+RewriteRuleWith<void>
+transformer::applyFirst(ArrayRef<RewriteRuleWith<void>> Rules) {
   RewriteRule R;
   for (auto &Rule : Rules)
     R.Cases.append(Rule.Cases.begin(), Rule.Cases.end());
   return R;
 }
 
 std::vector<DynTypedMatcher>
-transformer::detail::buildMatchers(const RewriteRule &Rule) {
+transformer::detail::buildMatchers(const RewriteRuleBase &Rule) {
   // Map the cases into buckets of matchers -- one for each "root" AST kind,
   // which guarantees that they can be combined in a single anyOf matcher. Each
   // case is paired with an identifying number that is converted to a string id
   // in `taggedMatchers`.
-  std::map<ASTNodeKind, SmallVector<std::pair<size_t, RewriteRule::Case>, 1>>
+  std::map<ASTNodeKind,
+           SmallVector<std::pair<size_t, RewriteRuleBase::Case>, 1>>
       Buckets;
   const SmallVectorImpl<RewriteRule::Case> &Cases = Rule.Cases;
   for (int I = 0, N = Cases.size(); I < N; ++I) {
     assert(hasValidKind(Cases[I].Matcher) &&
            "Matcher must be non-(Qual)Type node matcher");
     Buckets[Cases[I].Matcher.getSupportedKind()].emplace_back(I, Cases[I]);
   }
 
DynTypedMatcher M = DynTypedMatcher::constructVariadic(
         taggedMatchers("Tag", Bucket.second, TK_AsIs));
     M.setAllowBind(true);
     // `tryBind` is guaranteed to succeed, because `AllowBind` was set to true.
     Matchers.push_back(M.tryBind(RootID)->withTraversalKind(TK_AsIs));
   }
   return Matchers;
 }
 
-DynTypedMatcher transformer::detail::buildMatcher(const RewriteRule &Rule) {
+DynTypedMatcher transformer::detail::buildMatcher(const RewriteRuleBase &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(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.
   return Result.SourceManager->getExpansionLoc(
       Root->second.getSourceRange().getBegin());
 }
 
 // Finds the case that was "selected" -- that is, whose matcher triggered the
 // `MatchResult`.
-const RewriteRule::Case &
-transformer::detail::findSelectedCase(const MatchResult &Result,
-                                  const RewriteRule &Rule) {
+size_t transformer::detail::findSelectedCase(const MatchResult &Result,
+                                             const RewriteRuleBase &Rule) {
   if (Rule.Cases.size() == 1)
-    return Rule.Cases[0];
+    return 0;
 
   auto &NodesMap = Result.Nodes.getMap();
   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];
+      return i;
   }
   llvm_unreachable("No tag found for this rule.");
 }
-
-const llvm::StringRef RewriteRule::RootID = ::clang::transformer::RootID;

clang/lib/Tooling/Transformer/Transformer.cpp

 #include "clang/ASTMatchers/ASTMatchersInternal.h"
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Tooling/Refactoring/AtomicChange.h"
 #include "llvm/Support/Error.h"
 #include <map>
 #include <utility>
 #include <vector>
 
-using namespace clang;
-using namespace tooling;
+namespace clang {
+namespace tooling {
 
-using ast_matchers::MatchFinder;
+using ::clang::ast_matchers::MatchFinder;
 
-void Transformer::registerMatchers(MatchFinder *MatchFinder) {
-  for (auto &Matcher : transformer::detail::buildMatchers(Rule))
-    MatchFinder->addDynamicMatcher(Matcher, this);
-}
+namespace detail {
 
-void Transformer::run(const MatchFinder::MatchResult &Result) {
+void TransformerImpl::onMatch(
+    const ast_matchers::MatchFinder::MatchResult &Result) {
   if (Result.Context->getDiagnostics().hasErrorOccurred())
     return;
 
-  transformer::RewriteRule::Case Case =
-      transformer::detail::findSelectedCase(Result, Rule);
-  auto Transformations = Case.Edits(Result);
-  if (!Transformations) {
-    Consumer(Transformations.takeError());
-    return;
-  }
+  onMatchImpl(Result);
 
-  if (Transformations->empty())
-    return;
-
+llvm::Expected<llvm::SmallVector<AtomicChange, 1>>
+TransformerImpl::convertToAtomicChanges(
+    const llvm::SmallVectorImpl<transformer::Edit> &Edits,
+    const MatchFinder::MatchResult &Result) {
   // Group the transformations, by file, into AtomicChanges, each anchored by
   // the location of the first change in that file.
   std::map<FileID, AtomicChange> ChangesByFileID;
-  for (const auto &T : *Transformations) {
+  for (const auto &T : Edits) {
     auto ID = Result.SourceManager->getFileID(T.Range.getBegin());
     auto Iter = ChangesByFileID
                     .emplace(ID, AtomicChange(*Result.SourceManager,
                                               T.Range.getBegin(), T.Metadata))
                     .first;
     auto &AC = Iter->second;
     switch (T.Kind) {
     case transformer::EditKind::Range:
       if (auto Err =
               AC.replace(*Result.SourceManager, T.Range, T.Replacement)) {
-        Consumer(std::move(Err));
-        return;
+        return std::move(Err);
       }
       break;
     case transformer::EditKind::AddInclude:
       AC.addHeader(T.Replacement);
       break;
     }
   }
 
   llvm::SmallVector<AtomicChange, 1> Changes;
   Changes.reserve(ChangesByFileID.size());
   for (auto &IDChangePair : ChangesByFileID)
     Changes.push_back(std::move(IDChangePair.second));
-  Consumer(llvm::MutableArrayRef<AtomicChange>(Changes));
+
+  return Changes;
 }
+
+void NoMetadataImpl::onMatchImpl(const MatchFinder::MatchResult &Result) {
+  size_t I = transformer::detail::findSelectedCase(Result, Rule);
+  auto Transformations = Rule.Cases[I].Edits(Result);
+  if (!Transformations) {
+    Consumer(Transformations.takeError());
+    return;
+  }
+
+  if (Transformations->empty())
+    return;
+
+  auto Changes = convertToAtomicChanges(*Transformations, Result);
+  if (!Changes) {
+    Consumer(Changes.takeError());
+    return;
+  }
+
+  Consumer(llvm::MutableArrayRef<AtomicChange>(*Changes));
+}
+
+} // namespace detail
+
+void Transformer::registerMatchers(MatchFinder *MatchFinder) {
+  for (auto &Matcher : Impl->buildMatchers())
+    MatchFinder->addDynamicMatcher(Matcher, this);
+}
+
+void Transformer::run(const MatchFinder::MatchResult &Result) {
+  if (Result.Context->getDiagnostics().hasErrorOccurred())
+    return;
+
+  Impl->onMatch(Result);
+}
+
+} // namespace tooling
+} // namespace clang

clang/unittests/Tooling/TransformerTest.cpp

 using ::clang::transformer::before;
 using ::clang::transformer::cat;
 using ::clang::transformer::changeTo;
 using ::clang::transformer::editList;
 using ::clang::transformer::makeRule;
 using ::clang::transformer::member;
 using ::clang::transformer::name;
 using ::clang::transformer::node;
+using ::clang::transformer::noEdits;
 using ::clang::transformer::remove;
 using ::clang::transformer::rewriteDescendants;
 using ::clang::transformer::RewriteRule;
+using ::clang::transformer::RewriteRuleWith;
 using ::clang::transformer::statement;
 using ::testing::ElementsAre;
 using ::testing::IsEmpty;
 using ::testing::ResultOf;
 using ::testing::UnorderedElementsAre;
 
 constexpr char KHeaderContents[] = R"cc(
   struct string {
protected:
   }
 
   Transformer::ChangeSetConsumer consumer() {
     return [this](Expected<MutableArrayRef<AtomicChange>> C) {
       if (C) {
         Changes.insert(Changes.end(), std::make_move_iterator(C->begin()),
                        std::make_move_iterator(C->end()));
       } else {
-        // FIXME: stash this error rather then printing.
+        // FIXME: stash this error rather than printing.
         llvm::errs() << "Error generating changes: "
                      << llvm::toString(C.takeError()) << "\n";
         ++ErrorCount;
       }
     };
   }
 
-  template <typename R>
-  void testRule(R Rule, StringRef Input, StringRef Expected) {
+  auto consumerWithStringMetadata() {
+    return [this](Expected<Transformer::Result<std::string>> C) {
+      if (C) {
+        Changes.insert(Changes.end(),
+                       std::make_move_iterator(C->Changes.begin()),
+                       std::make_move_iterator(C->Changes.end()));
+        StringMetadata.push_back(std::move(C->Metadata));
+      } else {
+        // FIXME: stash this error rather than printing.
+        llvm::errs() << "Error generating changes: "
+                     << llvm::toString(C.takeError()) << "\n";
+        ++ErrorCount;
+      }
+    };
+  }
+
+  void testRule(RewriteRule Rule, StringRef Input, StringRef Expected) {
     Transformers.push_back(
         std::make_unique<Transformer>(std::move(Rule), consumer()));
     Transformers.back()->registerMatchers(&MatchFinder);
     compareSnippets(Expected, rewrite(Input));
   }
 
-  template <typename R> void testRuleFailure(R Rule, StringRef Input) {
+  void testRule(RewriteRuleWith<std::string> Rule, StringRef Input,
+                StringRef Expected) {
+    Transformers.push_back(std::make_unique<Transformer>(
+        std::move(Rule), consumerWithStringMetadata()));
+    Transformers.back()->registerMatchers(&MatchFinder);
+    compareSnippets(Expected, rewrite(Input));
+  }
+
+  void testRuleFailure(RewriteRule Rule, StringRef Input) {
     Transformers.push_back(
         std::make_unique<Transformer>(std::move(Rule), consumer()));
     Transformers.back()->registerMatchers(&MatchFinder);
     ASSERT_FALSE(rewrite(Input)) << "Expected failure to rewrite code";
   }
 
+  void testRuleFailure(RewriteRuleWith<std::string> Rule, StringRef Input) {
+    Transformers.push_back(std::make_unique<Transformer>(
+        std::move(Rule), consumerWithStringMetadata()));
+    Transformers.back()->registerMatchers(&MatchFinder);
+    ASSERT_FALSE(rewrite(Input)) << "Expected failure to rewrite code";
+  }
+
   // Transformers are referenced by MatchFinder.
   std::vector<std::unique_ptr<Transformer>> Transformers;
   clang::ast_matchers::MatchFinder MatchFinder;
   // Records whether any errors occurred in individual changes.
   int ErrorCount = 0;
   AtomicChanges Changes;
+  std::vector<std::string> StringMetadata;
 
 private:
   FileContentMappings FileContents = {{"header.h", ""}};
 };
 
 class TransformerTest : public ClangRefactoringTestBase {
 protected:
   TransformerTest() { appendToHeader(KHeaderContents); }
 };
 
 // Given string s, change strlen($s.c_str()) to REPLACED.
-static RewriteRule ruleStrlenSize() {
+static RewriteRuleWith<std::string> ruleStrlenSize() {
   StringRef StringExpr = "strexpr";
   auto StringType = namedDecl(hasAnyName("::basic_string", "::string"));
   auto R = makeRule(
       callExpr(callee(functionDecl(hasName("strlen"))),
                hasArgument(0, cxxMemberCallExpr(
                                   on(expr(hasType(isOrPointsTo(StringType)))
                                          .bind(StringExpr)),
                                   callee(cxxMethodDecl(hasName("c_str")))))),
testRule(makeRule(ifStmt(hasCondition(expr().bind(C)),
                                           cat("{ /* then */ }"))),
                             editList({changeTo(statement(std::string(E)),
                                                cat("{ /* else */ }"))}))),
            Input, Expected);
 }
 
 TEST_F(TransformerTest, OrderedRuleUnrelated) {
   StringRef Flag = "flag";
-  RewriteRule FlagRule = makeRule(
+  RewriteRuleWith<std::string> FlagRule = makeRule(
       cxxMemberCallExpr(on(expr(hasType(cxxRecordDecl(
                                     hasName("proto::ProtoCommandLineFlag"))))
                                .bind(Flag)),
                         unless(callee(cxxMethodDecl(hasName("GetProto"))))),
-      changeTo(node(std::string(Flag)), cat("PROTO")));
+      changeTo(node(std::string(Flag)), cat("PROTO")), cat(""));
 
   std::string Input = R"cc(
     proto::ProtoCommandLineFlag flag;
     int x = flag.foo();
     int y = flag.GetProto().foo();
     int f(string s) { return strlen(s.c_str()); }
   )cc";
   std::string Expected = R"cc(
std::string Source = R"cc(#include "input.h"
                             })cc";
   int ErrorCount = 0;
   std::vector<AtomicChanges> ChangeSets;
   clang::ast_matchers::MatchFinder MatchFinder;
   Transformer T(
       makeRule(callExpr(callee(functionDecl(hasName("Func"))),
                         forEachArgumentWithParam(expr().bind("arg"),
                                                  parmVarDecl().bind("param"))),
-               editList({changeTo(node("arg"), cat("ARG")),
-                         changeTo(node("param"), cat("PARAM"))})),
+               {changeTo(node("arg"), cat("ARG")),
+                changeTo(node("param"), cat("PARAM"))}),
       [&](Expected<MutableArrayRef<AtomicChange>> Changes) {
         if (Changes)
           ChangeSets.push_back(AtomicChanges(Changes->begin(), Changes->end()));
         else
           ++ErrorCount;
       });
   T.registerMatchers(&MatchFinder);
   auto Factory = newFrontendActionFactory(&MatchFinder);
   EXPECT_TRUE(runToolOnCodeWithArgs(
       Factory->create(), Source, std::vector<std::string>(), "input.cc",
       "clang-tool", std::make_shared<PCHContainerOperations>(),
       {{"input.h", Header}}));
 
   EXPECT_EQ(ErrorCount, 0);
   EXPECT_THAT(
       ChangeSets,
       UnorderedElementsAre(UnorderedElementsAre(
           ResultOf([](const AtomicChange &C) { return C.getFilePath(); },
                    "input.cc"),
           ResultOf([](const AtomicChange &C) { return C.getFilePath(); },
                    "./input.h"))));
 }
 
+TEST_F(TransformerTest, GeneratesMetadata) {
+  std::string Input = R"cc(int target = 0;)cc";
+  std::string Expected = R"cc(REPLACE)cc";
+  RewriteRuleWith<std::string> Rule = makeRule(
+      varDecl(hasName("target")), changeTo(cat("REPLACE")), cat("METADATA"));
+  testRule(std::move(Rule), Input, Expected);
+  EXPECT_EQ(ErrorCount, 0);
+  EXPECT_THAT(StringMetadata, UnorderedElementsAre("METADATA"));
+}
+
+TEST_F(TransformerTest, GeneratesMetadataWithNoEdits) {
+  std::string Input = R"cc(int target = 0;)cc";
+  RewriteRuleWith<std::string> Rule = makeRule(
+      varDecl(hasName("target")).bind("var"), noEdits(), cat("METADATA"));
+  testRule(std::move(Rule), Input, Input);
+  EXPECT_EQ(ErrorCount, 0);
+  EXPECT_THAT(StringMetadata, UnorderedElementsAre("METADATA"));
+}
+
+TEST_F(TransformerTest, PropagateMetadataErrors) {
+  class AlwaysFail : public transformer::MatchComputation<std::string> {
+    llvm::Error eval(const ast_matchers::MatchFinder::MatchResult &,
+                     std::string *) const override {
+      return llvm::createStringError(llvm::errc::invalid_argument, "ERROR");
+    }
+    std::string toString() const override { return "AlwaysFail"; }
+  };
+  std::string Input = R"cc(int target = 0;)cc";
+  RewriteRuleWith<std::string> Rule = makeRule<std::string>(
+      varDecl(hasName("target")).bind("var"), changeTo(cat("REPLACE")),
+      std::make_shared<AlwaysFail>());
+  testRuleFailure(std::move(Rule), Input);
+  EXPECT_EQ(ErrorCount, 1);
+}
+
 } // namespace