Check DuplicatesStrategy for merging transformers

docs/changes/README.md

@@ -5,6 +5,8 @@
 
 ### Added
 
+- Check `DuplicatesStrategy` for merging transformers. ([#2026](https://github.com/GradleUp/shadow/pull/2026))  
+  This will log warnings if the wrong `DuplicatesStrategy`s are applied in the Gradle configurations for built-in `ResourceTransformer`s.
 - Expose `patternSet` of `ComponentsXmlResourceTransformer` as `public`. ([#2028](https://github.com/GradleUp/shadow/pull/2028))
 - Expose `patternSet` of `GroovyExtensionModuleTransformer` as `public`. ([#2028](https://github.com/GradleUp/shadow/pull/2028))
 - Expose `patternSet` of `Log4j2PluginsCacheFileTransformer` as `public`. ([#2028](https://github.com/GradleUp/shadow/pull/2028))

src/functionalTest/kotlin/com/github/jengelman/gradle/plugins/shadow/transformers/ServiceFileTransformerTest.kt

@@ -1,6 +1,7 @@
 package com.github.jengelman.gradle.plugins.shadow.transformers
 
 import assertk.assertThat
+import assertk.assertions.contains
 import assertk.assertions.containsMatch
 import assertk.assertions.isEqualTo
 import com.github.jengelman.gradle.plugins.shadow.testkit.getContent
@@ -221,7 +222,15 @@ class ServiceFileTransformerTest : BaseTransformerTest() {
   ) {
     writeDuplicatesStrategy(strategy)
 
-    runWithSuccess(shadowJarPath)
+    val result = runWithSuccess(shadowJarPath)
+
+    if (strategy == EXCLUDE) {
+      assertThat(result.output)
+        .contains(
+          "META-INF/services/com.acme.Foo' is matched by com.github.jengelman.gradle.plugins.shadow.transformers.ServiceFileTransformer (a merging transformer) but its DuplicatesStrategy is EXCLUDE — duplicates may be silently dropped before merging.",
+          "META-INF/services/org.apache.maven.Shade' is matched by com.github.jengelman.gradle.plugins.shadow.transformers.ServiceFileTransformer (a merging transformer) but its DuplicatesStrategy is EXCLUDE — duplicates may be silently dropped before merging.",
+        )
+    }
 
     assertThat(outputShadowedJar).useAll {
       getContent(ENTRY_SERVICES_SHADE).isEqualTo(firstValue)

src/main/kotlin/com/github/jengelman/gradle/plugins/shadow/internal/DuplicatesStrategyChecker.kt

@@ -0,0 +1,29 @@
+package com.github.jengelman.gradle.plugins.shadow.internal
+
+import com.github.jengelman.gradle.plugins.shadow.transformers.ResourceTransformer
+import org.gradle.api.file.DuplicatesStrategy
+import org.gradle.api.file.FileCopyDetails
+import org.gradle.api.file.FileTreeElement
+import org.gradle.api.logging.Logging
+
+internal fun ResourceTransformer.checkDupStrategy(
+  canTransformResource: Boolean,
+  element: FileTreeElement,
+) {
+  when {
+    !canTransformResource -> return
+    element !is FileCopyDetails -> return
+    element.duplicatesStrategy == DuplicatesStrategy.EXCLUDE -> {
+      val logger = Logging.getLogger(this::class.java)
+      logger.warn(
+        """
+          ${element.path}' is matched by ${this::class.qualifiedName} (a merging transformer) but its DuplicatesStrategy is ${element.duplicatesStrategy} — duplicates may be silently dropped before merging.
+          Set it to INCLUDE or WARN to ensure all duplicates are processed by the transformer.
+          See https://gradleup.com/shadow/configuration/merging/#handling-duplicates-strategy for more details.
+        """
+          .trimIndent()
+      )
+    }
+    else -> Unit
+  }
+}

src/main/kotlin/com/github/jengelman/gradle/plugins/shadow/transformers/AppendingTransformer.kt

@@ -1,5 +1,6 @@
 package com.github.jengelman.gradle.plugins.shadow.transformers
 
+import com.github.jengelman.gradle.plugins.shadow.internal.checkDupStrategy
 import com.github.jengelman.gradle.plugins.shadow.internal.property
 import com.github.jengelman.gradle.plugins.shadow.internal.zipEntry
 import java.io.ByteArrayOutputStream
@@ -32,7 +33,9 @@ constructor(final override val objectFactory: ObjectFactory) : ResourceTransform
   @get:Input public open val separator: Property<String> = objectFactory.property(DEFAULT_SEPARATOR)
 
   override fun canTransformResource(element: FileTreeElement): Boolean {
-    return resource.get().equals(element.path, ignoreCase = true)
+    return resource.get().equals(element.path, ignoreCase = true).also { flag ->
+      checkDupStrategy(flag, element)
+    }
   }
 
   override fun transform(context: TransformerContext) {

src/main/kotlin/com/github/jengelman/gradle/plugins/shadow/transformers/DeduplicatingResourceTransformer.kt

@@ -1,5 +1,6 @@
 package com.github.jengelman.gradle.plugins.shadow.transformers
 
+import com.github.jengelman.gradle.plugins.shadow.internal.checkDupStrategy
 import com.github.jengelman.gradle.plugins.shadow.tasks.FindResourceInClasspath
 import java.io.File
 import javax.inject.Inject
@@ -66,8 +67,10 @@ public open class DeduplicatingResourceTransformer(
     val file = element.file
     val hash = file.sha256Hex()
 
-    val pathInfos =
-      sources.computeIfAbsent(element.path) { PathInfos(patternSpec.isSatisfiedBy(element)) }
+    val flag = patternSpec.isSatisfiedBy(element)
+    checkDupStrategy(flag, element)
+
+    val pathInfos = sources.computeIfAbsent(element.path) { PathInfos(flag) }
     val retainInOutput = pathInfos.addFile(hash, file)
 
     return !retainInOutput

src/main/kotlin/com/github/jengelman/gradle/plugins/shadow/transformers/PatternFilterableResourceTransformer.kt

@@ -1,5 +1,6 @@
 package com.github.jengelman.gradle.plugins.shadow.transformers
 
+import com.github.jengelman.gradle.plugins.shadow.internal.checkDupStrategy
 import com.github.jengelman.gradle.plugins.shadow.internal.unsafeLazy
 import org.gradle.api.file.FileTreeElement
 import org.gradle.api.specs.Spec
@@ -24,7 +25,7 @@ public abstract class PatternFilterableResourceTransformer(
   }
 
   override fun canTransformResource(element: FileTreeElement): Boolean {
-    return patternSpec.isSatisfiedBy(element)
+    return patternSpec.isSatisfiedBy(element).also { flag -> checkDupStrategy(flag, element) }
   }
 
   @Input // Trigger task executions after includes changed.

src/main/kotlin/com/github/jengelman/gradle/plugins/shadow/transformers/PreserveFirstFoundResourceTransformer.kt

@@ -1,5 +1,6 @@
 package com.github.jengelman.gradle.plugins.shadow.transformers
 
+import com.github.jengelman.gradle.plugins.shadow.internal.checkDupStrategy
 import com.github.jengelman.gradle.plugins.shadow.internal.setProperty
 import com.github.jengelman.gradle.plugins.shadow.internal.unsafeLazy
 import com.github.jengelman.gradle.plugins.shadow.tasks.ShadowJar
@@ -48,6 +49,8 @@ public open class PreserveFirstFoundResourceTransformer(
   override fun canTransformResource(element: FileTreeElement): Boolean {
     // Init once before patternSpec is accessed.
     includeResources
-    return patternSpec.isSatisfiedBy(element) && !found.add(element.path)
+    val flag = patternSpec.isSatisfiedBy(element)
+    checkDupStrategy(flag, element)
+    return flag && !found.add(element.path)
   }
 }

src/main/kotlin/com/github/jengelman/gradle/plugins/shadow/transformers/PropertiesFileTransformer.kt

@@ -1,6 +1,7 @@
 package com.github.jengelman.gradle.plugins.shadow.transformers
 
 import com.github.jengelman.gradle.plugins.shadow.internal.ReproducibleProperties
+import com.github.jengelman.gradle.plugins.shadow.internal.checkDupStrategy
 import com.github.jengelman.gradle.plugins.shadow.internal.mapProperty
 import com.github.jengelman.gradle.plugins.shadow.internal.property
 import com.github.jengelman.gradle.plugins.shadow.internal.setProperty
@@ -137,7 +138,7 @@ constructor(final override val objectFactory: ObjectFactory) : ResourceTransform
       path in paths -> true
       paths.any { it.toRegex().containsMatchIn(path) } -> true
       else -> mappings.isEmpty() && paths.isEmpty() && path.endsWith(PROPERTIES_SUFFIX)
-    }
+    }.also { checkDupStrategy(it, element) }
   }
 
   override fun transform(context: TransformerContext) {

src/main/kotlin/com/github/jengelman/gradle/plugins/shadow/transformers/XmlAppendingTransformer.kt

@@ -1,5 +1,6 @@
 package com.github.jengelman.gradle.plugins.shadow.transformers
 
+import com.github.jengelman.gradle.plugins.shadow.internal.checkDupStrategy
 import com.github.jengelman.gradle.plugins.shadow.internal.property
 import com.github.jengelman.gradle.plugins.shadow.internal.zipEntry
 import java.io.IOException
@@ -38,7 +39,9 @@ constructor(final override val objectFactory: ObjectFactory) : ResourceTransform
   @get:Input public open val resource: Property<String> = objectFactory.property("")
 
   override fun canTransformResource(element: FileTreeElement): Boolean {
-    return resource.get().equals(element.path, ignoreCase = true)
+    return resource.get().equals(element.path, ignoreCase = true).also { flag ->
+      checkDupStrategy(flag, element)
+    }
   }
 
   override fun transform(context: TransformerContext) {