Introduce non-nullable types in reactive integrations where appropriate #3393

qwwdfsad · 2022-08-05T14:19:31Z

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.

build.gradle

reactive/kotlinx-coroutines-rx2/src/RxAwait.kt

reactive/kotlinx-coroutines-rx2/src/RxChannel.kt

cpovirk

Thanks for making this happen!

cpovirk · 2022-09-20T14:03:51Z

reactive/kotlinx-coroutines-rx3/src/RxAwait.kt

-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) }


This bound seems to be properly propagated from the declaration site

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:

https://github.com/ReactiveX/RxJava/blob/8e53e0ee1637791923a81fdb9d17dfe8c1569250/src/main/java/io/reactivex/rxjava3/core/MaybeObserver.java#L76

I think that's why, with the current parameter type of T, the build fails with the flags I posted in the build script.

Thanks for pointing it out. T & Any is required since Kotlin 1.8.0, it was an omission on the language site

reactive/kotlinx-coroutines-rx3/src/RxChannel.kt

cpovirk · 2022-09-20T14:06:21Z

build.gradle

@@ -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"]


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 += "-Xnullability-annotations=@io.reactivex.rxjava3.annotations: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"

qwwdfsad requested a review from dkhalanskyjb August 5, 2022 14:19

qwwdfsad mentioned this pull request Sep 12, 2022

Update code to prepare for nullness annotations in rxjava3. #3007

Closed

qwwdfsad added the for 1.7 label Sep 12, 2022

dkhalanskyjb requested changes Sep 14, 2022

View reviewed changes

~update nullability, fix some warnings

34c08dd

qwwdfsad requested a review from dkhalanskyjb September 20, 2022 10:57

dkhalanskyjb reviewed Sep 20, 2022

View reviewed changes

reactive/kotlinx-coroutines-rx2/src/RxChannel.kt Outdated Show resolved Hide resolved

reactive/kotlinx-coroutines-rx2/src/RxChannel.kt Outdated Show resolved Hide resolved

~

508ce3a

qwwdfsad requested a review from dkhalanskyjb September 20, 2022 13:49

dkhalanskyjb approved these changes Sep 20, 2022

View reviewed changes

~

6b6fbd2

cpovirk reviewed Sep 20, 2022

View reviewed changes

~make changes compatible with Kotlin 1.8.0

fa472f9

qwwdfsad merged commit 61ba10d into develop Sep 20, 2022

qwwdfsad deleted the non-nullable-types branch September 20, 2022 18:27

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Introduce non-nullable types in reactive integrations where appropriate #3393

Introduce non-nullable types in reactive integrations where appropriate #3393

qwwdfsad commented Aug 5, 2022

cpovirk left a comment

cpovirk Sep 20, 2022

qwwdfsad Sep 20, 2022

cpovirk Sep 20, 2022

qwwdfsad Sep 20, 2022

cpovirk Sep 20, 2022 •

edited

Introduce non-nullable types in reactive integrations where appropriate #3393

Introduce non-nullable types in reactive integrations where appropriate #3393

Conversation

qwwdfsad commented Aug 5, 2022

cpovirk left a comment

Choose a reason for hiding this comment

cpovirk Sep 20, 2022

Choose a reason for hiding this comment

qwwdfsad Sep 20, 2022

Choose a reason for hiding this comment

cpovirk Sep 20, 2022

Choose a reason for hiding this comment

qwwdfsad Sep 20, 2022

Choose a reason for hiding this comment

cpovirk Sep 20, 2022 • edited

Choose a reason for hiding this comment

cpovirk Sep 20, 2022 •

edited