Skip to content

Confusing nullability on subscribe(BiConsumer) #7330

New issue

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

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

Already on GitHub? # to your account

Closed
ZacSweers opened this issue Sep 5, 2021 · 5 comments · Fixed by #7331
Closed

Confusing nullability on subscribe(BiConsumer) #7330

ZacSweers opened this issue Sep 5, 2021 · 5 comments · Fixed by #7331
Labels

Comments

@ZacSweers
Copy link
Contributor

With RxJava 3.1.x's improve nullability annotations and Kotlin 1.5.30's improved recognition of them, I came across an interesting nullability mismatch with the subscribe(BiConsumer) case. In short - the generic T is always assumed non null now, but the nature of this method is that both parameters to the consumer are actually nullable (since only one is guaranteed to be present).

I'm not sure what the right solution is for this off-hand, but raising here for visibility.

image

@ZacSweers
Copy link
Contributor Author

For now, I'm explicitly typing these parameters as nullable rather than allowing type inference to assume otherwise

@akarnokd
Copy link
Member

akarnokd commented Sep 5, 2021

Probably the declaration-side override could work:

Disposable subscribe(@NonNull BiConsumer<@Nullable ? super T, @Nullable ? super Throwable> onCallback)

Not sure how to verify it works with Kotlin. Could you create a mockup method with this kind of signature and see if the inference works for you?

@akarnokd akarnokd added the 3.x label Sep 5, 2021
@ZacSweers
Copy link
Contributor Author

Yup I'll give that a try and report back

@ZacSweers
Copy link
Contributor Author

Confirmed that works with this example

public class Example<@NonNull T> {
  void subscribe(BiConsumer<@Nullable T, @Nullable Throwable> consumer) {

  }
}

image

@ZacSweers
Copy link
Contributor Author

I'll make a PR

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

Successfully merging a pull request may close this issue.

2 participants