SplitPackageBinding might be added as child of PlainPackageBinding (e…

…clipse-jdt#2749) Unit test: This test represents a combination of explicit and auto modules with shared package prefixes which triggers the compiler to add a SplitPackageBinding as child of a PlainPackageBinding which should never happen. Fix: When combining packages bindings from different modules, an auto-module should consider all modules declaring the package, because it implicitly reads all modules and getAllRequiredModules() only works with explicit dependencies. Pending follow-up: eclipse-jdt#2764 fixes eclipse-jdt#2748 --------- Signed-off-by: Paul Pazderski <[email protected]> Co-authored-by: Paul Pazderski <[email protected]> Co-authored-by: Stephan Herrmann <[email protected]>
carstenartur · Jul 25, 2024 · ed787f5 · ed787f5
1 parent fb223eb
commit ed787f5
Show file tree

Hide file tree

Showing 2 changed files with 121 additions and 1 deletion.
diff --git a/...e.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/lookup/ModuleBinding.java b/...e.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/lookup/ModuleBinding.java
@@ -680,7 +680,7 @@ PackageBinding combineWithPackagesFromOtherRelevantModules(PackageBinding curren
 	}
 
 	List<ModuleBinding> otherRelevantModules(char[][] declaringModuleNames) {
-		if (isUnnamed() && declaringModuleNames != null) {
+		if ((isUnnamed() || isAutomatic()) && declaringModuleNames != null) {
 			// unnamed module reads all named modules,
 			// so all modules declaring the given package are relevant:
 			return Arrays.stream(declaringModuleNames)

diff --git a/...s.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/ModuleCompilationTests.java b/...s.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/ModuleCompilationTests.java
@@ -21,6 +21,7 @@
 import java.nio.file.FileVisitResult;
 import java.nio.file.Files;
 import java.nio.file.Path;
+import java.nio.file.Paths;
 import java.nio.file.SimpleFileVisitor;
 import java.nio.file.attribute.BasicFileAttributes;
 import java.util.ArrayList;
@@ -6356,4 +6357,123 @@ public void accept(SplitPackageBinding t) {
 			SplitPackageBinding.instanceListener = null;
 		}
 	}
+
+	public void testGH2748() throws IOException {
+		File outputDirectory = new File(OUTPUT_DIR);
+		Util.flushDirectoryContent(outputDirectory);
+
+		List<String> modules = new ArrayList<>();
+		// compile two explicit modules
+		for (String m : new String[] { "A", "B" }) {
+			String out = OUTPUT_DIR + File.separator + "module" + m + File.separator + "bin";
+			String src = OUTPUT_DIR + File.separator + "module" + m + File.separator + "src";
+			modules.add(out);
+
+			List<String> files = new ArrayList<>();
+			writeFileCollecting(files, src,
+					"module-info.java",
+					"module split.module" + m + " {\n" +
+					"	exports pkg.bug.split.sub" + m + ";\n" +
+					"}");
+			writeFileCollecting(files, Paths.get(src, "pkg", "bug", "split", "sub" + m).toString(),
+					"SubModule" + m + ".java",
+					"package pkg.bug.split.sub" + m + ";\n" +
+					"\n" +
+					"public class SubModule" + m + " {\n" +
+					"	\n" +
+					"}");
+
+			StringBuilder buffer = new StringBuilder();
+			buffer.append("-d " + out )
+				.append(" -9 ")
+				.append(" -proc:none ")
+				.append(" -classpath \"")
+				.append(Util.getJavaClassLibsAsString())
+				.append("\" ");
+			runConformModuleTest(
+					files,
+					buffer,
+					"",
+					"",
+					false);
+		}
+
+		// compile a jar which serves as auto-module
+		String[] sources = {
+			"pkg/bug/service/IService.java",
+			"""
+				package pkg.bug.service;
+
+				public interface IService {
+
+				}""",
+		};
+		String jarPath = OUTPUT_DIR + File.separator + "autoModule.jar";
+		Util.createJar(sources, jarPath, "1.8");
+		modules.add(jarPath);
+
+		// compile the main code which requires the other modules
+		String outFinal = OUTPUT_DIR + File.separator + "modularizedApp" + File.separator + "bin";
+		String srcFinal = OUTPUT_DIR + File.separator + "modularizedApp" + File.separator + "src";
+
+		List<String> files = new ArrayList<>();
+		writeFileCollecting(files, srcFinal,
+				"module-info.java",
+				"""
+					module split.app {\t
+						requires autoModule;
+						requires split.moduleA;
+						requires split.moduleB;
+					\t
+						exports pkg.bug.app;
+					}""");
+		writeFileCollecting(files, Paths.get(srcFinal, "pkg", "bug", "app").toString(),
+				"App.java",
+				"""
+					package pkg.bug.app;
+
+					import pkg.bug.split.subA.SubModuleA;
+
+					public class App {
+						public static void main(String[] args) {
+							new SubModuleA();
+						}
+					}""");
+		writeFileCollecting(files, Paths.get(srcFinal, "pkg", "bug", "service", "impl").toString(),
+				"AppServiceImpl.java",
+				"""
+					package pkg.bug.service.impl;
+
+					import pkg.bug.service.IService;
+
+					public class AppServiceImpl implements IService {
+
+					}""");
+
+		String modulePath = String.join(File.pathSeparator, modules);
+		StringBuilder buffer = new StringBuilder();
+		buffer.append("-d " + outFinal )
+			.append(" -9 ")
+			.append(" -proc:none ")
+			.append(" --module-path \"")
+			.append(Util.getJavaClassLibsAsString())
+			.append(File.pathSeparatorChar)
+			.append(modulePath)
+			.append("\" ")
+			.append(" --add-modules autoModule ");
+		runConformModuleTest(
+				files,
+				buffer,
+				"",
+				"""
+					----------
+					1. WARNING in ---OUTPUT_DIR_PLACEHOLDER---/modularizedApp/src/module-info.java (at line 2)
+						requires autoModule;
+						         ^^^^^^^^^^
+					Name of automatic module 'autoModule' is unstable, it is derived from the module's file name.
+					----------
+					1 problem (1 warning)
+					""",
+				false, outFinal);
+	}
 }