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.

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

.print add uneeded parenthsis even without modifications #1082

Open
dedesite opened this issue Apr 15, 2022 · 3 comments · May be fixed by #1128
Open

.print add uneeded parenthsis even without modifications #1082

dedesite opened this issue Apr 15, 2022 · 3 comments · May be fixed by #1128

Comments

@dedesite
Copy link

dedesite commented Apr 15, 2022

The README states that :

The magic of Recast is that it reprints only those parts of the syntax tree that you modify. In other words, the following identity is guaranteed:

recast.print(recast.parse(source)).code === source

But I came with a code that do not guaranty the above statement : https://astexplorer.net/#/gist/e94f3e50e6148f56b57022fbf5f04f68/latest

I know it's not a big deal, but it could be fixed, it would be great 👍
Otherwise reapplying our code formatter at the end will do the tricks.

@sincerexie
Copy link

Hi, have you solved the problem now?

@gnprice
Copy link
Contributor

gnprice commented May 25, 2022

Quoting the input and output from that link for handy reference in the thread:

// input:
const SubClass = (MainClass) =>
  class extends MainClass {
  };

// output:
const SubClass = (MainClass) =>
  (class extends MainClass {
  });

So a pair of parens is getting added around the body of an arrow function, in a case where it's not needed. Specifically, when the body is a class expression / ClassExpression.

I don't have a diagnosis, but this looks likely related to the same area of code that was involved in issue #743 / PR #1068.

gnprice added a commit to gnprice/recast that referenced this issue Jun 12, 2022
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
@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
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants