================
@@ -27,19 +29,44 @@ class UncheckedOptionalAccessCheck : public ClangTidyCheck {
UncheckedOptionalAccessCheck(StringRef Name, ClangTidyContext *Context)
: ClangTidyCheck(Name, Context),
ModelOptions{
- Options.getLocalOrGlobal("IgnoreSmartPointerDereference", false)}
{}
+ Options.getLocalOrGlobal("IgnoreSmartPointerDereference", false)},
+ ignore_test_tus_(Options.getLocalOrGlobal("IgnoreTestTUs", false)) {}
void registerMatchers(ast_matchers::MatchFinder *Finder) override;
void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+ void onStartOfTranslationUnit() override;
bool isLanguageVersionSupported(const LangOptions &LangOpts) const override {
return LangOpts.CPlusPlus;
}
void storeOptions(ClangTidyOptions::OptionMap &Opts) override {
Options.store(Opts, "IgnoreSmartPointerDereference",
ModelOptions.IgnoreSmartPointerDereference);
+ Options.store(Opts, "IgnoreTestTUs", ignore_test_tus_);
}
private:
dataflow::UncheckedOptionalAccessModelOptions ModelOptions;
+
+ // Tracks the Option of whether we should ignore test TUs (e.g., googletest).
+ // Currently we have many false positives in tests, making it difficult to
+ // find true positives and developers end up ignoring the warnings in tests,
+ // reducing the check's effectiveness.
+ // Reasons for false positives (once fixed we could remove this option):
+ // - has_value() checks wrapped in googletest assertion macros are not
handled
+ // (e.g., EXPECT_TRUE() and fall through, or ASSERT_TRUE() and crash,
+ // or more complex ones like ASSERT_THAT(x, Not(Eq(std::nullopt))))
+ // - we don't handle state carried over from test fixture constructors/setup
+ // to test case bodies (constructor may initialize an optional to a value)
+ // - developers may make shortcuts in tests making assumptions and
+ // use the test runs (expecially with sanitizers) to check assumptions.
+ // This is different from production code in that test code should have
+ // near 100% coverage (if not covered by the test itself, it is dead code).
+ bool ignore_test_tus_ = false;
+
+ // Records whether the current TU includes the test-specific headers (e.g.,
+ // googletest), in which case we assume it is a test TU of some sort.
+ // This along with the setting `ignore_test_tus_` allows us to disable
+ // checking for all test TUs.
+ bool is_test_tu_ = false;
----------------
5chmidti wrote:
nit: var casing should be `CamelCase`
https://github.com/llvm/llvm-project/pull/115051
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
