Disallow returning futures from `async` functions.

[lrhn]

In an async function with return type Future<T>, Dart currently allow you to return either a T or Future<T>.

That made (some) sense in Dart 1, where the type system wasn't particularly helpful and we didn't have type inference. Also, it was pretty much an accident of implementation because the return was implemented as completing a Completer, and Completer.complete accepted both a value and a future. If the complete method had only accepted a value, then I'm fairly sure the language wouldn't have allowed returning a future either.

In Dart 2, with its inference pushing context types into expressions, the return statement accepting a FutureOr<T> is more of a liability than an advantage (see, fx, dart-lang/sdk#40856).

I suggest that we change Dart to not accept a Future<T> in returns in an async function.
Then the context type of the return expression becomes T (the "future return type" of the function).

The typing around returns gets significantly simpler. There is no flatten on the expression, and currently an async return needs to check whether the returned value is a Future<T>, and if so, await it.
If T is Object, then that's always a possibility, so every return needs to dynamically check whether the value is a future, even if the author knows it's not.
This is one of the most complicated cases of implicit future handling in the language specification, and we'd just remove all the complication in one fell swoop.

And it would improve the type inference for users.

It would be a breaking change.
Any code currently returning a Future<T> or a FutureOr<T> will have to insert an explicit await.
This is statically detectable.
The one case which cannot be detected statically is returning a top type in a function with declared return type Future<top type>. Those needs to be manually inspected to see whether they intend to wait for any futures that may occur.
Alternatively, we can always insert the await in the migration, since awaiting non-futures changes nothing. It would only be a problem if the return type is Future<Future<X>> and the dynamic-typed value is a Future<X>. A Future<Future<...>> type is exceedingly rare (and shouldn't happen, ever, in well-designed code), so always awaiting dynamic is probably a viable approach. It may change timing, which has its own issues for badly designed code that relies on specific interleaving of asynchronous events.

That is the entirety of the migration, and it can be done ahead of time without changing program behavior^*. We could add a lint discouraging returning a future, and code could insert awaits to get rid of the lint warning. Then they'd be prepared for the language change too.

The change would remove a complication which affects both our implementation and our users negatively, It would make an implicit await in returns into an explicit await, which will also make the code more readable, and it will get rid of the implementation/specification discrepancy around async returns.

^*: Well, current implementations actually do not await the returned value, as the specification says they should, which means that an error future can get past a try { return errorFuture; } catch (e) {}. That causes much surprise and very little joy when it happens. I've also caught mistakes related to this in code reviews.

[natebosch]

This is statically detectable.
The one case which cannot be detected statically is returning a top type in a function with declared return type Future<top type>. Those needs to be manually inspected to see whether they intend to wait for any futures that may occur.

If we allow implicit cast from dynamic we wouldn't warn for any cases where the expression has static type dynamic, right? Would we special case that situation?

[lrhn]

No static warnings for dynamic, as usual.
The expression would be implicitly cast to T before being returned, just as a return statement in a non-async function.

Currently the expression should first be evaluated to an object, then that object should be checked for being a Future<T>, and if it is, it is awaited and the result becomes the new value, otherwise we use the original value. Then the resulting value is implicitly cast to T.

The issue here is not new, it's just that it might hide something which becomes a run-time error.

[festelo]

I think implementing this as analyzer rule is better, at least it doesn't require migration.

[jamesderlin]

BTW, this is the opposite of what the unnecessary_await_in_return lint recommends, so if this is done, the lint will need to be removed or disabled, and maybe analysis_options.yaml files will need to be updated during migration.

[mateusfccp]

BTW, this is the opposite of what the unnecessary_await_in_return lint recommends, so if this is done, the lint will need to be removed or disabled, and maybe analysis_options.yaml files will need to be updated during migration.

This is not the same case.

unnecessary_await_in_return prefers

Future<int> future;
Future<int> f1() => future;

over

Future<int> future;
Future<int> f1() async => await future;

Both, however, would be valid in this issue proposal, as the first case is not async and returns a Future<T>, and the second case is async and return a T (because of await).

What would not be valid is

Future<int> future;
Future<int> f1() async => future;

[natebosch]

BTW, this is the opposite of what the unnecessary_await_in_return lint recommends, so if this is done, the lint will need to be removed or disabled

Yes. The lint will need to be changed to only catch the case mentioned by @mateusfccp but allow it otherwise.

[natebosch]

There is a subtle inference detail that I think this could help with. Currently the inference type that gets used for an expression which is returned is FutureOr, which may not be what the user expects. If there is a call to a generic method as the returned expression it can have user visible impacts on the inference.

T someMethod<T>(T arg) {
  print(T);
  return arg;
}

Future<S> returnIt<S>(S arg) async {
  return someMethod(arg); // Infers someMethod<FutureOr<T>>
}

Future<S> assignIt<S>(S arg) async {
  final result = someMethod(arg); // Infers someMethod<T>
  return result;
}

void main() {
  returnIt(1);
  assignIt(1);
}

If we require the await to get to a T, that inference will work like user expectations.

[natebosch]

We could add a hint in the analyzer ahead of this change so that existing code can start to migrate ahead of time.