Skip to content
This repository has been archived by the owner on May 12, 2020. It is now read-only.

Support ParametersAreNonnullByDefault #51

Open
kageiit opened this issue Sep 24, 2017 · 8 comments
Open

Support ParametersAreNonnullByDefault #51

kageiit opened this issue Sep 24, 2017 · 8 comments

Comments

@kageiit
Copy link

kageiit commented Sep 24, 2017

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

@kageiit
Copy link
Author

kageiit commented Sep 24, 2017

@naturalwarren
Copy link
Contributor

naturalwarren commented Sep 25, 2017

@kageiit
Copy link
Author

kageiit commented Sep 25, 2017

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

@naturalwarren
Copy link
Contributor

naturalwarren commented Sep 25, 2017

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

@msridhar
Copy link

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?

@naturalwarren
Copy link
Contributor

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

@kageiit
Copy link
Author

kageiit commented Sep 25, 2017

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.

@behroozkhorashadi
Copy link
Contributor

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.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants