(idempotence violation) Empty line after parenthesized array item

[pushkine]

Prettier 2.7.1
Playground link

--parser babel

Input:

[
  a = b,
  
  c //
]

Output:

[
  (a = b),

  c, //
];

Second Output:

[
  (a = b),
  c, //
];

[solarized-fox]

I haven't been able to replicate this. I'm on macOS and ran a script to compare the output 1000 times, and they were all identical.

it may be difficult to determine the cause without more information

[kachkaev]

@solarized-fox the issue is to do with the second output not being equal to the first output, which means that idempotency is violated. This is still true for the latest Prettier:

Prettier 2.8.7
Playground link

--parser babel

Input:

[
  a = b,
  
  c //
]

Output:

[
  (a = b),

  c, //
];

Second Output:

[
  (a = b),
  c, //
];

I checked Prettier 3.0.0-alpha.6 locally, got the same results.

[solarized-fox]

hmm, what OS are you on? I'm running macOS 12.6.x, and this script:

echo "[
  a = b,

  c //
]" > foo.js

git checkout 3.0.0-alpha.6
printf "$(yarn prettier --parser babel foo.js)\n"
printf "$(yarn prettier --parser babel foo.js)\n"

produced this output on my machine:

HEAD is now at 432976de1 Release 3.0.0-alpha.6
[
  (a = b),

  c, //
];
[
  (a = b),

  c, //
];

could it be possible the issue is connected to the environment somehow?

[kachkaev]

I’m on macOS too, yep. Can you try adding --write to prettier commands and see what happens? By default, Prettier does not save the results of formatting so you get:

source → result 1
source → result 1

instead of

source → result 1
result 1 → result 2

If the idempotency principle is followed, we should get:

source → result 1
result 1 → result 1
result 1 → result 1
... ∞ ...

[solarized-fox]

ohhh I understand, the second output is supposed to use the input of the first. this gives me a few ideas, I’ll see if I can’t track this bug down

[solarized-fox]

I believe I've identified the problem!

the node representing a = b, in the first input has a range ending on the character , while the node representing (a = b), has a range ending on the character ) for some reason. the code tries to skip to the end of the line by passing over the characters ,; \t of which ) is not one

it seems to me that the problem is with the range assigned to the node in the first place. for what it's worth, the actual location where the newline is omitted on the second output is line 181 in array.js, where a ternary evaluates differently between the two inputs

incidentally since the problem seems to lie simply with the parens, I'm fairly confident that this issue can be boiled down to "these two statements should format the same":

_ = [
  a = b,

  c //
]

_ = [
  (a = b),

  c, //
];

I'll try and see if I can find a solution but the only part of the codebase I've really read so far is language-js/print so if the problem lies outside of that area (as I suspect it might) this might take me a bit

[solarized-fox]

okay after looking into it it appears that parens are just not (or not always) part of the character range covered by their respective node. this means that the last node on a line could be followed by a ")". so it seems the correct thing to do is just modify skipToLineEnd to also ignore ")".

yay one character diff, I'll get a PR ready!

[fisker]

@solarized-fox there will be comments, more parens

[
  (((((
    a = b/* comment */))/* comment */))),
  
  c //
]

[solarized-fox]

@fisker adding that to the tests, although it seems the code already accounts for that. it's just the parens that trip it up, apparently