Skip to content

Commit 2af6c12

Browse files
wilxwilx
committed
Fix off by one column in diagnostic when using "forked" execution of javac.
1 parent 36237da commit 2af6c12

3 files changed

Lines changed: 74 additions & 16 deletions

File tree

‎plexus-compilers/plexus-compiler-javac/src/main/java/org/codehaus/plexus/compiler/javac/JavacCompiler.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1016,7 +1016,7 @@ static CompilerMessage parseModernError(int exitCode, String error) {
10161016
msgBuffer.append(EOL);
10171017

10181018
final String message = msgBuffer.toString();
1019-
final int startcolumn = Objects.requireNonNull(pointer).indexOf("^");
1019+
final int startcolumn = Objects.requireNonNull(pointer).indexOf("^") + 1;
10201020
int endcolumn = (context == null) ? startcolumn : context.indexOf(" ", startcolumn);
10211021
if (endcolumn == -1) {
10221022
endcolumn = Objects.requireNonNull(context).length();

‎plexus-compilers/plexus-compiler-javac/src/test/java/org/codehaus/plexus/compiler/javac/ErrorMessageParserTest.java‎

Lines changed: 26 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@ public void testDeprecationMessage() throws Exception {
6969

7070
assertThat(compilerError.getMessage(), is("Date(java.lang.String) in java.util.Date has been deprecated"));
7171

72-
assertThat(compilerError.getStartColumn(), is(63));
72+
assertThat(compilerError.getStartColumn(), is(64));
7373

7474
assertThat(compilerError.getEndColumn(), is(66));
7575

@@ -93,7 +93,7 @@ public void testWarningMessage() {
9393

9494
assertThat(compilerError.getMessage(), is("finally clause cannot complete normally"));
9595

96-
assertThat(compilerError.getStartColumn(), is(26));
96+
assertThat(compilerError.getStartColumn(), is(27));
9797

9898
assertThat(compilerError.getEndColumn(), is(27));
9999

@@ -114,7 +114,7 @@ public void testErrorMessage() {
114114

115115
assertThat(compilerError.getMessage(), is("not a statement"));
116116

117-
assertThat(compilerError.getStartColumn(), is(9));
117+
assertThat(compilerError.getStartColumn(), is(10));
118118

119119
assertThat(compilerError.getEndColumn(), is(11));
120120

@@ -123,6 +123,17 @@ public void testErrorMessage() {
123123
assertThat(compilerError.getEndLine(), is(7));
124124
}
125125

126+
@Test
127+
public void testErrorColumnIsOneBased() {
128+
String error = "Test.java:5: error: not a statement" + EOL + " null;" + EOL + " ^" + EOL;
129+
130+
CompilerMessage compilerError = JavacCompiler.parseModernError(1, error);
131+
132+
assertThat(compilerError.getStartLine(), is(5));
133+
assertThat(compilerError.getStartColumn(), is(2));
134+
assertThat(compilerError.toString(), is("Test.java:[5,2] not a statement"));
135+
}
136+
126137
@Test
127138
public void testUnknownSymbolError() {
128139
String error = "./org/codehaus/foo/UnknownSymbol.java:7: cannot find symbol" + EOL + "symbol : method foo()"
@@ -142,7 +153,7 @@ public void testUnknownSymbolError() {
142153
is("cannot find symbol" + EOL + "symbol : method foo()" + EOL
143154
+ "location: class org.codehaus.foo.UnknownSymbol"));
144155

145-
assertThat(compilerError.getStartColumn(), is(8));
156+
assertThat(compilerError.getStartColumn(), is(9));
146157

147158
assertThat(compilerError.getEndColumn(), is(14));
148159

@@ -231,7 +242,7 @@ public void testUnixFileNames() {
231242
CompilerMessage compilerError = JavacCompiler.parseModernError(1, error);
232243

233244
assertThat(
234-
String.valueOf(compilerError), is("/my/prj/src/main/java/test/prj/App.java:[11,45] not a statement"));
245+
String.valueOf(compilerError), is("/my/prj/src/main/java/test/prj/App.java:[11,46] not a statement"));
235246
}
236247

237248
@Test
@@ -247,7 +258,7 @@ public void testWindowsDriveLettersMCOMPILER140() {
247258
assertThat(
248259
String.valueOf(compilerError),
249260
is(
250-
"c:\\Documents and Settings\\My Self\\Documents\\prj\\src\\main\\java\\test\\prj\\App.java:[11,45] not a statement"));
261+
"c:\\Documents and Settings\\My Self\\Documents\\prj\\src\\main\\java\\test\\prj\\App.java:[11,46] not a statement"));
251262
}
252263

253264
/**
@@ -652,7 +663,7 @@ public void testCRLF_windows() throws Exception {
652663
error1.getMessage(),
653664
is("[deprecation] ThreadSafeClientConnManager in org.apache.http.impl.conn.tsccm has been deprecated"));
654665
assertThat("line", error1.getStartLine(), is(31));
655-
assertThat("column", error1.getStartColumn(), is(38));
666+
assertThat("column", error1.getStartColumn(), is(39));
656667
CompilerMessage error2 = compilerErrors.get(2);
657668
assertThat(
658669
"file",
@@ -663,7 +674,7 @@ public void testCRLF_windows() throws Exception {
663674
error2.getMessage(),
664675
is("[deprecation] ThreadSafeClientConnManager in org.apache.http.impl.conn.tsccm has been deprecated"));
665676
assertThat("line", error2.getStartLine(), is(151));
666-
assertThat("column", error2.getStartColumn(), is(8));
677+
assertThat("column", error2.getStartColumn(), is(9));
667678
CompilerMessage error3 = compilerErrors.get(3);
668679
assertThat(
669680
"file",
@@ -674,7 +685,7 @@ public void testCRLF_windows() throws Exception {
674685
error3.getMessage(),
675686
is("[deprecation] ThreadSafeClientConnManager in org.apache.http.impl.conn.tsccm has been deprecated"));
676687
assertThat("line", error3.getStartLine(), is(152));
677-
assertThat("column", error3.getStartColumn(), is(16));
688+
assertThat("column", error3.getStartColumn(), is(17));
678689
}
679690

680691
@Test
@@ -703,7 +714,7 @@ public void testJava6Error() throws Exception {
703714
message1.getMessage(),
704715
is("cannot find symbol" + EOL + "symbol : class Properties" + EOL + "location: class Error"));
705716

706-
assertThat(message1.getStartColumn(), is(16));
717+
assertThat(message1.getStartColumn(), is(17));
707718

708719
assertThat(message1.getEndColumn(), is(26));
709720

@@ -719,7 +730,7 @@ public void testJava6Error() throws Exception {
719730
message2.getMessage(),
720731
is("cannot find symbol" + EOL + "symbol : class Properties" + EOL + "location: class Error"));
721732

722-
assertThat(message2.getStartColumn(), is(35));
733+
assertThat(message2.getStartColumn(), is(36));
723734

724735
assertThat(message2.getEndColumn(), is(48));
725736

@@ -755,7 +766,7 @@ public void testJava7Error() throws Exception {
755766
message1.getMessage(),
756767
is("cannot find symbol" + EOL + " symbol: class Properties" + EOL + " location: class Error"));
757768

758-
assertThat(message1.getStartColumn(), is(16));
769+
assertThat(message1.getStartColumn(), is(17));
759770

760771
assertThat(message1.getEndColumn(), is(26));
761772

@@ -771,7 +782,7 @@ public void testJava7Error() throws Exception {
771782
message2.getMessage(),
772783
is("cannot find symbol" + EOL + " symbol: class Properties" + EOL + " location: class Error"));
773784

774-
assertThat(message2.getStartColumn(), is(35));
785+
assertThat(message2.getStartColumn(), is(36));
775786

776787
assertThat(message2.getEndColumn(), is(48));
777788

@@ -1053,7 +1064,7 @@ public void testAnchoredWarning() throws IOException {
10531064
"C:\\repo\\src\\it\\includes-output-when-compiler-forked\\src\\main\\java\\MyClass" + ".java",
10541065
CompilerMessage.Kind.WARNING,
10551066
23,
1056-
27,
1067+
28,
10571068
23,
10581069
30,
10591070
"[divzero] division by zero"),
@@ -1084,7 +1095,7 @@ public void testMixedWarnings() throws IOException {
10841095
"C:\\repo\\src\\it\\includes-output-when-compiler-forked\\src\\main\\java\\MyClass" + ".java",
10851096
CompilerMessage.Kind.WARNING,
10861097
23,
1087-
27,
1098+
28,
10881099
23,
10891100
30,
10901101
"[divzero] division by zero"),

‎plexus-compilers/plexus-compiler-javac/src/test/java/org/codehaus/plexus/compiler/javac/JavacCompilerTest.java‎

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,23 @@
11
package org.codehaus.plexus.compiler.javac;
22

33
import java.io.BufferedReader;
4+
import java.io.File;
45
import java.io.IOException;
56
import java.io.StringReader;
7+
import java.nio.charset.StandardCharsets;
8+
import java.nio.file.Files;
9+
import java.nio.file.Path;
10+
import java.util.Arrays;
611
import java.util.List;
712
import java.util.stream.Stream;
813

14+
import org.codehaus.plexus.compiler.CompilerConfiguration;
915
import org.codehaus.plexus.compiler.CompilerMessage;
16+
import org.codehaus.plexus.compiler.CompilerResult;
1017
import org.codehaus.plexus.compiler.javac.JavacCompiler.JavaVersion;
1118
import org.junit.jupiter.api.BeforeEach;
1219
import org.junit.jupiter.api.Test;
20+
import org.junit.jupiter.api.io.TempDir;
1321
import org.junit.jupiter.params.ParameterizedTest;
1422
import org.junit.jupiter.params.provider.Arguments;
1523
import org.junit.jupiter.params.provider.MethodSource;
@@ -123,4 +131,43 @@ void testExtractMajorAndMinorVersion() {
123131
assertEquals("21", JavacCompiler.extractMajorAndMinorVersion("javac 21"));
124132
assertEquals("1.8", JavacCompiler.extractMajorAndMinorVersion("javac 1.8.0_432"));
125133
}
134+
135+
@Test
136+
void testForkedAndInProcessDiagnosticsAreEqual(@TempDir Path tempDirectory) throws Exception {
137+
Path sourceDirectory = tempDirectory.resolve("src");
138+
Files.createDirectories(sourceDirectory);
139+
Files.write(
140+
sourceDirectory.resolve("Test.java"),
141+
Arrays.asList("class Test {", "Object foo() {", " null;", "}", "}"),
142+
StandardCharsets.UTF_8);
143+
144+
CompilerResult inProcess = compile(tempDirectory, sourceDirectory, false);
145+
CompilerResult forked = compile(tempDirectory, sourceDirectory, true);
146+
147+
assertFalse(inProcess.isSuccess());
148+
assertFalse(forked.isSuccess());
149+
assertThat(inProcess.getCompilerMessages(), hasSize(1));
150+
assertThat(forked.getCompilerMessages(), hasSize(1));
151+
assertEquals(
152+
inProcess.getCompilerMessages().toString(),
153+
forked.getCompilerMessages().toString());
154+
}
155+
156+
private CompilerResult compile(Path tempDirectory, Path sourceDirectory, boolean fork) throws Exception {
157+
File buildDirectory =
158+
tempDirectory.resolve(fork ? "forked" : "in-process").toFile();
159+
Files.createDirectories(buildDirectory.toPath());
160+
161+
CompilerConfiguration configuration = new CompilerConfiguration();
162+
configuration.setFork(fork);
163+
configuration.setWorkingDirectory(tempDirectory.toFile());
164+
configuration.setBuildDirectory(buildDirectory);
165+
configuration.setOutputLocation(new File(buildDirectory, "classes").getAbsolutePath());
166+
configuration.addSourceLocation(sourceDirectory.toString());
167+
configuration.setSourceVersion("8");
168+
configuration.setTargetVersion("8");
169+
configuration.addCompilerCustomArgument("-Xlint:-options", null);
170+
171+
return getCompiler().performCompile(configuration);
172+
}
126173
}

0 commit comments

Comments
 (0)