Keep the comment prefix when wrapping a markdown docstring (palantir #1776) - #102
Merged
Merged
Conversation
From JDK 23 on, javac returns a run of `///` lines (JEP 467) as a single comment token,
so every line after the first still carries its source indentation when
`JavaCommentsHelper.wrapLineComments` sees it. The prefix was read from index 0 of that
untrimmed line, came back empty, and the wrapped remainder was emitted with no `///` at
all -- as bare code. The same indentation was then counted twice in the width test, so
lines under the column limit were wrapped as well.
In Palantir style the result compiles, which is the dangerous part:
class P1 {
/// Summary line.
/// This second markdown line is long enough to overflow the palantir one hundred twenty column limit @deprecated
void f() {}
}
formats to the comment, then `@Deprecated` on a line of its own, then the method.
`javap -v` goes from zero Deprecated markers to four, `javac` exits 0, and nothing
downstream notices that the formatter deprecated a method. In Google style the same
input produces output that does not compile (`illegal start of type`), and formatting it
again fails to parse. Both need the `///` block to be indented, which is why a
`column0 == 0` case does not reproduce it.
The prefix and the width budget now come from the trimmed line, which
`indentLineComments` trims and re-indents anyway. An unbreakable token is left long
instead of producing an empty `///` followed by bare text, and the missing-space rule
(`///foo` -> `/// foo`) now applies to every line of the run, so the same source no
longer formats differently depending on whether the running JDK hands the run over as
one token or as one token per line.
JavaCommentsHelperTest builds the multi-line comment token directly, so the JDK 21 test
task covers the bug that only a JDK 23 or later parser can produce; four of its six
tests fail without this change. The end-to-end test in FormatterTest is gated at 23.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
(cherry picked from commit 0f3a437)
abashev
enabled auto-merge
October 3, 2026 18:31
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
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.
Port of palantir#1776 by asm0dey, cherry-picked with the author kept. It fixes palantir#1803: from JDK 23 on, javac hands a run of
///markdown doc lines over as one comment token, every line after the first still carries its source indentation, andJavaCommentsHelper.wrapLineCommentsread the slash prefix, and the width budget, from that untrimmed line. The prefix came back empty, so the wrapped remainder was written out as bare code.Reproduced on main (2fbf175)
The class from palantir#1803, formatted on JDK 25 and 27, comes out exactly as reported:
javacthen fails with';' expected. The default CLI run reports the same errors and exits with 2, because the import pass re-parses the broken result; that is the crash the issue shows in Spotless. On JDK 21 each///line is its own token and the output is right.It is not only a contrived case: in the JDK 27 sources,
jdk.compiler/com/sun/source/tree/VariableTree.javahas a 141-column///line. On JDK 27, main turns its overflow into bare text and the CLI refuses the file with= expected; with this change the overflow wraps as a///line.The change
wrapLineCommentstrims the line before reading the prefix and measuring it: six lines inJavaCommentsHelper. From the upstream description: a continuation line keeps///when it wraps; a line at the column limit is no longer wrapped (the indentation was counted twice); an unbreakable token is left long instead of becoming an empty///followed by bare text;///foobecomes/// fooon every line of the run, so the same source no longer formats differently depending on whether the running JDK hands the run over as one token or one per line.Tests:
JavaCommentsHelperTestbuilds the multi-line token directly, so the JDK 21 test task covers the bug; four of its six tests fail on main. The end-to-end test inFormatterTestis gated at JDK 23 and runs in the JDK 25, 26 and 27 jobs.Adapted to the fork
Style.OJFforStyle.PALANTIR.JavaCommentsHelpertakes theJavaInputrather than a line separator, so the test builds one fromclass T {}.JUnitClassModifiersandJUnitMethodDeclarationdemand.Checks
:open-java-format:teston JDK 21 (gated test skipped), 25 and 27 (gated test passes): 1604 tests, 0 failures.VariableTree.javaabove, which main could not format at all.