Implement a lint for unnecessary async

[a14n]

From style guide: DON’T use async when it has no useful effect.

[a14n]

One potentially issue is that it changes function's behaviour regarding exceptions.

For instance:

import 'dart:async';

void f1 () {
  throw Error();
}
Future<int> f2 () => Future.value(1);
Future<int> f3() async {
  f1();
  return f2();
}

f4() {
  f3().then(print).catchError(print);
}
f5() async {
  try{
    print(await f3());
  } catch (e) {
    print(e);
  }
}

A unnecessary_async would flag async on f3. But removing this async will lead to an uncaught error in f4 (f5 will have the same behaivour).

If all async code use async/await (and avoid then/catchError) it should be safe. As the style guide also promotes the usage of async/await I don't think it's a blocking issue and we should ignore this corner case.

WDYT?

[bwilkerson]

I think the key is the phrase "when it has no useful effect". In any code that might throw an exception (which is most code) it has an effect. Even without any code it has an effect in that it wraps the result in a future:

void f6() async {}

Determining whether this effect is "useful" is probably complex enough that it isn't worth implementing. About the only case I can think of where it has no effect is something trivial like

Future<int> f7() async => f2();

But if that's all we could lint, then it just isn't worth it. Can you think of other cases that would be safe (as in no false positives)?

[a14n]

Even removing async from f7 could not be safe depending on f2. For instance:

Future<int> f2 () {
  if (cond) throw Error();
  return Future.value(1);
}

Exceptions/errors make it quite impossible to create a lint for this rule.

[bwilkerson]

You're right, and I agree.

[a14n]

The only context that is safe to make the changes is when the call site is inside an async body. Without future.then(...) or future.catchError(...) (like f5) there's no problem to change f2. Perhaps the combinasion with the other rule PREFER async/await over using raw futures could improve the situation.

[a14n]

The only cases I can think of are :

returning_future() {
  Future asyncFunction() => throw Error();
  Future f1() async => Future.value(); // LINT
  Future f2() async // LINT
  {
    return Future.value();
  }

  Future f3_use_await() async // OK
  {
    return await Future.value();
  }

  Future f4_return_function() async // OK
      =>
      asyncFunction();
  Future f5_return_function() async // OK
  {
    return asyncFunction();
  }
}

returning_async_function() {
  Future asyncFunction() async {}
  Future f1() async => asyncFunction(); // LINT
  Future f2() async // LINT
  {
    return asyncFunction();
  }
}

[bwilkerson]

It isn't obvious to me that this would add enough value to be worth doing.

[annagrin]

unnecessary_async would be useful in this: https://github.com/dart-lang/webdev/pull/1982.

Additionally (or alternatively), we could suggest changing the return value if it is unnecessarily returning a future, if foo is not an implementation of some interface:

Future<String>  foo() async { // lint: unnecessary async - consider returning a value?
  // no awaits here
  return value;
}

Could that lead to performance improvement for the user code? See a bunch of examples in the PR above.

[incendial]

For anyone looking for this rule and willing to try DCM - we have it available https://dcm.dev/docs/rules/common/avoid-redundant-async/, any feedback is welcome!

[felipecastrosales]

any news about it?

[srawlins]

No news.