Allow defined and automatic modules to co-exist

In a maven project, a maven module creates two artifacts:

- a main artifact with a module-info.class file that defines a JPMS
  module "foo.bar". The jar is called foo-bar-1.0.jar
- a test artifact which is not a JPMS module. It is called
  foo-bar-1.0-tests.jar

Another module declares dependencies on both modules:

<dependencies>
  <dependency>
    <groupId>xxx</groupId>
    <artifactId>foo-bar</artifactId>
    <version>1.0</version>
  </dependency>

  <dependency>
    <groupId>xxx</groupId>
    <artifactId>foo-bar</artifactId>
    <version>1.0</version>
    <classifier>tests</classifier>
    <scope>test</scope>
  </dependency>
</dependencies>

This is a common use case in large projects. The LocationManager now
creates two JavaModuleDescriptors for the "foo.bar" JPMS module. One
from the main artifact and its module-info.class (automatic == false)
and one from the test artifact (automatic == true).

The current code considers these duplicates and drops one. As a result,
the test code no longer compiles because a dependency is missing.

The patch separates out modules with a module descriptor and automatic
modules.

Then it adds the modules with module descriptors to the module path.
Any duplicate is dropped for modules with module descriptors.

Finally, it adds the automatic modules; if a module with the same module
id already exists on the module path, it adds it to the class path.

In the case above, this will allow the compile to successfully compile
test code (the tests dependency drops to the class path, while the main
artifact is on the module path).

Author: hgschmie

plexus-java/src/main/java/org/codehaus/plexus/languages/java/jpms/LocationManager.java

@@ -34,6 +34,7 @@
 import java.util.Map;
 import java.util.Map.Entry;
 import java.util.Set;
+import java.util.function.Consumer;
 
 import org.codehaus.plexus.languages.java.jpms.JavaModuleDescriptor.JavaProvides;
 
@@ -249,22 +250,47 @@ public String extract(Path path) throws IOException {
                     request.isIncludeStatic());
         }
 
-        // in case of identical module names, first one wins
+        Map<T, JavaModuleDescriptor> definedModules = new HashMap<>();
+        Map<T, JavaModuleDescriptor> automaticModules = new HashMap<>();
+
+        pathElements.forEach((k, v) -> {
+            if (v != null && !v.isAutomatic()) {
+                definedModules.put(k, v);
+            } else {
+                automaticModules.put(k, v);
+            }
+        });
+
+        // in case of identical module names, first one wins, others drop onto classpath
+        // if they are automatic modules.
         Set<String> collectedModules = new HashSet<>(requiredNamedModules.size());
 
