Bug 543604 - [9] Error when hovering a HashMapI20190515-1800

The bug was observed under the following conditions:

1. Debug a snippet with a map variable on Java 9 or above JRE.
2. Enable "Show Logical Structure" in the variables view.
3. Expand a local variable or a parameter of type java.util.Map or
inheriting, either in the debug hover in the Java source editor or in
the Variables view.
4. Observe an error text instead of content.

The error is caused by JDT debug attempting to compile a snippet as
follows:

package java.util;
abstract class Map___ implements java.util.Map {
void ___run() throws Throwable {
return entrySet().toArray();
}
}

This snippet is constructed in order to validate whether the expression
"return entrySet().toArray();" compiles in the context of the current
breakpoint.

On Java 9 and above, this snippet is not legal. java.util is already
contained in the JRE libraries; its not possible to define a package
like this and compile it without extra compile arguments (in particular
--patch-module).

To fix this problem, we add a special compile mode to JDT core, used by
JDT debug. When in this mode, the JDT compiler will ignore split package
problems. The compiled expression "return entrySet().toArray();" is then
used for evaluation against the underlying java.util.Map, which does
work with JRE 9+.

Change-Id: Iaaf3edde97e5006aecc8792df9da789b8eddbfce
Signed-off-by: Simeon Andreev <simeon.danailov.andreev@gmail.com>
Signed-off-by: Andrey Loskutov <loskutov@gmx.de>

6 files changed, 77 insertions, 11 deletions

diff --git a/org.eclipse.jdt.core.tests.model/src/org/eclipse/jdt/core/tests/model/ModuleBuilderTests.java b/org.eclipse.jdt.core.tests.model/src/org/eclipse/jdt/core/tests/model/ModuleBuilderTests.java
index 69256ef59c..bba2f4865e 100644
--- a/org.eclipse.jdt.core.tests.model/src/org/eclipse/jdt/core/tests/model/ModuleBuilderTests.java
+++ b/org.eclipse.jdt.core.tests.model/src/org/eclipse/jdt/core/tests/model/ModuleBuilderTests.java
@@ -3317,6 +3317,37 @@ public class ModuleBuilderTests extends ModifyingResourceTests {
 			deleteProject("com.greetings");
 		}
 	}
