Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Flip the safety restrictions from 3rd party jars. #1119

Open
xenoterracide opened this issue Dec 30, 2024 · 2 comments
Open

Flip the safety restrictions from 3rd party jars. #1119

xenoterracide opened this issue Dec 30, 2024 · 2 comments

Comments

@xenoterracide
Copy link

I guess this is two related requests?

https://github.com/uber/NullAway/wiki/Configuration#acknowledge-more-restrictive-annotations-from-third-party-jars

First, a question, this is a boolean, how does this help when this is a problem with only one jar (looking at you jgit!) feels like it should take packages like other options.

Secondly, can we get an option to flip this? which seems more safe to me. The new option would be worded as follows.

For example, when this flag is turned on, a method Object foo(Object o) in an unannotated package would be assumed to not take a nullable argument and return a nullable. However, a method Object bar(@Nullable Object o) will be interpreted by NullAway as taking a nullable argument, and a method @NonNull Object baz(Object o) will be acknowledged as not returning a null, even in code otherwise marked as unannotated. If this flag is turned off, which is the default, all three examples are treated as the first and optimistic defaults are assumed to reduce the number of warnings unless the package is marked as fully annotated.

I personally feel this would be safer.

@msridhar
Copy link
Collaborator

msridhar commented Jan 1, 2025

Hi @xenoterracide,

First, a question, this is a boolean, how does this help when this is a problem with only one jar (looking at you jgit!) feels like it should take packages like other options.

We haven't had a request for finer-grained control like that before. Do you have examples of jars with incorrect @NonNull annotations on parameters or @nullable annotations on return values?

Secondly, can we get an option to flip this? which seems more safe to me. The new option would be worded as follows.

For example, when this flag is turned on, a method Object foo(Object o) in an unannotated package would be assumed to not take a nullable argument and return a nullable. However, a method Object bar(@Nullable Object o) will be interpreted by NullAway as taking a nullable argument, and a method @NonNull Object baz(Object o) will be acknowledged as not returning a null, even in code otherwise marked as unannotated. If this flag is turned off, which is the default, all three examples are treated as the first and optimistic defaults are assumed to reduce the number of warnings unless the package is marked as fully annotated.

I personally feel this would be safer.

This indeed would be safer. We never added this option since we thought it would add way too much noise. The Checker Framework, which strives for soundness, has an option to treat libraries in this way, but they disable it by default, presumably for the same reason.

Maybe you can share more details on what you are trying to accomplish? If you are trying to obtain very strong guarantees about the null safety of your code, to be 100% honest NullAway may not be the best option, as there are a number of ways that it can miss possible NPEs. (See Section 4 and Section 6 of this paper for some details / examples.) You might instead want to use the Checker Framework Nullness Checker with the option linked above set to treat libraries conservatively.

If you'd like jgit to be annotated for nullness, maybe we can work with the project authors on that, using the NullAway Annotator to help.

@xenoterracide
Copy link
Author

We haven't had a request for finer-grained control like that before. Do you have examples of jars with incorrect @nonnull annotations on parameters or https://github.com/nullable annotations on return values?

not incorrect, jgit is just incomplete. However, in its case its safer to assume that input must not be null, and output is nullable. AFAIK I have no other mixed libraries. They are all either fully annotated, or not annotated at all. I would think it would be more performant (not that I'm having a performance issue) to tell the system to only look for individual annotations in jgit packages; not just every library marked as unknown.

This indeed would be safer. We never added this option since we thought it would add way too much noise. The Checker Framework, which strives for soundness, has an option to treat libraries in this way, but they disable it by default, presumably for the same reason

which I feel like it makes sense for adding this to a large-ish project that has much library usage. However, in a greenfield where you can handle as they come.

I've kind of re-thought this a bit, thinking of apache commons lang3. It might be nicer to be able to change the defaults easily per package. Since commons lang3 is basically known to allow nullable inputs, but also have nullable outputs.

Where perhaps we could just set a list of packages with default nullable input and another list with default nullable output. Of course how this impacts.

Checker Framework Nullness Checker

every time I've tried I've found it obnoxious in other ways. I'm not certain I want/need 100% comprehensiveness.

If you'd like jgit to be annotated for nullness, maybe we can work with the project authors on that,

I opened a ticket for that eclipse-jgit/jgit#121 note: somewhere there's a ticket for apache commons to be annotated, but they haven't been willing yet. I don't know if jspecify has ever been mentioned on that ticket.

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

No branches or pull requests

2 participants