Fix skipStringConstants per-relocator behavior: use continue instead of return (#1968)

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: Goooler <10363352+Goooler@users.noreply.github.com>

Author: Copilot

docs/changes/README.md

@@ -20,6 +20,7 @@
 ### Fixed
 
 - Fix interaction with Gradle artifact transforms. ([#1345](https://github.com/GradleUp/shadow/pull/1345))
+- Fix skipStringConstants per-relocator behavior in mapName. ([#1968](https://github.com/GradleUp/shadow/pull/1968))
 
 ## [9.3.2](https://github.com/GradleUp/shadow/releases/tag/9.3.2) - 2026-02-27
 

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

@@ -11,7 +11,7 @@ private val classPattern: Pattern = Pattern.compile("([\\[()BCDFIJSZ]*)?L([^;]+)
 internal fun Set<Relocator>.mapName(
   name: String,
   mapLiterals: Boolean = false,
-  onModified: () -> Unit,
+  onModified: () -> Unit = {},
 ): String {
   // Maybe a list of types.
   val newName = name.split(';').joinToString(";") { realMap(it, mapLiterals) }
@@ -35,7 +35,7 @@ private fun Set<Relocator>.realMap(name: String, mapLiterals: Boolean): String {
 
   for (relocator in this) {
     if (mapLiterals && relocator.skipStringConstants) {
-      return name
+      continue
     } else if (relocator.canRelocateClass(newName)) {
       return prefix + relocator.relocateClass(newName) + suffix
     } else if (relocator.canRelocatePath(newName)) {

src/test/kotlin/com/github/jengelman/gradle/plugins/shadow/relocation/RelocatorsTest.kt

@@ -3,6 +3,7 @@ package com.github.jengelman.gradle.plugins.shadow.relocation
 import assertk.assertThat
 import assertk.assertions.isEqualTo
 import com.github.jengelman.gradle.plugins.shadow.internal.mapName
+import org.junit.jupiter.api.Test
 import org.junit.jupiter.params.ParameterizedTest
 import org.junit.jupiter.params.provider.Arguments
 import org.junit.jupiter.params.provider.MethodSource
@@ -11,12 +12,34 @@ class RelocatorsTest {
   @ParameterizedTest
   @MethodSource("signaturePatternsProvider")
   fun relocateSignaturePatterns(input: String, expected: String) {
-    val actual =
-      setOf(SimpleRelocator("org.package", "shadow.org.package"))
-        .mapName(name = input, onModified = {})
+    val actual = setOf(SimpleRelocator("org.package", "shadow.org.package")).mapName(name = input)
     assertThat(actual).isEqualTo(expected)
   }
 
+  /**
+   * Verifies that a relocator with [Relocator.skipStringConstants] = true is skipped individually
+   * when mapping literals, rather than short-circuiting the entire set of relocators.
+   */
+  @Test
+  fun skipStringConstantsIsPerRelocator() {
+    val skippingRelocator =
+      SimpleRelocator("org.package", "shadow.org.package", skipStringConstants = true)
+    val normalRelocator = SimpleRelocator("com.example", "shadow.com.example")
+    val relocators = linkedSetOf(skippingRelocator, normalRelocator)
+
+    // The skipping relocator cannot match "com.example.Foo", but the normal one can.
+    // Ensure the normal relocator is still applied even though the first one has
+    // skipStringConstants.
+    assertThat(relocators.mapName("com.example.Foo", mapLiterals = true))
+      .isEqualTo("shadow.com.example.Foo")
+    // When mapLiterals=true, the skipping relocator should not apply to its own pattern.
+    assertThat(relocators.mapName("org.package.Bar", mapLiterals = true))
+      .isEqualTo("org.package.Bar")
+    // When mapLiterals=false, the skipping relocator applies normally.
+    assertThat(relocators.mapName("org.package.Bar", mapLiterals = false))
+      .isEqualTo("shadow.org.package.Bar")
+  }
+
   private companion object {
     val primitiveTypes = setOf('B', 'C', 'D', 'F', 'I', 'J', 'S', 'Z')