Skip to content

Commit c101ee9

Browse files
cushongoogle-java-format Team
authored and
google-java-format Team
committed
Fix column numbers in diagnostics
Despite documentation to the contrary, the column numbers are already 1-indexed. The comment was added in unknown commit when g-j-f was still implemented using ecj instead of javac, so maybe it was true then? PiperOrigin-RevId: 681451766
1 parent 8c652ed commit c101ee9

File tree

4 files changed

+48
-18
lines changed

4 files changed

+48
-18
lines changed

core/src/main/java/com/google/googlejavaformat/FormatterDiagnostic.java

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ public int line() {
4949
}
5050

5151
/**
52-
* Returns the 0-indexed column number on which the error occurred, or {@code -1} if the error
52+
* Returns the 1-indexed column number on which the error occurred, or {@code -1} if the error
5353
* does not have a column.
5454
*/
5555
public int column() {
@@ -61,14 +61,14 @@ public String message() {
6161
return message;
6262
}
6363

64+
@Override
6465
public String toString() {
6566
StringBuilder sb = new StringBuilder();
6667
if (lineNumber >= 0) {
6768
sb.append(lineNumber).append(':');
6869
}
6970
if (column >= 0) {
70-
// internal column numbers are 0-based, but diagnostics use 1-based indexing by convention
71-
sb.append(column + 1).append(':');
71+
sb.append(column).append(':');
7272
}
7373
if (lineNumber >= 0 || column >= 0) {
7474
sb.append(' ');

core/src/main/java/com/google/googlejavaformat/java/FormatterException.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@ public String formatDiagnostics(String path, String input) {
6969
if (line != -1 && column != -1) {
7070
sb.append(CharMatcher.breakingWhitespace().trimTrailingFrom(lines.get(line - 1)))
7171
.append(System.lineSeparator());
72-
sb.append(" ".repeat(column)).append('^').append(System.lineSeparator());
72+
sb.append(" ".repeat(column - 1)).append('^').append(System.lineSeparator());
7373
}
7474
}
7575
return sb.toString();

core/src/test/java/com/google/googlejavaformat/java/DiagnosticTest.java

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,7 @@ public void parseError() throws Exception {
7979

8080
int result = main.format(path.toString());
8181
assertThat(stdout.toString()).isEmpty();
82-
assertThat(stderr.toString()).contains("InvalidSyntax.java:2:29: error: <identifier> expected");
82+
assertThat(stderr.toString()).contains("InvalidSyntax.java:2:28: error: <identifier> expected");
8383
assertThat(result).isEqualTo(1);
8484
}
8585

@@ -119,7 +119,7 @@ public void oneFileParseError() throws Exception {
119119

120120
int result = main.format(pathOne.toString(), pathTwo.toString());
121121
assertThat(stdout.toString()).isEqualTo(two);
122-
assertThat(stderr.toString()).contains("One.java:1:13: error: reached end of file");
122+
assertThat(stderr.toString()).contains("One.java:1:12: error: reached end of file");
123123
assertThat(result).isEqualTo(1);
124124
}
125125

@@ -141,7 +141,7 @@ public void oneFileParseErrorReplace() throws Exception {
141141

142142
int result = main.format("-i", pathOne.toString(), pathTwo.toString());
143143
assertThat(stdout.toString()).isEmpty();
144-
assertThat(stderr.toString()).contains("One.java:1:14: error: class, interface");
144+
assertThat(stderr.toString()).contains("One.java:1:13: error: class, interface");
145145
assertThat(result).isEqualTo(1);
146146
// don't edit files with parse errors
147147
assertThat(Files.readAllLines(pathOne, UTF_8)).containsExactly("class One {}}");
@@ -164,7 +164,7 @@ public void parseError2() throws FormatterException, IOException, UsageException
164164
int exitCode = main.format(args);
165165

166166
assertThat(exitCode).isEqualTo(1);
167-
assertThat(err.toString()).contains("A.java:2:6: error: ';' expected");
167+
assertThat(err.toString()).contains("A.java:2:5: error: ';' expected");
168168
}
169169

170170
@Test
@@ -179,7 +179,7 @@ public void parseErrorStdin() throws FormatterException, IOException, UsageExcep
179179
int exitCode = main.format(args);
180180

181181
assertThat(exitCode).isEqualTo(1);
182-
assertThat(err.toString()).contains("<stdin>:2:6: error: ';' expected");
182+
assertThat(err.toString()).contains("<stdin>:2:5: error: ';' expected");
183183
}
184184

185185
@Test
@@ -198,7 +198,7 @@ public void lexError2() throws FormatterException, IOException, UsageException {
198198
int exitCode = main.format(args);
199199

200200
assertThat(exitCode).isEqualTo(1);
201-
assertThat(err.toString()).contains("A.java:2:5: error: unclosed character literal");
201+
assertThat(err.toString()).contains("A.java:2:4: error: unclosed character literal");
202202
}
203203

204204
@Test
@@ -212,6 +212,6 @@ public void lexErrorStdin() throws FormatterException, IOException, UsageExcepti
212212
int exitCode = main.format(args);
213213

214214
assertThat(exitCode).isEqualTo(1);
215-
assertThat(err.toString()).contains("<stdin>:2:5: error: unclosed character literal");
215+
assertThat(err.toString()).contains("<stdin>:2:4: error: unclosed character literal");
216216
}
217217
}

