dart-lang / sdk

The Dart SDK, including the VM, dart2js, core libraries, and more.
https://dart.dev
BSD 3-Clause "New" or "Revised" License
9.94k stars 1.53k forks source link

I would like a `--strict-await` static analysis option (2.1+) #33823

Open matanlurey opened 5 years ago

matanlurey commented 5 years ago

Branched from https://github.com/dart-lang/sdk/issues/33415#issuecomment-404253308 and original work by @MichaelRFairhurst.

There are many remaining places in the language where await can be misused, and some of them (like await <void>) didn't make the cut for Dart 2.0, and others (like await 5) are, for some reason, important to retain in the language. In the set of optional strict static analysis checks (see https://github.com/dart-lang/sdk/issues/33749), I'd like to see these addressed:

void aVoidMethod() {
  /* ... */
}

Future<void> asyncMethod() async {
  // ERROR: strict_await_void
  await aVoidMethod();

  // ERROR: strict_await_not_a_future
  await 5;
  await 'Hello';

  // ERROR: strict_await_unnecessary
  return await new Future.value();
}

(My rationale for not making this a lint is the same as #33749)

MichaelRFairhurst commented 5 years ago

One comment, I am still trying to land #33415. Its officially punted, but, I hope I can clean up the code to where its an easy decision to turn on or not.

That said, even if I manage to do that in a timely manner, the decision may still be that its not worth the effort given that it can be accomplished with lints (and in fact, such a lint already exists: await_only_futures).

Though notably that should maybe be split up, since awaiting void is much more suspect than awaiting a primitive (for some reason), or an Object which may in fact just be a Future at runtime.

The other benefit of my cleanup is I will be able turn such a lint (but not await_only_futures) after that work is done, even if the language restriction doesn't land.

jmesserly commented 5 years ago

My rationale for not making this a lint is the same as #33749

I looked at that issue, but I didn't see the rationale for implementing something as an Analyzer flag versus a lint, would it be possible to elaborate on that? (Apologies if I missed it).

I'm trying to get a mental model for when to choose one implementation strategy over the other. (I implemented no-implicit-* as Analyzer flags because they depended on strong mode, but I'm not sure if that rationale still applies now that Dart 2 is released.)