Don't allow cascades on expressions with infix operators. about language

[rrousselGit]

Someone made a tweet about how they got confused by this behavior https://twitter.com/CptPixel/status/1580596198221545476

From the answers, it is pretty clear that this behavior is unexpected to most people.

I would say that a ?? b..foo() clashes with that of a ?? b.foo().

[lrhn]

The 13 levels of syntax that we would change the behavior of are:

test ? e1 : e2 ..cascade // conditionalExpression
e1 ?? e2..cascade // ifNullExpression
e1 || e2..cascade  // logicalOrExpression
e1 && e2..cascade  // logicalAndExpression
e1 == e2..cascade  // equalityExpression
e1 is Type..cascade  // relationalExpression
e1 as Type..cascade  // relationalExpression (potentially useful)
e1 < e2..cascade  // relationalExpression
e1 | e2..cascade // bitwiseOrExpression
e1 ^ e2..cascade // bitwiseXorExpression 
e1 & e2..cascade // bitwiseAndExpression
e1 << e2..cascade // shiftExpression
e1 + e2..cascade // additiveExpression
e1 * e2..cascade // multiplicativeExpression
~e1..cascade // unaryExpression
await e1..cascade // unaryExpression
++e1..cascade  // unaryExpression

Of these, only e1 as Type..cascade and the unary expressions actually do what they look like. Everything else looks like the cascade should bind to the last operand because . traditionally binds stronger than anything with a space in it. So, yes, I do think that it's better at every level except unaryExpression.

I am considering whether we should use unaryExpression instead of postfixExpression as where we allow cascades. It's probably better, so let's do that.

The e1 as Type..cascade might be useful, but is still much more readable as (e1 as Type)..cascade (and I want a suffix cast operation anyway, in which case it would be moot).

[leafpetersen]

Just hit this one as well, anything we can do about it?

  var a = Future.value(3);
  var x = await (int == String) ? a : a;
  print("Surprise! ${x.runtimeType} != int");

[eernstg]

The grammar change will move the first part of a cascade 13 levels down the derivation hierarchy, which means that we will outlaw 13 different shapes of expression. I do recognize that many of them will yield boolean values and in other ways be "unlikely" candidates for the intended target of a cascade, but it might also be rather massively breaking. So we need to check for that. Otherwise, I like the idea of making cascade targets less expressive such that the cascade won't swallow too much.

For the await issue we'd have the same danger that it might be massively breaking, but we could consider the following (where I haven't included the change for cascades):

expression
    :    (formalParameterPart functionExpressionBodyPrefix) =>
         functionExpression
    |    awaitExpression
    |    throwExpression
    |    (assignableExpression assignmentOperator) =>
         assignableExpression assignmentOperator expression
    |    conditionalExpression cascadeSection*
    ;

expressionWithoutCascade
    :    (formalParameterPart functionExpressionBodyPrefix) =>
         functionExpressionWithoutCascade
    |    awaitExpression
    |    throwExpressionWithoutCascade
    |    (assignableExpression assignmentOperator) =>
         assignableExpression assignmentOperator expressionWithoutCascade
    |    conditionalExpression
    ;

unaryExpression
    :    (prefixOperator ~SUPER) => prefixOperator unaryExpression
    # Delete this case: |    (awaitExpression) => awaitExpression
    |    postfixExpression
    |    (minusOperator | tildeOperator) SUPER
    |    incrementOperator assignableExpression
    ;

awaitExpression
    :    AWAIT expression
    ;

This would make await swallow everything, forcing (await e) whenever anything else is intended. We could find some intermediate level, of course, and I'm not sure where the "natural" placement would be.

[lrhn]

@leafpetersen
I'm not sure I want to change await syntactically. When I see an await, I expect it to apply to the following "dense expression", but not past any operators, including the ? and : of the conditional expression.

await a + b  // (await a) + b
await a ? b : c  // (await a) ? b : c
await a++   // await (a++)
await -a  // await (-a)
await foo.bar = 42  // allowed? If so, then await (foo.bar = 42)

So, I very much want an awaitExpression to be AWAIT unaryExpression, it exactly corresponds to how I expect it to work.

What comes back to bite us here is that we allowed awaiting a bool, even though it's useless. If we didn't, your program would have been stopped there. That I would like to change (while still allowing FutureOr).

I would also like to change throw to take a unaryExpression. I don't remember the exact expressions that were an issue, just that here were issues.

(In either case, that's a separate topic)

[eernstg]

The original reason for putting cascade at a very global position (such that it binds very weakly) was that this is required in order to enable cascadeSection to include assignments,

e1..cascade = e2
    ..foo(42)
    ..cascade = e3;

which are otherwise only present at the very top level (expression). I suspect that we will have to fight with that if we try to move cascades down to a much more tightly binding level.

It sounds like you're saying that this looks like

e1..cascade = ((e2..foo(42))..cascade = e3);

which is probably a bridge too far.

[lrhn]

The way I specified it wasn't to move cascades down as unaryExpression :: prefixExpression cascadeSection*, but to keep it at expression :: prefixExpression cascadeSection* | conditionalExpression.

I don't think that will change the behavior (also because cascadeSection uses expressionNoCascade).

[srawlins]

One of the very typical cases of "unused parentheses" is to make the precedence of expressions like this explicit. It is called out in the description: https://dart-lang.github.io/linter/lints/unnecessary_parenthesis.html

Before such a language change, we could imagine a new lint rule, "required unnecessary parentheses".