summaryrefslogtreecommitdiffstats
path: root/clang-tools-extra
diff options
context:
space:
mode:
authorYitzhak Mandelbaum <yitzhakm@google.com>2019-06-26 16:04:38 +0000
committerYitzhak Mandelbaum <yitzhakm@google.com>2019-06-26 16:04:38 +0000
commit039af0ea03f987654a162b6160399a00fd22b1b0 (patch)
tree152c66521e8734b2beef0da296d593c07fdb0348 /clang-tools-extra
parent71ad22707cdf2997469ee8bbbbd9ea03cf096726 (diff)
downloadbcm5719-llvm-039af0ea03f987654a162b6160399a00fd22b1b0.tar.gz
bcm5719-llvm-039af0ea03f987654a162b6160399a00fd22b1b0.zip
[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 llvm-svn: 364442
Diffstat (limited to 'clang-tools-extra')
-rw-r--r--clang-tools-extra/clang-tidy/utils/TransformerClangTidyCheck.cpp31
-rw-r--r--clang-tools-extra/clang-tidy/utils/TransformerClangTidyCheck.h27
-rw-r--r--clang-tools-extra/unittests/clang-tidy/TransformerClangTidyCheckTest.cpp62
3 files changed, 105 insertions, 15 deletions
diff --git a/clang-tools-extra/clang-tidy/utils/TransformerClangTidyCheck.cpp b/clang-tools-extra/clang-tidy/utils/TransformerClangTidyCheck.cpp
index 80a82980868..1dedb0571c3 100644
--- a/clang-tools-extra/clang-tidy/utils/TransformerClangTidyCheck.cpp
+++ b/clang-tools-extra/clang-tidy/utils/TransformerClangTidyCheck.cpp
@@ -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) {
diff --git a/clang-tools-extra/clang-tidy/utils/TransformerClangTidyCheck.h b/clang-tools-extra/clang-tidy/utils/TransformerClangTidyCheck.h
index faf946ceb0f..0cd386f5b07 100644
--- a/clang-tools-extra/clang-tidy/utils/TransformerClangTidyCheck.h
+++ b/clang-tools-extra/clang-tidy/utils/TransformerClangTidyCheck.h
@@ -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
diff --git a/clang-tools-extra/unittests/clang-tidy/TransformerClangTidyCheckTest.cpp b/clang-tools-extra/unittests/clang-tidy/TransformerClangTidyCheckTest.cpp
index 6b8763810e9..6e42be25d5f 100644
--- a/clang-tools-extra/unittests/clang-tidy/TransformerClangTidyCheckTest.cpp
+++ b/clang-tools-extra/unittests/clang-tidy/TransformerClangTidyCheckTest.cpp
@@ -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, Basic) {
)";
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
OpenPOWER on IntegriCloud