Bug 537934 - Build never comes to an end and working is not possibleI20181021-1800I20181020-1800

- test & fix for the selection issue

Change-Id: I8a35d300d81d9d0f5f7953ec0f33e5d69026d15f

4 files changed, 182 insertions, 19 deletions

diff --git a/org.eclipse.jdt.core.tests.model/src/org/eclipse/jdt/core/tests/model/ResolveTests9.java b/org.eclipse.jdt.core.tests.model/src/org/eclipse/jdt/core/tests/model/ResolveTests9.java
index 63bfa4cec6..4f369a718a 100644
--- a/org.eclipse.jdt.core.tests.model/src/org/eclipse/jdt/core/tests/model/ResolveTests9.java
+++ b/org.eclipse.jdt.core.tests.model/src/org/eclipse/jdt/core/tests/model/ResolveTests9.java
@@ -16,6 +16,8 @@ package org.eclipse.jdt.core.tests.model;
 
 import java.io.File;
 import java.io.IOException;
+import java.util.ArrayList;
+import java.util.List;
 
 import org.eclipse.core.resources.IFile;
 import org.eclipse.core.resources.IFolder;
@@ -23,6 +25,7 @@ import org.eclipse.core.resources.IMarker;
 import org.eclipse.core.resources.IResource;
 import org.eclipse.core.resources.IncrementalProjectBuilder;
 import org.eclipse.core.runtime.CoreException;
+import org.eclipse.core.runtime.IPath;
 import org.eclipse.core.runtime.IProgressMonitor;
 import org.eclipse.core.runtime.Path;
 import org.eclipse.jdt.core.IClasspathAttribute;
