Bug: Trailing line comments should retain position with respect to immediately preceding code on same line

[dustin-rcg]

This is not a stylist issue; it is a bug in handling the semantics of the language, and it can cause incorrect code.

Prettier 2.8.7
Playground link

Input:

const FROM_NATIVES = Cases // My Comment about this line
  .when((x): x is boolean => typeof x === 'boolean', bool)
  .when((x): x is number => typeof x === 'number', number)
  .when((x): x is string => typeof x === 'string', string);

Output:

const FROM_NATIVES = Cases.when( // My Comment about this line
  (x): x is boolean => typeof x === "boolean",
  bool
)
  .when((x): x is number => typeof x === "number", number)
  .when((x): x is string => typeof x === "string", string);

or for printWidth of 120

const FROM_NATIVES = Cases.when((x): x is boolean => typeof x === "boolean", bool) // My Comment about this line
  .when((x): x is number => typeof x === "number", number)
  .when((x): x is string => typeof x === "string", string);

Expected behavior:

Trailing line comments (//) are semantically attached to the code that immediately precedes them on that line and should be treated as a hard boundary around which code should not be moved. This is why the language makes them comment out the remainder of that line. The language does not have "whole-line" comments nor require that comments appear on a line by themselves, so Prettier should not invent such a requirement.

For line comments that appear on a line by themselves, generally they are semantically attached to the subsequent code. So whether a line comment is trailing other code on the same line or on a line by itself gives completely different semantics.

The bug can actually produce incorrect code if we consider that actual trailing code was commented out prior to formatting, since Prettier will relocate that code. Subsequently uncommenting the code will result in an incorrect program, which may not be obvious depending on how the code was relocated. Transient commenting/uncommenting of code is common during development. This is especially problematic for users with "format on save" enabled.

Additionally, given the long discussion about stylistic behavior, allowing trailing line comments to force Prettier not to change the wrapping around the comment seems like a natural and fundamental way to allow users to tell Prettier where the line should wrap.

Traceback

This issue dealt with a similar bug, where the trailing comment was moved with respect to its immediately preceding code. So, this bug seems like a regression of that fix, and may be related to whatever caused that issue as well. Probably, the snapshot needs to add tests with trailing line comments mixed with chained properties and function calls.

[kachkaev]

👋 @dustin-rcg! Could you please check existing issues tagged with area:comments? It is possible that we are already tracking this exact bug. If you find a match, feel free to close this issue as a duplicate and link to it from another issue for more context. Thanks for the write-up!

[dustin-rcg]

@kachkaev Thanks for your reply. Looking through each one, I see many special cases of a general pattern of Prettier moving comments -- both line and block comments -- out of place relative to the adjacent lexical tokens. However, I don't see my special case.

It is seems that fixing these issues one-by-one would be like playing whack-a-mole, and will be a continuous source of bugs that will never get fixed. There are so many examples just in the first few results:

• Sequential block comments produces syntax error #11273
• Unstable comment formatting in for-in and for-of loops #12880
• Incorrect Comment Movement In Empty if/else Statement #13079
• Prettier reorders comments and parenthesis, reducing the clarity of comments #11572
• Unstable comment (never stabilizing) on type assertion #10851
• Improve comments inside for loop #10046

Since I am new to the internals of Prettier, does Prettier operate on token streams?

Looking at the ages of these issues, it seems that Prettier is in dire need of a (1) a strict and simple rule about moving comments relative to the adjacent code and (2) a form of automated testing other than adding more and more examples to one snapshot test.

RE (1), I don't see why it would be desirable for the formatter to relocate comments relative to the adjacent lexical tokens. To do so opens up a can of worms and seems more often not to do what the coder wants. What is the official position of Prettier on this point?

RE (2), because of the number of special cases reflected in the issues, either the snapshot tests need to be broken out into each general language construct to be tested (so that implementors can be required to test each construct), or some additional automation would be useful, for iterating through all tested language constructs, across all supported languages.

I am naive about how Prettier works, but was but was able to find this example, so I guess VSCode is just giving Prettier code as strings line-by-line. If Prettier doesn't use tokenizers for each language, then it seems like it ought to to allow test automation. If it does, then it seems like it should be possible to write some test builders that can generate some general lexical structures, from which each of the supported languages can be emitted.

It should be possible to generate a token stream (including comments) separately from Prettier consuming it. Comments would be inserted at various places within the lexical constructs, and code emitted. Then tokenize the code and keep in memory. Then run Prettier. Then tokenize the result. If the code in those tests is generated in such as way that commas, parentheses, etc. remain constant during formatting, then it seems like in general the token streams should be identical, since Prettier would be operating on whitespace.

As I said I'm naive about Prettier's internals, and I'm interested in learning more, especially what obvious things I have missed in my simple analysis.