[PATCH] D63288: [clang-tidy] Generalize TransformerClangTidyCheck to take a rule generator.

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel created this revision.
ymandel added a reviewer: gribozavr.
Herald added a subscriber: xazax.hun.
Herald added a project: clang.
ymandel added a parent revision: D63287: [clang-tidy] Make ClangTidyCheck::OptionsView public..

Tidy check behavior often depends on language and/or clang-tidy options. This revision allows a user of TranformerClangTidyCheck to pass rule _generator_ in place of a rule, where the generator takes both the language and clang-tidy options. Additionally, the generator returns an `Optional` to allow for the case where the check is deemed irrelevant/disable based on those options.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D63288

Files:
  clang-tools-extra/clang-tidy/utils/TransformerClangTidyCheck.cpp
  clang-tools-extra/clang-tidy/utils/TransformerClangTidyCheck.h


Index: clang-tools-extra/clang-tidy/utils/TransformerClangTidyCheck.h
===================================================================
--- clang-tools-extra/clang-tidy/utils/TransformerClangTidyCheck.h
+++ clang-tools-extra/clang-tidy/utils/TransformerClangTidyCheck.h
@@ -31,19 +31,32 @@
 // };
 class TransformerClangTidyCheck : public ClangTidyCheck {
 public:
-  // All cases in \p R 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).
+  // \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(std::function<Optional<tooling::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.
   TransformerClangTidyCheck(tooling::RewriteRule R, StringRef Name,
                             ClangTidyContext *Context);
+
   void registerMatchers(ast_matchers::MatchFinder *Finder) final;
   void check(const ast_matchers::MatchFinder::MatchResult &Result) final;
 
 private:
-  tooling::RewriteRule Rule;
+  Optional<tooling::RewriteRule> Rule;
 };
 
 } // namespace utils
Index: clang-tools-extra/clang-tidy/utils/TransformerClangTidyCheck.cpp
===================================================================
--- clang-tools-extra/clang-tidy/utils/TransformerClangTidyCheck.cpp
+++ clang-tools-extra/clang-tidy/utils/TransformerClangTidyCheck.cpp
@@ -25,9 +25,23 @@
          " explicitly provide an empty explanation if none is desired");
 }
 
+TransformerClangTidyCheck::TransformerClangTidyCheck(
+    std::function<Optional<RewriteRule>(const LangOptions &,
+                                        const OptionsView &)>
+        MakeRule,
+    StringRef Name, ClangTidyContext *Context)
+    : ClangTidyCheck(Name, Context), Rule(MakeRule(getLangOpts(), Options)) {
+  assert(llvm::all_of(Rule.Cases, [](const RewriteRule::Case &C) {
+                       return C.Explanation != nullptr;
+                     }) &&
+         "clang-tidy checks must have an explanation by default;"
+         " explicitly provide an empty explanation if none is desired");
+}
+
 void TransformerClangTidyCheck::registerMatchers(
     ast_matchers::MatchFinder *Finder) {
-  Finder->addDynamicMatcher(tooling::detail::buildMatcher(Rule), this);
+  if (Rule)
+    Finder->addDynamicMatcher(tooling::detail::buildMatcher(*Rule), this);
 }
 
 void TransformerClangTidyCheck::check(
@@ -43,7 +57,8 @@
       Root->second.getSourceRange().getBegin());
   assert(RootLoc.isValid() && "Invalid location for Root node of match.");
 
-  RewriteRule::Case Case = tooling::detail::findSelectedCase(Result, Rule);
+  assert(Rule && "check() should not fire if Rule is None");
+  RewriteRule::Case Case = tooling::detail::findSelectedCase(Result, *Rule);
   Expected<SmallVector<tooling::detail::Transformation, 1>> Transformations =
       tooling::detail::translateEdits(Result, Case.Edits);
   if (!Transformations) {


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D63288.204581.patch
Type: text/x-patch
Size: 4112 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20190613/1f29213f/attachment-0001.bin>