Method visibility rule is too aggressive classes nested in intefaces

[vpavic]

While trying to align Spring Session with Spring Java Format 0.0.15 I've found the newly introduced method visibility rule (introduced in #128) to be too aggressive.

For instance, this rule errors out due to methods of Spring Session's CookieSerializer.CookieValue being public. Complying with this would be a breaking API change. This is quite dangerous for a default check.

Another example are @Bean factory methods on (typically static and package-private) inner @Configuration classes as those are no longer allowed to be public. Personally I'd prefer to have all @Bean factory methods public (possibly due to still having the old behavior in the back of my head - see spring-projects/spring-framework#11829) and find this a bit too prescriptive for being among default checks.

With that in mind, my proposal is to remove method visibility rule from automatically applied checks, and have it enabled manually in projects that prefer it, like Spring Boot. Otherwise I feel it greatly impacts the adoption of this project across Spring projects.

[philwebb]

The issue with CookieSerializer.CookieValue looks like a bug because we're not considering it a public class and we should be. We've got another one of those in #132.

As for changing the default, I'm not totally convinced that we should drop the rule, but I'd like to see what the rest of the team think. We found it identified a few placed in Spring Boot where we were accidentally has a package private classes in the API that should have been public.

It's pretty easy to add a <suppress files=".*" ... rule in the meantime if you want to just suppress it for all files.

[vpavic]

Thanks for the quick feedback.

Yeah, the rule certainly does have some value. If first situation is a bug, and second one can be handled nicely somehow, I'd leave it enabled by default.

What's your take on @Bean factory method visibility remark? IMO it's valid to have those with same visibility across the project.

[wilkinsona]

I'd prefer not to drop the rule as well. As another data point, I adopted it in Spring REST Docs with minimal hassle.

FWIW, I think @Bean methods should be treated the same as any other method and, therefore, their visibility should be minimised.