Skip to content
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

unawaited_futures lint is too aggressive - reports Future.whenComplete #338

Closed
Hixie opened this issue Nov 22, 2016 · 6 comments
Closed
Labels
false-positive P3 A lower priority bug or feature request set-none Does not affect any rules in a defined rule set (e.g., core, recommended, flutter) type-bug Incorrect behavior (everything from a crash to more subtle misbehavior)

Comments

@Hixie
Copy link

Hixie commented Nov 22, 2016

Consider the following code:

  Future<Transaction> openTransaction() async {
    await _connect();
    if (_currentTransaction != null)
      await _currentTransaction.done;
    Transaction result = new Transaction(this);
    result.done.whenComplete(() {
      _currentTransaction = null;
    });
    return result;
  }

This causes:

   info • Await for future expression statements inside async function bodies. • lib/src/foo.dart:136:5

...but doing what the lint suggests would change the semantics of the code and not be correct.

@zoechi
Copy link

zoechi commented Nov 22, 2016

does

var _ = result.done.whenComplete(() {

fix the linter warning?

It was clear when this lint was introduced that there are always cases where one doesn't want to await and therefore false positives can't be avoided.
Another always available option is using // ignore: lint_rule to suppress a lint

@Hixie
Copy link
Author

Hixie commented Nov 22, 2016

oh is the problem that whenComplete returns a Future?

That's one Future that should probably be ignorable... (generally, exceptions from that are like exceptions from a finally, i.e. not expected and it's ok if they end up uncaught and reported by the root zone).

@zoechi
Copy link

zoechi commented Nov 22, 2016

Not sure. I didn't see where exactly you got the linter warning.

@alexeieleusis
Copy link
Contributor

alexeieleusis commented Nov 22, 2016

I have too many open bugs right now :(
I'll assign to the rule author and come back if it is still open once I close the ones I have open, IIUC the code in question is here https://github.com/dart-lang/linter/blob/master/lib/src/rules/unawaited_futures.dart
...
I cannot assign it, so through mention cc @ochafik

@pq pq added false-positive type-bug Incorrect behavior (everything from a crash to more subtle misbehavior) and removed type-bug Incorrect behavior (everything from a crash to more subtle misbehavior) labels Jun 27, 2018
@srawlins srawlins changed the title "Await for future expression statements" lint is too aggressive unawaited_futures lint is too aggressive Sep 3, 2022
@bwilkerson bwilkerson added P3 A lower priority bug or feature request set-none Does not affect any rules in a defined rule set (e.g., core, recommended, flutter) labels Nov 7, 2022
@srawlins srawlins changed the title unawaited_futures lint is too aggressive unawaited_futures lint is too aggressive - reports Future.whenComplete Jun 28, 2024
@lrhn
Copy link
Member

lrhn commented Jul 1, 2024

Since this was just linked to, I'll point out that the warning here is reasonable.
The future returned by whenComplete is not awaited. If it completes with an error, that error will be unhandled. That's precisely the kind of potential error the lint is intended to catch

Using unawaited(...) to make that explicit that the missing await is deliberate (and likely that the author considered it and decided that there won't be any errors) seems like the recommended behavior.
(And they can use .ignore() instead of unawaited if there might be an error, but it shouldn't be handled here.)

@bwilkerson
Copy link
Member

I think it's fairly clear that this isn't likely to be changed, so I'm going to close it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
false-positive P3 A lower priority bug or feature request set-none Does not affect any rules in a defined rule set (e.g., core, recommended, flutter) type-bug Incorrect behavior (everything from a crash to more subtle misbehavior)
Projects
None yet
Development

No branches or pull requests

6 participants