Check that 'Automatic-Module-Names' are not changed accidentally (#85)

See #79

Author: jjohannes

src/main/java/org/gradlex/javamodule/moduleinfo/ExtraJavaModuleInfoTransform.java

@@ -64,9 +64,9 @@
 import java.util.zip.ZipEntry;
 import java.util.zip.ZipException;
 
-import static org.gradlex.javamodule.moduleinfo.ModuleNameUtil.automaticModulNameFromFileName;
 import static org.gradlex.javamodule.moduleinfo.FilePathToModuleCoordinates.gaCoordinatesFromFilePathMatch;
 import static org.gradlex.javamodule.moduleinfo.FilePathToModuleCoordinates.versionFromFilePath;
+import static org.gradlex.javamodule.moduleinfo.ModuleNameUtil.automaticModulNameFromFileName;
 
 /**
  * An artifact transform that applies additional information to Jars without module information.
@@ -130,18 +130,28 @@ public void transform(TransformOutputs outputs) {
             if (realModule && !((ModuleInfo) moduleSpec).patchRealModule) {
                 throw new RuntimeException("Patching of real modules must be explicitly enabled with 'patchRealModule()'");
             }
+            String definedName = moduleSpec.getModuleName();
+            String expectedName = autoModuleName(originalJar);
+            if (expectedName != null && !definedName.equals(expectedName) && !moduleSpec.overrideModuleName) {
+                throw new RuntimeException("The name '" + definedName + "' is different than the Automatic-Module-Name '" + expectedName + "'; explicitly allow override via 'overrideName()'");
+            }
             addModuleDescriptor(originalJar, getModuleJar(outputs, originalJar), (ModuleInfo) moduleSpec);
         } else if (moduleSpec instanceof AutomaticModuleName) {
             if (realModule) {
                 throw new RuntimeException("Patching of real modules must be explicitly enabled with 'patchRealModule()' and can only be done with 'module()'");
             }
+            String definedName = moduleSpec.getModuleName();
+            String expectedName = autoModuleName(originalJar);
+            if (expectedName != null && (moduleSpec.getMergedJars().isEmpty() || !definedName.equals(expectedName)) && !moduleSpec.overrideModuleName) {
+                throw new RuntimeException("'" + definedName + "' already has the Automatic-Module-Name '" + expectedName + "'; explicitly allow override via 'overrideName()'");
+            }
             if (parameters.getFailOnAutomaticModules().get()) {
                 throw new RuntimeException("Use of 'automaticModule()' is prohibited. Use 'module()' instead: " + originalJar.getName());
             }
             addAutomaticModuleName(originalJar, getModuleJar(outputs, originalJar), (AutomaticModuleName) moduleSpec);
         } else if (realModule) {
             outputs.file(originalJar);
-        } else if (isAutoModule(originalJar)) {
+        } else if (autoModuleName(originalJar) != null) {
             if (parameters.getFailOnAutomaticModules().get()) {
                 throw new RuntimeException("Found an automatic module: " + originalJar.getName());
             }
@@ -210,10 +220,11 @@ private boolean containsMultiReleaseJarEntry(JarInputStream jarStream) {
         return manifest != null && Boolean.parseBoolean(manifest.getMainAttributes().getValue("Multi-Release"));
     }
 
-    private boolean isAutoModule(File jar) {
+    @Nullable
+    private String autoModuleName(File jar) {
         try (JarInputStream inputStream = new JarInputStream(Files.newInputStream(jar.toPath()))) {
             Manifest manifest = inputStream.getManifest();
-            return manifest != null && manifest.getMainAttributes().getValue("Automatic-Module-Name") != null;
+            return manifest != null ? manifest.getMainAttributes().getValue("Automatic-Module-Name") : null;
         } catch (IOException e) {
             throw new RuntimeException(e);
         }

src/main/java/org/gradlex/javamodule/moduleinfo/ModuleSpec.java

@@ -36,6 +36,8 @@ public abstract class ModuleSpec implements Serializable {
     private final String moduleName;
     private final List<String> mergedJars = new ArrayList<>();
 
+    boolean overrideModuleName;
+
     protected ModuleSpec(String identifier, String moduleName) {
         validateIdentifier(identifier);
         validateModuleName(moduleName);
@@ -77,4 +79,11 @@ public void mergeJar(Provider<MinimalExternalModuleDependency> alias) {
     public List<String> getMergedJars() {
         return mergedJars;
     }
+
+    /**
+     * If the Module already has an Automatic-Module-Name, allow changing that name
+     */
+    public void overrideModuleName() {
+        this.overrideModuleName = true;
+    }
 }

