This revision was automatically updated to reflect the committed changes.
Closed by commit rG068da2c749a5: [clang-tidy] Allow `TransformerClangTidyCheck`
clients to set the rule directly. (authored by ymandel).
Changed prior to commit:
https://reviews.llvm.org/D91544?vs=305516&id=306151#toc
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D91544/new/
https://reviews.llvm.org/D91544
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
@@ -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<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 @@
/// 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;
};
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
@@ -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<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::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<SmallVector<transformer::Edit, 1>> Edits = Case.Edits(Result);
if (!Edits) {
llvm::errs() << "Rewrite failed: " << llvm::toString(Edits.takeError())
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
