Author: ymandel Date: Wed Jun 26 09:04:38 2019 New Revision: 364442 URL: http://llvm.org/viewvc/llvm-project?rev=364442&view=rev Log: [clang-tidy] Generalize TransformerClangTidyCheck to take a rule generator.
Summary: 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. Reviewers: gribozavr Subscribers: xazax.hun, cfe-commits Tags: #clang Differential Revision: https://reviews.llvm.org/D63288 Modified: clang-tools-extra/trunk/clang-tidy/utils/TransformerClangTidyCheck.cpp clang-tools-extra/trunk/clang-tidy/utils/TransformerClangTidyCheck.h clang-tools-extra/trunk/unittests/clang-tidy/TransformerClangTidyCheckTest.cpp Modified: clang-tools-extra/trunk/clang-tidy/utils/TransformerClangTidyCheck.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/utils/TransformerClangTidyCheck.cpp?rev=364442&r1=364441&r2=364442&view=diff ============================================================================== --- clang-tools-extra/trunk/clang-tidy/utils/TransformerClangTidyCheck.cpp (original) +++ clang-tools-extra/trunk/clang-tidy/utils/TransformerClangTidyCheck.cpp Wed Jun 26 09:04:38 2019 @@ -14,20 +14,40 @@ namespace tidy { namespace utils { using tooling::RewriteRule; +static bool hasExplanation(const RewriteRule::Case &C) { + return C.Explanation != nullptr; +} + +// 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, +// we would be accessing `getLangOpts` and `Options` before the underlying +// `ClangTidyCheck` instance was properly initialized. +TransformerClangTidyCheck::TransformerClangTidyCheck( + std::function<Optional<RewriteRule>(const LangOptions &, + const OptionsView &)> + MakeRule, + StringRef Name, ClangTidyContext *Context) + : ClangTidyCheck(Name, Context), Rule(MakeRule(getLangOpts(), Options)) { + 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::TransformerClangTidyCheck(RewriteRule R, StringRef Name, ClangTidyContext *Context) : ClangTidyCheck(Name, Context), Rule(std::move(R)) { - assert(llvm::all_of(Rule.Cases, [](const RewriteRule::Case &C) { - return C.Explanation != nullptr; - }) && + 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"); } 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 +63,8 @@ void TransformerClangTidyCheck::check( 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) { Modified: clang-tools-extra/trunk/clang-tidy/utils/TransformerClangTidyCheck.h URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/utils/TransformerClangTidyCheck.h?rev=364442&r1=364441&r2=364442&view=diff ============================================================================== --- clang-tools-extra/trunk/clang-tidy/utils/TransformerClangTidyCheck.h (original) +++ clang-tools-extra/trunk/clang-tidy/utils/TransformerClangTidyCheck.h Wed Jun 26 09:04:38 2019 @@ -31,19 +31,32 @@ namespace utils { // }; 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 Modified: clang-tools-extra/trunk/unittests/clang-tidy/TransformerClangTidyCheckTest.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clang-tidy/TransformerClangTidyCheckTest.cpp?rev=364442&r1=364441&r2=364442&view=diff ============================================================================== --- clang-tools-extra/trunk/unittests/clang-tidy/TransformerClangTidyCheckTest.cpp (original) +++ clang-tools-extra/trunk/unittests/clang-tidy/TransformerClangTidyCheckTest.cpp Wed Jun 26 09:04:38 2019 @@ -18,16 +18,16 @@ namespace clang { namespace tidy { namespace utils { namespace { +using tooling::change; using tooling::RewriteRule; +using tooling::text; +using tooling::stencil::cat; // Invert the code of an if-statement, while maintaining its semantics. RewriteRule invertIf() { using namespace ::clang::ast_matchers; - using tooling::change; using tooling::node; using tooling::statement; - using tooling::text; - using tooling::stencil::cat; StringRef C = "C", T = "T", E = "E"; RewriteRule Rule = tooling::makeRule( @@ -65,6 +65,62 @@ TEST(TransformerClangTidyCheckTest, Basi )"; EXPECT_EQ(Expected, test::runCheckOnCode<IfInverterCheck>(Input)); } + +// A trivial rewrite-rule generator that requires Objective-C code. +Optional<RewriteRule> needsObjC(const LangOptions &LangOpts, + const ClangTidyCheck::OptionsView &Options) { + if (!LangOpts.ObjC) + return None; + return tooling::makeRule(clang::ast_matchers::functionDecl(), + change(cat("void changed() {}")), text("no message")); +} + +class NeedsObjCCheck : public TransformerClangTidyCheck { +public: + NeedsObjCCheck(StringRef Name, ClangTidyContext *Context) + : TransformerClangTidyCheck(needsObjC, Name, Context) {} +}; + +// Verify that the check only rewrites the code when the input is Objective-C. +TEST(TransformerClangTidyCheckTest, DisableByLang) { + const std::string Input = "void log() {}"; + EXPECT_EQ(Input, + test::runCheckOnCode<NeedsObjCCheck>(Input, nullptr, "input.cc")); + + EXPECT_EQ("void changed() {}", + test::runCheckOnCode<NeedsObjCCheck>(Input, nullptr, "input.mm")); +} + +// A trivial rewrite rule generator that checks config options. +Optional<RewriteRule> noSkip(const LangOptions &LangOpts, + const ClangTidyCheck::OptionsView &Options) { + if (Options.get("Skip", "false") == "true") + return None; + return tooling::makeRule(clang::ast_matchers::functionDecl(), + change(cat("void nothing()")), text("no message")); +} + +class ConfigurableCheck : public TransformerClangTidyCheck { +public: + ConfigurableCheck(StringRef Name, ClangTidyContext *Context) + : TransformerClangTidyCheck(noSkip, Name, Context) {} +}; + +// Tests operation with config option "Skip" set to true and false. +TEST(TransformerClangTidyCheckTest, DisableByConfig) { + const std::string Input = "void log(int);"; + const std::string Expected = "void nothing();"; + ClangTidyOptions Options; + + Options.CheckOptions["test-check-0.Skip"] = "true"; + EXPECT_EQ(Input, test::runCheckOnCode<ConfigurableCheck>( + Input, nullptr, "input.cc", None, Options)); + + Options.CheckOptions["test-check-0.Skip"] = "false"; + EXPECT_EQ(Expected, test::runCheckOnCode<ConfigurableCheck>( + Input, nullptr, "input.cc", None, Options)); +} + } // namespace } // namespace utils } // namespace tidy _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
