Warn when returning a function invocation when within a try block in an async function

[kevmoo]

Often if you're in a try block, you want to catch exceptions.

But if you omit the await errors might flow through.

See https://dartpad.dev/c8deb87cee33b48316fc0f5cf4b1891f

Future<void> main() async {
  for (var func in [goodCatch, badCatch]) {
    print('Running $func');
    try {
      await func();
    } catch (e) {
      print('Why was this not caught? $e');
    }
    print('');
  }
}

Future<void> goodCatch() async {
  try {
    // `await` here is CRITICAL!
    return await badAsync();
  } on StateError catch (e) {
    print('Caught "$e"!');
  }
}

Future<void> badCatch() async {
  try {
    // No await, so the state error is not caught!
    return badAsync();
  } on StateError catch (e) {
    print('Caught "$e"!');
  }
}

Future<void> badAsync() async {
  throw StateError('bad!');
}

Running Closure 'goodCatch'
Caught "Bad state: bad!"!

Running Closure 'badCatch'
Why was this not caught? Bad state: bad!

[kevmoo]

This conflicts with unnecessary_await_in_return, too – I wonder if we should special-case return someFuture(); within a try block?

[natebosch]

You shouldn't use unnecessary_await_in_return at all. I think we should probably remove that lint.

We have some discussion about making the await always required at the language level. No concrete plans, but I do think it would be a nice thing to do. It would remove the ambiguity and the need to control this with lints at all. If we do anything here we could have an always_await_future_even_in_return or update unawaited_futures so that it doesn't allow return as an escape hatch. This would bring the lint in line with the potential future of the language. https://github.com/dart-lang/language/issues/870

[mnordine]

Just my 2 cents, but I'd strongly prefer a new lint, rather than updating unawaited_futures. They seem to be 2 different things. I think everyone would want always_await_future_even_in_return, so it could prob go in pedantic, but not everyone will want to wrap their calls in unawaited()

[kevmoo]

@mnordine – Agreed we want a separate lint. But also we should resolve any conflicts between the two lints!

[eernstg]

Actually, this particular behavior should not occur, cf. https://github.com/dart-lang/sdk/issues/44395. So it shouldn't be necessary to help developers remember to include the await, because it must be included by the semantics in any case.

[otmi100]

I agree unnecessary_await_in_return might be a common pitfall and either be removed, have a better explanation or an enhanced logic to detect if it is "unnecessary" for sure.. The lint sounds very optimistic about that, but in fact it has a different outcome when trying to catch errors: https://dartpad.dev/340b7252b1e979b6d6fc397e12cb6af8?

[Reprevise]

Since dart-lang/sdk#44395 is now on hold in favor of dart-lang/language#870, and that there's no clear timeframe on it landing (it's not even accepted into the language yet), can we get a lint for this in the meantime?

[rrousselGit]

Agreed. Modifying the language is way more difficult than adding a lint.

I think it's valuable to add the lint now, for a quick win. Then once the language is updated, we can depreciate the lint.