Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
matanlurey opened this issue Jul 11, 2018 · 2 comments
Open

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

matanlurey opened this issue Jul 11, 2018 · 2 comments
Labels
analyzer-warning Issues with the analyzer's Warning codes area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. P2 A bug or feature request we're likely to work on type-enhancement A request for a change that isn't a bug

Comments

@matanlurey
Copy link
Contributor

Branched from #33415 (comment) 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 #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
Copy link
Contributor

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.

@dgrove dgrove added area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. type-enhancement A request for a change that isn't a bug labels Jul 15, 2018
@jmesserly
Copy link

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.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
analyzer-warning Issues with the analyzer's Warning codes area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. P2 A bug or feature request we're likely to work on type-enhancement A request for a change that isn't a bug
Projects
None yet
Development

No branches or pull requests

5 participants