Update default set of root modules for unnamed modules (#3572)

+ Implement new (simplified) rules as of https://bugs.openjdk.org/browse/JDK-8205169 + New methods to query the default root modules in a version-aware way + Deprecate old version-agnostic methods Fixes #2786
eclipse-jdt · Jan 18, 2025 · 831cde9 · 831cde9
1 parent 6117b71
commit 831cde9
Show file tree

Hide file tree

Showing 5 changed files with 110 additions and 15 deletions.
diff --git a/...eclipse.jdt.core.tests.model/src/org/eclipse/jdt/core/tests/model/ModuleBuilderTests.java b/...eclipse.jdt.core.tests.model/src/org/eclipse/jdt/core/tests/model/ModuleBuilderTests.java
@@ -9028,6 +9028,57 @@ public void testIssue23() throws CoreException {
 			deleteProject(p1);
 		}
 	}
+	public void testIssue2786_10() throws CoreException {
+		// module java.smartcardio is not in default root modules according to old rules of JEP 261
+		IJavaProject p10 = createJava10Project("J10", new String[] {"src"});
+		p10.setOption(JavaCore.COMPILER_RELEASE, JavaCore.ENABLED);
+		try {
+			createFolder("/J10/src/p1");
+			createFile("/J10/src/p1/X.java",
+					"package p1;\n" +
+					"import javax.smartcardio.Card;\n" +
+					"public class X {\n" +
+					"	Card card;\n" +
+					"}");
+
+			waitForManualRefresh();
+			waitForAutoBuild();
+			p10.getProject().build(IncrementalProjectBuilder.FULL_BUILD, null);
+			IMarker[] markers = p10.getProject().findMarkers(null, true, IResource.DEPTH_INFINITE);
+			sortMarkers(markers);
+			assertMarkers("unexpected markers",
+					"The import javax.smartcardio cannot be resolved\n" +
+					"Card cannot be resolved to a type",
+					markers);
+		} finally {
+			deleteProject(p10);
+		}
+	}
+	public void testIssue2786_11() throws CoreException {
+		// since JDK-8205169 module java.smartcardio is indeed in default root modules
+		IJavaProject p11 = createJava11Project("J11", new String[] {"src"});
+		p11.setOption(JavaCore.COMPILER_RELEASE, JavaCore.ENABLED);
+		try {
+			createFolder("/J11/src/p1");
+			createFile("/J11/src/p1/X.java",
+					"package p1;\n" +
+					"import javax.smartcardio.Card;\n" +
+					"public class X {\n" +
+					"	Card card;\n" +
+					"}");
+
+			waitForManualRefresh();
+			waitForAutoBuild();
+			p11.getProject().build(IncrementalProjectBuilder.FULL_BUILD, null);
+			IMarker[] markers = p11.getProject().findMarkers(null, true, IResource.DEPTH_INFINITE);
+			assertMarkers("Unexpected Markers",
+					"",
+					markers);
+		} finally {
+			deleteProject(p11);
+		}
+	}
+
 	protected void assertNoErrors() throws CoreException {
 		for (IProject p : getWorkspace().getRoot().getProjects()) {
 			int maxSeverity = p.findMaxProblemSeverity(null, true, IResource.DEPTH_INFINITE);

diff --git a/org.eclipse.jdt.core/model/org/eclipse/jdt/core/JavaCore.java b/org.eclipse.jdt.core/model/org/eclipse/jdt/core/JavaCore.java
@@ -6500,11 +6500,24 @@ public static IModuleDescription getAutomaticModuleDescription(IJavaElement elem
 	 * @param allSystemRoots all physically available system modules, represented by their package fragment roots
 	 * @return the list of names of default root modules
 	 * @since 3.14
+	 * @deprecated This method cannot distinguish strategies for old (9/10) vs new (11+) JDK versions. Please use {@link #defaultRootModules(Iterable, String)}
 	 */
+	@Deprecated
 	public static List<String> defaultRootModules(Iterable<IPackageFragmentRoot> allSystemRoots) {
 		return JavaProject.defaultRootModules(allSystemRoots);
 	}
 
+	/**
+	 * Filter the given set of system roots by the rules for root modules from JEP 261.
+	 * @param allSystemRoots all physically available system modules, represented by their package fragment roots
+	 * @param release JDK version to select strategies before / after resolution of https://bugs.openjdk.org/browse/JDK-8205169
+	 * @return the list of names of default root modules
+	 * @since 3.41
+	 */
+	public static List<String> defaultRootModules(Iterable<IPackageFragmentRoot> allSystemRoots, String release) {
+		return JavaProject.defaultRootModules(allSystemRoots, release);
+	}
+
 	/**
 	 * Compile the given module description in the context of its enclosing Java project
 	 * and add class file attributes using the given map of attribute values.

diff --git a/org.eclipse.jdt.core/model/org/eclipse/jdt/internal/core/JavaProject.java b/org.eclipse.jdt.core/model/org/eclipse/jdt/internal/core/JavaProject.java
@@ -741,7 +741,9 @@ public void computePackageFragmentRoots(
 								if (limitModules != null) {
 									rootModules = Arrays.asList(limitModules.split(",")); //$NON-NLS-1$
 								} else if (isUnNamedModule()) {
-									rootModules = defaultRootModules((Iterable) imageRoots);
+									String release = JavaCore.ENABLED.equals(getOption(JavaCore.COMPILER_RELEASE, true))
+											? getOption(JavaCore.COMPILER_COMPLIANCE, true) : null;
+									rootModules = defaultRootModules((Iterable) imageRoots, release);
 								}
 								if (rootModules != null) {
 									imageRoots = filterLimitedModules(entryPath, imageRoots, rootModules);
@@ -792,30 +794,50 @@ public void computePackageFragmentRoots(
 		}
 	}
 
-	/** Implements selection of root modules per JEP 261. */
+	/**
+	 * Implements selection of root modules per JEP 261.
+	 * @deprecated This method cannot distinguish strategies for old (9/10) vs new (11+) JDK versions. Please use {@link #defaultRootModules(Iterable, String)}
+	 */
+	@Deprecated
 	public static List<String> defaultRootModules(Iterable<IPackageFragmentRoot> allSystemRoots) {
+		return defaultRootModules(allSystemRoots, JavaCore.VERSION_11); // 11 is fix version of https://bugs.openjdk.org/browse/JDK-8205169
+	}
+
+	/**
+	 * Implements selection of root modules per JEP 261.
+	 * @param allSystemRoots all modules found in the JRT system
+	 * @param releaseVersion the release version which decides about the strategy for selecting root modules
+	 */
+	public static List<String> defaultRootModules(Iterable<IPackageFragmentRoot> allSystemRoots, String releaseVersion) {
 		return internalDefaultRootModules(allSystemRoots,
 				IPackageFragmentRoot::getElementName,
-				r ->  (r instanceof JrtPackageFragmentRoot) ? ((JrtPackageFragmentRoot) r).getModule() : null);
+				r ->  (r instanceof JrtPackageFragmentRoot) ? ((JrtPackageFragmentRoot) r).getModule() : null,
+				releaseVersion);
 	}
 
-	public static <T> List<String> internalDefaultRootModules(Iterable<T> allSystemModules, Function<T,String> getModuleName, Function<T,IModule> getModule) {
+	public static <T> List<String> internalDefaultRootModules(Iterable<T> allSystemModules, Function<T,String> getModuleName, Function<T,IModule> getModule, String releaseVersion) {
 		List<String> result = new ArrayList<>();
+		boolean beforeJDK8205169 = JavaCore.VERSION_9.equals(releaseVersion) || JavaCore.VERSION_10.equals(releaseVersion);
 		boolean hasJavaDotSE = false;
-		for (T mod : allSystemModules) {
-			String moduleName = getModuleName.apply(mod);
-			if ("java.se".equals(moduleName)) { //$NON-NLS-1$
-				result.add(moduleName);
-				hasJavaDotSE = true;
-				break;
+		if (beforeJDK8205169) {
+			for (T mod : allSystemModules) {
+				String moduleName = getModuleName.apply(mod);
+				if ("java.se".equals(moduleName)) { //$NON-NLS-1$
+					result.add(moduleName);
+					hasJavaDotSE = true;
+					break;
+				}
 			}
 		}
 		for (T mod : allSystemModules) {
 			String moduleName = getModuleName.apply(mod);
-			boolean isJavaDotStart = moduleName.startsWith("java."); //$NON-NLS-1$
-			boolean isPotentialRoot = !isJavaDotStart;	// always include non-java.*
-			if (!hasJavaDotSE)
-				isPotentialRoot |= isJavaDotStart;		// no java.se => add all java.*
+			boolean isPotentialRoot = true;  // since JDK-8205169 all system modules are considered
+			if (beforeJDK8205169) {
+				boolean isJavaDotStart = moduleName.startsWith("java."); //$NON-NLS-1$
+				isPotentialRoot = !isJavaDotStart;	// always include non-java.*
+				if (!hasJavaDotSE)
+					isPotentialRoot |= isJavaDotStart;		// no java.se => add all java.*
+			}
 
 			if (isPotentialRoot) {
 				IModule module = getModule.apply(mod);

diff --git a/org.eclipse.jdt.core/model/org/eclipse/jdt/internal/core/builder/ClasspathJrt.java b/org.eclipse.jdt.core/model/org/eclipse/jdt/internal/core/builder/ClasspathJrt.java
@@ -242,14 +242,18 @@ protected Collection<String> selectModules(Set<String> keySet, Collection<String
 		result.retainAll(limitModules);
 		rootModules = result;
 	} else {
-		rootModules = JavaProject.internalDefaultRootModules(keySet, s -> s, this::getModule);
+		rootModules = JavaProject.internalDefaultRootModules(keySet, s -> s, this::getModule, getReleaseVersion());
 	}
 	Set<String> allModules = new HashSet<>(rootModules);
 	for (String mod : rootModules)
 		addRequired(mod, allModules);
 	return allModules;
 }
 
+protected String getReleaseVersion() {
+	return null;
+}
+
 protected void addRequired(String mod, Set<String> allModules) {
 	IModule iMod = getModule(mod);
 	if(iMod == null) {

diff --git a/...e.jdt.core/model/org/eclipse/jdt/internal/core/builder/ClasspathJrtWithReleaseOption.java b/...e.jdt.core/model/org/eclipse/jdt/internal/core/builder/ClasspathJrtWithReleaseOption.java
@@ -115,6 +115,11 @@ private String getReleaseOptionFromCompliance(String comp) throws CoreException
 		}
 	}
 
+	@Override
+	protected String getReleaseVersion() {
+		return this.release;
+	}
+
 	public void loadModules() {
 		if (this.fs == null || !this.ctSym.isJRE12Plus()) {
 			ClasspathJrt.loadModules(this);