Fix only_add trailing commas strategy causing MAX_WIDTH overflow (#610)

[lwasyl]

See #610

[meta-codesync]

@hick209 has imported this pull request. If you are a Meta employee, you can view this in D100997972.

[cortinico]

Review automatically exported from Phabricator review in Meta.

[hick209]

Running the tests locally in our codebase here I found issues.
I investigated it with an agent and got the following report (which makes sense given what we saw in our codebase)

Root Cause

The pipeline reorder PR introduced a regression for trailing comma insertion.

Here's the mechanism:

Old pipeline: dropRedundantElements → prettyPrint → addRedundantElements

1. prettyPrint reformats code, breaking long parameter lists across multiple lines
2. addRedundantElements sees multi-line lists (via element.text.contains("\n") check in TrailingCommas.Suggestor at TrailingCommas.kt:80) and adds trailing commas
3. Trailing commas survive because nothing runs after

New pipeline: dropRedundantElements → addRedundantElements → prettyPrint

1. addRedundantElements runs on the pre-formatted source — if the input has single-line parameter lists, contains("\n") is false, so no trailing commas are added
2. prettyPrint then breaks lists across multiple lines, but since no trailing comma exists in the input, KotlinInputAstVisitor.visitEachCommaSeparated sees hasTrailingComma = false and doesn't emit one
3. Result: multi-line parameter lists without trailing commas

The fix for the MAX_WIDTH issue (trailing comma pushing lines over the limit) inadvertently broke the case where trailing commas need to be added to lists that only become multi-line during formatting.

What's happening

The pipeline reorder made ktfmt non-idempotent — it takes two runs to converge:

Run 1 (input has single-line params from codegen):

1. addRedundantElements — input params are single-line, element.text.contains("\n") is false → no trailing commas added
2. prettyPrint — breaks params across lines, but since no trailing comma exists, doesn't emit one
3. Output: multi-line params without trailing commas

Run 2 (input already has multi-line params from Run 1):

1. addRedundantElements — input params ARE multi-line now, contains("\n") is true → trailing commas added
2. prettyPrint — sees parameterList.trailingComma != null, preserves them
3. Output: multi-line params with trailing commas ← matches fixture

Run 3+: Idempotent, no changes.

The regression

Before the pipeline reorder, addRedundantElements ran after prettyPrint, so it always saw already-formatted (multi-line) parameter lists and correctly added trailing commas in a single pass.
The reorder broke this because addRedundantElements now runs on the pre-formatted source where parameter lists may still be single-line.

This is a non-idempotency bug — a single ktfmt invocation should produce the final formatted output.
The fact that it requires two passes to converge is a regression from this PR.

[lwasyl]

Thanks for the overview! My original idea was to run addRedundantElements twice (before and after pretty-printing) but at the time I wanted to keep 100% of the existing tests passing. After relaxing that requirement, looks like I can go back to that idea. I added two tests based on your description that indeed were failing, now they pass.

I recognize that just adding elements multiple times is maybe not the most efficient approach 😅 At the same time I think it's the simplest at this point

[meta-codesync]

@hick209 merged this pull request in 17ea8f2.