src/test/groovy/org/gradlex/javamodule/moduleinfo/test/EdgeCasesFunctionalTest.groovy

@@ -158,7 +158,7 @@ class EdgeCasesFunctionalTest extends Specification {
             module org.gradle.sample.app {
                 exports org.gradle.sample.app;
                 
-                requires org.kohsuke.github;
+                requires org.kohsuke.github.api;
             }
         """
         buildFile << """          
@@ -167,7 +167,7 @@ class EdgeCasesFunctionalTest extends Specification {
             }
             
             extraJavaModuleInfo {
-                module("org.kohsuke:github-api", "org.kohsuke.github") {
+                module("org.kohsuke:github-api", "org.kohsuke.github.api") {
                     exportAllPackages()
                     requires("org.apache.commons.lang3")
                 }
@@ -194,4 +194,77 @@ class EdgeCasesFunctionalTest extends Specification {
         def result = failRun()
         result.output.contains "nd4j.native.api: Invalid module name: 'native' is not a Java identifier"
     }
+
+    def "fail if module name does not correspond to Automatic-Module-Name - module"() {
+        given:
+        buildFile << """          
+            dependencies {
+                implementation("org.apache.commons:commons-lang3:3.10")
+            }
+            
+            extraJavaModuleInfo {
+                module("org.apache.commons:commons-lang3", "org.apache.commons.lang") {
+                    exportAllPackages()
+                }
+            }
+        """
+
+        expect:
+        def result = failRun()
+        result.output.contains "The name 'org.apache.commons.lang' is different than the Automatic-Module-Name 'org.apache.commons.lang3'; explicitly allow override via 'overrideName()'"
+    }
+
+    def "fail if module name does not correspond to Automatic-Module-Name - automaticModule"() {
+        given:
+        buildFile << """          
+            dependencies {
+                implementation("org.apache.commons:commons-lang3:3.10")
+            }
+            
+            extraJavaModuleInfo {
+                automaticModule("org.apache.commons:commons-lang3", "org.apache.commons.lang")
+            }
+        """
+
+        expect:
+        def result = failRun()
+        result.output.contains "'org.apache.commons.lang' already has the Automatic-Module-Name 'org.apache.commons.lang3'; explicitly allow override via 'overrideName()'"
+    }
+
+    def "do not fail if overrideModuleName is set - module"() {
+        given:
+        buildFile << """          
+            dependencies {
+                implementation("org.apache.commons:commons-lang3:3.10")
+            }
+            
+            extraJavaModuleInfo {
+                module("org.apache.commons:commons-lang3", "org.apache.commons.lang") {
+                    overrideModuleName()
+                    exportAllPackages()
+                }
+            }
+        """
+
+        expect:
+        build()
+    }
+
+    def "do not fail if overrideModuleName is set - automaticModule"() {
+        given:
+        buildFile << """          
+            dependencies {
+                implementation("org.apache.commons:commons-lang3:3.10")
+            }
+            
+            extraJavaModuleInfo {
+                automaticModule("org.apache.commons:commons-lang3", "org.apache.commons.lang") {
+                    overrideModuleName()
+                }
+            }
+        """
+
+        expect:
+        build()
+    }
 }