core/src/test/java/com/google/googlejavaformat/java/MainTest.java

Lines changed: 37 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -307,7 +307,7 @@ public void importRemoveErrorParseError() throws Exception {
307307
new PrintWriter(err, true),
308308
new ByteArrayInputStream(joiner.join(input).getBytes(UTF_8)));
309309
assertThat(main.format("-")).isEqualTo(1);
310-
assertThat(err.toString()).contains("<stdin>:4:3: error: class, interface");
310+
assertThat(err.toString()).contains("<stdin>:4:2: error: class, interface");
311311

312312
} finally {
313313
Locale.setDefault(backupLocale);
@@ -508,7 +508,7 @@ public void assumeFilename_error() throws Exception {
508508
new PrintWriter(err, true),
509509
new ByteArrayInputStream(joiner.join(input).getBytes(UTF_8)));
510510
assertThat(main.format("--assume-filename=Foo.java", "-")).isEqualTo(1);
511-
assertThat(err.toString()).contains("Foo.java:1:15: error: class, interface");
511+
assertThat(err.toString()).contains("Foo.java:1:14: error: class, interface");
512512
}
513513

514514
@Test
@@ -637,11 +637,13 @@ public void reorderModifiersOptionTest() throws Exception {
637637
}
638638

639639
@Test
640-
public void badIdentifier() throws Exception {
640+
public void syntaxError() throws Exception {
641641
Path path = testFolder.newFile("Test.java").toPath();
642642
String[] input = {
643643
"class Test {", //
644-
" void f(int package) {}",
644+
" void f(int package) {",
645+
" int",
646+
" }",
645647
"}",
646648
"",
647649
};
@@ -653,9 +655,37 @@ public void badIdentifier() throws Exception {
653655
int errorCode = main.format(path.toAbsolutePath().toString());
654656
assertWithMessage("Error Code").that(errorCode).isEqualTo(1);
655657
String[] expected = {
656-
path + ":2:14: error: <identifier> expected", //
657-
" void f(int package) {}",
658-
" ^",
658+
path + ":2:13: error: <identifier> expected",
659+
" void f(int package) {",
660+
" ^",
661+
path + ":3:5: error: not a statement",
662+
" int",
663+
" ^",
664+
path + ":3:8: error: ';' expected",
665+
" int",
666+
" ^",
667+
"",
668+
};
669+
assertThat(err.toString()).isEqualTo(joiner.join(expected));
670+
}
671+
672+
@Test
673+
public void syntaxErrorBeginning() throws Exception {
674+
Path path = testFolder.newFile("Test.java").toPath();
675+
String[] input = {
676+
"error", //
677+
};
678+
String source = joiner.join(input);
679+
Files.writeString(path, source, UTF_8);
680+
StringWriter out = new StringWriter();
681+
StringWriter err = new StringWriter();
682+
Main main = new Main(new PrintWriter(out, true), new PrintWriter(err, true), System.in);
683+
int errorCode = main.format(path.toAbsolutePath().toString());
684+
assertWithMessage("Error Code").that(errorCode).isEqualTo(1);
685+
String[] expected = {
686+
path + ":1:1: error: reached end of file while parsing", //
687+
"error",
688+
"^",
659689
"",
660690
};
661691
assertThat(err.toString()).isEqualTo(joiner.join(expected));

0 commit comments

Comments
 (0)