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

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.

Differential Revision: https://reviews.llvm.org/D91544

diff --git a/clang-tools-extra/clang-tidy/utils/TransformerClangTidyCheck.cpp b/clang-tools-extra/clang-tidy/utils/TransformerClangTidyCheck.cpp
index ae2ae86..e93f3fb 100644 (file)
--- a/clang-tools-extra/clang-tidy/utils/TransformerClangTidyCheck.cpp
+++ b/clang-tools-extra/clang-tidy/utils/TransformerClangTidyCheck.cpp
@@ -21,6 +21,18 @@ static bool hasExplanation(const RewriteRule::Case &C) {
 }
 #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,
@@ -31,24 +43,21 @@ TransformerClangTidyCheck::TransformerClangTidyCheck(
                                         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(
@@ -58,8 +67,8 @@ void TransformerClangTidyCheck::registerPPCallbacks(
 
 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);
 }
 
@@ -68,8 +77,7 @@ void TransformerClangTidyCheck::check(
   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())

diff --git a/clang-tools-extra/clang-tidy/utils/TransformerClangTidyCheck.h b/clang-tools-extra/clang-tidy/utils/TransformerClangTidyCheck.h
index 404f474..9736e64 100644 (file)
--- a/clang-tools-extra/clang-tidy/utils/TransformerClangTidyCheck.h
+++ b/clang-tools-extra/clang-tidy/utils/TransformerClangTidyCheck.h
@@ -19,7 +19,7 @@ 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:
@@ -38,24 +38,23 @@ namespace utils {
 //      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);
 
@@ -68,8 +67,17 @@ public:
   /// 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;
 };