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.

Sign up for GitHub

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

Jump to bottom

Over-parenthesization with ArrowFunctionExpression #914

Open
NickHeiner opened this issue May 7, 2021 · 1 comment · May be fixed by #1128
Open

Over-parenthesization with ArrowFunctionExpression #914

NickHeiner opened this issue May 7, 2021 · 1 comment · May be fixed by #1128

Comments

@NickHeiner
Copy link
Contributor

When I parse and print an AST without changing it, I get extra parens inserted.

const recast = require('recast');

const ast = recast.parse(`
  const e = () => function(g, h) {
    return i;
  };

`)

console.log(recast.print(ast).code);

This outputs:

  const e = () => (function(g, h) {
    return i;
  });

We can see that the body of the ArrowFunctionExpression got an extra ( ).

Maybe related: #327, #533, #81

Using recast@0.20.4.
@NickHeiner NickHeiner mentioned this issue Sep 8, 2021
Open
gnprice added a commit to gnprice/recast that referenced this issue Jun 12, 2022
@gnprice
parens: Fix arrow-function bodies getting excess parens
d83b22e 
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
@gnprice gnprice linked a pull request Jun 12, 2022 that will close this issue
Open
@gnprice
Copy link
Contributor

gnprice commented Jun 12, 2022

I've sent a PR that should fix this issue: #1128.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Fix arrow-function bodies getting excess parens gnprice/recast
2 participants
@gnprice @NickHeiner