+
+	// test that the compilation of a class using same package defined in the java.util module
+	// works if a special option is given
+	public void test_no_conflicting_packages_for_debugger() throws CoreException {
+		deleteProject("P1");
+		Hashtable<String, String> javaCoreOptions = JavaCore.getOptions();
+		try {
+			Hashtable<String, String> newOptions=new Hashtable<>(javaCoreOptions);
+			newOptions.put(CompilerOptions.OPTION_JdtDebugCompileMode, JavaCore.ENABLED);
+			JavaCore.setOptions(newOptions);
+			String[] sources = new String[] {
+					"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);
+			assertNoErrors();
+		} finally {
+			deleteProject("debugger_project");
+			JavaCore.setOptions(javaCoreOptions);
+		}
+	}
 	// test that a package declared in a module conflicts with an accessible package
 	// of the same name declared in another required module
 	public void test_conflicting_packages_declaredvsaccessible() throws CoreException {
diff --git a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/ImportReference.java b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/ImportReference.java
index 438946eddc..e9aee50f6a 100644
--- a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/ImportReference.java
+++ b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/ImportReference.java
@@ -18,6 +18,7 @@ import java.util.Set;
 
 import org.eclipse.jdt.internal.compiler.ASTVisitor;
 import org.eclipse.jdt.internal.compiler.classfmt.ClassFileConstants;
+import org.eclipse.jdt.internal.compiler.impl.CompilerOptions;
 import org.eclipse.jdt.internal.compiler.lookup.*;
 
 public class ImportReference extends ASTNode {
@@ -70,7 +71,11 @@ public class ImportReference extends ASTNode {
 					declaringMods.add(incarnation.enclosingModule);
 			}
 			if (!declaringMods.isEmpty()) {
-				scope.problemReporter().conflictingPackagesFromOtherModules(this, declaringMods);
+				CompilerOptions compilerOptions = scope.compilerOptions();
+				boolean inJdtDebugCompileMode = compilerOptions.enableJdtDebugCompileMode;
+				if (!inJdtDebugCompileMode) {
+					scope.problemReporter().conflictingPackagesFromOtherModules(this, declaringMods);
+				}
 			}
 		}
 	}
diff --git a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/ModuleDeclaration.java b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/ModuleDeclaration.java
index 2a2aa534f1..af0e947fc9 100644
--- a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/ModuleDeclaration.java
+++ b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/ModuleDeclaration.java
@@ -29,6 +29,7 @@ import org.eclipse.jdt.internal.compiler.ASTVisitor;
 import org.eclipse.jdt.internal.compiler.ClassFile;
 import org.eclipse.jdt.internal.compiler.CompilationResult;
 import org.eclipse.jdt.internal.compiler.classfmt.ClassFileConstants;
+import org.eclipse.jdt.internal.compiler.impl.CompilerOptions;
 import org.eclipse.jdt.internal.compiler.impl.ReferenceContext;
 import org.eclipse.jdt.internal.compiler.lookup.BlockScope;
 import org.eclipse.jdt.internal.compiler.lookup.CompilationUnitScope;
@@ -332,8 +333,13 @@ public class ModuleDeclaration extends ASTNode implements ReferenceContext {
 	{
 		for (PackageBinding pack : requiredModule.getExports()) {
 			Set<ModuleBinding> mods = pack2mods.get(String.valueOf(pack.readableName()));
-			if (mods != null && mods.size() > 1)
-				skope.problemReporter().conflictingPackagesFromModules(pack, mods, requiresStat.sourceStart, requiresStat.sourceEnd);
+			if (mods != null && mods.size() > 1) {
+				CompilerOptions compilerOptions = skope.compilerOptions();
+				boolean inJdtDebugCompileMode = compilerOptions.enableJdtDebugCompileMode;
+				if (!inJdtDebugCompileMode) {
+					skope.problemReporter().conflictingPackagesFromModules(pack, mods, requiresStat.sourceStart, requiresStat.sourceEnd);
+				}
+			}
 		}
 	}
 
diff --git a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/QualifiedTypeReference.java b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/QualifiedTypeReference.java
index a9d93097a3..8178dec1f9 100644
--- a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/QualifiedTypeReference.java
+++ b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/QualifiedTypeReference.java
@@ -15,6 +15,7 @@ package org.eclipse.jdt.internal.compiler.ast;
 
 import org.eclipse.jdt.core.compiler.CharOperation;
 import org.eclipse.jdt.internal.compiler.ASTVisitor;
+import org.eclipse.jdt.internal.compiler.impl.CompilerOptions;
 import org.eclipse.jdt.internal.compiler.lookup.*;
 import org.eclipse.jdt.internal.compiler.problem.AbortCompilation;
 
@@ -125,10 +126,14 @@ public class QualifiedTypeReference extends TypeReference {
 	    if (packageBinding != null) {
 	    	PackageBinding uniquePackage = packageBinding.getVisibleFor(scope.module(), false);
 	    	if (uniquePackage instanceof SplitPackageBinding) {
-	    		SplitPackageBinding splitPackage = (SplitPackageBinding) uniquePackage;
-    			scope.problemReporter().conflictingPackagesFromModules(splitPackage, scope.module(), this.sourceStart, (int)this.sourcePositions[typeStart-1]);
-    			this.resolvedType = new ProblemReferenceBinding(this.tokens, null, ProblemReasons.Ambiguous);
-    			return null;
+	    		CompilerOptions compilerOptions = scope.compilerOptions();
+	    		boolean inJdtDebugCompileMode = compilerOptions.enableJdtDebugCompileMode;
+	    		if (!inJdtDebugCompileMode) {
+	    			SplitPackageBinding splitPackage = (SplitPackageBinding) uniquePackage;
+	    			scope.problemReporter().conflictingPackagesFromModules(splitPackage, scope.module(), this.sourceStart, (int)this.sourcePositions[typeStart-1]);
+	    			this.resolvedType = new ProblemReferenceBinding(this.tokens, null, ProblemReasons.Ambiguous);
+	    			return null;
+	    		}
 	    	}
 	    }
 	    rejectAnnotationsOnPackageQualifiers(scope, packageBinding);
diff --git a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/impl/CompilerOptions.java b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/impl/CompilerOptions.java
index ebd6b67561..5fda6a7089 100644
--- a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/impl/CompilerOptions.java
+++ b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/impl/CompilerOptions.java
@@ -205,6 +205,9 @@ public class CompilerOptions {
 	public static final String OPTION_EnablePreviews = "org.eclipse.jdt.core.compiler.problem.enablePreviewFeatures"; //$NON-NLS-1$
 	public static final String OPTION_ReportPreviewFeatures = "org.eclipse.jdt.core.compiler.problem.reportPreviewFeatures"; //$NON-NLS-1$
 
+	// Internally used option to allow debug framework compile evaluation snippets in context of modules, see bug 543604
+	public static final String OPTION_JdtDebugCompileMode = "org.eclipse.jdt.internal.debug.compile.mode"; //$NON-NLS-1$
+
 	/**
 	 * Possible values for configurable options
 	 */
@@ -528,6 +531,9 @@ public class CompilerOptions {
 	/** Master flag to enabled/disable all preview features */
 	public boolean enablePreviewFeatures;
 
+	/** Enable a less restrictive compile mode for JDT debug. */
+	public boolean enableJdtDebugCompileMode;
+
 	// keep in sync with warningTokenToIrritant and warningTokenFromIrritant
 	public final static String[] warningTokens = {
 		"all", //$NON-NLS-1$
@@ -1565,6 +1571,8 @@ public class CompilerOptions {
 
 		this.complainOnUninternedIdentityComparison = false;
 		this.enablePreviewFeatures = false;
+
+		this.enableJdtDebugCompileMode = false;
 	}
 
 	public void set(Map<String, String> optionsMap) {
@@ -2087,6 +2095,14 @@ public class CompilerOptions {
 		}
 		if ((optionValue = optionsMap.get(OPTION_ReportPreviewFeatures)) != null) 
 			updateSeverity(PreviewFeatureUsed, optionValue);
+
+		if ((optionValue = optionsMap.get(OPTION_JdtDebugCompileMode)) != null) {
+			if (ENABLED.equals(optionValue)) {
+				this.enableJdtDebugCompileMode = true;
+			} else if (DISABLED.equals(optionValue)) {
+				this.enableJdtDebugCompileMode = false;
+			}
+		}
 	}
 
 	private String[] stringToNameList(String optionValue) {
diff --git a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/lookup/CompilationUnitScope.java b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/lookup/CompilationUnitScope.java
index 453f97c4ab..c5f9f3700c 100644
--- a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/lookup/CompilationUnitScope.java
+++ b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/lookup/CompilationUnitScope.java
@@ -392,7 +392,10 @@ void faultInImports() {
 	this.tempImports = new ImportBinding[numberOfImports];
 	this.tempImports[0] = getDefaultImports()[0];
 	this.importPtr = 1;
-	
+
+	CompilerOptions compilerOptions = compilerOptions();
+	boolean inJdtDebugCompileMode = compilerOptions.enableJdtDebugCompileMode;
+
 	// keep static imports with normal imports until there is a reason to split them up
 	// on demand imports continue to be packages & types. need to check on demand type imports for fields/methods
 	// single imports change from being just types to types or fields
@@ -418,7 +421,7 @@ void faultInImports() {
 			}
 			if (importBinding instanceof PackageBinding) {
 				PackageBinding uniquePackage = ((PackageBinding)importBinding).getVisibleFor(module(), false);
-				if (uniquePackage instanceof SplitPackageBinding) {
+				if (uniquePackage instanceof SplitPackageBinding && !inJdtDebugCompileMode) {
 					SplitPackageBinding splitPackage = (SplitPackageBinding) uniquePackage;
 					problemReporter().conflictingPackagesFromModules(splitPackage, module(), importReference.sourceStart, importReference.sourceEnd);
 					continue nextImport;
@@ -431,7 +434,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) {
+			if (importBinding instanceof SplitPackageBinding && !inJdtDebugCompileMode) {
 				SplitPackageBinding splitPackage = (SplitPackageBinding) importBinding;
 				int sourceEnd = (int)(importReference.sourcePositions[splitPackage.compoundName.length-1] & 0xFFFF);
 				problemReporter().conflictingPackagesFromModules((SplitPackageBinding) importBinding, module(), importReference.sourceStart, sourceEnd);
@@ -462,7 +465,7 @@ void faultInImports() {
 					importedPackage = (PackageBinding) findImport(importedPackage.compoundName, false, true);
 					if (importedPackage != null)
 						importedPackage = importedPackage.getVisibleFor(module(), true);
-					if (importedPackage instanceof SplitPackageBinding) {
+					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);