Handle classifiers in FilePathToModuleCoordinates

CHANGELOG.md

@@ -1,5 +1,9 @@
 # Extra Java Module Info Gradle Plugin - Changelog
 
+## Version 1.14.1
+* [Fixed] [#246](https://github.com/gradlex-org/extra-java-module-info/issues/246) - More correct classifier handling (Thanks to [Geolykt](https://github.com/Geolykt) for reporting)
+  * ⚠️ For matching a Jar with classifier, the _identifier_ now needs to be included - e.g. `org.dojotoolkit:dojo|distribution` instead of `org.dojotoolkit:dojo`
+
 ## Version 1.14
 * [New] [#207](https://github.com/gradlex-org/extra-java-module-info/issues/207) - Add 'exportAllPackagesExcept("org.exception", ...)' (Thanks [Tim Hurman](https://github.com/timhamoni) for contributing)
 * [New] [#212](https://github.com/gradlex-org/extra-java-module-info/issues/212) - Add 'provides("service", "service.impl", ...)' (Thanks [Sandy Dunlop](https://github.com/sandydunlop) for contributing)

build.gradle.kts

@@ -19,10 +19,6 @@ publishingConventions {
 
 testingConventions { testGradleVersions("6.8.3", "6.9.4", "7.6.5", "8.14.2") }
 
-// Turn off classfile lint as long as we still compile with Java 8
-// /org/objectweb/asm/ClassReader.class: Cannot find annotation method 'forRemoval()' in type 'Deprecated'
-tasks.compileJava { options.compilerArgs.add("-Xlint:-classfile") }
-
 // === the following custom configuration should be removed once tests are migrated to Java
 apply(plugin = "groovy")
 

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

@@ -39,7 +39,6 @@
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 import java.util.stream.Collectors;
-import java.util.stream.Stream;
 import java.util.zip.ZipEntry;
 import java.util.zip.ZipException;
 import javax.annotation.Nullable;
@@ -224,22 +223,11 @@ private ModuleSpec findModuleSpec(File originalJar) {
         Map<String, ModuleSpec> moduleSpecs = getParameters().getModuleSpecs().get();
 
         Optional<ModuleSpec> moduleSpec = moduleSpecs.values().stream()
-                .filter(spec -> gaCoordinatesFromFilePathMatch(originalJar.toPath(), spec.getIdentifier()))
+                .filter(spec -> gaCoordinatesFromFilePathMatch(
+                        originalJar.toPath(), spec.getIdentifier(), spec.getClassifier()))
                 .findFirst();
         if (moduleSpec.isPresent()) {
-            String ga = moduleSpec.get().getIdentifier();
-            if (moduleSpecs.containsKey(ga)) {
-                return moduleSpecs.get(ga);
-            } else {
-                // maybe with classifier
-                Stream<String> idsWithClassifier = moduleSpecs.keySet().stream().filter(id -> id.startsWith(ga + "|"));
-                for (String idWithClassifier : idsWithClassifier.collect(Collectors.toList())) {
-                    if (nameHasClassifier(originalJar, moduleSpecs.get(idWithClassifier))) {
-                        return moduleSpecs.get(idWithClassifier);
-                    }
-                }
-            }
-            return null;
+            return moduleSpec.get();
         }
 
         String originalJarName = originalJar.getName();
@@ -749,10 +737,6 @@ private String moduleNameFromSharedMapping(String ga) {
         return null;
     }
 
-    private boolean nameHasClassifier(File jar, ModuleSpec spec) {
-        return jar.getName().endsWith("-" + spec.getClassifier() + ".jar");
-    }
-
     private static boolean isModuleInfoClass(String jarEntryName) {
         return "module-info.class".equals(jarEntryName)
                 || MODULE_INFO_CLASS_MRJAR_PATH.matcher(jarEntryName).matches();

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

@@ -4,8 +4,8 @@
 import java.nio.file.Path;
 import java.util.stream.Collectors;
 import java.util.stream.StreamSupport;
-import javax.annotation.Nullable;
 import org.jspecify.annotations.NullMarked;
+import org.jspecify.annotations.Nullable;
 
 /**
  * Attempts to parse 'group', 'name', 'version' coordinates from a paths like:
@@ -17,51 +17,79 @@ final class FilePathToModuleCoordinates {
 
     @Nullable
     static String versionFromFilePath(Path path) {
-        if (isInGradleCache(path)) {
+        if (null != isInGradleCache(path)) {
             return getVersionFromGradleCachePath(path);
         }
-        if (isInM2Cache(path)) {
+        if (null != isInM2Cache(path)) {
             return getVersionFromM2CachePath(path);
         }
         return null;
     }
 
-    static boolean gaCoordinatesFromFilePathMatch(Path path, String ga) {
+    static boolean gaCoordinatesFromFilePathMatch(Path path, String gaAndClassifier) {
+        if (gaAndClassifier.contains("|")) {
+            String[] split = gaAndClassifier.split("\\|");
+            return gaCoordinatesFromFilePathMatch(path, split[0], split[1]);
+        }
+        return gaCoordinatesFromFilePathMatch(path, gaAndClassifier, null);
+    }
+
+    static boolean gaCoordinatesFromFilePathMatch(Path path, String ga, @Nullable String classifier) {
         String name = nameCoordinateFromFilePath(path);
         String group = groupCoordinateFromFilePath(path);
         if (name == null || group == null) {
             return false;
         }
-        return (isInGradleCache(path) && ga.equals(group + ":" + name))
-                || (isInM2Cache(path) && (group + ":" + name).endsWith("." + ga));
+
+        String coordinatesFromPath = group + ":" + name;
+
+        String classifierFromGradleCache = isInGradleCache(path);
+        if (classifierFromGradleCache != null) {
+            if (classifierFromGradleCache.isEmpty()) {
+                return coordinatesFromPath.equals(ga);
+            }
+            return coordinatesFromPath.equals(ga) && classifierFromGradleCache.equals(classifier);
+        }
+
+        String classifierFromM2Cache = isInM2Cache(path);
+        if (classifierFromM2Cache != null) {
+            if (classifierFromM2Cache.isEmpty()) {
+                return coordinatesFromPath.endsWith(ga);
+            }
+            return coordinatesFromPath.endsWith(ga) && classifierFromM2Cache.equals(classifier);
+        }
+
+        return false;
     }
 
     @Nullable
     private static String groupCoordinateFromFilePath(Path path) {
-        if (isInGradleCache(path)) {
+        if (null != isInGradleCache(path)) {
             return path.getName(path.getNameCount() - 5).toString();
         }
-        if (isInM2Cache(path)) {
+        if (null != isInM2Cache(path)) {
             return StreamSupport.stream(path.subpath(0, path.getNameCount() - 3).spliterator(), false)
                     .map(Path::toString)
                     .collect(Collectors.joining("."));
         }
         return null;
     }
 
-    static boolean isInGradleCache(Path path) {
+    @Nullable
+    static String isInGradleCache(Path path) {
         String name = nameCoordinateFromFilePath(path);
         if (name == null) {
-            return false;
+            return null;
         }
         String version = getVersionFromGradleCachePath(path);
         return matchesPath(path, name, version);
     }
 
-    static boolean isInM2Cache(Path path) {
+    @Nullable
+    static String isInM2Cache(Path path) {
         String name = nameCoordinateFromFilePath(path);
         if (name == null) {
-            return false;
+            return null;
         }
         String version = getVersionFromM2CachePath(path);
         return matchesPath(path, name, version);
@@ -75,12 +103,12 @@ private static String nameCoordinateFromFilePath(Path path) {
 
         String nameFromGradleCachePath = path.getName(path.getNameCount() - 4).toString();
         String versionFromGradleCachePath = getVersionFromGradleCachePath(path);
-        if (matchesPath(path, nameFromGradleCachePath, versionFromGradleCachePath)) {
+        if (null != matchesPath(path, nameFromGradleCachePath, versionFromGradleCachePath)) {
             return nameFromGradleCachePath;
         }
         String nameFromM2CachePath = path.getName(path.getNameCount() - 3).toString();
         String versionFromM2CachePath = getVersionFromM2CachePath(path);
-        if (matchesPath(path, nameFromM2CachePath, versionFromM2CachePath)) {
+        if (null != matchesPath(path, nameFromM2CachePath, versionFromM2CachePath)) {
             return nameFromM2CachePath;
         }
 
@@ -95,8 +123,34 @@ private static String getVersionFromM2CachePath(Path path) {
         return path.getName(path.getNameCount() - 2).toString();
     }
 
-    private static boolean matchesPath(Path path, String name, String version) {
+    /**
+     * @return the classifier or 'null' if there is no match.
+     */
+    @Nullable
+    private static String matchesPath(Path path, String potentialName, String potentialVersion) {
         String jarFileName = path.getFileName().toString();
-        return jarFileName.startsWith(name + "-") && !jarFileName.startsWith(version);
+        boolean fuzzyMatchVersion = potentialVersion.contains("-");
+        String potentialVersionNormalized =
+                fuzzyMatchVersion ? potentialVersion.substring(0, potentialVersion.indexOf("-")) : potentialVersion;
+
+        if (jarFileName.startsWith(potentialVersionNormalized)) {
+            return null;
+        }
+
+        String baseName = potentialName + "-" + potentialVersionNormalized;
+        if (jarFileName.startsWith(baseName)) {
+            if (fuzzyMatchVersion) {
+                // Potential classifiers are ignored as we do not know what is version and what is classifier.
+                // For example in '33.2.1-android' is '-android' part of the version or a classifier?
+                return "";
+            } else {
+                if (jarFileName.startsWith(baseName + ".")) {
+                    return "";
+                }
+                return jarFileName.substring(baseName.length() + 1, jarFileName.length() - 4);
+            }
+        }
+
+        return null;
     }
 }

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

@@ -30,15 +30,24 @@ class FilePathToModuleCoordinatesTest extends Specification {
         def path = path('/Users/someone/.gradle/caches/modules-2/files-2.1/org.slf4j/slf4j-api/1.7.36/6c62681a2f655b49963a5983b8b0950a6120ae14/slf4j-api-1.7.36.jar')
 
         expect:
-        gaCoordinatesFromFilePathMatch(path, "org.slf4j:slf4j-api")
+        gaCoordinatesFromFilePathMatch(path, "org.slf4j:slf4j-api", null)
     }
 
     def "ga coordinates from gradle cache file path (version in file name does not match)"() {
         given:
         def path = path('/Users/jendrik/.gradle/caches/modules-2/files-2.1/com.google.guava/guava/33.2.1-jre/818e780da2c66c63bbb6480fef1f3855eeafa3e4/guava-33.2.1-android.jar')
 
         expect:
-        gaCoordinatesFromFilePathMatch(path, "com.google.guava:guava")
+        gaCoordinatesFromFilePathMatch(path, "com.google.guava:guava", null)
+    }
+
+    def "ga coordinates from gradle cache file path with classifier"() {
+        given:
+        def path = path('/Users/jendrik/.gradle/caches/modules-2/files-2.1/org.lwjgl/lwjgl-glfw/3.3.6/399b42b491c5dfa6595ae6fd79dcba61b93538a7/lwjgl-glfw-3.3.6-natives-macos-arm64.jar')
+
+        expect:
+        !gaCoordinatesFromFilePathMatch(path, "org.lwjgl:lwjgl-glfw", null)
+        gaCoordinatesFromFilePathMatch(path, "org.lwjgl:lwjgl-glfw", "natives-macos-arm64")
     }
 
     def "version from m2 repo file path"() {
@@ -64,21 +73,30 @@ class FilePathToModuleCoordinatesTest extends Specification {
         jarPath = path('/Users/someone/.m2/repository/com/google/code/findbugs/jsr305/3.0.2/jsr305-3.0.2.jar')
 
         then:
-        gaCoordinatesFromFilePathMatch(jarPath, "com.google.code.findbugs:jsr305")
+        gaCoordinatesFromFilePathMatch(jarPath, "com.google.code.findbugs:jsr305", null)
 
         when:
         jarPath = path('/Users/someone/.m2/repository/de/odysseus/juel/juel-impl/2.2.7/juel-impl-2.2.7.jar')
 
         then:
-        gaCoordinatesFromFilePathMatch(jarPath, "de.odysseus.juel:juel-impl")
+        gaCoordinatesFromFilePathMatch(jarPath, "de.odysseus.juel:juel-impl", null)
     }
 
     def "ga coordinates from m2 repo file path (version in file name does not match)"() {
         given:
         def path = path('/Users/someone/.m2/repository/com/google/guava/guava/33.2.1-jre/guava-33.2.1-android.jar')
 
         expect:
-        gaCoordinatesFromFilePathMatch(path, "com.google.guava:guava")
+        gaCoordinatesFromFilePathMatch(path, "com.google.guava:guava", null)
+    }
+
+    def "ga coordinates from m2 repo file path with classifier"() {
+        given:
+        def path = path('/Users/someone/.m2/repository/org/lwjgl/lwjgl-glfw/3.3.6/lwjgl-glfw-3.3.6-natives-macos-arm64.jar')
+
+        expect:
+        !gaCoordinatesFromFilePathMatch(path, "org.lwjgl:lwjgl-glfw", null)
+        gaCoordinatesFromFilePathMatch(path, "org.lwjgl:lwjgl-glfw", "natives-macos-arm64")
     }
 
     private Path path(String path) {

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

@@ -63,4 +63,41 @@ class ClassifiedJarsFunctionalTest extends Specification {
         build().task(':compileJava').outcome == TaskOutcome.SUCCESS
     }
 
+    def "does not accidentally patch classified Jar"() {
+        given:
+        file("src/main/java/org/gradle/sample/app/Main.java") << """
+            package org.gradle.sample.app;
+            public class Main {
+                public static void main(String[] args) {
+                    org.lwjgl.glfw.GLFW.glfwInit();
+                }
+            }
+        """
+        file("src/main/java/module-info.java") << """
+            module org.gradle.sample.app {               
+                requires org.lwjgl.glfw;
+            }
+        """
+        buildFile << """
+            dependencies {
+                implementation("org.lwjgl:lwjgl-glfw:3.2.2")
+                implementation("org.lwjgl:lwjgl-glfw:3.2.2:natives-macos")
+                implementation("org.lwjgl:lwjgl:3.2.2:natives-macos")
+            }
+            extraJavaModuleInfo {
+                module("org.lwjgl:lwjgl", "org.lwjgl") {
+                    preserveExisting()
+                    requiresStatic("org.lwjgl.natives")
+                }
+                module("org.lwjgl:lwjgl-glfw", "org.lwjgl.glfw") {
+                    preserveExisting()
+                    requiresStatic("org.lwjgl.glfw.natives")
+                }
+            }
+        """
+
+        expect:
+        build().task(':compileJava').outcome == TaskOutcome.SUCCESS
+    }
+
 }

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

@@ -74,7 +74,7 @@ class EdgeCasesFunctionalTest extends Specification {
                 failOnMissingModuleInfo.set(false)
                 automaticModule("org.apache.qpid:qpid-broker-core", "org.apache.qpid.broker") {
                     mergeJar("org.apache.qpid:qpid-broker-plugins-management-http")
-                    mergeJar("org.dojotoolkit:dojo") // This is a Zip, selected by 'distribution' classifier in dependencies of 'qpid-broker-plugins-management-http'  
+                    mergeJar("org.dojotoolkit:dojo|distribution") // This is a Zip, selected by 'distribution' classifier in dependencies of 'qpid-broker-plugins-management-http'  
                     mergeJar("org.webjars.bower:dgrid")
                     mergeJar("org.webjars.bower:dstore")
                 }