Sdk: Warnings on missing async are very unhelpful

[lukechurch]

Consider the following code:

main() {
  var x = await foo();
}

foo() async {
  print ("Hi");
}

The return from Analyzer is:
info: The value of the local variable 'x' is not used
error: Expected to find ';'
warning: Undefined name 'await'

The return from Dart2JS is:

error: line 2 pos 17: semicolon expected
Warning: Cannot resolve 'await'.
  var x = await foo();

The return from the VM is:
error: line 2 pos 17: semicolon expected

====

None of these help the user fix the problem, which is that the main method is missing an async.

I suggest that we add a specific case here that reports something like

"await may only be used in a method marked async. consider replacing 'main() {' with 'main() async {'"

@Gilad - do you agree? Is there a wording change we need of 'marked async'?

[lukechurch]

cc @lrhn.

[gbracha]

This isn't a language issue per se. It is a QoI issue. Nothing in the spec prevents an implementation from say, recognizing that the last token was await (or yield) and spitting out a "did you mean to declare the surrounding function async or async*" etc.

---

cc @mhausner.

[DartBot]

_This comment was originally written by @mhausner_

---

This is a language issue. A compiler or analyzer cannot give a meaningful hint because await is just a normal identifier in a non-async code. Consider this:

await -1;

This is legal in both async and normal functions, but has different meanings. The example given in comment 0 is not legal, but in order to give a warning the compiler would have to somehow recognize that it has seen the word "await" and that the syntax error is related to that word.

Instead of building AI into a compiler, we should make the async/await names proper keywords in all of Dart.

[lrhn]

I agree that "await -x;" can't be a compile-time error, and its runtime semantics is fixed to be a NoSuchMethodError, so there isn't much the VM can do. It's also an error that is unlikely to happen because "-x" isn't a Future expression, so I'm not too worried about that.

If "await" isn't declared as an identifier, then static analysis must still give a warning, and nothing says that this warning can't suggest that you add an async marker to the function instead of just saying that "await" is not declared, so the analyzer and offline compilers can give a better warning for this case.
If "await" is declared as an identifier, then it's probably deliberate and everything is fine.

The more likely error is just "await x;" or "await expr;" where expr is some expression evaluating to a future, and which is often not valid syntax in a non-async function.
The VM can give error messages when the syntax is not valid, and again, the language specification does not say what that error must contain - the only requirement is that compilation fails - so it's perfectly fine to say "'await expr' is only valid in async functions. Did you mean to make the function async?".

The one semi-likely case that we cannot catch is "await (expr);". Again, that can only be caught by static analysis, not the VM.

There is no language change needed for giving these errors when the program is an error. I don't see any reasonable language change that can help that.

This request is about giving better error messages for existing compilation errors and static warnings when they look like an attempt to do await in a non-async function.

The VM can only do the compilation errors.
The analyzer can only to the static warnings.
Dart2js and DDC can do both.
There is no need for a language or spec change to support this, it's purely a quality-of-life change for users.

[gbracha]

What Lasse said.

[lukechurch]

Brian,

Could we implement first in the Analyzer?

---

cc @bwilkerson.
cc @stereotype441.
cc @danrubel.
_Set owner to @bwilkerson._
_Removed Area-Language label._
_Added Area-Analyzer, Analyzer-UX labels._

[bwilkerson]

Could we implement first in the Analyzer?

I think we need to improve the messages in both the VM and in the analyzer, and personally I don't care what order the work happens in.

[iposva-google]

Regarding comment #­4 there is no concept in the VM that some expression evaluates to a Future. So given the current syntax rules, there is really not much indication what the user wanted.

You all make it sound like you are advocating to make await a keyword, because all your arguments are centered around: If the user wrote await, then he really meant to use the async-await combinations and all other possibilities are invalid.

[lrhn]

The only thing I'm asking the VM for is that syntax errors of the form "await expression" (inside non-async functions, necessarily) are reported more helpfully, for example by suggesting to make the function async.

[sigmundch]

FWIW - I just created a change in dart2js to give these suggestions too: https://codereview.chromium.org/1094903002/

[sigmundch]

seems like this and #22625 are the same, so I'll close this issue as a duplicate.

[sethladd]

Love the dart2js message, when it encounters await without an async. Very cool.

https://codereview.chromium.org/1094903002/diff/80001/pkg/compiler/lib/src/warnings.dart

[lukechurch]

+1

[trinarytree]

I just got burned by this yet again. It makes you waste your time looking everywhere for a mismatched parenthesis or a missing semicolon or something like that. Is there any ETA on a fix?