benjamn / recast

JavaScript syntax tree transformer, nondestructive pretty-printer, and automatic source map generator
MIT License
5.01k stars 350 forks source link

Reprinting file causes line break changes #215

Open sophiebits opened 9 years ago

sophiebits commented 9 years ago

Parsing and reprinting

// I have a parent
newNode.parent
  && (
    (// Or I did but its different than the one I have now.
      !originalNode.parent ||
      newNode.parent.key !== originalNode.parent.key
    )
  )

outputs

// I have a parent
newNode.parent
  && (
    (// Or I did but its different than the one I have now.
      (!originalNode.parent || newNode.parent.key !== originalNode.parent.key)
    )
  )

which is inconvenient for codemods. Verified using 0.10.32 by running

console.log(recast.print(recast.parse("// I have a parent\nnewNode.parent\n  && (\n    (// Or I did but its different than the one I have now.\n      !originalNode.parent ||\n      newNode.parent.key !== originalNode.parent.key\n    )\n  )")).code)

in node.

benjamn commented 9 years ago

Does the same problem occur if the // Or I did... comment is moved to the line before the && (?

I'm pretty sure the problem lies here. Specifically, hasParens is kind of dumb and doesn't know how to skip over comments to find existing parentheses, so this code is paranoid and forcibly adds additional parentheses.

sophiebits commented 9 years ago

It does not. I'll see if I have time to send a PR.

gnprice commented 2 years ago

This issue happily no longer reproduces (as of 0.21.1.)

Running the very handy one-step original repro now gives:

> console.log(recast.print(recast.parse("// I have a parent\nnewNode.parent\n  && (\n    (// Or I did but its different than the one I have now.\n      !originalNode.parent ||\n      newNode.parent.key !== originalNode.parent.key\n    )\n  )")).code)
// I have a parent
newNode.parent
  && (
    (// Or I did but its different than the one I have now.
      !originalNode.parent ||
      newNode.parent.key !== originalNode.parent.key
    )
  )

just like the original version of the code.

Given this diagnosis:

Specifically, hasParens is kind of dumb and doesn't know how to skip over comments to find existing parentheses, so this code is paranoid and forcibly adds additional parentheses.

the change that fixed it was probably #537, which gave hasParens the ability to look at the input's token stream instead of its character stream.