@@ -43,6 +46,10 @@ import org.eclipse.jdt.internal.core.search.indexing.IndexRequest;
 import junit.framework.Test;
 
 public class ResolveTests9 extends AbstractJavaModelTests {
+	private static final int MODULE = 1;
+	private static final int WITHOUT_TEST = 2;
+	private static final int TEST = 4;
+
 	ICompilationUnit wc = null;
 
 	static {
@@ -91,6 +98,30 @@ public class ResolveTests9 extends AbstractJavaModelTests {
 		}
 		super.tearDown();
 	}
+
+	void addProjectEntry(IJavaProject thisProject, IJavaProject otherProject, int flags) throws JavaModelException {
+		addClasspathEntry(thisProject,
+				JavaCore.newProjectEntry(otherProject.getPath(), null, false, attributes(flags), false));
+	}
+	IClasspathAttribute[] attributes(int flags) {
+		List<IClasspathAttribute> attrs = new ArrayList<>();
+		if ((flags & MODULE) != 0)
+			attrs.add(JavaCore.newClasspathAttribute(IClasspathAttribute.MODULE, "true"));
+		if ((flags & WITHOUT_TEST) != 0)
+			attrs.add(JavaCore.newClasspathAttribute(IClasspathAttribute.WITHOUT_TEST_CODE, "true"));
+		if ((flags & TEST) != 0)
+			attrs.add(JavaCore.newClasspathAttribute(IClasspathAttribute.TEST, "true"));
+		return attrs.toArray(new IClasspathAttribute[attrs.size()]);
+	}
+	void addTestSrc(IJavaProject prj) throws CoreException {
+		IPath path = prj.getProject().getFullPath();
+		IClasspathEntry testSrc = JavaCore.newSourceEntry(path.append(new Path("src-test")), 
+				null, null, path.append(new Path("bin-test")),
+				new IClasspathAttribute[] {JavaCore.newClasspathAttribute(IClasspathAttribute.TEST, "true")});
+		addClasspathEntry(prj, testSrc);
+		createFolder(prj.getElementName() + "/src-test");
+	}
+
 	public void testModuleInfo_serviceImplementation_OK() throws CoreException {
 		IFile modInfo = null;
 		try {
@@ -398,4 +429,131 @@ public class ResolveTests9 extends AbstractJavaModelTests {
 			deleteProject(mod);
 		}
 	}
+
+	public void testBug537934() throws Exception {
+		if (!isJRE9) {
+			System.err.println("Test "+getName()+" requires a JRE 9");
+			return;
+		}
+		IJavaProject gui = null;
+		IJavaProject model = null;
+		IJavaProject type = null;
+		IJavaProject logg = null;
+		try {
+			// ---- module log:
+			//      - has log4j on the module path
+			logg = createJava9ProjectWithJREAttributes("com.igorion.log", new String[] {"src"}, attributes(MODULE));
+			String jarAbsPath = logg.getProject().getLocation()+"/log4j.jar";
+			createJar(new String[] {
+					"log4j/Dummy.java",
+					"package log4j;\n" +
+					"public class Dummy {}\n"
+				},
+				jarAbsPath);
+			addLibraryEntry(logg, new Path(jarAbsPath), null, null, null, null, attributes(MODULE), false);
+			createFolder("com.igorion.log/src/com/igorion/log");
+			createFile("com.igorion.log/src/com/igorion/log/ILog.java",
+					"package com.igorion.log;\n public interface ILog {}\n");
+			createFile("com.igorion.log/src/module-info.java",
+					"module com.igorion.log {\n" +
+					"	requires log4j;\n" +
+					"	exports com.igorion.log;\n" +
+					"}\n");
+			logg.getProject().build(IncrementalProjectBuilder.FULL_BUILD, null);
+			IMarker[] markers = logg.getProject().findMarkers(null, true, IResource.DEPTH_INFINITE);
+			assertMarkers("markers in com.igorion.log",
+					"Name of automatic module \'log4j\' is unstable, it is derived from the module\'s file name.",
+					markers);
+
+			// ---- module type:
+			//      - has test sources
+			type = createJava9Project("com.igorion.type");
+			createFolder("com.igorion.type/src/com/igorion/type");
+			createFile("com.igorion.type/src/com/igorion/type/IOther.java",
+					"package com.igorion.type;\n public interface IOther {}\n");
+			createFile("com.igorion.type/src/module-info.java",
+					"module com.igorion.type {\n" +
+					"	exports com.igorion.type;\n" +
+					"}\n");
+			addTestSrc(type);
+			type.getProject().build(IncrementalProjectBuilder.FULL_BUILD, null);
+			markers = type.getProject().findMarkers(null, true, IResource.DEPTH_INFINITE);
+			assertMarkers("markers in com.igorion.type", "", markers);
+
+			// ---- module model:
+			//      - has test sources
+			//      - has log4j on the module path
+			//      - has modules log & type on the module path without_test_code
+			model = createJava9ProjectWithJREAttributes("com.igorion.model", new String[] {"src"}, attributes(MODULE));
+			createFolder("com.igorion.model/src/com/igorion/model/define");
+			createFile("com.igorion.model/src/com/igorion/model/IModel.java",
+					"package com.igorion.model;\n public interface IModel {}\n");
+			createFile("com.igorion.model/src/com/igorion/model/define/Model.java",
+					"package com.igorion.model.define;\n" +
+					"import com.igorion.model.IModel;\n" +
+					"import java.util.Optional;\n" +
+					"public class Model {\n" +
+					"	public static synchronized Optional<IModel> instance() { return Optional.empty(); }\n" +
+					"}\n");
+			createFile("com.igorion.model/src/module-info.java",
+					"module com.igorion.model {\n" +
+					"	requires com.igorion.log;\n" +
+					"	exports com.igorion.model;\n" +
+					"	exports com.igorion.model.define;\n" +
+					"}\n");
+			addTestSrc(model);
+			addLibraryEntry(model, new Path(jarAbsPath), null, null, null, null, attributes(MODULE), false);
+			addProjectEntry(model, logg, MODULE|WITHOUT_TEST);
+			addProjectEntry(model, type, MODULE|WITHOUT_TEST);
+			model.getProject().build(IncrementalProjectBuilder.FULL_BUILD, null);
+			markers = model.getProject().findMarkers(null, true, IResource.DEPTH_INFINITE);
+			assertMarkers("markers in com.igorion.model", "", markers);
+
+			// ---- module gui:
+			//      - has log4j on the module path for test code
+			//      - has modules type, model, log on the module path without_test_code (order is significant)
+			gui = createJava9ProjectWithJREAttributes("com.igorion.gui", new String[] {"src"}, attributes(MODULE));
+			addTestSrc(gui);
+			createFolder("com.igorion.gui/src/com/igorion/gui");
+			String source = 
+					"package com.igorion.gui;\n" +
+					"import com.igorion.model.IModel;\n" +
+					"import com.igorion.model.define.Model;\n" +
+					"import java.util.Optional;\n" +
+					"public class Reproduce {\n" +
+					"	static void meth() {\n" +
+					"		Optional<IModel> oModel = Model.instance();\n" +
+					"		if (oModel.isPresent())\n" +
+					"			oModel.get();\n" +
+					"	}\n" +
+					"}\n";
+			createFile("com.igorion.gui/src/com/igorion/gui/Reproduce.java", source);
+			createFile("com.igorion.gui/src/module-info.java",
+					"module com.igorion.gui {\n" +
+					"	requires com.igorion.type;\n" +
+					"	requires com.igorion.model;\n" +
+					"}\n");
+			addLibraryEntry(gui, new Path(jarAbsPath), null, null, null, null, attributes(MODULE|TEST), false);
+			addProjectEntry(gui, type, MODULE|WITHOUT_TEST);
+			addProjectEntry(gui, model, MODULE|WITHOUT_TEST);
+			addProjectEntry(gui, logg, MODULE|WITHOUT_TEST);
+			gui.getProject().build(IncrementalProjectBuilder.FULL_BUILD, null);
+			markers = gui.getProject().findMarkers(null, true, IResource.DEPTH_INFINITE);
+			assertMarkers("markers in com.igorion.gui", "", markers);
+
+			// test that selection finds a fully resolved type Optional<IModel>:
+			ICompilationUnit unit = getCompilationUnit("com.igorion.gui/src/com/igorion/gui/Reproduce.java");
+			int start = source.indexOf("get(");
+			IJavaElement[] selected = unit.codeSelect(start, 3);
+			assertElementsEqual(
+					"Unexpected elements",
+					"get() [in Optional [in Optional.class [in java.util [in <module:java.base>]]]]",
+					selected);
+		} finally {
+			deleteProject(gui);
+			deleteProject(model);
+			deleteProject(type);
+			deleteProject(logg);
+		}
+	}
 }
diff --git a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/lookup/LookupEnvironment.java b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/lookup/LookupEnvironment.java
index 31672e0519..90f4431270 100644
--- a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/lookup/LookupEnvironment.java
+++ b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/lookup/LookupEnvironment.java
@@ -762,7 +762,7 @@ private PackageBinding computePackageFrom(char[][] constantPoolName, boolean isM
 						}
 					}
 				} else {
-					packageBinding = this.module.getVisiblePackage(parent, constantPoolName[i]);
+					packageBinding = this.module.getVisiblePackage(parent, constantPoolName[i], true);
 				}
 			}
 			if (packageBinding == null || packageBinding == TheNotFoundPackage) {
diff --git a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/lookup/ModuleBinding.java b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/lookup/ModuleBinding.java
index 2c9911efac..7cf8b0ed0e 100644
--- a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/lookup/ModuleBinding.java
+++ b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/lookup/ModuleBinding.java
@@ -487,7 +487,7 @@ public class ModuleBinding extends Binding implements IUpdatableModule {
 		binding = this.environment.getPackage0(name);
 		if (binding != null)
 			return binding;
-		binding = getVisiblePackage(null, name);
+		binding = getVisiblePackage(null, name, true);
 		// remember:
 		if (binding != null) {
 			this.environment.knownPackages.put(name, binding);
@@ -515,7 +515,7 @@ public class ModuleBinding extends Binding implements IUpdatableModule {
 		return binding;
 	}
 	// Given parent is visible in this module, see if there is sub package named name visible in this module
-	PackageBinding getVisiblePackage(PackageBinding parent, char[] name) {
+	PackageBinding getVisiblePackage(PackageBinding parent, char[] name, boolean considerRequiredModules) {
 		// check caches:
 		char[][] parentName = parent == null ? CharOperation.NO_CHAR_CHAR : parent.compoundName;
 		char[][] subPkgCompoundName = CharOperation.arrayConcat(parentName, name);
@@ -535,6 +535,7 @@ public class ModuleBinding extends Binding implements IUpdatableModule {
 		}
 
 		PackageBinding binding = null;
+		boolean packageMayBeIncomplete = !considerRequiredModules;
 		if (this.environment.useModuleSystem) {
 			IModuleAwareNameEnvironment moduleEnv = (IModuleAwareNameEnvironment) this.environment.nameEnvironment;
 			char[][] declaringModuleNames = moduleEnv.getModulesDeclaringPackage(parentName, name, nameForLookup());
@@ -542,19 +543,23 @@ public class ModuleBinding extends Binding implements IUpdatableModule {
 				if (!this.isUnnamed() && CharOperation.containsEqual(declaringModuleNames, this.moduleName)) {
 					// declared here, not yet known, so create it now:
 					binding = new PackageBinding(subPkgCompoundName, parent, this.environment, this);
-				} else {
+				} else if (considerRequiredModules) {
 					// visible but foreign (when current is unnamed or auto):
 					for (char[] declaringModuleName : declaringModuleNames) {
 						ModuleBinding declaringModule = this.environment.root.getModule(declaringModuleName);
-						if (declaringModule != null && !declaringModule.isPackageLookupActive) {
-							PackageBinding declaredPackage = declaringModule.getDeclaredPackage(parentName, name);
-							if (declaredPackage != null) {
-								// don't add foreign package to 'parent' (below), but to its own parent:
-								if (declaredPackage.parent != null)
-									declaredPackage.parent.addPackage(declaredPackage, declaringModule, true);
-								parent = null;
-								//
-								binding = SplitPackageBinding.combine(declaredPackage, binding, this);
+						if (declaringModule != null) {
+							if (declaringModule.isPackageLookupActive) {
+								packageMayBeIncomplete = true;
+							} else {
+								PackageBinding declaredPackage = declaringModule.getDeclaredPackage(parentName, name);
+								if (declaredPackage != null) {
+									// don't add foreign package to 'parent' (below), but to its own parent:
+									if (declaredPackage.parent != null)
+										declaredPackage.parent.addPackage(declaredPackage, declaringModule, true);
+									parent = null;
+									//
+									binding = SplitPackageBinding.combine(declaredPackage, binding, this);
+								}
 							}
 						}
 					}
@@ -566,11 +571,11 @@ public class ModuleBinding extends Binding implements IUpdatableModule {
 		}
 
 		// enrich with split-siblings from visible modules:
-		if (!isUnnamed()) {
+		if (!isUnnamed() && considerRequiredModules) {
 			binding = combineWithPackagesFromRequired(binding, subPkgCompoundName);
 		}
 		if (binding == null || !binding.isValidBinding()) {
-			if (parent != null)
+			if (parent != null && !packageMayBeIncomplete) // don't remember package that may still lack some siblings
 				parent.knownPackages.put(name, binding == null ? LookupEnvironment.TheNotFoundPackage : binding);
 			return null;
 		}
@@ -601,7 +606,7 @@ public class ModuleBinding extends Binding implements IUpdatableModule {
 
 		// check each sub package
 		for (int i = 1; i < qualifiedPackageName.length; i++) {
-			PackageBinding binding = getVisiblePackage(parent, qualifiedPackageName[i]); 
+			PackageBinding binding = getVisiblePackage(parent, qualifiedPackageName[i], true); 
 			if (binding == null || binding == LookupEnvironment.TheNotFoundPackage) {
 				return null;
 			}
@@ -622,12 +627,12 @@ public class ModuleBinding extends Binding implements IUpdatableModule {
 		// Returns a package binding if there exists such a package in the context of this module and it is observable
 		// A package is observable if it is declared in this module or it is exported by some required module
 		if (parentPackageName == null || parentPackageName.length == 0) {
-			return getVisiblePackage(null, packageName);
+			return getVisiblePackage(null, packageName, true);
 		}
 		PackageBinding binding = null;
 		PackageBinding parent = getVisiblePackage(parentPackageName);
 		if (parent != null && parent != LookupEnvironment.TheNotFoundPackage) {
-			binding = getVisiblePackage(parent, packageName);
+			binding = getVisiblePackage(parent, packageName, true);
 		}
 		if (binding != null)
 			return addPackage(binding, false);
diff --git a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/lookup/SplitPackageBinding.java b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/lookup/SplitPackageBinding.java
index f6133e8955..ee2f191342 100644
--- a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/lookup/SplitPackageBinding.java
+++ b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/lookup/SplitPackageBinding.java
@@ -112,7 +112,7 @@ public class SplitPackageBinding extends PackageBinding {
 				ModuleBinding moduleBinding = incarnation.enclosingModule;
 				if (moduleBinding == module)
 					continue;
-				PackageBinding next = moduleBinding.getVisiblePackage(incarnation, name); // TODO(SHMOD): reduce split-package work during this invocation?
+				PackageBinding next = moduleBinding.getVisiblePackage(incarnation, name, false);
 				childPackage = combine(next, childPackage, primaryModule);
 			}
 			return childPackage;