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) {
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) {
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
