Bug 519444: [9] Add --add-exports support in IDE

- integrate with reconciler (SearchableNameEnvironment), too
- refactoring & documentation

Change-Id: I2ef79502d10792255ec9b311b87dbff8a27db435

6 files changed, 138 insertions, 48 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 bfa1c2ae30..357ba8958c 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
@@ -4671,6 +4671,17 @@ public class ModuleBuilderTests extends ModifyingResourceTests {
 			p2.getProject().getWorkspace().build(IncrementalProjectBuilder.INCREMENTAL_BUILD, null);
 			markers = p2.getProject().findMarkers(null, true, IResource.DEPTH_INFINITE);
 			assertMarkers("Unexpected markers",	"",  markers);
+
+			// check that reconcile respects --add-reads, too:
+			this.problemRequestor.reset();
+			ICompilationUnit cu = getCompilationUnit("/com.greetings/src/com/greetings/MyTest.java");
+			cu.getWorkingCopy(this.wcOwner, null);
+			assertProblems(
+				"Unexpected problems",
+				"----------\n" +
+				"----------\n",
+				this.problemRequestor);
+
 		} finally {
 			deleteProject("org.astro");
 			deleteProject("com.greetings");
diff --git a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/env/IModuleAwareNameEnvironment.java b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/env/IModuleAwareNameEnvironment.java
index e692682b9d..cbddb7c0e1 100644
--- a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/env/IModuleAwareNameEnvironment.java
+++ b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/env/IModuleAwareNameEnvironment.java
@@ -119,5 +119,10 @@ public interface IModuleAwareNameEnvironment extends INameEnvironment {
 	IModule getModule(char[] moduleName);
 	IModule[] getAllAutomaticModules();
 
+	/**
+	 * Ask the name environment to perform any updates (add-exports or add-reads) to the given module.
+	 * @param module the compiler representation of the module to updates
+	 * @param kind selects what kind of updates should be performed
+	 */
 	default void applyModuleUpdates(IUpdatableModule module, IUpdatableModule.UpdateKind kind) { /* default: do nothing */ }
 }
diff --git a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/env/IUpdatableModule.java b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/env/IUpdatableModule.java
index a055a3b3fa..e4a6ca7073 100644
--- a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/env/IUpdatableModule.java
+++ b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/env/IUpdatableModule.java
@@ -21,10 +21,14 @@ import java.util.function.Consumer;
 
 /**
  * Interface to a module as needed to implement the updates for {@code --add-reads}
- * and {@code --add-exports} command line options.
+ * and {@code --add-exports} command line options (or corresponding classpath attributes).
  */
 public interface IUpdatableModule {
-	
+
+	/**
+	 * The compiler first wires modules only, before then wiring packages based on the module graph.
+	 * This enum selects one of these phases when asking the environment to perform its updates.
+	 */
 	enum UpdateKind { MODULE, PACKAGE }
 
 	/** Structure for update operations, sorted by {@link UpdateKind}. */
diff --git a/org.eclipse.jdt.core/model/org/eclipse/jdt/internal/core/ModuleUpdater.java b/org.eclipse.jdt.core/model/org/eclipse/jdt/internal/core/ModuleUpdater.java
new file mode 100644
index 0000000000..4275879625
--- /dev/null
+++ b/org.eclipse.jdt.core/model/org/eclipse/jdt/internal/core/ModuleUpdater.java
@@ -0,0 +1,95 @@
+/*******************************************************************************
+ * Copyright (c) 2017 GK Software AG, and others.
+ * All rights reserved. This program and the accompanying materials
+ * are made available under the terms of the Eclipse Public License v1.0
+ * which accompanies this distribution, and is available at
+ * http://www.eclipse.org/legal/epl-v10.html
+ *
+ * This is an implementation of an early-draft specification developed under the Java
+ * Community Process (JCP) and is made available for testing and evaluation purposes
+ * only. The code is not compatible with any specification of the JCP.
+ *
+ * Contributors:
+ *     Stephan Herrmann - initial API and implementation
+ *******************************************************************************/
+package org.eclipse.jdt.internal.core;
+
+import java.util.HashMap;
+import java.util.Map;
+import java.util.function.Consumer;
+
+import org.eclipse.core.runtime.IStatus;
+import org.eclipse.jdt.core.IClasspathAttribute;
+import org.eclipse.jdt.core.IClasspathEntry;
+import org.eclipse.jdt.core.compiler.CharOperation;
+import org.eclipse.jdt.internal.compiler.env.IModuleAwareNameEnvironment;
+import org.eclipse.jdt.internal.compiler.env.IUpdatableModule;
+import org.eclipse.jdt.internal.compiler.env.IUpdatableModule.UpdateKind;
+import org.eclipse.jdt.internal.compiler.env.IUpdatableModule.UpdatesByKind;
+import org.eclipse.jdt.internal.compiler.lookup.ModuleBinding;
+import org.eclipse.jdt.internal.core.util.Util;
+
+/**
+ * An instance of this class collects add-exports and add-reads options from a project's
+ * class path entries, and performs the corresponding updates when requested by the compiler.
+ */
+public class ModuleUpdater {
+
+	private Map<String,UpdatesByKind> moduleUpdates = new HashMap<>();
+
+	/**
+	 * Detects any ADD_EXPORTS or ADD_READS classpath attributes, parses the value,
+	 * and collects the resulting module updates.
+	 * @param entry a classpath entry of the current project.
+	 */
+	public void computeModuleUpdates(IClasspathEntry entry) {
+		for (IClasspathAttribute attribute : entry.getExtraAttributes()) {
+			String attributeName = attribute.getName();
+			if (attributeName.equals(IClasspathAttribute.ADD_EXPORTS)) {
+				String value = attribute.getValue(); // format: <source-module>/<package>=<target-module>(,<target-module>)*
+				int slash = value.indexOf('/');
+				int equals = value.indexOf('=');
+				if (slash != -1 && equals != -1) {
+					String modName = value.substring(0, slash);
+					char[] packName = value.substring(slash+1, equals).toCharArray();
+					char[][] targets = CharOperation.splitOn(',', value.substring(equals+1).toCharArray());
+					addModuleUpdate(modName, m -> m.addExports(packName, targets), UpdateKind.PACKAGE);
+				} else {
+					Util.log(IStatus.WARNING, "Invalid argument to add-exports: "+value); //$NON-NLS-1$
+				}
+			} else if (attributeName.equals(IClasspathAttribute.ADD_READS)) {
+				String value = attribute.getValue(); // format: <source-module>=<target-module>
+				int equals = value.indexOf('=');
+				if (equals != -1) {
+					String srcMod = value.substring(0, equals);
+					char[] targetMod = value.substring(equals+1).toCharArray();
+					addModuleUpdate(srcMod, m -> m.addReads(targetMod), UpdateKind.MODULE);
+				} else {
+					Util.log(IStatus.WARNING, "Invalid argument to add-reads: "+value); //$NON-NLS-1$
+				}
+			}
+		}
+	}
+
+	private void addModuleUpdate(String moduleName, Consumer<IUpdatableModule> update, UpdateKind kind) {
+		UpdatesByKind updates = this.moduleUpdates.get(moduleName);
+		if (updates == null) {
+			this.moduleUpdates.put(moduleName, updates = new UpdatesByKind());
+		}
+		updates.getList(kind, true).add(update);
+	}
+
+	/**
+	 * @see IModuleAwareNameEnvironment#applyModuleUpdates(IUpdatableModule, UpdateKind)
+	 */
+	public void applyModuleUpdates(IUpdatableModule compilerModule, UpdateKind kind) {
+		char[] name = compilerModule.name();
+		if (name != ModuleBinding.UNNAMED) { // can't update the unnamed module
+			UpdatesByKind updates = this.moduleUpdates.get(String.valueOf(name));
+			if (updates != null) {
+				for (Consumer<IUpdatableModule> update : updates.getList(kind, false))
+					update.accept(compilerModule);
+			}
+		}
+	}
+}
diff --git a/org.eclipse.jdt.core/model/org/eclipse/jdt/internal/core/SearchableEnvironment.java b/org.eclipse.jdt.core/model/org/eclipse/jdt/internal/core/SearchableEnvironment.java
index 227be17670..027978c711 100644
--- a/org.eclipse.jdt.core/model/org/eclipse/jdt/internal/core/SearchableEnvironment.java
+++ b/org.eclipse.jdt.core/model/org/eclipse/jdt/internal/core/SearchableEnvironment.java
@@ -32,7 +32,9 @@ import org.eclipse.jdt.internal.compiler.env.ICompilationUnit;
 import org.eclipse.jdt.internal.compiler.env.IModule;
 import org.eclipse.jdt.internal.compiler.env.IModuleAwareNameEnvironment;
 import org.eclipse.jdt.internal.compiler.env.ISourceType;
+import org.eclipse.jdt.internal.compiler.env.IUpdatableModule;
 import org.eclipse.jdt.internal.compiler.env.NameEnvironmentAnswer;
+import org.eclipse.jdt.internal.compiler.env.IUpdatableModule.UpdateKind;
 import org.eclipse.jdt.internal.compiler.impl.CompilerOptions;
 import org.eclipse.jdt.internal.compiler.lookup.ModuleBinding;
 import org.eclipse.jdt.internal.compiler.lookup.TypeConstants;
@@ -63,6 +65,8 @@ public class SearchableEnvironment
 	// moduleName -> IJavaProject | IJavaPackageFragmentRoot (lazily populated)
 	private Map<String,IJavaElement> knownModuleLocations; // null indicates: not using JPMS
 
+	private ModuleUpdater moduleUpdater;
+
 	/**
 	 * Creates a SearchableEnvironment on the given project
 	 */
@@ -81,6 +85,11 @@ public class SearchableEnvironment
 				}
 			}
 		}
