[BUG] [JSR-199] ECJ cannot resolve JPMS modules if using a user-provided

file manager #958 ClasspathJsr199: + various drafty additions for handling modules ModuleFinder: + find automatic modules, too + allow invocation without a parser for binary-only scans EclipseFileManager: + allow scanning all packages (by specifying an empty package name) + establish symmetry between setLocation() and setLocationFromPaths() + catch disruptive IAE from ModuleFinder.scanForModule()
eclipse-jdt · Dec 12, 2024 · c1212d0 · c1212d0
1 parent c2a99e6
commit c1212d0
Show file tree

Hide file tree

Showing 7 changed files with 171 additions and 14 deletions.
diff --git a/org.eclipse.jdt.compiler.tool.tests/resources/module_locations/GH954/module-info.java b/org.eclipse.jdt.compiler.tool.tests/resources/module_locations/GH954/module-info.java
@@ -0,0 +1,3 @@
+module org.example {
+    requires org.example.adder;
+}
diff --git a/...jdt.compiler.tool.tests/resources/module_locations/GH954/org/example/impl/AddNumbers.java b/...jdt.compiler.tool.tests/resources/module_locations/GH954/org/example/impl/AddNumbers.java
@@ -0,0 +1,11 @@
+package org.example.impl;
+
+import org.example.adder.Adder;
+
+public class AddNumbers {
+    public static void main(String[] args) {
+        int a = 123;
+        int b = 456;
+        System.out.println(new Adder().add(a, b));
+    }
+}
diff --git a/org.eclipse.jdt.compiler.tool.tests/resources/module_locations/automod_GH954.jar b/org.eclipse.jdt.compiler.tool.tests/resources/module_locations/automod_GH954.jar
diff --git a/...t.compiler.tool.tests/src/org/eclipse/jdt/compiler/tool/tests/CompilerToolJava9Tests.java b/...t.compiler.tool.tests/src/org/eclipse/jdt/compiler/tool/tests/CompilerToolJava9Tests.java
@@ -21,6 +21,9 @@
 import java.io.PrintWriter;
 import java.io.StringWriter;
 import java.io.Writer;
+import java.lang.reflect.InvocationHandler;
+import java.lang.reflect.Method;
+import java.lang.reflect.Proxy;
 import java.net.URL;
 import java.nio.charset.Charset;
 import java.nio.file.Files;
@@ -954,6 +957,67 @@ public void testBug533830_1() throws IOException {
 		//-- We must now also have a diagnostic
 		assertTrue("The diagnostic listener did not receive an error for the illegal option", b.listener().hasDiagnostic("option -source is not supported when --release is used"));
 	}
