Report the column of a syntax error where javac puts it - #72
Merged
Merged
Conversation
For "class Foo { void f() {\n g() } }" the formatter reported
"A.java:2:6: error: ';' expected", one column to the right of where
javac puts its caret. FormatterDiagnostic added one to the column on
the way out, on the assumption that it held a 0-based value; but the
columns come from javac's line map and diagnostics, which are 1-based
already. The comment saying otherwise dates from the time the formatter
ran on ecj.
This ports google/google-java-format#1167 by Liam Miller-Cushon: the
column is printed as it is, and column() is documented as 1-based,
which is what it always returned. The eleven expectations in
DiagnosticTest and MainTest move one column to the left; javac's caret
for the two inputs of DiagnosticTest stands at columns 5 and 4, as the
tests now say.
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.
Brings over google/google-java-format#1167, a fix from google-java-format 1.24.
The bug
For
the formatter reported
A.java:2:6: error: ';' expected, while javac's caret for the same file stands under column 5.FormatterDiagnostic.toStringadded one to the column on the way out, on the assumption that it held a 0-based value; but the columns come from javac's line map and diagnostics, which are 1-based already. The comment saying otherwise dates from the time the formatter ran on ecj.From upstream, with the author kept
FormatterDiagnostic.column()is documented as 1-based, which is what it always returned, so the SPI's behaviour does not change. The eleven expectations inDiagnosticTestandMainTestmove one column to the left, as upstream's did. Upstream also fixed the caret line of its "problem line as context" output, which this project does not print.Checked
DiagnosticTeststands at columns 5 and 4, the values the tests now expect../gradlew :open-java-format:teston JDK 21: 1497 tests, all green.