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

recast generates invalid code when printing destructuring assignment expressions preceded by comments #575

Open
not-an-aardvark opened this issue Feb 16, 2019 · 1 comment · May be fixed by #1129

Comments

@not-an-aardvark
Copy link

Using recast 0.17.3:

const recast = require('recast');
const ast = recast.parse('(/**/{foo} = 1)');
const assignmentExpressionText = recast.print(ast.program.body[0].expression).code;
console.log(assignmentExpressionText);

Expected output: something like (/**/{foo} = 1) or {foo} = 1
Actual output: (/**/)({foo} = 1)

The output is invalid code because the initial parentheses are only surrounding a comment and nothing else.

gnprice added a commit to gnprice/recast that referenced this issue Jun 12, 2022
We were taking code like the following:

    (/**/{foo} = 1)

and wrapping the comment in parens, which is invalid syntax:

    (/**/)({foo} = 1)

The symptom appears when using the default parser, but not esprima;
and only when you call the printer directly on the AST node for the
expression, not the whole program.

The bug here was that in needsParens, we check to see if `node` is
an AssignmentExpression with ObjectPattern, and if so we say that
we do need parens... but we were making that check at the very top
of the function, before we'd checked that `node` was actually the
thing we wanted to be operating on at all.

If in fact the path currently points to a comment -- as it can
when needsParens gets called by the printer invoked by the
`print(commentPath)` in printLeadingComment or printTrailingComment
-- then the comment doesn't count as a node, and so `getNode` will
actually return the parent.  If that parent meets the
AssignmentExpression / ObjectPattern condition, then we would
buggily decide that the comment needed parens too.

Fix it by checking that `node` is the actual top of the stack,
i.e. `this.getValue()`, before doing anything with `node`.
Also add a regression test.

Fixes: benjamn#575
@gnprice
Copy link
Contributor

gnprice commented Jun 12, 2022

I've just sent a PR that fixes this issue: #1129.

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.

2 participants