linter icon indicating copy to clipboard operation
linter copied to clipboard

proposal: `unnecessary_future_value_in_async_return`

Open asashour opened this issue 3 years ago • 4 comments

unnecessary_future_value_in_async_return

AVOID returning a Future value in the return statement of an async function.

Details

There is no need to wrap the returned value in Future.value in the return of an async function.

Good Examples

Future<int> f() async {
  return 1;
}
Future<int>? g() => null;

Future<int> f() async {
  var r = g();
  return r ?? Future.value(1);
}

Bad Examples

Future<int> f() async {
  return Future.value(1);
}

Discussion

Add any other motivation or useful context here.

Discussion checklist

  • [ ] List any existing rules this proposal modifies, complements, overlaps or conflicts with.
  • [ ] List any relevant issues (reported here, the SDK Tracker, or elsewhere).
  • [ ] If there's any prior art (e.g., in other linters), please add references here.
  • [ ] If this proposal corresponds to Effective Dart or Flutter Style Guide advice, please call it out. (If there isn’t any corresponding advice, should there be?)
  • [ ] If this proposal is motivated by real-world examples, please provide as many details as you can. Demonstrating potential impact is especially valuable.

asashour avatar Sep 01 '22 05:09 asashour

The lint name seems too general to me. I thought about unnecessary_future_in_async, but that still isn't quite as specific as the proposed rule. I considered unnecessary_future_value_in_async, but I'm assuming that the lint would only flagging the use in return statements. The name unnecessary_future_value_in_return_in_async is closer, but (a) it's really long and (b) it still misses some cases. For the latter, I'm assuming that something like the following would be considered valid:

Future<int>? g() => null;

Future<int> f() async {
  // Code that uses `await`.
  var r = g();
  return r ?? Future.value(0);
}

The description and details would also need to be updated to capture the full semantics of the lint.

But given the limited number of cases that would be caught by this (recent changes to the analyzer package aside), I have to wonder how valuable this lint would be. It might be useful to run something like it over a few large packages to find out.

bwilkerson avatar Sep 01 '22 14:09 bwilkerson

Consider a name of await_future_in_async_return.

Description:

In a return from an async function, don't return an unawaited future, but instead await the future and return the result.

Details

Dart currently allows you to return either an int or a Future<int> from a async function with return type Future<int>, but intends to remove the ability to return the Future<int> at some point. You should always return the int, which may mean awaiting a future before returning its result.

Good Examples

Future<int> f(String input) async {
  Future<int> intermediate = computation(input);
  return await intermediate;
}

Bad Examples

Future<int> f(String input) async {
  Future<int> intermediate = computation(input);
  return intermediate; // No await, returning a future.
}

lrhn avatar Oct 06 '22 14:10 lrhn

Thanks for hinting, I would change the description more, but I guess the intention is something else, the return statement can directly return a value (e.g. 1) instead of Future.value(1);

Good:

Future<int> f() async {
  return 1;
}

Bad:

Future<int> f() async {
  return Future.value(1);
}

and not:

Future<int> f() async {
  return await Future.value(1);
}

asashour avatar Oct 06 '22 16:10 asashour

ACK, if this lint is just checking for return Future.value(e);, then it's fine. (I can see that happening if you convert a non-async function to async.) I was thinking the lint would be more aggressive and encourage return await someOtherAsyncFunction(); over return someOtherAsyncFunction() too, but that's not this lint.

With that in mind, my new suggestion is:

Description:

AVOID creating a new future just to return a value from an async function.

Details

Dart currently allows you to return either an int or a Future<int> from a async function with return type Future<int>. You should not wrap a value in a new future in order to return it, but instead just return the value directly.

Be particularly aware of this when converting a non-async function returning a Future to an async function.

Good Examples

Future<int> f(String input) async {
  /// Something.
  return input.length;
}

Bad Examples

Future<int> f(String input) async { 
  /// Something.
  return Future.value(input.length); // Valid, but unnecessary.
}

lrhn avatar Oct 10 '22 12:10 lrhn