Skip to content

Commit 8804303

Browse files
committed
Throw CompilerException in performCompile|getEcj, not in constructor
This shifts the focus from a supposed Maven component wiring error to the root cause, expressed in the error message: Fatal error compiling: ECJ needs JRE 17+ We need to add safeguards throwing the exception in both methods accessing class EclipseJavaCompilerDelegate, because we cannot predict who calls which one first. Omitting one safeguard might lead to the return of the infamous "No such compiler 'eclipse'" error. The new static boolean field EclipseJavaCompiler.isJdkSupported also enables us to write a simple unit test, asserting on the error message, if the field value is false. Relates to #347.
1 parent d372db6 commit 8804303

File tree

2 files changed

+73
-4
lines changed

2 files changed

+73
-4
lines changed

plexus-compilers/plexus-compiler-eclipse/src/main/java/org/codehaus/plexus/compiler/eclipse/EclipseJavaCompiler.java

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@
6464
public class EclipseJavaCompiler extends AbstractCompiler {
6565
public EclipseJavaCompiler() {
6666
super(CompilerOutputStyle.ONE_OUTPUT_FILE_PER_INPUT_FILE, ".java", ".class", null);
67-
if (Runtime.version().feature() < 17) throw new EcjFailureException("ECJ only works on Java 17+");
67+
if (!isJdkSupported) return;
6868
try {
6969
// Do not directly import EclipseJavaCompilerDelegate or any ECJ classes compiled to target 17.
7070
// This ensures that the plugin still runs on Java 11 and can report the error above.
@@ -82,9 +82,10 @@ public EclipseJavaCompiler() {
8282
// ----------------------------------------------------------------------
8383
// Compiler Implementation
8484
// ----------------------------------------------------------------------
85+
static boolean isJdkSupported = Runtime.version().feature() >= 17;
8586
boolean errorsAsWarnings = false;
86-
private final MethodHandle getClassLoaderMH;
87-
private final MethodHandle batchCompileMH;
87+
private MethodHandle getClassLoaderMH;
88+
private MethodHandle batchCompileMH;
8889

8990
@Override
9091
public String getCompilerId() {
@@ -93,6 +94,9 @@ public String getCompilerId() {
9394

9495
@Override
9596
public CompilerResult performCompile(CompilerConfiguration config) throws CompilerException {
97+
// Safeguard before using method handle accessing EclipseJavaCompilerDelegate
98+
if (!isJdkSupported) throw new CompilerException("ECJ needs JRE 17+");
99+
96100
List<String> args = new ArrayList<>();
97101
args.add("-noExit"); // Make sure ecj does not System.exit on us 8-/
98102

@@ -507,7 +511,9 @@ private static boolean haveSourceOrReleaseArgument(List<String> args) {
507511
return false;
508512
}
509513

510-
private JavaCompiler getEcj() {
514+
private JavaCompiler getEcj() throws CompilerException {
515+
// Safeguard before using method handle accessing EclipseJavaCompilerDelegate
516+
if (!isJdkSupported) throw new CompilerException("ECJ needs JRE 17+");
511517
ClassLoader classLoader;
512518
try {
513519
classLoader = (ClassLoader) getClassLoaderMH.invoke();
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
package org.codehaus.plexus.compiler.eclipse;
2+
3+
import javax.inject.Inject;
4+
import javax.inject.Named;
5+
6+
import java.io.File;
7+
import java.util.Set;
8+
import java.util.stream.Collectors;
9+
10+
import org.codehaus.plexus.compiler.Compiler;
11+
import org.codehaus.plexus.compiler.CompilerConfiguration;
12+
import org.codehaus.plexus.compiler.CompilerException;
13+
import org.codehaus.plexus.testing.PlexusTest;
14+
import org.codehaus.plexus.util.FileUtils;
15+
import org.hamcrest.MatcherAssert;
16+
import org.hamcrest.Matchers;
17+
import org.junit.jupiter.api.AfterAll;
18+
import org.junit.jupiter.api.BeforeAll;
19+
import org.junit.jupiter.api.Test;
20+
import org.junit.jupiter.api.parallel.Isolated;
21+
22+
import static org.codehaus.plexus.testing.PlexusExtension.getBasedir;
23+
import static org.junit.jupiter.api.Assertions.assertThrows;
24+
25+
@PlexusTest
26+
@Isolated("changes static variable")
27+
public class EclipseCompilerUnsupportedJdkTest {
28+
static final boolean IS_JDK_SUPPORTED = EclipseJavaCompiler.isJdkSupported;
29+
30+
@Inject
31+
@Named("eclipse")
32+
Compiler compiler;
33+
34+
@BeforeAll
35+
public static void setUpClass() {
36+
EclipseJavaCompiler.isJdkSupported = false;
37+
}
38+
39+
@AfterAll
40+
public static void cleanUpClass() {
41+
EclipseJavaCompiler.isJdkSupported = IS_JDK_SUPPORTED;
42+
}
43+
44+
@Test
45+
public void testUnsupportedJdk() {
46+
CompilerException error = assertThrows(CompilerException.class, () -> compiler.performCompile(getConfig()));
47+
MatcherAssert.assertThat(error.getMessage(), Matchers.containsString("ECJ needs JRE 17+"));
48+
}
49+
50+
private CompilerConfiguration getConfig() throws Exception {
51+
String sourceDir = getBasedir() + "/src/test-input/src/main";
52+
Set<File> sourceFiles = FileUtils.getFileNames(new File(sourceDir), "**/*.java", null, false, true).stream()
53+
.map(File::new)
54+
.collect(Collectors.toSet());
55+
CompilerConfiguration compilerConfig = new CompilerConfiguration();
56+
compilerConfig.addSourceLocation(sourceDir);
57+
compilerConfig.setOutputLocation(getBasedir() + "/target/eclipse/classes");
58+
compilerConfig.setSourceFiles(sourceFiles);
59+
compilerConfig.setTargetVersion("1.8");
60+
compilerConfig.setSourceVersion("1.8");
61+
return compilerConfig;
62+
}
63+
}

0 commit comments

Comments
 (0)