dart-lang / sdk

The Dart SDK, including the VM, JS and Wasm compilers, analysis, core libraries, and more.
https://dart.dev
BSD 3-Clause "New" or "Revised" License
10.28k stars 1.59k forks source link

new lint: `always_return_future_from_async ` #58164

Open pq opened 4 years ago

pq commented 4 years ago

DO: return Future<void> from async functions as opposed to void.


There's been a bit of lively discussion about this already. A few culled highlights:

@mehmetf @natebosch @lrhn @davidmorgan @matanlurey @dballesteros7: please feel free to chime in w/ nuances 🙏

TODO:

bwilkerson commented 4 years ago

As a side note, I'll suggest that we find a name that doesn't include "prefer". Perhaps something like always_return_future_from_async?

pq commented 4 years ago

Thanks Brian. Updated.

a14n commented 4 years ago

What would be the difference with avoid_void_async?

lrhn commented 4 years ago

I disagree with the lint in general. There is a place and reason for having void methods which do asynchronous computation. Disallowing a void/async function just forces people to do complicated workarounds. Example:

void log(String text) async {
  (await getLogger()).log(text);
}

becomes

void log(String text) {
  getLogger().then((logger) { logger.log(text); });
}

or

void log(String text) {
  () async {
    (await getLogger()).log(text); 
  }();
}

The reason to have a lint is that writing void by accident is a hard-to-detect problem. The author might have meant to return a Future<void> and forgot to write the Future<...>. This happens for other types too, but only top type are non-errors, and only void will suggest that the returned future is not handled.

If there is a way to detect some deliberate uses of void, then those should not be prohibited. There most likely isn't. A Future<void> returning async function looks exactly like a function that returns nothing.

So, if you go with the lint, it's because you consider the risk of using void/async functions too great. All async functions must visibly return a future, and all futures must be awaited, with all exceptions visibly marked in the source. That's a valid position to take. In that case I would not allow them to be used to implement a void interface method either. If anything, it's even easier to make a mistake when you didn't pick the return type yourself.