-        for (Entry<T, JavaModuleDescriptor> entry : pathElements.entrySet()) {
-            if (entry.getValue() != null
-                    && requiredNamedModules.contains(entry.getValue().name())) {
-                if (collectedModules.add(entry.getValue().name())) {
-                    result.getModulepathElements()
-                            .put(
-                                    entry.getKey(),
-                                    moduleNameSources.get(entry.getValue().name()));
+        Consumer<Map<T, JavaModuleDescriptor>> moduleAcceptor = moduleSet -> {
+            for (Entry<T, JavaModuleDescriptor> entry : moduleSet.entrySet()) {
+                if (entry.getValue() != null
+                        && requiredNamedModules.contains(entry.getValue().name())) {
+                    if (collectedModules.add(entry.getValue().name())) {
+                        result.getModulepathElements()
+                                .put(
+                                        entry.getKey(),
+                                        moduleNameSources.get(entry.getValue().name()));
+                        // if the module is an automatic module, add it to the classpath
+                    } else if (entry.getValue().isAutomatic()) {
+                        result.getClasspathElements().add(entry.getKey());
+                    }
+                } else {
+                    result.getClasspathElements().add(entry.getKey());
                 }
-            } else {
-                result.getClasspathElements().add(entry.getKey());
             }
-        }
+        };
+
+        // process defined modules first. This fixes a corner case where a project creates
+        // a main artifact that is a JPMS module and a tests artifact that is not. If the
+        // main artifact and the test artifact happen to have the same module id (one from
+        // the module-info, one from the automatic module naming), the main artifact will
+        // be on the module path and the test artifact will be on the class path.
+        moduleAcceptor.accept(definedModules);
+        moduleAcceptor.accept(automaticModules);
 
         return result;
     }

plexus-java/src/test/java/org/codehaus/plexus/languages/java/jpms/LocationManagerTest.java

@@ -166,6 +166,32 @@ public void testIdenticalModuleNames() throws Exception {
         ResolvePathsRequest<Path> request =
                 ResolvePathsRequest.ofPaths(Arrays.asList(pj1, pj2)).setMainModuleDescriptor(mockModuleInfoJava);
 
+        when(asmParser.getModuleDescriptor(pj1))
+                .thenReturn(JavaModuleDescriptor.newModule("plexus.java").build());
+        when(asmParser.getModuleDescriptor(pj2))
+                .thenReturn(JavaModuleDescriptor.newModule("plexus.java").build());
+
+        ResolvePathsResult<Path> result = locationManager.resolvePaths(request);
+
+        assertThat(result.getMainModuleDescriptor(), is(descriptor));
+        assertThat(result.getPathElements().size(), is(2));
+        assertThat(result.getModulepathElements().size(), is(1));
+        assertThat(result.getModulepathElements().containsKey(pj1), is(true));
+        assertThat(result.getModulepathElements().containsKey(pj2), is(false));
+        assertThat(result.getClasspathElements().size(), is(0));
+        assertThat(result.getPathExceptions().size(), is(0));
+    }
+
+    @Test
+    public void testIdenticalAutomaticModuleNames() throws Exception {
+        Path pj1 = Paths.get("src/test/resources/jar.empty/plexus-java-1.0.0-SNAPSHOT.jar");
+        Path pj2 = Paths.get("src/test/resources/jar.empty.2/plexus-java-2.0.0-SNAPSHOT.jar");
+        JavaModuleDescriptor descriptor =
+                JavaModuleDescriptor.newModule("base").requires("plexus.java").build();
+        when(qdoxParser.fromSourcePath(any(Path.class))).thenReturn(descriptor);
+        ResolvePathsRequest<Path> request =
+                ResolvePathsRequest.ofPaths(Arrays.asList(pj1, pj2)).setMainModuleDescriptor(mockModuleInfoJava);
+
         when(asmParser.getModuleDescriptor(pj1))
                 .thenReturn(
                         JavaModuleDescriptor.newAutomaticModule("plexus.java").build());
@@ -180,7 +206,34 @@ public void testIdenticalModuleNames() throws Exception {
         assertThat(result.getModulepathElements().size(), is(1));
         assertThat(result.getModulepathElements().containsKey(pj1), is(true));
         assertThat(result.getModulepathElements().containsKey(pj2), is(false));
-        assertThat(result.getClasspathElements().size(), is(0));
+        assertThat(result.getClasspathElements().size(), is(1));
+        assertThat(result.getPathExceptions().size(), is(0));
+    }
+
+    @Test
+    public void testMainJarModuleAndTestJarAutomatic() throws Exception {
+        Path pj1 = Paths.get("src/test/resources/jar.tests/plexus-java-1.0.0-SNAPSHOT.jar");
+        Path pj2 = Paths.get("src/test/resources/jar.tests/plexus-java-1.0.0-SNAPSHOT-tests.jar");
+        JavaModuleDescriptor descriptor =
+                JavaModuleDescriptor.newModule("base").requires("plexus.java").build();
+        when(qdoxParser.fromSourcePath(any(Path.class))).thenReturn(descriptor);
+        ResolvePathsRequest<Path> request =
+                ResolvePathsRequest.ofPaths(Arrays.asList(pj1, pj2)).setMainModuleDescriptor(mockModuleInfoJava);
+
+        when(asmParser.getModuleDescriptor(pj1))
+                .thenReturn(JavaModuleDescriptor.newModule("plexus.java").build());
+        when(asmParser.getModuleDescriptor(pj2)).thenReturn(null);
+
+        ResolvePathsResult<Path> result = locationManager.resolvePaths(request);
+
+        assertThat(result.getMainModuleDescriptor(), is(descriptor));
+        assertThat(result.getPathElements().size(), is(2));
+        assertThat(result.getModulepathElements().size(), is(1));
+        assertThat(result.getModulepathElements().containsKey(pj1), is(true));
+        assertThat(result.getModulepathElements().containsKey(pj2), is(false));
+        assertThat(result.getClasspathElements().size(), is(1));
+        assertThat(result.getClasspathElements().contains(pj2), is(true));
+        assertThat(result.getClasspathElements().contains(pj1), is(false));
         assertThat(result.getPathExceptions().size(), is(0));
     }