From da047d7793b4e621e013a1f027821adb3b4b7405 Mon Sep 17 00:00:00 2001 From: Liam Miller-Cushon Date: Thu, 24 Sep 2026 23:07:49 +0300 Subject: [PATCH] Report the column of a syntax error where javac puts it 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. --- .../javaformat/java/FormatterDiagnostic.java | 5 ++--- .../palantir/javaformat/java/DiagnosticTest.java | 14 +++++++------- .../com/palantir/javaformat/java/MainTest.java | 6 +++--- 3 files changed, 12 insertions(+), 13 deletions(-) diff --git a/open-java-format-spi/src/main/java/com/palantir/javaformat/java/FormatterDiagnostic.java b/open-java-format-spi/src/main/java/com/palantir/javaformat/java/FormatterDiagnostic.java index 3f4794ab5..26f0a6ace 100644 --- a/open-java-format-spi/src/main/java/com/palantir/javaformat/java/FormatterDiagnostic.java +++ b/open-java-format-spi/src/main/java/com/palantir/javaformat/java/FormatterDiagnostic.java @@ -48,7 +48,7 @@ public int line() { } /** - * Returns the 0-indexed column number on which the error occurred, or {@code -1} if the error does not have a + * Returns the 1-indexed column number on which the error occurred, or {@code -1} if the error does not have a * column. */ public int column() { @@ -67,8 +67,7 @@ public String toString() { sb.append(lineNumber).append(':'); } if (column >= 0) { - // internal column numbers are 0-based, but diagnostics use 1-based indexing by convention - sb.append(column + 1).append(':'); + sb.append(column).append(':'); } if (lineNumber >= 0 || column >= 0) { sb.append(' '); diff --git a/open-java-format/src/test/java/com/palantir/javaformat/java/DiagnosticTest.java b/open-java-format/src/test/java/com/palantir/javaformat/java/DiagnosticTest.java index 8002e269d..d5dae53d2 100644 --- a/open-java-format/src/test/java/com/palantir/javaformat/java/DiagnosticTest.java +++ b/open-java-format/src/test/java/com/palantir/javaformat/java/DiagnosticTest.java @@ -78,7 +78,7 @@ public void parseError() throws Exception { int result = main.format(path.toString()); assertThat(stdout.toString()).isEmpty(); - assertThat(stderr.toString()).contains("InvalidSyntax.java:2:29: error: expected"); + assertThat(stderr.toString()).contains("InvalidSyntax.java:2:28: error: expected"); assertThat(result).isEqualTo(2); } @@ -116,7 +116,7 @@ public void oneFileParseError() throws Exception { int result = main.format(pathOne.toString(), pathTwo.toString()); assertThat(stdout.toString()).isEqualTo(two); - assertThat(stderr.toString()).contains("One.java:1:13: error: reached end of file"); + assertThat(stderr.toString()).contains("One.java:1:12: error: reached end of file"); assertThat(result).isEqualTo(2); } @@ -137,7 +137,7 @@ public void oneFileParseErrorReplace() throws Exception { int result = main.format("-i", pathOne.toString(), pathTwo.toString()); assertThat(stdout.toString()).isEmpty(); - assertThat(stderr.toString()).contains("One.java:1:14: error: class, interface"); + assertThat(stderr.toString()).contains("One.java:1:13: error: class, interface"); assertThat(result).isEqualTo(2); // don't edit files with parse errors assertThat(Files.readAllLines(pathOne, UTF_8)).containsExactly("class One {}}"); @@ -159,7 +159,7 @@ public void parseError2() throws FormatterException, IOException, UsageException int exitCode = main.format(args); assertThat(exitCode).isEqualTo(2); - assertThat(err.toString()).contains("A.java:2:6: error: ';' expected"); + assertThat(err.toString()).contains("A.java:2:5: error: ';' expected"); } @Test @@ -174,7 +174,7 @@ public void parseErrorStdin() throws FormatterException, IOException, UsageExcep int exitCode = main.format(args); assertThat(exitCode).isEqualTo(2); - assertThat(err.toString()).contains(":2:6: error: ';' expected"); + assertThat(err.toString()).contains(":2:5: error: ';' expected"); } @Test @@ -192,7 +192,7 @@ public void lexError2() throws FormatterException, IOException, UsageException { int exitCode = main.format(args); assertThat(exitCode).isEqualTo(2); - assertThat(err.toString()).contains("A.java:2:5: error: unclosed character literal"); + assertThat(err.toString()).contains("A.java:2:4: error: unclosed character literal"); } @Test @@ -206,6 +206,6 @@ public void lexErrorStdin() throws FormatterException, IOException, UsageExcepti int exitCode = main.format(args); assertThat(exitCode).isEqualTo(2); - assertThat(err.toString()).contains(":2:5: error: unclosed character literal"); + assertThat(err.toString()).contains(":2:4: error: unclosed character literal"); } } diff --git a/open-java-format/src/test/java/com/palantir/javaformat/java/MainTest.java b/open-java-format/src/test/java/com/palantir/javaformat/java/MainTest.java index a87238af0..73b5efd8b 100644 --- a/open-java-format/src/test/java/com/palantir/javaformat/java/MainTest.java +++ b/open-java-format/src/test/java/com/palantir/javaformat/java/MainTest.java @@ -351,7 +351,7 @@ public void importRemoveErrorParseError() throws Exception { new PrintWriter(err, true), new ByteArrayInputStream(joiner.join(input).getBytes(UTF_8))); assertThat(main.format("-")).isEqualTo(2); - assertThat(err.toString()).contains(":4:3: error: class, interface"); + assertThat(err.toString()).contains(":4:2: error: class, interface"); } finally { Locale.setDefault(backupLocale); @@ -584,7 +584,7 @@ public void exitIfChangedLosesToParseError() throws Exception { .isEqualTo(1); assertThat(main.format("-n", "--set-exit-if-changed", unformatted.toString(), broken.toString())) .isEqualTo(2); - assertThat(err.toString()).contains("Broken.java:1:16: error: reached end of file"); + assertThat(err.toString()).contains("Broken.java:1:15: error: reached end of file"); } @Test @@ -599,7 +599,7 @@ public void assumeFilename_error() throws Exception { new PrintWriter(err, true), new ByteArrayInputStream(joiner.join(input).getBytes(UTF_8))); assertThat(main.format("--assume-filename=Foo.java", "-")).isEqualTo(2); - assertThat(err.toString()).contains("Foo.java:1:15: error: class, interface"); + assertThat(err.toString()).contains("Foo.java:1:14: error: class, interface"); } @Test