-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
Introduce non-nullable types in reactive integrations where appropriate #3393
Conversation
qwwdfsad
commented
Aug 5, 2022
- For RxJava2, use them in internal implementations where appropriate
- For RxJava3, introduce & Any bound to generic argument in our extensions to avoid errors in Kotlin 1.8.0 due to non-nullability rx3 annotations being part of generics upper bound. This change went through commitee, and all the "unsound" declarations such as "RxSignature<Foo?>" were properly highlighted as warning that will become an error.
* For RxJava2, use them in internal implementations where appropriate * For RxJava3, introduce & Any bound to generic argument in our extensions to avoid errors in Kotlin 1.8.0 due to non-nullability rx3 annotations being part of generics upper bound. This change went through commitee, and all the "unsound" declarations such as "RxSignature<Foo?>" were properly highlighted as warning that will become an error.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for making this happen!
public suspend fun <T> MaybeSource<T>.awaitSingleOrNull(): T? = suspendCancellableCoroutine { cont -> | ||
subscribe(object : MaybeObserver<T> { | ||
public suspend fun <T> MaybeSource<T & Any>.awaitSingleOrNull(): T? = suspendCancellableCoroutine { cont -> | ||
subscribe(object : MaybeObserver<T & Any> { | ||
override fun onSubscribe(d: Disposable) { cont.disposeOnCancellation(d) } | ||
override fun onComplete() { cont.resume(null) } | ||
override fun onSuccess(t: T) { cont.resume(t) } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
T & Any
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This bound seems to be properly propagated from the declaration site
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since the object is implementing MaybeObserver<T & Any>
, I think the method parameter needs to be T & Any
to match.
Or you know what, actually, even if we were implementing plain MaybeObserver<T>
, we'd probably still need the method parameter to be T & Any
, since the interface declares its method as requiring a @NonNull T
:
I think that's why, with the current parameter type of T
, the build fails with the flags I posted in the build script.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for pointing it out. T & Any is required since Kotlin 1.8.0, it was an omission on the language site
@@ -176,8 +176,6 @@ configure(subprojects.findAll { !sourceless.contains(it.name) }) { | |||
tasks.withType(AbstractKotlinCompile).all { | |||
kotlinOptions.freeCompilerArgs += OptInPreset.optInAnnotations.collect { "-Xopt-in=" + it } | |||
kotlinOptions.freeCompilerArgs += "-progressive" | |||
// Disable KT-36770 for RxJava2 integration | |||
kotlinOptions.freeCompilerArgs += "-XXLanguage:-ProhibitUsingNullableTypeParameterAgainstNotNullAnnotated" | |||
// Remove null assertions to get smaller bytecode on Android | |||
kotlinOptions.freeCompilerArgs += ["-Xno-param-assertions", "-Xno-receiver-assertions", "-Xno-call-assertions"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure exactly what -progressive
enables, but I notice that I can get a few more errors (noted above) if I enable the flags from #3007. (Thanks for pointing me here from there!)
// Recognize rxjava3 nullness annotations even before that becomes the default (which may happen in 1.8): https://kotlinlang.org/docs/java-interop.html#nullability-annotations
kotlinOptions.freeCompilerArgs += "[email protected]:strict"
// Recognize nullness annotations on type arguments, etc.: https://kotlinlang.org/docs/java-interop.html#annotating-type-arguments-and-type-parameters
kotlinOptions.freeCompilerArgs += "-Xtype-enhancement-improvements-strict-mode"