Update map literal formatting test

[danrubel]

This test case is invalid Dart syntax that the old analyzer parser let slide by.
From section 17.2 of the language spec...

An expression statement consists of an expression other than a non-constant map literal (16.8) that has no explicit type arguments. The restriction on maps is designed to resolve an ambiguity in the grammar, when a statement begins with {.

[munificent]

Is this change necessary? The expression in the expression statement isn't a map literal, it's a method invocation (.someLongMethod()) whose receiver is a map literal. Is that not valid?

[danrubel]

Yes, the change is necessary.
No, the expression statement is not valid.
Although it's valid as an expression, it's not valid as an expression statement.

The problem is that the statement begins with { and to the parser when it's parsing a statement, the { looks like the start of a block. When parsing a statement (not an expression), if the parser supported { as either a block or an expression statement, then it would have an arbitrarily long lookahead to figure out if the thing after the { was an expression followed by a : and only then would it be able to decide whether to parse the { as a block or an expression statement.

The spec has the clause above so that the parser does not have an arbitrarily long lookahead (and possible ambiguity) each time it parsed a statement beginning with {.

[bwilkerson]

@leafpetersen @lrhn For clarification of the spec.

[leafpetersen]

The spec language doesn't seem to me to cover this case. As best as I can tell, none of our release mode compilers (dart2js, dart) support this or any related things that I've tests (cascade, subscript, update), so I think it would be de facto non-breaking to make the spec reflect what seems to be the intention. That is:

An {\em expression statement} consists of an expression other than a non-constant map literal (\ref{maps}) that has no explicit type arguments.

Should probably be:

An {\em expression statement} consists of an expression that does not begin with a non-constant map literal (\ref{maps}) that has no explicit type arguments.

[leafpetersen]

@munificent Do you mind if this gets landed to unblock the analyzer/parser integration? We'll have to sort out the spec decision and if necessary all of the implementations separately, and I don't see any reason to block on this now.

[munificent]

@leafpetersen your understanding aligns with mine on the spec. I'm happy to change the test to match the intention of the spec.

[lrhn]

We should specify what the implementations do.
Did this work in dart2js/VM before they moved to CFE? If so, it's a CFE bug. If not, it's probably safer to just not make it work now either.

The grammar works either way - if a block ends with }.foo(); then it's not a code block, so it must be a map, and the grammar is not ambiguous. (It might not even be ambiguous without the trailing call, unless the block is empty, since any statement inside will be recursively distinguishable from an expression by, eventually, ending in a semicolon).

Allowing leading maps does make parsing need arbitrary look-ahead to determine whether something starting with {foo: verylargeexpression is a map or a code block starting with a labeled statement. (If it's followed by ';' it's a statement, if it's followed by '}', it's a map.
It's not enough to just look after the end-brace (we, foolishly, allow empty statements outside of loop bodies), so:
{foo: {bar: {baz: {qux: 0 ; } } } };
is two statements, but could be an expression without the semicolon if we allowed it.

So, the simplest is to just keep the current CFEbehavior and codify it in the spec.
(The "begins with expression" is too vague for my part. I'd prefer to just say that an expression statement cannot start with a '{' character).

[bwilkerson]

@munificent Thanks!

We also need the updated version of dart_style to be pulled into the SDK. Could you publish it and update the DEPS file to bring it it?

[leafpetersen]

For the record, in SDK 1.13 (which is before the VM and the dart2js switched to the CFE, I believe), all of these examples were rejected by the VM and dart2js:

void main() {
  {"foo" : "bar"}.containsKey("foo");
  {"foo" : "bar"}..containsKey("foo");
  {"foo" : "bar"}["foo"];
  {"foo" : "bar"}["foo"] = "baz";
}

[lrhn]

Then I see no problem in keeping them disallowed, let's just do that. I'll fix the specification.
https://dart-review.googlesource.com/c/sdk/+/71247