Bring over upstream #1784: run the string wrapping pass to a fixed point - #46
Merged
Merged
Conversation
From palantir#1784 by Zayan Khan, which fixes palantir#1343 (#34 here). The indentation StringWrapper picks for a text block is derived from the layout around it, but the same pass can move that layout, so one round is not always a fixed point. In the nested array initialisers of palantir#1343 the second text block moved four columns on a second run, so a file that --replace had just written was reported by --set-exit-if-changed. wrap already recomputed its replacements after reformatting, twice; it now repeats the pass while it keeps changing the source, up to MAX_ROUNDS = 5. Adapted while bringing it over: paths moved from palantir-java-format/ to open-java-format/, and the changelog entry left out. Of the 15,747 files of the JDK 21 sources one formats differently, java/lang/Module.java, whose text block argument the old code moved again on a second run; one run now gives that second result. Formatting the sources takes no longer. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
A text block that is a method argument with more arguments after it, and a text block in each branch of a ternary, came out of one run at one indentation and out of the next at another. The inputs come from google/google-java-format#805 and google/google-java-format#1378, where they show other symptoms; the first is also the shape of java/lang/Module.java, the one JDK 21 file the fixed point changes. With the string wrapping pass run to a fixed point, one run gives the second result; without it, both new goldens fail (#34).
abashev
enabled auto-merge
September 23, 2026 07:54
abashev
disabled auto-merge
September 23, 2026 08:28
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #34. Brings over palantir#1784, which fixes palantir#1343.
From upstream, with the author kept
Co-authored-bytrailer.StringWrapperderives a text block's indentation from the layout around it, and the same pass can move that layout, so one round is not always a fixed point.wrapnow repeats the pass while it keeps changing the source, up toMAX_ROUNDS = 5.palantir-java-format/toopen-java-format/; the changelog entry is left out.palantir-issue-1343-text-block-indent-stablecomes with it.Added here
Checked
./gradlew :open-java-format:teston JDK 21: 1429 tests, all green.java/lang/Module.java, with a text block argument like the first new golden. The old code moved it again on a second run; the new first run gives exactly the old second run's result and is stable. Formatting the sources takes no longer (30–34 s before, 32–33 s after, within noise).Version
The output changes only for files that were never stable: on a file that is already a fixed point, one round is the same as before. #34 asks to show that on a corpus before it ships; the numbers above are that run.