OPTION_JdtDebugCompileMode has surprising effects on imports (#2340)

fixes #445 Revisiting the 3 changes in faultInImports() from Bug 543604: + the check during resolution of on-demand imports remains, it is good + the second change was wrong, moved into findImport() this avoids answering a SplitPackageBinding (for conflict reporting) where in fact a type is being imported + the third change is being pulled up, because the whole purpose of that block is to detect package conflicts, which we are not interested in.
eclipse-jdt · Apr 13, 2024 · 597ca36 · 597ca36
1 parent 9512d3f
commit 597ca36
Show file tree

Hide file tree

Showing 3 changed files with 111 additions and 11 deletions.
diff --git a/...ore.compiler.batch/src/org/eclipse/jdt/internal/compiler/lookup/CompilationUnitScope.java b/...ore.compiler.batch/src/org/eclipse/jdt/internal/compiler/lookup/CompilationUnitScope.java
@@ -523,7 +523,7 @@ void faultInImports() {
 			recordImportBinding(new ImportBinding(compoundName, true, importBinding, importReference));
 		} else {
 			Binding importBinding = findSingleImport(compoundName, Binding.TYPE | Binding.FIELD | Binding.METHOD, importReference.isStatic());
-			if (importBinding instanceof SplitPackageBinding && !inJdtDebugCompileMode) {
+			if (importBinding instanceof SplitPackageBinding) {
 				SplitPackageBinding splitPackage = (SplitPackageBinding) importBinding;
 				int sourceEnd = (int)(importReference.sourcePositions[splitPackage.compoundName.length-1] & 0xFFFF);
 				problemReporter().conflictingPackagesFromModules((SplitPackageBinding) importBinding, module(), importReference.sourceStart, sourceEnd);
@@ -550,15 +550,17 @@ void faultInImports() {
 						problemReporter().importProblem(importReference, importedPackage);
 						continue nextImport;
 					}
-					// re-get to find a possible split package:
-					importedPackage = (PackageBinding) findImport(importedPackage.compoundName, false, true);
-					if (importedPackage != null)
-						importedPackage = importedPackage.getVisibleFor(module(), true);
-					if (importedPackage instanceof SplitPackageBinding && !inJdtDebugCompileMode) {
-						SplitPackageBinding splitPackage = (SplitPackageBinding) importedPackage;
-						int sourceEnd = (int) importReference.sourcePositions[splitPackage.compoundName.length-1];
-						problemReporter().conflictingPackagesFromModules(splitPackage, module(), importReference.sourceStart, sourceEnd);
-						continue nextImport;
+					if (!inJdtDebugCompileMode ) {
+						// re-get to find a possible split package:
+						importedPackage = (PackageBinding) findImport(importedPackage.compoundName, false, true);
+						if (importedPackage != null)
+							importedPackage = importedPackage.getVisibleFor(module(), true);
+						if (importedPackage instanceof SplitPackageBinding) {
+							SplitPackageBinding splitPackage = (SplitPackageBinding) importedPackage;
+							int sourceEnd = (int) importReference.sourcePositions[splitPackage.compoundName.length-1];
+							problemReporter().conflictingPackagesFromModules(splitPackage, module(), importReference.sourceStart, sourceEnd);
+							continue nextImport;
+						}
 					}
 				}
 			}
@@ -635,7 +637,7 @@ private Binding findImport(char[][] compoundName, int length) {
 			}
 			if (!(binding instanceof PackageBinding)) {
 				PackageBinding visibleFor = packageBinding.getVisibleFor(module, false); // filter out empty parent-packages
-				if (visibleFor instanceof SplitPackageBinding)
+				if (visibleFor instanceof SplitPackageBinding && !compilerOptions().enableJdtDebugCompileMode)
 					return visibleFor;
 				break foundNothingOrType;
 			}

diff --git a/....tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/UnnamedModuleTest.java b/....tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/UnnamedModuleTest.java
@@ -208,4 +208,60 @@ public void testConflictWithUnnamedModule() {
 			"----------\n";
 	runner.runNegativeTest();
 }
+public void testGH445_1() {
+	// ensure soundness of OPTION_JdtDebugCompileMode
+	String path = this.getCompilerTestsPluginDirectoryPath() + File.separator + "workspace" + File.separator + "ignore-unnamed-module-test.jar";
+	String[] defaultLibs = getDefaultClassPaths();
+	int len = defaultLibs.length;
+	String[] libs = new String[len+1];
+	System.arraycopy(defaultLibs, 0, libs, 0, len);
+	libs[len] = path;
+	Map<String, String> compilerOptions = getCompilerOptions();
+	compilerOptions.put(CompilerOptions.OPTION_JdtDebugCompileMode, CompilerOptions.ENABLED);
+	this.runConformTest(
+			true,
+			new String[] {
+				"X.java",
+				"import org.xml.sax.SAXException;\n" +
+				"public class X {\n" +
+				"	void foo() {\n" +
+				"		SAXException s;\n" +
+				"	}\n" +
+				"}"
+			},
+			libs,
+			compilerOptions,
+			"",
+			"",
+			"",
+			JavacTestOptions.DEFAULT);
+}
+public void testGH445_2() {
+	// ensure soundness of OPTION_JdtDebugCompileMode
+	String path = this.getCompilerTestsPluginDirectoryPath() + File.separator + "workspace" + File.separator + "ignore-unnamed-module-test.jar";
+	String[] defaultLibs = getDefaultClassPaths();
+	int len = defaultLibs.length;
+	String[] libs = new String[len+1];
+	System.arraycopy(defaultLibs, 0, libs, 0, len);
+	libs[len] = path;
+	Map<String, String> compilerOptions = getCompilerOptions();
+	compilerOptions.put(CompilerOptions.OPTION_JdtDebugCompileMode, CompilerOptions.ENABLED);
+	this.runConformTest(
+			true,
+			new String[] {
+				"X.java",
+				"import org.xml.sax.*;\n" +
+				"public class X {\n" +
+				"	void foo() {\n" +
+				"		SAXException s;\n" +
+				"	}\n" +
+				"}"
+			},
+			libs,
+			compilerOptions,
+			"",
+			"",
+			"",
+			JavacTestOptions.DEFAULT);
+}
 }
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
@@ -3431,6 +3431,48 @@ public void test_no_conflicting_packages_for_debugger_global() throws CoreExcept
 		}
 	}
 
