Skip to content

Commit

Permalink
Fix package path computation in ClasspathScanner
Browse files Browse the repository at this point in the history
Prior to this commit no trailing `/` character was
appended to the computed package path.

Now, except for the default package `""`, a `/` is
appended to package path. This leads to corrected
and documented behavior even if two modules start
with the same name elements.

Fixes #2500
  • Loading branch information
sormuras authored Jan 26, 2021
1 parent e08ab9e commit 33f7d6e
Show file tree
Hide file tree
Showing 9 changed files with 54 additions and 12 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,8 @@ on GitHub.

==== Bug Fixes

* ❓
* Method `scanForClassesInPackage(String)` in `ClasspathScanner` now returns a valid list
of class names when the package name is equal to the name of a module on the module path.

==== Deprecations and Breaking Changes

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -211,7 +211,11 @@ private ClassLoader getClassLoader() {
}

private static String packagePath(String packageName) {
return packageName.replace(PACKAGE_SEPARATOR_CHAR, CLASSPATH_RESOURCE_PATH_SEPARATOR);
if (packageName.isEmpty()) {
return "";
}
String path = packageName.replace(PACKAGE_SEPARATOR_CHAR, CLASSPATH_RESOURCE_PATH_SEPARATOR);
return path + CLASSPATH_RESOURCE_PATH_SEPARATOR;
}

private List<URI> getRootUrisForPackage(String basePackageName) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
import java.util.function.Predicate;
import java.util.logging.Level;
import java.util.logging.LogRecord;
import java.util.spi.ToolProvider;
import java.util.stream.Collectors;

import org.junit.jupiter.api.Test;
Expand Down Expand Up @@ -180,22 +181,45 @@ void scanForClassesInPackage() {

@Test
// #2500
void scanForClassesInPackageWithinModuleSharingNames() throws Exception {
var jarfile = getClass().getResource("/[email protected]");
void scanForClassesInPackageWithinModulesSharingNamePrefix(@TempDir Path temp) throws Exception {
var moduleSourcePath = Path.of(getClass().getResource("/modules-2500/").toURI()).toString();
run("javac", "--module", "foo,foo.bar", "--module-source-path", moduleSourcePath, "-d", temp.toString());

var module = "com.greetings";
checkModules2500(ModuleFinder.of(temp)); // exploded modules

var foo = temp.resolve("foo.jar");
var bar = temp.resolve("foo.bar.jar");
run("jar", "--create", "--file", foo.toString(), "-C", temp.resolve("foo").toString(), ".");
run("jar", "--create", "--file", bar.toString(), "-C", temp.resolve("foo.bar").toString(), ".");

checkModules2500(ModuleFinder.of(foo, bar)); // jarred modules

System.gc(); // required on Windows in order to release JAR file handles
}

private static int run(String tool, String... args) {
return ToolProvider.findFirst(tool).orElseThrow().run(System.out, System.err, args);
}

private void checkModules2500(ModuleFinder finder) {
var root = "foo.bar";
var before = ModuleFinder.of();
var finder = ModuleFinder.of(Path.of(jarfile.toURI()));
var boot = ModuleLayer.boot();
var configuration = boot.configuration().resolveAndBind(before, finder, Set.of(module));
var configuration = boot.configuration().resolve(before, finder, Set.of(root));
var parent = ClassLoader.getPlatformClassLoader();
var layer = ModuleLayer.defineModulesWithOneLoader(configuration, List.of(boot), parent).layer();

var classpathScanner = new ClasspathScanner(() -> layer.findLoader(module), ReflectionUtils::tryToLoadClass);

var classes = classpathScanner.scanForClassesInPackage("com.greetings", allClasses);
var classNames = classes.stream().map(Class::getName).collect(Collectors.toList());
assertThat(classNames).hasSize(1).contains("com.greetings.Main");
var classpathScanner = new ClasspathScanner(() -> layer.findLoader(root), ReflectionUtils::tryToLoadClass);
{
var classes = classpathScanner.scanForClassesInPackage("foo", allClasses);
var classNames = classes.stream().map(Class::getName).collect(Collectors.toList());
assertThat(classNames).hasSize(2).contains("foo.Foo", "foo.bar.FooBar");
}
{
var classes = classpathScanner.scanForClassesInPackage("foo.bar", allClasses);
var classNames = classes.stream().map(Class::getName).collect(Collectors.toList());
assertThat(classNames).hasSize(1).contains("foo.bar.FooBar");
}
}

@Test
Expand Down
Binary file removed platform-tests/src/test/resources/[email protected]
Binary file not shown.
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
package foo.bar;

public class FooBar {}
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
open module foo.bar {
requires foo;
}
3 changes: 3 additions & 0 deletions platform-tests/src/test/resources/modules-2500/foo/Foo.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
package foo;

public class Foo {}
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
module foo {
exports foo;
}
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,7 @@ private static List<String> compileModules(Path temp, Writer out, Writer err, Fu
try (var walk = Files.walk(base)) {
var projects = walk.filter(path -> path.endsWith("module-info.java")) //
.map(base::relativize) //
.filter(path -> !path.startsWith("platform-tests")) //
.filter(path -> !path.startsWith("platform-tooling-support-tests")) //
.map(base::resolve) //
.map(Project::new) //
Expand Down

0 comments on commit 33f7d6e

Please sign in to comment.