Trailing line comments are being turned into leading comments...

[zertosh]

but only line comments, not block comments.

var recast = require('recast');

var ast = recast.parse([
  'module.exports = {};',
  '// trailing line comment',
  '/**',
  ' * trailing block comment',
  ' */'
].join('\n'));

recast.visit(ast, {
  visitNode: function(path) {
    this.traverse(path);
    path.value.original = null;
  }
});

console.log(recast.print(ast).code);

Actual:

// trailing line comment
module.exports = {};
/**
 * trailing block comment
 */;

Expected:

module.exports = {};
// trailing line comment
/**
 * trailing block comment
 */;

[benjamn]

I believe this behavior is a consequence of an imperfect decision that I made a while back to reclassify trailing line comments as leading comments when pretty printing, since printing trailing comments (especially end-of-line ones) is tricky.

Please keep in mind that this bug only manifests itself when recast.print falls back to pretty printing the node that the comments are attached to. Otherwise, recast.print does a pretty good job of preserving tricky formatting. For example, if you change only the text of the comments, they will be reprinted in place, without triggering the pretty printer. Setting node.original = null is a reliable way to force the pretty printer to kick in, so I understand why you used it for this reproduction. I just hope the scope of the problem is clear: this bug reflects a limitation of the pretty printer, and recast.print avoids the pretty printer as much as possible.

Let me explain what's tricky, and you can tell me if you see a less-bad solution?

Imagine you have a multi-line function call with comments for the arguments:

addNumbers(
  a, // the first number
  b, // the second number
  c // the third number
);

All of these are trailing comments, and the comment attachment code gets that right.

If you call recast.prettyPrint or set .original = null for the a, b, and c identifiers, however, it's difficult to pretty-print those nodes with trailing line comments, because the comments ideally should start after the , characters, but unfortunately the commas are not contained by the .loc.{start,end} locations associated with the identifier nodes. The CallExpression is responsible for printing the commas between its arguments, whereas the identifiers are responsible for printing their own comments, and I just don't know how to implement logic for having children print separate chunks of code around syntax printed by the parent, especially since there is no information in the AST about where those commas appear.

My imperfect solution has been to reclassify trailing line comments as leading comments, so that you end up with this code:

addNumbers(
  // the first number
  a,
  // the second number
  b,
  // the third number
  c
);

There are more lines here than you might like, but at least the relationship between the comments and the identifiers still makes sense.

Here are some alternatives that seem worse to me, but feel free to disagree!

Trailing line comments always introduce a newline:

addNumbers(
  a // the first number
  ,
  b // the second number
  ,
  c // the third number
);

Trailing line comments get turned into multiline comments:

addNumbers(
  a /* the first number */,
  b /* the second number */,
  c /* the third number */
);

What do you think?

[zertosh]

What you're saying makes sense and it's better than the alternatives. Thanks for the explanation.

I am just wrapping up several rounds of modifying all of the js at SoundCloud. I only need this to work once, so no worries on stressing over a proper fix. I'm working around it by regex'ing for ending comments and adding a 1; to them. That no longer makes them trailing comments of a body, but leading comments of the 1;. Then after the pretty printing, I remove lone 1;s. It really gross, but it works.

[benjamn]

Awesome, it's really great to hear this tool is (mostly) working for your needs! Thanks for understanding and being willing to work around the edge cases.

From my own experience, I know that one-off hacks like that are often necessary for real refactorings, even if there's no hope of permanently incorporating them into the tool itself. Your workaround sounds both sensible and familiar to me :)

Good luck!

[ipeychev]

This also moves the source maps comment like this:

//# sourceMappingURL=sm-compiled.js.map

to the top of the file which browsers like Chrome don't like. They are supposed to be on bottom of the file. If the comment is above, the breakpoints are after that incorrectly set :(

[ipeychev]

Also, I wasn't able to workaround this issue by adding:

1;

to the comments. I tried:

1; //# sourceMappingURL=sm-compiled.js.map

//# sourceMappingURL=sm-compiled.js.map
1;

These didn't work. I tried adding function and whatever else in front - Recast was moving these on top :(

I am not using prettyPrint, just print.

[benjamn]

@ipeychev that's an interesting use case that I would definitely like to support. In your case, was the //# sourceMappingURL comment in the original source, and you just want it to be preserved in the output, or are you adding the comment during an AST transformation, or something else?

[ipeychev]

On Aug 5, 2015 2:35 PM, "Ben Newman" notifications@github.com wrote:

@ipeychev that's an interesting use case that I would definitely like to support. In your case, was the //# sourceMappingURL comment in the original source, and you just want it to be preserved in the output, or are you adding the comment during an AST transformation, or something else?

It was in the original source already. I wanted to preserve it.

Thanks,

— Reply to this email directly or view it on GitHub.

[zertosh]

@ipeychev Here is my hacky workaround. After I'm done transforming a file a cleanup the placeholders.

[ipeychev]

On Aug 5, 2015 4:23 PM, "Andres Suarez" notifications@github.com wrote:

@ipeychev Here is my hacky workaround. After I'm done transforming a file a cleanup the placeholders.

Thanks!

— Reply to this email directly or view it on GitHub.

[dgreensp]

I wonder if there's a simple scheme that works for the case where a single-line comment is on a line by itself. We have an eslint rule in force that bans inline comments, anyway.