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

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added inline comments.


================
Comment at: clang-tidy/misc/NonPrivateMemberVariablesInClassesCheck.cpp:26
+
+AST_MATCHER(clang::CXXRecordDecl, hasNonStaticMethod) {
+  return hasMethod(unless(isStaticStorageClass()))
----------------
lebedev.ri wrote:
> 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.)
Ok.


================
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}]}' --
----------------
lebedev.ri wrote:
> 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.
Ok, if you prefer this.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D52771