Retain comments inside return statements

[weswigham]

by calculating its position for comment emit. Some transformations may cause comments to be lost; but in the general case when a node is transformed into a node of the same kind, they should be preserved.

Fixes #17532

[DanielRosenwasser]

I am slightly concerned about adding another token node for every return statement. At what point does the tree become concrete for all terminal nodes? Is that a cost we're willing to pay?

[weswigham]

@DanielRosenwasser I think I could get the correct position by getting the statement start, skipping trivia, then adding "return".length; but that's another special case in the emitter for return statements, which also should be evaluated. If it works (it should?), then I'm OK swapping to doing it that way; its just a fair bit of late calculation for something we already recorded way back in the parser, but decided not to keep.

It's a matter of AST size vs emit time (a small delta, either way); so which is preferable to minimize?

[rbuckton]

I think I could get the correct position by getting the statement start, skipping trivia, then adding "return".length

This is the approach we took for PropertyAccess and it works well enough. Currently, emitReturnStatement uses writeToken which already does position tracking for source maps. We could augment that to emit comments as well, and possibly end up with higher fidelity for comment emit in other places.

[rbuckton]

I'd rather not increase the size of the node. Instead, consider modifying writeTokenText in emitter to emit comments as well as source maps.

Keep in mind that writeTokenText is used quite a bit throughout the emitter. Changes could have an impact on emit performance. If the impact is minimal that change would be preferable. If it is significant then we either need to find ways to mitigate the cost or consider the approach in the current PR.

[scf37]

This issue seems to affect all rhs expressions:

function f() {
  var a = /* foo */ 1;
  var b = (1 + /* bar */ a);
  return /* foobar */ (a + b);
}

compiles to

function f() {
    var a = 1;
    var b = (1 + a);
    return (a + b);
}

[weswigham]

@scf37 It's not all RHS expressions, but rather all trailing comments of a token which appears in the middle of a parsed node. Once this fix is merged, I plan on looking into doing a similar operation to try to retain comments on other such tokens.

[rbuckton]

@weswigham Is there an issue for the other token positions where we don't preserve comments? If not, can you add one?

[weswigham]

Opened - I was able to find a general issue about generally better comment emit - #2546 - but nothing specific to this area.