ClassCanBeStatic: Exclude JUnit @Nested classes

[ljrmorgan]

Exclude inner classes annotated with JUnit's @Nested annotation from the ClassCanBeStatic check. JUnit requires them to not be static.

From the JUnit docs:

Only non-static nested classes (i.e. inner classes) can serve as @Nested test classes.

Fixes #956.

[Stephan202]

Nice one. In our code base I find quite a number of these:

@Nested
@SuppressWarnings("ClassCanBeStatic" /* @Nested classes cannot be static. */)

[cushon]

Thanks, LGTM overall.

We've been starting to consolidate more of the heuristics for situations where Error Prone shouldn't change modifiers etc. by using @Keep, or treating annotations as if they were annotated with @Keep.

error-prone/annotations/src/main/java/com/google/errorprone/annotations/Keep.java

Lines 30 to 31 in bb9ede9

	* Indicates that the annotated element should not be removed, and that its visibility, modifiers,
	* type, and name should not be modified.

What do you think about using ASTHelpers.shoudlKeep here, and adding @Nested to this list:

error-prone/check_api/src/main/java/com/google/errorprone/util/ASTHelpers.java

Lines 1043 to 1044 in bb9ede9

	private static final ImmutableSet<String> ANNOTATIONS_CONSIDERED_KEEP =
	ImmutableSet.of("org.apache.beam.sdk.transforms.DoFn.ProcessElement");

[ljrmorgan]

@cushon thank you for taking a look at the PR, and for the suggestion!

I think that makes sense - the JavaDoc for @Keep made me think initially of checks like UnusedMethod etc., but I suppose the only reason that @Nested classes can't be static is because of how they are used via reflection, which fits the description of @Keep. I guess my only concern is that we might lose some valid ClassCanBeStatic warnings on types that happen to be annotated with @Keep, but you are much better placed than I am to say whether that is a legitimate concern 😁

I've pushed that change as a separate commit, I'm happy to squash the two commits into one if you'd prefer, just let me know.

Thanks!

[ljrmorgan]

Hi @cushon, I wondered if you'd had a chance to take a look at the latest changes? Thanks!