uber-archive/rave

Support ParametersAreNonnullByDefault

Opened this issue · 8 comments

See https://static.javadoc.io/com.google.code.findbugs/jsr305/3.0.1/javax/annotation/ParametersAreNonnullByDefault.html

This can be used to annotate entire packages/classes and reduce annotation overhead. Currently Rave does not honor this annotation. It would be great to check for the presence of this annotation on packages/classes during validation

Sure. It was more about the idea of doing it with package level annotations. It would be great to add this support for method return types in Rave

Great! I think this is a worthwhile improvement. Is there a specific use case you're thinking of so I can get this prioritized?

cc @behroozkhorashadi

Not 100% sure about this one. We already have strict mode for RAVE (which assumes NonNull by default for returns), and this can be configured on a per-validator-factory basis. Maybe that's good enough?

We'd only be improving the DEFAULT mode by hooking into local package-info files.

DEFAULT may not apply to someone in a situation where they use third party libs that do return null from methods (but some dont). This annotation is a good way to get more fine grained results.

Rave won't apply to 3rd party methods/classes anyway since Rave doesn't validate things that don't explicitly use it. Adding this to package-info would probably make things more confusing since you couldn't be sure what takes precedence (the validator mode or the package mode).

It would allow you to get more granular control (if you wanted to have default mode in one package but strict mode in another, for example). This would probably be useful for larger code bases when migrating between one mode or another but I'm not sure its worth the effort to do for just those cases.