dart-lang / linter

Linter for Dart.
https://dart.dev/tools/linter-rules
BSD 3-Clause "New" or "Revised" License
628 stars 172 forks source link

Question about `cancel_subscriptions` #4950

Open bwilkerson opened 2 months ago

bwilkerson commented 2 months ago

Is it expected that the lint cancel_subscriptions does not generate a diagnostic for the following code

import 'dart:async';

void f(Stream stream) {
  stream.listen((_) {});
}

but will generate a diagnostic for the following code:

import 'dart:async';

void f(Stream stream) {
  var s = stream.listen((_) {});
}

@pq @srawlins

srawlins commented 2 months ago

I think that's intentional. Or a known limitation. In the latter example, we have a variable, such that we can track whether it is canceled. In the former example, we don't, so we throw our hands up. But I could see it being a false negative.

bwilkerson commented 2 months ago

Yeah, if it isn't assigned to a variable, and it isn't passed as an argument to an invocation, then I think we can conclude that it isn't cancelled.

I don't know that I care deeply about it; I was just surprised when I discovered that it didn't get reported. (I was writing an example, and adding the variable creates an unused_local_variable diagnostic, which is unfortunate for my purposes.)

lrhn commented 2 months ago

If you're not expecting errors, or is using cancelOnError: true, consider using stream.forEach(...) instead. (Then you'll get yelled at for not handling the returned future, which you probably should, instead of using an onDone handler.)