[PATCH] D52771: [clang-tidy] Non-private member variables in classes (MISRA, CppCoreGuidelines, HICPP)

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added inline comments.


================
Comment at: clang-tidy/misc/NonPrivateMemberVariablesInClassesCheck.cpp:26
+
+AST_MATCHER(clang::CXXRecordDecl, hasNonStaticMethod) {
+  return hasMethod(unless(isStaticStorageClass()))
----------------
JonasToth wrote:
> I think this and the next matcher can be a normal variable.
> 
> ```
> auto HasNonStaticMethod = hasMethod(unless(isStaticStorageClass()));
> 
> ...
> ... allOf(anyOf(isStruct(), isClass()), HasMethods, HasNonStaticMethod)
> ...
> ```
They could, but these have a very meaningful meaning, so i'd argue it would not be worse to keep them 
out here refactored as proper matchers, in case someone else needs them elsewhere (so he could just
move them from here into more general place, as opposed to writing a duplicating matcher.)


================
Comment at: test/clang-tidy/misc-non-private-member-variables-in-classes.cpp:1
+// RUN: %check_clang_tidy -check-suffix=NONPRIVATE %s misc-non-private-member-variables-in-classes %t
+// RUN: %check_clang_tidy -check-suffix=NONPRIVATE %s misc-non-private-member-variables-in-classes %t -- -config='{CheckOptions: [{key: misc-non-private-member-variables-in-classes.IgnorePublicMemberVariables, value: 0}, {key: misc-non-private-member-variables-in-classes.IgnoreClassesWithAllMemberVariablesBeingPublic, value: 0}]}' --
----------------
JonasToth wrote:
> I would prefer multiple test-files instead of mixing them all together.
Hmm, no. Then the tests will have to be duplicated in N files,
because i really do want to see what each of these 4 configurations do on the *same* input.

It only looks ugly because the script only supports `-check-suffix=`,
and not `-check-suffixes=`, which would significantly cut down on check-lines.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D52771