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

Transforming destructure reassignment results in extra parenthesis #533

Open
Retsam opened this issue Aug 12, 2018 · 2 comments
Open

Transforming destructure reassignment results in extra parenthesis #533

Retsam opened this issue Aug 12, 2018 · 2 comments
Assignees

Comments

@Retsam
Copy link

Retsam commented Aug 12, 2018

If I translate code with a destructuring reassignment, i.e. destructuring syntax, outside of a VariableDeclaration, e.g. ({ foo } = bar);, then the result ends up wrapped in an additional, unnecessary layer of parentheses, e.g. ({ foo } = bar)).

(Example from AST Explore)

I believe this to be largely the same underlying causes as #327 , so I suspect there's nothing to be done about it, at the moment, but I thought it'd be worth opening an issue to track it nonetheless.

(Sorry for some edit noise, here. I accidentally posted this way before it was ready to be posted)

@Retsam Retsam changed the title Transforming Transforming destructure reassignment results in extra parenthesis Aug 12, 2018
@benjamn benjamn self-assigned this Aug 12, 2018
@benjamn
Copy link
Owner

benjamn commented Aug 12, 2018

I think you're right about the similarity to #327, but I'm happy to keep this issue open because you've found a slightly different test case.

benjamn added a commit that referenced this issue Sep 10, 2018
Recast has suffered for a long time because it did not have reliable
access to the lexical analysis of source tokens during reprinting.

Most importantly, accurate token information could be used to detect
whether a node was originally wrapped with parentheses, even if the
parentheses are separated from the node by comments or other incidental
non-whitespace text, such as trailing commas. Here are just some of the
issues that have resulted from the lack of reliable token information:
- #533
- #528
- #513
- #512
- #366
- #327
- #286

With this change, every node in the AST returned by recast.parse will now
have a node.loc.tokens array representing the entire sequence of original
source tokens, as well as node.loc.{start,end}.token indexes into this
array of tokens, such that

  node.loc.tokens.slice(
    node.loc.start.token,
    node.loc.end.token
  )

returns a complete list of all source tokens contained by the node. Note
that some nodes (such as comments) may contain no source tokens, in which
case node.loc.start.token === node.loc.end.token, which will be the index
of the first token *after* the position where the node appeared.

Most parsers can expose token information for free / very cheaply, as a
byproduct of the parsing process. In case a custom parser is provided that
does not expose token information, we fall back to Esprima's tokenizer.
While there is considerable variation between different parsers in terms
of AST format, there is much less variation in tokenization, so the
Esprima tokenizer should be adequate in most cases (even for JS dialects
like TypeScript). If it is not adequate, the caller should simply ensure
that the custom parser exposes an ast.tokens array containing token
objects with token.loc.{start,end}.{line,column} information.
benjamn added a commit that referenced this issue Sep 11, 2018
Recast has suffered for a long time because it did not have reliable
access to the lexical analysis of source tokens during reprinting.

Most importantly, accurate token information could be used to detect
whether a node was originally wrapped with parentheses, even if the
parentheses are separated from the node by comments or other incidental
non-whitespace text, such as trailing commas. Here are just some of the
issues that have resulted from the lack of reliable token information:
- #533
- #528
- #513
- #512
- #366
- #327
- #286

With this change, every node in the AST returned by recast.parse will now
have a node.loc.tokens array representing the entire sequence of original
source tokens, as well as node.loc.{start,end}.token indexes into this
array of tokens, such that

  node.loc.tokens.slice(
    node.loc.start.token,
    node.loc.end.token
  )

returns a complete list of all source tokens contained by the node. Note
that some nodes (such as comments) may contain no source tokens, in which
case node.loc.start.token === node.loc.end.token, which will be the index
of the first token *after* the position where the node appeared.

Most parsers can expose token information for free / very cheaply, as a
byproduct of the parsing process. In case a custom parser is provided that
does not expose token information, we fall back to Esprima's tokenizer.
While there is considerable variation between different parsers in terms
of AST format, there is much less variation in tokenization, so the
Esprima tokenizer should be adequate in most cases (even for JS dialects
like TypeScript). If it is not adequate, the caller should simply ensure
that the custom parser exposes an ast.tokens array containing token
objects with token.loc.{start,end}.{line,column} information.
@gnprice
Copy link
Contributor

gnprice commented Jun 12, 2022

It looks like this issue no longer reproduces.

Here's the output at the given AST Explorer link, at recast 0.21.1:
({ foo } = bar)

So that's the one necessary layer of parentheses (if it were just { foo } = bar, the leading { wouldn't introduce an expression at all but instead a BlockStatement, and then the = would be a syntax error), but no unnecessary additional layer.

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

No branches or pull requests

3 participants