[clang-tidy] Allow `TransformerClangTidyCheck` clients to set the rule directly.

[ymandel]

Summary

Adds support for setting the Rule field. In the process, refactors the code that accesses that field and adds a constructor that doesn't require a rule argument.

This feature is needed by checks that must set the rule *after* the check class
is constructed. For example, any check that maintains state to be accessed from
the rule needs this support. Since the object's fields are not initialized when
the superclass constructor is called, they can't be (safely) captured by a rule
passed to the existing constructor. This patch allows constructing the check
superclass fully before setting the rule.

As a driveby fix, removed the "optional" from the rule, since rules are just a
set of cases, so empty rules are evident.

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

 #include "IncludeSorter.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
 #include "clang/Tooling/Transformer/Transformer.h"
 
 namespace clang {
 namespace tidy {
 namespace utils {
 
-// A base class for defining a ClangTidy check based on a `RewriteRule`.
+/// A base class for defining a ClangTidy check based on a `RewriteRule`.
 //
 // For example, given a rule `MyCheckAsRewriteRule`, one can define a tidy check
 // as follows:
 //
 // class MyCheck : public TransformerClangTidyCheck {
 //  public:
 //   MyCheck(StringRef Name, ClangTidyContext *Context)
 //       : TransformerClangTidyCheck(MyCheckAsRewriteRule, Name, Context) {}
 // };
 //
 // `TransformerClangTidyCheck` recognizes this clang-tidy option:
 //
 //  * IncludeStyle. A string specifying which file naming convention is used by
 //      the source code, 'llvm' or 'google'.  Default is 'llvm'. The naming
 //      convention influences how canonical headers are distinguished from other
 //      includes.
 class TransformerClangTidyCheck : public ClangTidyCheck {
 public:
-  // \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.
-  //
-  // All cases in the rule generated by \p MakeRule 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).
+  TransformerClangTidyCheck(StringRef Name, ClangTidyContext *Context);
+
+  /// 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);
 
-  // Convenience overload of the constructor when the rule doesn't depend on any
-  // of the language or clang-tidy options.
+  /// Convenience overload of the constructor when the rule doesn't have any
+  /// dependies.
   TransformerClangTidyCheck(transformer::RewriteRule 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);
+
 private:
-  Optional<transformer::RewriteRule> Rule;
+  transformer::RewriteRule 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

 using transformer::RewriteRule;
 
 #ifndef NDEBUG
 static bool hasExplanation(const RewriteRule::Case &C) {
   return C.Explanation != nullptr;
 }
 #endif
 
+static void verifyRule(const RewriteRule &Rule) {
+  assert(llvm::all_of(Rule.Cases, hasExplanation) &&
+         "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 &,
                                         const OptionsView &)>
         MakeRule,
     StringRef Name, ClangTidyContext *Context)
-    : ClangTidyCheck(Name, Context), Rule(MakeRule(getLangOpts(), Options)),
-      Inserter(
-          Options.getLocalOrGlobal("IncludeStyle", IncludeSorter::IS_LLVM)) {
-  if (Rule)
-    assert(llvm::all_of(Rule->Cases, hasExplanation) &&
-           "clang-tidy checks must have an explanation by default;"
-           " explicitly provide an empty explanation if none is desired");
+    : TransformerClangTidyCheck(Name, Context) {
+  if (Optional<RewriteRule> R = MakeRule(getLangOpts(), Options))
+    setRule(std::move(*R));
 }
 
 TransformerClangTidyCheck::TransformerClangTidyCheck(RewriteRule R,
                                                      StringRef Name,
                                                      ClangTidyContext *Context)
-    : ClangTidyCheck(Name, Context), Rule(std::move(R)),
-      Inserter(
-          Options.getLocalOrGlobal("IncludeStyle", IncludeSorter::IS_LLVM)) {
-  assert(llvm::all_of(Rule->Cases, hasExplanation) &&
-         "clang-tidy checks must have an explanation by default;"
-         " explicitly provide an empty explanation if none is desired");
+    : TransformerClangTidyCheck(Name, Context) {
+  setRule(std::move(R));
+}
+
+void TransformerClangTidyCheck::setRule(transformer::RewriteRule 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)
-    for (auto &Matcher : transformer::detail::buildMatchers(*Rule))
+  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;
 
-  assert(Rule && "check() should not fire if Rule is None");
-  RewriteRule::Case Case = transformer::detail::findSelectedCase(Result, *Rule);
+  RewriteRule::Case Case = transformer::detail::findSelectedCase(Result, Rule);
   Expected<SmallVector<transformer::Edit, 1>> Edits = Case.Edits(Result);
   if (!Edits) {
     llvm::errs() << "Rewrite failed: " << llvm::toString(Edits.takeError())
                  << "\n";
     return;
   }
 
   // No rewrite applied, but no error encountered either.