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

Allow models to override @Nullable on third-party functional interfaces #326

Merged
merged 3 commits into from Jun 27, 2019

Conversation

lazaroclapp
Copy link
Collaborator

This PR does two things, one uncontroversial and the other more opinionated

  1. [Uncontroversial] It acknowledges LibraryModels.nonNullParameters() when deciding the nullability of the arguments in a lambda with a functional interface which otherwise marks the parameters as @Nullable. Before we did acknowledge this when calling the FI methods directly, but not for lambda expression based overriding.

  2. [Opinionated] We override the nullability of Guava's Function and Predicate functional interfaces. This means that lambdas implementing these in our code will need to return @NonNull but need not check the incoming arguments.

This has implications for checking Guava code with NullAway, but not for treating it as annotated vs unannotated with restrictive annotations enabled.

cc: @kurtisnelson

@lazaroclapp
Copy link
Collaborator Author

lazaroclapp commented Jun 25, 2019

This is actually a WIP. ToDo:

@shubhamugare
Copy link
Contributor

@lazaroclapp
Copy link
Collaborator Author

What is to be done about this?

See latest commit. The idea being that to compute if an unbound instance method ref can be passed to a method that expects a particular functional interface, one thing must be true: the first argument (the receiver for an instance method) must be @NonNull. We were checking this already, but what defined whether this parameter was nullable or not was 100% the bytecode annotations, without acknowledging the distinction between annotated and unannotated code, or any overrides due to handlers (including library models).

Copy link
Collaborator

@msridhar msridhar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wish we had generics support to properly handle Function and Predicate but this looks fine for now

Copy link
Contributor

@navahgar navahgar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@lazaroclapp lazaroclapp merged commit e1a1166 into master Jun 27, 2019
@lazaroclapp lazaroclapp deleted the lazaro_library_models_nonnull_lambda_override branch June 27, 2019 18:18
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

Successfully merging this pull request may close these issues.

None yet

4 participants