diff --git a/java-compiler-testing/src/main/java/io/github/ascopes/jct/containers/impl/ContainerGroupRepositoryImpl.java b/java-compiler-testing/src/main/java/io/github/ascopes/jct/containers/impl/ContainerGroupRepositoryImpl.java index 142d34c65..d5d7532c2 100644 --- a/java-compiler-testing/src/main/java/io/github/ascopes/jct/containers/impl/ContainerGroupRepositoryImpl.java +++ b/java-compiler-testing/src/main/java/io/github/ascopes/jct/containers/impl/ContainerGroupRepositoryImpl.java @@ -30,6 +30,7 @@ import javax.tools.JavaFileManager.Location; import org.apiguardian.api.API; import org.apiguardian.api.API.Status; +import org.jspecify.annotations.Nullable; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -64,8 +65,13 @@ public ContainerGroupRepositoryImpl(String release) { * have all modules extracted from them and registered individually rather than the provided path * being stored. * + *

Adding modules to an output location must ensure that the output location + * has a file system path associated with it as a package path already, otherwise the operation + * will likely fail + * * @param location the location to add. * @param pathRoot the path root to register with the location. + * @throws IllegalStateException if the location is an output location and is misconfigured. */ public void addPath(Location location, PathRoot pathRoot) { if (location instanceof ModuleLocation) { @@ -139,10 +145,11 @@ public void copyContainers(Location from, Location to) { public void createEmptyLocation(Location location) { if (location instanceof ModuleLocation) { throw new IllegalArgumentException("Cannot ensure a module location exists"); - } - - if (location.isOutputLocation()) { - getOrCreateOutputContainerGroup(location); + } else if (location.isOutputLocation()) { + throw new IllegalArgumentException( + "Cannot create an empty output location. It must be created by " + + "registering at least one file system path to enable output to." + ); } else if (location.isModuleOrientedLocation()) { getOrCreateModuleContainerGroup(location); } else { @@ -164,6 +171,7 @@ public void flush() { * @param location the location to get the container group for. * @return the container group, or {@code null} if no group is associated with the location. */ + @Nullable public ContainerGroup getContainerGroup(Location location) { ContainerGroup group = outputs.get(location); if (group == null) { @@ -172,100 +180,113 @@ public ContainerGroup getContainerGroup(Location location) { group = packageInputs.get(location); } } - return group; } /** - * Get a package container group. + * Get a module container group. * * @param location the location associated with the group to get. * @return the container group, or {@code null} if no group is associated with the location. */ - public PackageContainerGroup getPackageContainerGroup(Location location) { - return packageInputs.get(location); + @Nullable + public ModuleContainerGroup getModuleContainerGroup(Location location) { + return moduleInputs.get(location); } /** - * Get a snapshot of all the package container groups for inputs. + * Get a snapshot of all the module container groups for inputs. * - * @return the package container groups. + * @return the module container groups. */ - public Collection getPackageContainerGroups() { - return Set.copyOf(packageInputs.values()); + public Collection getModuleContainerGroups() { + return Set.copyOf(moduleInputs.values()); } /** - * Get a package-oriented container group from the input packages or the outputs. - * - *

This also accepts {@link ModuleLocation module locations} and will resolve them - * correctly. + * Get a module-oriented container group from the input modules or the outputs. * * @param location the location associated with the group to get. * @return the container group, or {@code null} if no group is associated with the location. */ - public PackageContainerGroup getPackageOrientedContainerGroup(Location location) { - if (location instanceof ModuleLocation) { - var moduleLocation = (ModuleLocation) location; - var group = getModuleOrientedContainerGroup(moduleLocation.getParent()); - return group == null - ? null - : group.getModule(moduleLocation.getModuleName()); - } - - var group = packageInputs.get(location); - return group == null - ? outputs.get(location) - : group; + @Nullable + public ModuleContainerGroup getModuleOrientedContainerGroup(Location location) { + var group = moduleInputs.get(location); + return group == null ? outputs.get(location) : group; } /** - * Get a module container group. + * Get an output container group. * * @param location the location associated with the group to get. * @return the container group, or {@code null} if no group is associated with the location. */ - public ModuleContainerGroup getModuleContainerGroup(Location location) { - return moduleInputs.get(location); + @Nullable + public OutputContainerGroup getOutputContainerGroup(Location location) { + return outputs.get(location); } /** - * Get a snapshot of all the module container groups for inputs. + * Get a snapshot of all the output container groups. * - * @return the module container groups. + * @return the output container groups. */ - public Collection getModuleContainerGroups() { - return Set.copyOf(moduleInputs.values()); + public Collection getOutputContainerGroups() { + return Set.copyOf(outputs.values()); } /** - * Get a module-oriented container group from the input modules or the outputs. + * Get a package container group. * * @param location the location associated with the group to get. * @return the container group, or {@code null} if no group is associated with the location. */ - public ModuleContainerGroup getModuleOrientedContainerGroup(Location location) { - var group = moduleInputs.get(location); - return group == null ? outputs.get(location) : group; + @Nullable + public PackageContainerGroup getPackageContainerGroup(Location location) { + return packageInputs.get(location); } /** - * Get an output container group. + * Get a snapshot of all the package container groups for inputs. + * + * @return the package container groups. + */ + public Collection getPackageContainerGroups() { + return Set.copyOf(packageInputs.values()); + } + + /** + * Get a package-oriented container group from the input packages or the outputs. + * + *

This also accepts {@link ModuleLocation module locations} and will resolve them + * correctly. * * @param location the location associated with the group to get. * @return the container group, or {@code null} if no group is associated with the location. */ - public OutputContainerGroup getOutputContainerGroup(Location location) { - return outputs.get(location); + @Nullable + public PackageContainerGroup getPackageOrientedContainerGroup(Location location) { + if (location instanceof ModuleLocation) { + var moduleLocation = (ModuleLocation) location; + var group = getModuleOrientedContainerGroup(moduleLocation.getParent()); + return group == null + ? null + : group.getModule(moduleLocation.getModuleName()); + } + + var group = packageInputs.get(location); + return group == null + ? outputs.get(location) + : group; } /** - * Get a snapshot of all the output container groups. + * Get the release. * - * @return the output container groups. + * @return the release. */ - public Collection getOutputContainerGroups() { - return Set.copyOf(outputs.values()); + public String getRelease() { + return release; } /** @@ -338,12 +359,10 @@ private void addModuleRoot(Location location, PathRoot pathRoot) { modules ); } else { - var group = location.isOutputLocation() - ? getOrCreateOutputContainerGroup(location) - : getOrCreateModuleContainerGroup(location); + var moduleGroup = getOrCreateModuleContainerGroup(location); modules.forEach((moduleName, modulePath) -> { - group.addModule(moduleName, new WrappingDirectoryImpl(modulePath)); + moduleGroup.addModule(moduleName, new WrappingDirectoryImpl(modulePath)); }); } } @@ -406,6 +425,11 @@ private ModuleContainerGroup getOrCreateModuleContainerGroup(Location location) ); } + // Note that by itself, this method should be considered unsafe. The caller MUST + // ensure at least one package root (which is not a module) gets registered upon + // creation to enable writing out files correctly to this location. If this is not + // done, then attempting to create a new module output will likely fail with errors + // since we will have no place to put the module on the file systems we manage. private OutputContainerGroup getOrCreateOutputContainerGroup(Location location) { return outputs.computeIfAbsent( location, diff --git a/java-compiler-testing/src/main/java/io/github/ascopes/jct/containers/impl/OutputContainerGroupImpl.java b/java-compiler-testing/src/main/java/io/github/ascopes/jct/containers/impl/OutputContainerGroupImpl.java index 1654be04d..3724089e1 100644 --- a/java-compiler-testing/src/main/java/io/github/ascopes/jct/containers/impl/OutputContainerGroupImpl.java +++ b/java-compiler-testing/src/main/java/io/github/ascopes/jct/containers/impl/OutputContainerGroupImpl.java @@ -146,7 +146,21 @@ private PackageContainerGroup newPackageGroup(ModuleLocation moduleLocation) { // For output locations, we only need the first root. We then just put a subdirectory // in there, as it reduces the complexity of this tenfold and means we don't have to // worry about creating more in-memory locations on the fly. + // + // The reason we have to do this relates to the fact that we will be provided an output + // directory to write to regardless of whether we want to write out modules or normal + // packages. var release = getRelease(); + var packages = getPackages(); + + if (packages.isEmpty()) { + // This *shouldn't* be reachable in most cases. + throw new IllegalStateException( + "Cannot add module " + moduleLocation + " to outputs. No output path has been " + + "provided for this location! Please register a package path to output generated " + + "modules to first before running the compiler." + ); + } // Use an anonymous class here to avoid the constraints that the PackageContainerGroupImpl // imposes on us. diff --git a/java-compiler-testing/src/main/java/io/github/ascopes/jct/filemanagers/JctFileManager.java b/java-compiler-testing/src/main/java/io/github/ascopes/jct/filemanagers/JctFileManager.java index cff99bf21..ceda81d18 100644 --- a/java-compiler-testing/src/main/java/io/github/ascopes/jct/filemanagers/JctFileManager.java +++ b/java-compiler-testing/src/main/java/io/github/ascopes/jct/filemanagers/JctFileManager.java @@ -39,18 +39,26 @@ public interface JctFileManager extends JavaFileManager { /** - * Add a path to a given location. + * Add a package-oriented path to a given location. + * + *

To add a module, first obtain the module location using + * {@link #getLocationForModule(Location, String)}, and pass that result to this call. * * @param location the location to use. * @param path the path to add. + * @see #getLocationForModule(Location, String) */ void addPath(Location location, PathRoot path); /** - * Add a collection of paths to a given location. + * Add a collection of package-oriented paths to a given location. + * + *

To add a module, first obtain the module location using + * {@link #getLocationForModule(Location, String)}, and pass that result to this call. * * @param location the location to use. * @param paths the paths to add. + * @see #getLocationForModule(Location, String) */ void addPaths(Location location, Collection paths); @@ -70,7 +78,15 @@ public interface JctFileManager extends JavaFileManager { * *

If the location already exists, then do not do anything. * + *

If the location is an output location, then this operation does not make any sense, since + * an empty location cannot have files output to it. In this case, you will likely get an + * exception. + * + *

Likewise, this operation does not make sense for module locations within a module-oriented + * location group, so this operation will fail with an error for those inputs as well. + * * @param location the location to apply an empty container for. + * @throws IllegalArgumentException if the location is an output location or a module location. */ void createEmptyLocation(Location location);