+		if (CompilerOptions.versionToJdkLevel(project.getOption(JavaCore.COMPILER_COMPLIANCE, true)) >= ClassFileConstants.JDK9) {
+			this.moduleUpdater = new ModuleUpdater();
+			for (IClasspathEntry entry : project.getRawClasspath())
+				this.moduleUpdater.computeModuleUpdates(entry);
+		}
 	}
 
 	/**
@@ -915,4 +924,10 @@ public class SearchableEnvironment
 	public IModule[] getAllAutomaticModules() {
 		return new IModule[0];
 	}
+
+	@Override
+	public void applyModuleUpdates(IUpdatableModule module, UpdateKind kind) {
+		if (this.moduleUpdater != null)
+			this.moduleUpdater.applyModuleUpdates(module, kind);
+	}
 }
diff --git a/org.eclipse.jdt.core/model/org/eclipse/jdt/internal/core/builder/NameEnvironment.java b/org.eclipse.jdt.core/model/org/eclipse/jdt/internal/core/builder/NameEnvironment.java
index 55b612b662..eeaf55b972 100644
--- a/org.eclipse.jdt.core/model/org/eclipse/jdt/internal/core/builder/NameEnvironment.java
+++ b/org.eclipse.jdt.core/model/org/eclipse/jdt/internal/core/builder/NameEnvironment.java
@@ -27,11 +27,8 @@ import org.eclipse.jdt.core.*;
 import org.eclipse.jdt.core.compiler.CharOperation;
 import org.eclipse.jdt.internal.compiler.classfmt.ClassFileConstants;
 import org.eclipse.jdt.internal.compiler.env.*;
-import org.eclipse.jdt.internal.compiler.env.IUpdatableModule.UpdateKind;
-import org.eclipse.jdt.internal.compiler.env.IUpdatableModule.UpdatesByKind;
 import org.eclipse.jdt.internal.compiler.impl.CompilerOptions;
 import org.eclipse.jdt.internal.compiler.lookup.AutoModule;
-import org.eclipse.jdt.internal.compiler.lookup.ModuleBinding;
 import org.eclipse.jdt.internal.compiler.problem.AbortCompilation;
 import org.eclipse.jdt.internal.compiler.util.SimpleLookupTable;
 import org.eclipse.jdt.internal.compiler.util.SimpleSet;
@@ -41,7 +38,6 @@ import org.eclipse.jdt.internal.core.*;
 
 import java.io.*;
 import java.util.*;
-import java.util.function.Consumer;
 import java.util.stream.Collectors;
 
 @SuppressWarnings({"rawtypes", "unchecked"})
@@ -56,7 +52,7 @@ BuildNotifier notifier;
 SimpleSet initialTypeNames; // assumed that each name is of the form "a/b/ClassName", or, if a module is given: "my.mod:a/b/ClassName"
 SimpleLookupTable additionalUnits;
 /** Tasks resulting from add-reads or add-exports classpath attributes. */