+
+	/**
+	 * Proxy that transparently delegates to a JavaFileManager impl, but hides the physical
+	 * implementation of the delegated file manager to prevent ECJ performing analysis based
+	 * on the superclasses of the file manager object.
+	 */
+	static class DemotingFileManagerProxy implements InvocationHandler {
+		private final JavaFileManager fileManager;
+
+		private DemotingFileManagerProxy(JavaFileManager fileManager) {
+			this.fileManager = fileManager;
+		}
+
+		@Override
+		public Object invoke(Object proxy, Method method, Object[] args) throws Throwable {
+			System.err.printf("Calling %s(%s)%n", method.getName(), Arrays.toString(args));
+			try {
+				var result = method.invoke(fileManager, args);
+				System.err.printf("Call to %s(%s) returned %s%n", method.getName(), Arrays.toString(args), result);
+				return result;
+			} catch (Throwable ex) {
+				ex.printStackTrace();
+				throw ex;
+			}
+		}
+
+		public static JavaFileManager demote(JavaFileManager fileManager) {
+			return (JavaFileManager) Proxy.newProxyInstance(
+					fileManager.getClass().getClassLoader(),
+					new Class<?>[]{ JavaFileManager.class },
+					new DemotingFileManagerProxy(fileManager)
+					);
+		}
+	}
+	public void testGH954() throws Exception {
+		File classOutput = new File(_tmpFolder);
+		JavaCompiler compiler = new EclipseCompiler();
+		StandardJavaFileManager standardFileManager = compiler.getStandardFileManager(null, null, null);
+
+		List<File> classPath = List.of(new File("resources/module_locations/automod_GH954.jar"));
+		List<File> sourcePath = List.of(new File("resources/module_locations/GH954"));
+		standardFileManager.setLocation(StandardLocation.CLASS_PATH, classPath);
+		standardFileManager.setLocation(StandardLocation.MODULE_PATH, classPath);
+		standardFileManager.setLocation(StandardLocation.SOURCE_PATH, sourcePath);
+		standardFileManager.setLocation(StandardLocation.CLASS_OUTPUT, List.of(classOutput));
+
+		// Make ECJ think we don't inherit StandardJavaFileManager by wrapping it.
+		JavaFileManager demotedFileManager = DemotingFileManagerProxy.demote(standardFileManager);
+		Iterable<JavaFileObject> compilationUnits = demotedFileManager.list(StandardLocation.SOURCE_PATH, "", Set.of(JavaFileObject.Kind.SOURCE), true);
+
+		CompilationTask task = compiler.getTask(
+				null,
+				demotedFileManager,
+				null,
+				List.of("--release", "11", "-verbose"),
+				List.of(),
+				compilationUnits
+		);
+
+		assertTrue(task.call());
+	}
 
 	/**
 	 * Helps with building a compiler invocation, handling the common parts of testing.

diff --git a/....jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/batch/ClasspathJsr199.java b/....jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/batch/ClasspathJsr199.java
@@ -18,6 +18,7 @@
 import java.io.File;
 import java.io.IOException;
 import java.io.InputStream;
+import java.nio.file.Path;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Collections;
@@ -26,13 +27,15 @@
 import java.util.List;
 import java.util.Set;
 import javax.tools.JavaFileManager;
+import javax.tools.JavaFileManager.Location;
 import javax.tools.JavaFileObject;
 import org.eclipse.jdt.core.compiler.CharOperation;
 import org.eclipse.jdt.internal.compiler.batch.FileSystem.Classpath;
 import org.eclipse.jdt.internal.compiler.classfmt.ClassFileReader;
 import org.eclipse.jdt.internal.compiler.classfmt.ClassFormatException;
 import org.eclipse.jdt.internal.compiler.env.IModule;
 import org.eclipse.jdt.internal.compiler.env.NameEnvironmentAnswer;
+import org.eclipse.jdt.internal.compiler.tool.ModuleLocationHandler.LocationWrapper;
 
 @SuppressWarnings({ "rawtypes", "unchecked" })
 public class ClasspathJsr199 extends ClasspathLocation {
@@ -100,7 +103,8 @@ public NameEnvironmentAnswer findClass(char[] typeName, String qualifiedPackageN
 			try (InputStream inputStream = jfo.openInputStream()) {
 				ClassFileReader reader = ClassFileReader.read(inputStream.readAllBytes(), qualifiedBinaryFileName);
 				if (reader != null) {
-					return new NameEnvironmentAnswer(reader, fetchAccessRestriction(qualifiedBinaryFileName));
+					char[] answerModule = this.module != null ? this.module.name() : null;
+					return new NameEnvironmentAnswer(reader, fetchAccessRestriction(qualifiedBinaryFileName), answerModule);
 				}
 			}
 		} catch (ClassFormatException e) {
@@ -156,12 +160,29 @@ public char[][][] findTypeNames(String aQualifiedPackageName, String moduleName)
 	public void initialize() throws IOException {
 		if (this.jrt != null) {
 			this.jrt.initialize();
+		} else if (this.location.isModuleOrientedLocation()) {
+			Iterable<Set<Location>> locationsForModules = this.fileManager.listLocationsForModules(this.location);
+			for (Set<Location> locs: locationsForModules) {
+				for (Location loc : locs) {
+					if (loc instanceof LocationWrapper wrapper) {
+						for (Path locPath : wrapper.getPaths()) {
+							File file = locPath.toFile();
+							IModule mod = ModuleFinder.scanForModule(this, file, null, true, null);
+							if (mod != null) {
+								return;
+							}
+						}
+					}
+				}
+			}
 		}
 	}
 
 	@Override
 	public void acceptModule(IModule mod) {
-		// do nothing
+		if (this.jrt != null)
+			return; // do nothing
+		this.module = mod;
 	}
 
 	@Override
@@ -195,6 +216,29 @@ public char[][] getModulesDeclaringPackage(String aQualifiedPackageName, String
 		return singletonModuleNameIf(result);
 	}
 
+	@Override
+	public char[][] listPackages() {
+		Set<String> packageNames = new HashSet<>();
+		try {
+			for (JavaFileObject fileObject : this.fileManager.list(this.location, "", fileTypes, true)) { //$NON-NLS-1$
+				String name = fileObject.getName();
+				int lastSlash = name.lastIndexOf('/');
+				if (lastSlash != -1) {
+					packageNames.add(name.substring(0, lastSlash).replace('/', '.'));
+				}
+			}
+			char[][] result = new char[packageNames.size()][];
+			int i = 0;
+			for (String s : packageNames) {
+				result[i++] = s.toCharArray();
+			}
+			return result;
+		} catch (IOException e) {
+			// ??
+		}
+		return CharOperation.NO_CHAR_CHAR;
+	}
+
 	@Override
 	public boolean hasCompilationUnit(String qualifiedPackageName, String moduleName) {
 		if (this.jrt != null)
@@ -250,6 +294,22 @@ public boolean hasAnnotationFileFor(String qualifiedTypeName) {
 	public Collection<String> getModuleNames(Collection<String> limitModules) {
 		if (this.jrt != null)
 			return this.jrt.getModuleNames(limitModules);
+		if (this.location.isModuleOrientedLocation()) {
+			Set<String> moduleNames = new HashSet<>();
+			try {
+				for (Set<Location> locs : this.fileManager.listLocationsForModules(this.location)) {
+					for (Location loc : locs) {
+						String moduleName = this.fileManager.inferModuleName(loc);
+						if (moduleName != null)
+							moduleNames.add(moduleName);
+					}
+				}
+				return moduleNames;
+			} catch (IOException e) {
+				// TODO Auto-generated catch block
+				e.printStackTrace();
+			}
+		}
 		return Collections.emptyList();
 	}
 
@@ -269,6 +329,11 @@ public IModule getModule(char[] name) {
 		return super.getModule(name);
 	}
 
+	@Override
+	public IModule getModule() {
+		return this.module;
+	}
+
 	@Override
 	public NameEnvironmentAnswer findClass(char[] typeName, String qualifiedPackageName,
 			String moduleName, String qualifiedBinaryFileName) {

diff --git a/...pse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/batch/ModuleFinder.java b/...pse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/batch/ModuleFinder.java
@@ -40,7 +40,7 @@ public class ModuleFinder {
 
 	public static List<FileSystem.Classpath> findModules(File f, String destinationPath, Parser parser, Map<String, String> options, boolean isModulepath, String release) {
 		List<FileSystem.Classpath> collector = new ArrayList<>();
-		scanForModules(destinationPath, parser, options, isModulepath, false, collector, f, release);
+		scanForModules(destinationPath, parser, options, isModulepath, !f.isDirectory(), collector, f, release);
 		return collector;
 	}
 
@@ -97,7 +97,8 @@ public boolean accept(File dir, String name) {
 						module = ModuleFinder.extractModuleFromClass(new File(file, fileName), modulePath);
 						break;
 					case IModule.MODULE_INFO_JAVA:
-						module = ModuleFinder.extractModuleFromSource(new File(file, fileName), parser, modulePath);
+						if (parser != null)
+							module = ModuleFinder.extractModuleFromSource(new File(file, fileName), parser, modulePath);
 						if (module == null)
 							return null;
 						String modName = new String(module.name());

diff --git a/...dt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/tool/EclipseFileManager.java b/...dt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/tool/EclipseFileManager.java
@@ -25,6 +25,7 @@
 import java.nio.file.Paths;
 import java.text.MessageFormat;
 import java.util.*;
+import java.util.function.Function;
 import javax.lang.model.SourceVersion;
 import javax.tools.FileObject;
 import javax.tools.JavaFileObject;
@@ -151,13 +152,14 @@ private void collectAllMatchingFiles(Location location, File file, String normal
 			Archive archive = this.getArchive(file);
 			if (archive != Archive.UNKNOWN_ARCHIVE) {
 				String key = normalizedPackageName;
+				boolean findAll = key.length() == 0;
 				if (!normalizedPackageName.endsWith("/")) {//$NON-NLS-1$
 					key += '/';
 				}
 				// we have an archive file
 				if (recurse) {
 					for (String packageName : archive.allPackages()) {
-						if (packageName.startsWith(key)) {
+						if (findAll || packageName.startsWith(key)) {
 							List<String[]> types = archive.getTypes(packageName);
 							if (types != null) {
 								for (String[] entry : types) {
@@ -1021,6 +1023,10 @@ private boolean isRunningJvm9() {
 	 */
 	@Override
 	public void setLocation(Location location, Iterable<? extends File> files) throws IOException {
+		internalSetLocation(location, files);
+		initModules(location, files, Function.identity());
+	}
+	private void internalSetLocation(Location location, Iterable<? extends File> files) {
 		if (location.isOutputLocation() && files != null) {
 			// output location
 			int count = 0;
@@ -1406,7 +1412,10 @@ public Iterable<? extends Path> getLocationAsPaths(Location location) {
 
 	@Override
 	public void setLocationFromPaths(Location location, Collection<? extends Path> paths) throws IOException {
-		setLocation(location, getFiles(paths));
+		internalSetLocation(location, getFiles(paths));
+		initModules(location, paths, Path::toFile);
+	}
+	private <P> void initModules(Location location, Iterable<? extends P> paths, Function<P,File> toFile) throws IOException {
 		if (location == StandardLocation.MODULE_PATH || location == StandardLocation.MODULE_SOURCE_PATH) {
 			// FIXME: same for module source path?
 			Map<String, String> options = new HashMap<>();
@@ -1421,15 +1430,19 @@ public void setLocationFromPaths(Location location, Collection<? extends Path> p
 						DefaultErrorHandlingPolicies.proceedWithAllProblems(),
 						compilerOptions,
 						new DefaultProblemFactory());
-			for (Path path : paths) {
-				List<Classpath> mp = ModuleFinder.findModules(path.toFile(), null,
-						new Parser(problemReporter, true), null, true, this.releaseVersion);
-				for (Classpath cp : mp) {
-					Collection<String> moduleNames = cp.getModuleNames(null);
-					for (String string : moduleNames) {
-						Path p = Paths.get(cp.getPath());
-						setLocationForModule(location, string,  Collections.singletonList(p));
+			for (P path : paths) {
+				try {
+					List<Classpath> mp = ModuleFinder.findModules(toFile.apply(path), null,
+							new Parser(problemReporter, true), null, true, this.releaseVersion);
+					for (Classpath cp : mp) {
+						Collection<String> moduleNames = cp.getModuleNames(null);
+						for (String string : moduleNames) {
+							Path p = Paths.get(cp.getPath());
+							setLocationForModule(location, string,  Collections.singletonList(p));
+						}
 					}
+				} catch (IllegalArgumentException iae) { // e.g., from ModuleFinder.scanForModule(Classpath, File, Parser, boolean, String)
+					// FIXME ignore for now
 				}
 			}
 		}