Reporting Issues with Functions Returning Disposable Objects

[commonsguy]

I have run across a couple of edge cases that I thought I'd point out. The attached project should reproduce them — at least, it does for me here (macOS, using Gradle Wrapper in the project).

package com.example.rxlinttest

import android.os.Bundle
import androidx.appcompat.app.AppCompatActivity
import io.reactivex.Observable

class MainActivity : AppCompatActivity() {
    override fun onCreate(savedInstanceState: Bundle?) {
        super.onCreate(savedInstanceState)

        Observable.just(true)
            .subscribe({}) {}

        Observable.just(true).loggingSubscribe {}

        loggingSubscribeAsAFunction(Observable.just(true)) { }
    }
}

Here, we have three leaked Disposable objects. The first one is a fairly obvious leak, and rxlint 1.7.8 complains at the call site as expected.

The second one leaks because loggingSubscribe() returns the Disposable, which the call site is not referencing:

fun <T> Observable<T>.loggingSubscribe(onNext: (T) -> Unit): Disposable =
    subscribe(onNext) { if (BuildConfig.DEBUG) throw it else Log.e("SCRAP", "Someting blow'd up", it) }

In this case, rxlint 1.7.8 reports the leak... but IMHO at the wrong spot. It complains about loggingSubscribe(). Technically, we don't know if it leaked yet there, as it depends on what the caller does with the Disposable that is returned. I don't know how practical this is, but IMHO ideally rxlint would treat our own functions returning Disposable the same as it does RxJava functions returning Disposable, with the Lint error showing up on the caller of the function.

The third one leaks because loggingSubscribeAsAFunction() returns a Disposable, which the call site is not referencing:

fun <T> loggingSubscribeAsAFunction(o: Observable<T>, onNext: (T) -> Unit): Disposable =
    o.subscribe(onNext) { if (BuildConfig.DEBUG) throw it else Log.e("SCRAP", "Someting blow'd up", it) }

This is the same code as the loggingSubscribe() extension function, just written as a Kotlin top-level function. Oddly, rxlint 1.7.8 does not complain — I would have expected the same behavior as with the extension function.

Let me know if you need more information!

rxlintTest.zip

[hvisser]

Yes, interesting idea. The way the check works now is that it checks if you call into Observable without using the result (if I remember correctly, it has been a while 😅 ). Checking every return of Disposable and/or tracing it up to the call site might be a whole different ballgame, but I'm not sure.

Re: extension functions, it might very well be that more work is needed to catch some of these more kotlin specific things. It has been a while since I did any rx related code, so I haven't really been keeping up on this lint check either in that sense.

PR's welcome though :)