Rxswift: When using takeUntil instead of DisposeBag, don´t warn about the unused Disposable return value

[sticksen]

Short description of the issue:

When using takeUntil(self.rx.deallocated) instead of .disposed(by: discoveryDisposeBag), the compiler warns about an unused result.

Instead, you could use @discardableResult

Expected outcome:

There should be no warning, because the memory is handled by using takeUntil

What actually happens:

The compiler warns about an unused result.

Self contained code example that reproduces the issue:

sequence
    .takeUntil(self.rx.deallocated)
    .subscribe {
        print($0)
    }

RxSwift/RxCocoa/RxBlocking/RxTest version/commit

3.5.0

Platform/Environment

• [ x] iOS

How easy is to reproduce? (chances of successful reproduce after running the self contained code)

• [x ] easy, 100% repro
• [ ] sometimes, 10%-100%
• [ ] hard, 2% - 10%
• [ ] extremely hard, %0 - 2%

Xcode version:

  8.3.3

:warning: Fields below are optional for general issues or in case those questions aren't related to your issue, but filling them out will increase the chances of getting your issue resolved. :warning:

Installation method:

• [ ] CocoaPods
• [x ] Carthage
• [ ] Git submodules

I have multiple versions of Xcode installed:
(so we can know if this is a potential cause of your issue)

• [ ] yes (which ones)
• [ x] no

Level of RxSwift knowledge:
(this is so we can understand your level of knowledge
and formulate the response in an appropriate manner)

• [ ] just starting
• [x ] I have a small code base
• [ ] I have a significant code base

[freak4pc]

This is not related to RxSwift. This is how Swift works.
If you get a result and you don't store it anywhere it will complain regardless of the type of result.

The easiest way of course is just to do
_ = myObservable.takeUntil(rx.deallocated)

The other way would be to add a @discardableResult to takeUntil, but I really don't think we would want to do that cc/ @kzaher

[raphaklr]

Why not? The return value of subscribe and takeUntil will often remain unused, so I don't see the point in forcing people to write ugly boilerplate code like _ = ..., but it's possible that I'm missing something here...

[freak4pc]

For subscribe() there isn't even a question IMHO. The fact you get a "not used" warning is to remind you that you're probably not disposing of it correctly (e.g. adding it to a DisposeBag or storing the Disposable itself and disposing of it manually if you prefer)

Since using a Dispose bag is the official method that is most commonly used, I don't see a good reason for making such a change.

Also when you use a subscribe() the compiler / type checker doesn't know that there's a takeUntil before, so even if we would add a @discardableResult to takeUntil, it would still return that warning on a subscribe if you're not handling the result in some way.

[sticksen]

Well, my idea would have been to have a subclass of Observable as the return value of takeUntil where you could insert the @discardableResult annotation. Something like this:

public class ObservableChecked<Element>: Observable<Element> {
    @discardableResult public override func subscribe<O>(_ observer: O) -> Disposable where O : ObserverType, O.E == Element {
        rxAbstractMethod()
    }
}

takeUntil signature:

public func takeUntil<O: ObservableType>(_ other: O)
        -> ObservableChecked<E>

Do you think that might work?

[freak4pc]

I'm personally against returning anything that isn't a regular Observable, this feels to me like a more personal use case kind of situation where you would want to have that extension in your code, but not in the main repo. Of course final decision on this is kzaher's but I'm personally against, It doesn't feel to me like it brings value to the majority of the users but just confusion on the different return types.

[kzaher]

You can use takeUntil in various scenarios and not just for .takeUntil(rx.deallocated). Using @discardableResult is those cases would be wrong.

It seems to me that writing _ = x.takeUntil(x.rx.deallocated) is a small price to pay.

[sticksen]

Alright, makes sense! Thanks

[chrislconover]

So short summary then, we could really use a trailing discardWhen(observable etc...) method that comes after subscribe (acts on Disposable), so it doesn't complicate subscribe and doesn't confuse the reader with an ignored subscription.

This follows the natural flow of logic "do something until", vs "do until, thing to do"

Thanks.

[freak4pc]

@chrisco314 There's no concept of "operators on Disposables" anywhere, on any Rx implementation. Operators act on Observables only.

You can use takeWhile that takes a closure vs. takeUntil that takes an Observable.

[sergdort]

I guess you can add your extension to Disposable

extension Disposable {
    func ignoreSubscription() {}
}