diff --git a/src/main/java/spoon/reflect/factory/PackageFactory.java b/src/main/java/spoon/reflect/factory/PackageFactory.java index 9ba0ca539b8..3fcf23a708a 100644 --- a/src/main/java/spoon/reflect/factory/PackageFactory.java +++ b/src/main/java/spoon/reflect/factory/PackageFactory.java @@ -11,8 +11,8 @@ import java.util.ArrayList; import java.util.Collection; import java.util.List; -import java.util.Objects; import java.util.StringTokenizer; +import spoon.SpoonException; import spoon.reflect.declaration.CtModule; import spoon.reflect.declaration.CtPackage; import spoon.reflect.declaration.CtPackageDeclaration; @@ -159,26 +159,73 @@ public CtPackage get(String qualifiedName) { throw new RuntimeException("Invalid package name " + qualifiedName); } - return factory.getModel().getAllModules().stream() - .map(module -> getPackageFromModule(qualifiedName, module)) - .filter(Objects::nonNull) - .findFirst() - .orElse(null); + // Find package with the most contained types. If a module exports package "foo.bar" and the + // other "foo.bar.baz", *both modules* will contain a "foo.bar" package in spoon. As + // javac (yes, javac. This is not in the spec but would be a colossal pain for many things if + // it were ever allowed) does not allow overlapping packages, one of them will be a synthetic + // package spoon creates as a parent for "foo.bar.baz". This package will *never* have any + // types in it. + // JDT does allow it, but the chances of a real-word program actually having overlap are slim. + // + // However, if the "foo.bar.baz" module is found first in "getAllModules()", we will find the + // synthetic "foo.bar" package in it. As that one contains no types, all queries for types in + // it will fail! + // + // To solve this we look for the package with at least one contained type, effectively + // filtering out any synthetic packages. + int foundPackageCount = 0; + CtPackage packageWithTypes = null; + CtPackage lastNonNullPackage = null; + for (CtModule module : factory.getModel().getAllModules()) { + CtPackage aPackage = getPackageFromModule(qualifiedName, module); + if (aPackage == null) { + continue; + } + lastNonNullPackage = aPackage; + if (!aPackage.getTypes().isEmpty()) { + packageWithTypes = aPackage; + foundPackageCount++; + } + } + + if (foundPackageCount > 1) { + throw new SpoonException( + "Ambiguous package name detected. If you believe the code you analyzed is correct, please" + + " file an issue and reference https://github.com/INRIA/spoon/issues/4051. " + + "Error details: Found " + foundPackageCount + " non-empty packages with name " + + "'" + qualifiedName + "'" + ); + } + + // Return a non synthetic package but if *no* package had any types we return the last one. + // This ensures that you can also retrieve empty packages with this API + return packageWithTypes != null ? packageWithTypes : lastNonNullPackage; } /** * @param qualifiedName Qualified name of a package. - * @param module A module in which to search for the package. + * @param ctModule A module in which to search for the package. * @return The package if found in this module, otherwise null. */ - private static CtPackage getPackageFromModule(String qualifiedName, CtModule module) { - StringTokenizer token = new StringTokenizer(qualifiedName, CtPackage.PACKAGE_SEPARATOR); - CtPackage current = module.getRootPackage(); - while (token.hasMoreElements() && current != null) { - current = current.getPackage(token.nextToken()); + private static CtPackage getPackageFromModule(String qualifiedName, CtModule ctModule) { + int index = 0; + int nextIndex; + CtPackage current = ctModule.getRootPackage(); + + if (qualifiedName.isEmpty() || current == null) { + return current; + } + + while ((nextIndex = qualifiedName.indexOf(CtPackage.PACKAGE_SEPARATOR_CHAR, index)) >= 0) { + current = current.getPackage(qualifiedName.substring(index, nextIndex)); + index = nextIndex + 1; + + if (current == null) { + return null; + } } - return current; + return current.getPackage(qualifiedName.substring(index)); } /** diff --git a/src/test/java/spoon/test/module/TestModule.java b/src/test/java/spoon/test/module/TestModule.java index cb867273169..5d9a1ac089e 100644 --- a/src/test/java/spoon/test/module/TestModule.java +++ b/src/test/java/spoon/test/module/TestModule.java @@ -376,4 +376,33 @@ public void testModuleComplianceLevelException() { throw e; } } + + @Test + public void testModuleOverlappingPackages() { + // contract: Non-synthetic package is returned for modules with overlapping packages + // Modules might contain "overlapping" (only in spoon! In java the packages are distinct) + // packages: + // first + // `- test + // `- parent <- This is only here because nested is a "sub package". This might be found + // first when looking for modules containing "test.parent". In that case + // we have a problem. + // `- nested <- This is exported and in first. + // `- Foo <- This is the actual class in the module + // second + // `- test + // `- parent + // `- Bar + Launcher launcher = new Launcher(); + launcher.getEnvironment().setComplianceLevel(9); + launcher.addInputResource(MODULE_RESOURCES_PATH + "/overlapping-packages"); + CtModel ctModel = launcher.buildModel(); + assertEquals(3, ctModel.getAllModules().size()); + assertNotNull( + "Wrong package picked, the synthetic one comes first in alphabetical order but" + + " doesn't have the classes we want!", + launcher.getFactory().Type().get("test.parent.Bar") + ); + assertNotNull("", launcher.getFactory().Type().get("test.parent.nested.Foo")); + } } diff --git a/src/test/resources/spoon/test/module/overlapping-packages/first/module-info-tpl b/src/test/resources/spoon/test/module/overlapping-packages/first/module-info-tpl new file mode 100644 index 00000000000..98d0f553d42 --- /dev/null +++ b/src/test/resources/spoon/test/module/overlapping-packages/first/module-info-tpl @@ -0,0 +1,3 @@ +module first { + exports test.parent.nested; +} diff --git a/src/test/resources/spoon/test/module/overlapping-packages/first/test/parent/nested/Foo.java b/src/test/resources/spoon/test/module/overlapping-packages/first/test/parent/nested/Foo.java new file mode 100644 index 00000000000..c9d548bc2e9 --- /dev/null +++ b/src/test/resources/spoon/test/module/overlapping-packages/first/test/parent/nested/Foo.java @@ -0,0 +1,3 @@ +package test.parent.nested; + +class Foo {} diff --git a/src/test/resources/spoon/test/module/overlapping-packages/second/module-info-tpl b/src/test/resources/spoon/test/module/overlapping-packages/second/module-info-tpl new file mode 100644 index 00000000000..c7d3cf2dbe7 --- /dev/null +++ b/src/test/resources/spoon/test/module/overlapping-packages/second/module-info-tpl @@ -0,0 +1,3 @@ +module second { + exports test.parent; +} diff --git a/src/test/resources/spoon/test/module/overlapping-packages/second/test/parent/Bar.java b/src/test/resources/spoon/test/module/overlapping-packages/second/test/parent/Bar.java new file mode 100644 index 00000000000..8526f032fc2 --- /dev/null +++ b/src/test/resources/spoon/test/module/overlapping-packages/second/test/parent/Bar.java @@ -0,0 +1,3 @@ +package test.parent; + +class Bar {}