+	public void test_no_conflicting_packages_for_debugger_both_named() throws CoreException {
+		// the current project is modular, hence we need to suppress a conflict between 2 named modules
+		Hashtable<String, String> javaCoreOptions = JavaCore.getOptions();
+		try {
+			String[] sources = new String[] {
+					"src/module-info.java",
+					"module Test {}\n",
+					"src/java/util/Map___.java",
+					"package java.util;\n" +
+					"abstract class Map___ implements java.util.Map {\n" +
+					"  Map___() {\n" +
+					"    super();\n" +
+					"  }\n" +
+					"  Object[] ___run() throws Throwable {\n" +
+					"    return entrySet().toArray();\n" +
+					"  }\n" +
+					"}"
+			};
+			IClasspathEntry dep = JavaCore.newContainerEntry(new Path(JavaCore.MODULE_PATH_CONTAINER_ID));
+			IJavaProject p1= setupModuleProject("debugger_project", sources, new IClasspathEntry[]{dep});
+			p1.getProject().getWorkspace().build(IncrementalProjectBuilder.FULL_BUILD, null);
+			IMarker[] markers = p1.getProject().findMarkers(null, true, IResource.DEPTH_INFINITE);
+			sortMarkers(markers);
+			assertMarkers("Unexpected markers",
+					"The package java.util conflicts with a package accessible from another module: java.base\n" +
+					"The package java.util is accessible from more than one module: Test, java.base\n" +
+					"The method entrySet() is undefined for the type Map___",
+					markers);
+
+			Hashtable<String, String> newOptions=new Hashtable<>(javaCoreOptions);
+			newOptions.put(CompilerOptions.OPTION_JdtDebugCompileMode, JavaCore.ENABLED);
+			JavaCore.setOptions(newOptions);
+			p1.getProject().getWorkspace().build(IncrementalProjectBuilder.FULL_BUILD, null);
+			assertNoErrors();
+
+			assertNull("Option should not be stored", JavaCore.getOption(CompilerOptions.OPTION_JdtDebugCompileMode));
+		} finally {
+			deleteProject("debugger_project");
+			JavaCore.setOptions(javaCoreOptions);
+		}
+	}
+
 	// test that the special OPTION_JdtDebugCompileMode cannot be persisted on a project
 	public void test_no_conflicting_packages_for_debugger_project() throws CoreException {
 		try {