File manager implementation tests, simplify OutputContainerGroup expectations

java-compiler-testing/src/main/java/io/github/ascopes/jct/containers/OutputContainerGroup.java

@@ -26,6 +26,10 @@
  * <p>These can behave as if they are module-oriented, or non-module-oriented.
  * It is down to the implementation to mediate access between modules and their files.
  *
+ * <p>Operations on modules should first {@link #getModule(String) get} or
+ * {@link #getOrCreateModule(String) create} the module, and then operate on that sub-container
+ * group. Operations on non-module packages should operate on this container group directly.
+ *
  * @author Ashley Scopes
  * @since 0.0.1
  */

java-compiler-testing/src/main/java/io/github/ascopes/jct/containers/impl/ContainerGroupRepositoryImpl.java

@@ -44,7 +44,7 @@
  */
 @API(since = "0.0.1", status = Status.STABLE)
 @ThreadSafe
-public class ContainerGroupRepositoryImpl {
+public final class ContainerGroupRepositoryImpl implements AutoCloseable {
 
   private static final Logger LOGGER = LoggerFactory.getLogger(ContainerGroupRepositoryImpl.class);
 
@@ -88,14 +88,10 @@ public void addPath(Location location, PathRoot pathRoot) {
     }
   }
 
-  /**
-   * A bulk-style call for {@link #addPath(Location, PathRoot)}.
-   *
-   * @param location  the location to add.
-   * @param pathRoots the path roots to register with the location.
-   */
-  public void addPaths(Location location, Iterable<? extends PathRoot> pathRoots) {
-    pathRoots.forEach(pathRoot -> addPath(location, pathRoot));
+  @Override
+  public void close() {
+    // Nothing to do here. This is a placeholder in case we ever need to allow closing logic
+    // in the future.
   }
 
   /**
@@ -158,6 +154,14 @@ public void createEmptyLocation(Location location) {
     }
   }
 
+  /**
+   * Perform any flushing operation, if needed.
+   */
+  public void flush() {
+    // Nothing to do here. This is a placeholder for a future implementation if we ever need to
+    // enable flushing.
+  }
+
   /**
    * Get a container group.
    *
@@ -421,5 +425,4 @@ private OutputContainerGroup getOrCreateOutputContainerGroup(Location location)
         outputLocation -> new OutputContainerGroupImpl(outputLocation, release)
     );
   }
-
 }

java-compiler-testing/src/main/java/io/github/ascopes/jct/containers/impl/OutputContainerGroupImpl.java

@@ -29,12 +29,10 @@
 import java.util.HashMap;
 import java.util.Map;
 import java.util.Set;
-import javax.annotation.Nullable;
 import javax.annotation.WillCloseWhenClosed;
 import javax.annotation.WillNotClose;
 import javax.annotation.concurrent.ThreadSafe;
 import javax.tools.JavaFileManager.Location;
-import javax.tools.JavaFileObject.Kind;
 import org.apiguardian.api.API;
 import org.apiguardian.api.API.Status;
 
@@ -44,6 +42,10 @@
  * <p>These can contain packages <strong>and</strong> modules of packages together, and thus
  * are slightly more complicated internally as a result.
  *
+ * <p>Operations on modules should first {@link #getModule(String) get} or
+ * {@link #getOrCreateModule(String) create} the module, and then operate on that sub-container
+ * group. Operations on non-module packages should operate on this container group directly.
+ *
  * @author Ashley Scopes
  * @since 0.0.1
  */
@@ -120,84 +122,6 @@ public PackageContainerGroup getModule(String module) {
         .orElse(null);
   }
 
-  @Override
-  @Nullable
-  public PathFileObject getFileForInput(String packageName, String relativeName) {
-    var moduleName = extractModuleName(packageName);
-
-    if (moduleName != null) {
-      var module = getModule(moduleName);
-
-      if (module != null) {
-        var realPackageName = extractPackageName(packageName);
-        var file = module.getFileForInput(realPackageName, relativeName);
-
-        if (file != null) {
-          return file;
-        }
-      }
-    }
-
-    return super.getFileForInput(packageName, relativeName);
-  }
-
-  @Override
-  @Nullable
-  public PathFileObject getFileForOutput(String packageName, String relativeName) {
-    var moduleName = extractModuleName(packageName);
-
-    if (moduleName != null) {
-      var module = getOrCreateModule(moduleName);
-      var realPackageName = extractPackageName(packageName);
-      var file = module.getFileForOutput(realPackageName, relativeName);
-
-      if (file != null) {
-        return file;
-      }
-    }
-
-    return super.getFileForOutput(packageName, relativeName);
-  }
-
-  @Override
-  @Nullable
-  public PathFileObject getJavaFileForInput(String className, Kind kind) {
-    var moduleName = extractModuleName(className);
-
-    if (moduleName != null) {
-      var module = getModule(moduleName);
-
-      if (module != null) {
-        var realClassName = extractPackageName(className);
-        var file = module.getJavaFileForInput(realClassName, kind);
-
-        if (file != null) {
-          return file;
-        }
-      }
-    }
-
-    return super.getJavaFileForInput(className, kind);
-  }
-
-  @Override
-  @Nullable
-  public PathFileObject getJavaFileForOutput(String className, Kind kind) {
-    var moduleName = extractModuleName(className);
-
-    if (moduleName != null) {
-      var module = getOrCreateModule(moduleName);
-      var realClassName = extractPackageName(className);
-      var file = module.getJavaFileForOutput(realClassName, kind);
-
-      if (file != null) {
-        return file;
-      }
-    }
-
-    return super.getJavaFileForOutput(className, kind);
-  }
-
   @Override
   public Set<Location> getLocationsForModules() {
     return Set.copyOf(modules.keySet());
@@ -238,19 +162,4 @@ private PackageContainerGroup newPackageGroup(ModuleLocation moduleLocation) {
     group.addPackage(pathWrapper);
     return group;
   }
-
-  @Nullable
-  private static String extractModuleName(String binaryName) {
-    // extractModuleName("foo.bar.Baz") -> null
-    // extractModuleName("some.module/foo.bar.Baz") -> "some.module"
-    var separatorIndex = binaryName.indexOf('/');
-    return separatorIndex == -1 ? null : binaryName.substring(0, separatorIndex);
-  }
-
-  private static String extractPackageName(String binaryName) {
-    // extractPackageName("foo.bar.Baz") -> "foo.bar.Baz"
-    // extractPackageName("some.module/foo.bar.Baz") -> "foo.bar.Baz"
-    var separatorIndex = binaryName.indexOf('/');
-    return separatorIndex == -1 ? binaryName : binaryName.substring(separatorIndex + 1);
-  }
 }

java-compiler-testing/src/main/java/io/github/ascopes/jct/filemanagers/impl/JctFileManagerImpl.java

@@ -69,16 +69,16 @@ public void addPath(Location location, PathRoot pathRoot) {
 
   @Override
   public void addPaths(Location location, Collection<? extends PathRoot> pathRoots) {
-    repository.addPaths(location, pathRoots);
+    pathRoots.forEach(pathRoot -> addPath(location, pathRoot));
   }
 
   @Override
   public void close() {
-    // Nothing to close here.
+    repository.close();
   }
 
   @Override
-  public boolean contains(Location location, FileObject fo) throws IOException {
+  public boolean contains(Location location, FileObject fo) {
     if (!(fo instanceof PathFileObject)) {
       return false;
     }
@@ -99,7 +99,7 @@ public void createEmptyLocation(Location location) {
 
   @Override
   public void flush() {
-    // Don't do anything else for now.
+    repository.flush();
   }
 
   @Nullable
@@ -144,25 +144,23 @@ public FileObject getFileForOutput(
   ) {
     requireOutputLocation(location);
 
-    // If we have a module, we may need to create a brand new location for it.
+    PackageContainerGroup group = null;
+
+    // If we have a module, we may need to create a brand-new location for it.
     if (location instanceof ModuleLocation) {
       var moduleLocation = ((ModuleLocation) location);
-      var group = repository.getOutputContainerGroup(moduleLocation.getParent());
+      var parentGroup = repository.getOutputContainerGroup(moduleLocation.getParent());
 
-      if (group != null) {
-        return group
-            .getOrCreateModule(moduleLocation.getModuleName())
-            .getFileForOutput(packageName, relativeName);
+      if (parentGroup != null) {
+        group = parentGroup.getOrCreateModule(moduleLocation.getModuleName());
       }
     } else {
-      var group = repository.getOutputContainerGroup(location);
-
-      if (group != null) {
-        return group.getFileForOutput(packageName, relativeName);
-      }
+      group = repository.getOutputContainerGroup(location);
     }
 
-    return null;
+    return group == null
+        ? null
+        : group.getFileForOutput(packageName, relativeName);
   }
 
   @Nullable
@@ -189,45 +187,45 @@ public JavaFileObject getJavaFileForOutput(
   ) {
     requireOutputLocation(location);
 
-    // If we have a module, we may need to create a brand new location for it.
+    PackageContainerGroup group = null;
+
+    // If we have a module, we may need to create a brand-new location for it.
     if (location instanceof ModuleLocation) {
       var moduleLocation = ((ModuleLocation) location);
-      var group = repository.getOutputContainerGroup(moduleLocation.getParent());
+      var parentGroup = repository.getOutputContainerGroup(moduleLocation.getParent());
 
-      if (group != null) {
-        return group
-            .getOrCreateModule(moduleLocation.getModuleName())
-            .getJavaFileForOutput(className, kind);
+      if (parentGroup != null) {
+        group = parentGroup.getOrCreateModule(moduleLocation.getModuleName());
       }
     } else {
-      var group = repository.getOutputContainerGroup(location);
-
-      if (group != null) {
-        return group.getJavaFileForOutput(className, kind);
-      }
+      group = repository.getOutputContainerGroup(location);
     }
 
-    return null;
+    return group == null
+        ? null
+        : group.getJavaFileForOutput(className, kind);
   }
 
   @Override
-  public Location getLocationForModule(Location location, String moduleName) {
-    // This checks that the input location is module/output oriented within the constructor,
-    // so we don't need to do it here as well.
+  public ModuleLocation getLocationForModule(Location location, String moduleName) {
+    // ModuleLocation will also validate this, but we do this to keep consistent
+    // error messages.
+    requireOutputOrModuleOrientedLocation(location);
+
     return new ModuleLocation(location, moduleName);
   }
 
   @Nullable
   @Override
-  public Location getLocationForModule(Location location, JavaFileObject fo) {
+  public ModuleLocation getLocationForModule(Location location, JavaFileObject fo) {
     requireOutputOrModuleOrientedLocation(location);
 
     if (fo instanceof PathFileObject) {
       var pathFileObject = (PathFileObject) fo;
       var moduleLocation = pathFileObject.getLocation();
 
       if (moduleLocation instanceof ModuleLocation) {
-        return moduleLocation;
+        return (ModuleLocation) moduleLocation;
       }
 
       // The expectation is to return null if this is not for a module. Certain frameworks like
@@ -237,7 +235,7 @@ public Location getLocationForModule(Location location, JavaFileObject fo) {
     }
 
     throw new IllegalArgumentException(
-        "File object " + fo + " does not appear to be registered to a module"
+        "File object " + fo + " is not compatible with this file manager"
     );
   }
 
@@ -284,7 +282,7 @@ public <S> ServiceLoader<S> getServiceLoader(Location location, Class<S> service
 
     if (group == null) {
       throw new NoSuchElementException(
-          "No container group for location " + location.getName() + " exists"
+          "No container group for location " + location.getName() + " exists in this file manager"
       );
     }
 
@@ -310,12 +308,12 @@ public String inferBinaryName(Location location, JavaFileObject file) {
     if (!(file instanceof PathFileObject)) {
       return null;
     }
-    var pathFileObject = (PathFileObject) file;
+
     var group = repository.getPackageOrientedContainerGroup(location);
 
     return group == null
         ? null
-        : group.inferBinaryName(pathFileObject);
+        : group.inferBinaryName((PathFileObject) file);
   }
 
   @Nullable
@@ -331,11 +329,7 @@ public String inferModuleName(Location location) {
   @Override
   public boolean isSameFile(@Nullable FileObject a, @Nullable FileObject b) {
     // Some annotation processors provide null values here for some reason.
-    if (a == null || b == null) {
-      return false;
-    }
-
-    return Objects.equals(a.toUri(), b.toUri());
+    return a != null && b != null && Objects.equals(a.toUri(), b.toUri());
   }
 
   @Override
@@ -367,7 +361,9 @@ public Iterable<Set<Location>> listLocationsForModules(Location location) {
 
   @Override
   public String toString() {
-    return new ToStringBuilder(this).toString();
+    return new ToStringBuilder(this)
+        .attribute("repository", repository)
+        .toString();
   }
 
   private static void requireOutputOrModuleOrientedLocation(Location location) {