diff --git a/clang-tools-extra/clang-tidy/utils/TransformerClangTidyCheck.h b/clang-tools-extra/clang-tidy/utils/TransformerClangTidyCheck.h --- a/clang-tools-extra/clang-tidy/utils/TransformerClangTidyCheck.h +++ b/clang-tools-extra/clang-tidy/utils/TransformerClangTidyCheck.h @@ -19,7 +19,7 @@ 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 @@ // 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( 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 @@ /// 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 Rule; + transformer::RewriteRule Rule; IncludeInserter Inserter; }; diff --git a/clang-tools-extra/clang-tidy/utils/TransformerClangTidyCheck.cpp b/clang-tools-extra/clang-tidy/utils/TransformerClangTidyCheck.cpp --- a/clang-tools-extra/clang-tidy/utils/TransformerClangTidyCheck.cpp +++ b/clang-tools-extra/clang-tidy/utils/TransformerClangTidyCheck.cpp @@ -21,6 +21,18 @@ } #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 @@ 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 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::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 @@ 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> Edits = Case.Edits(Result); if (!Edits) { llvm::errs() << "Rewrite failed: " << llvm::toString(Edits.takeError())