-Map<String,UpdatesByKind> moduleUpdates = new HashMap<>();
+ModuleUpdater moduleUpdater;
 
 NameEnvironment(IWorkspaceRoot root, JavaProject javaProject, SimpleLookupTable binaryLocationsPerProject, BuildNotifier notifier) throws CoreException {
 	this.isIncrementalBuild = false;
@@ -119,6 +115,7 @@ private void computeClasspathLocations(
 	Map<String, IModulePathEntry> moduleEntries = null;
 	if (CompilerOptions.versionToJdkLevel(javaProject.getOption(JavaCore.COMPILER_COMPLIANCE, true)) >= ClassFileConstants.JDK9) {
 		moduleEntries = new HashMap<>(classpathEntries.length);
+		this.moduleUpdater = new ModuleUpdater();
 	}
 	IModuleDescription mod = null;
 	
@@ -130,32 +127,8 @@ private void computeClasspathLocations(
 		if (target == null) continue nextEntry;
 		boolean isOnModulePath = entry.isAutomaticModule();
 
-		for (IClasspathAttribute attribute : entry.getExtraAttributes()) {
-			String attributeName = attribute.getName();
-			if (attributeName.equals(IClasspathAttribute.ADD_EXPORTS)) {
-				String value = attribute.getValue(); // format: <source-module>/<package>=<target-module>(,<target-module>)*
-				int slash = value.indexOf('/');
-				int equals = value.indexOf('=');
-				if (slash != -1 && equals != -1) {
-					String modName = value.substring(0, slash);
-					char[] packName = value.substring(slash+1, equals).toCharArray();
-					char[][] targets = CharOperation.splitOn(',', value.substring(equals+1).toCharArray());
-					addModuleUpdate(modName, m -> m.addExports(packName, targets), UpdateKind.PACKAGE);
-				} else {
-					org.eclipse.jdt.internal.core.util.Util.log(IStatus.WARNING, "Invalid argument to add-exports: "+value); //$NON-NLS-1$
-				}
-			} else if (attributeName.equals(IClasspathAttribute.ADD_READS)) {
-				String value = attribute.getValue(); // format: <source-module>=<target-module>
-				int equals = value.indexOf('=');
-				if (equals != -1) {
-					String srcMod = value.substring(0, equals);
-					char[] targetMod = value.substring(equals+1).toCharArray();
-					addModuleUpdate(srcMod, m -> m.addReads(targetMod), UpdateKind.MODULE);
-				} else {
-					org.eclipse.jdt.internal.core.util.Util.log(IStatus.WARNING, "Invalid argument to add-reads: "+value); //$NON-NLS-1$
-				}
-			}
-		}
+		if (this.moduleUpdater != null)
+			this.moduleUpdater.computeModuleUpdates(entry);
 
 		switch(entry.getEntryKind()) {
 			case IClasspathEntry.CPE_SOURCE :
@@ -642,22 +615,9 @@ public IModule[] getAllAutomaticModules() {
 			.collect(Collectors.toSet());
 	return set.toArray(new IModule[set.size()]);
 }
-void addModuleUpdate(String moduleName, Consumer<IUpdatableModule> update, UpdateKind kind) {
-	UpdatesByKind updates = this.moduleUpdates.get(moduleName);
-	if (updates == null) {
-		this.moduleUpdates.put(moduleName, updates = new UpdatesByKind());
-	}
-	updates.getList(kind, true).add(update);
-}
 @Override
 public void applyModuleUpdates(IUpdatableModule compilerModule, IUpdatableModule.UpdateKind kind) {
-	char[] name = compilerModule.name();
-	if (name != ModuleBinding.UNNAMED) { // can't update the unnamed module
-		UpdatesByKind updates = this.moduleUpdates.get(String.valueOf(name));
-		if (updates != null) {
-			for (Consumer<IUpdatableModule> update : updates.getList(kind, false))
-				update.accept(compilerModule);
-		}
-	}
+	if (this.moduleUpdater != null)
+		this.moduleUpdater.applyModuleUpdates(compilerModule, kind);
 }
 }