Fix arrow-function bodies getting excess parens #1128

This makes it a lot easier to see what's going on when test cases fail, because we can now see which ones passed and which failed in any given group. We leave the "Arithmetic" group as a single test, because it contains a very large number of test cases.

Printing the entire source code for each side of the comparison, rather than just the first node path where they differ, makes it easier to see what went wrong in printing.

I don't quite see why the justNeedsOpeningParen logic would be necessary. If having just an opening parenthesis is good enough for those nodes (and indeed it seems like it is), then it seems like it should be good enough for any node. I.e., we could treat justNeedsOpeningParen as always true. And if we do so, all the tests still pass. So I'm now pretty confident that we can in fact cut that logic out. This code gets quite a bit simpler when we do, so that's nice! Then in particular, cutting out this pair of calls to firstInStatement and canBeFirstInStatement leaves just one remaining call site for each of them. That makes things a lot simpler for us when we go to make changes to how those functions work -- which we're about to do, in order to fix some bugs.

We just cut the only call site that passed this flag, in hasParens.

There's another copy of these a few lines above.

Previously, no tests would fail if you commented out the check for ClassExpression in canBeFirstInStatement. As these new test cases show, that check is indeed necessary.

As it was, if there was a non-`n.Node` value in the middle of the stack (is that even possible?), then we'd recheck the same parent and child as we did on the last iteration. That can't do anything useful; instead, just have each iteration always check either the current parent and child, or nothing.

To get here, the child would have to already be a statement, so we'd have stopped at the previous step. All tests still pass. In fact, all tests still pass if instead of deleting this one replaces the body with `assert.fail()`. So that adds empirical evidence that this case is impossible.

…ement These are fundamentally in exactly the same situation as a CallExpression, a BinaryExpression, etc.

We're going to change how this works, to fix some bugs. Putting the logic here will simplify that.

We were inserting parentheses unnecessarily in the bodies of some arrow functions. In particular, `() => function(){}` would become `() => (function(){})`, and `() => class {}.foo.bar` would become `() => (class {}).foo.bar`. The cause turns out to be that we were taking the logic that applies if you have an *expression statement* starting with a FunctionExpression or ClassExpression -- which does indeed need parens, to avoid getting parsed as a FunctionDeclaration or ClassDeclaration respectively -- and applying it the same way if one of those expressions is instead at the start of a braceless arrow-function body, aka an ExpressionBody. In fact, while an ExpressionBody does call for similar intervention if it starts with a `{` token (so with an ObjectExpression node), that is the only case where it does. In the ES spec, this is expressed as lookahead constraints, on the one hand at ExpressionStatement: https://tc39.es/ecma262/#prod-ExpressionStatement and on the other hand at the two productions where ExpressionBody appears: https://tc39.es/ecma262/#prod-ConciseBody https://tc39.es/ecma262/#prod-AsyncConciseBody Adjust the logic accordingly to distinguish the two situations, and add tests. The ExpressionStatement lookahead constraints also point at one more edge case which we don't yet correctly handle. We'll fix that in the next commit for the sake of making the logic comprehensive, along with comments explaining how it corresponds to the spec. Fixes: benjamn#914 Fixes: benjamn#1082

This is a fun edge case I discovered from reading the ES spec, in particular here: https://tc39.es/ecma262/#prod-ExpressionStatement while working out whether there were cases we were missing in this logic, beyond the three we had. I believe this turns out to be the only such case. Add comments explaining why, with references to the language spec.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix arrow-function bodies getting excess parens #1128

Fix arrow-function bodies getting excess parens #1128

Commits on Jun 12, 2022

Fix arrow-function bodies getting excess parens #1128

Are you sure you want to change the base?

Fix arrow-function bodies getting excess parens #1128

Commits on Jun 12, 2022