Repository: GradleUp/shadow
Pull request #1076 (2024-12-01 15:09:09)

Sync `SimpleRelocator` changes from maven-shade-plugin
Updated to 

- https://github.com/apache/maven-shade-plugin/blob/f15f26b1b8100d0c3bfa15b9e8b233922432ee15/src/test/java/org/apache/maven/plugins/shade/relocation/SimpleRelocatorTest.java.
- https://github.com/Goooler/maven-shade-plugin/blob/49f6e13de5748700ef2185795f89f66be61554ef/src/main/java/org/apache/maven/plugins/shade/relocation/SimpleRelocator.java

---

- [ ] [CHANGELOG](https://github.com/GradleUp/shadow/blob/main/src/docs/changes/README.md)'s "Unreleased" section has been updated, if applicable.



Patch:
diff --git a/src/docs/changes/README.md b/src/docs/changes/README.md
index 8e1dabf3..63dc6b7f 100644
--- a/src/docs/changes/README.md
+++ b/src/docs/changes/README.md
@@ -3,6 +3,10 @@
 
 ## [Unreleased]
 
+**Added**
+
+- Sync `SimpleRelocator` changes from maven-shade-plugin. ([#1076](https://github.com/GradleUp/shadow/pull/1076))
+
 
 ## [v9.0.0-beta4] (2024-12-06)
 
diff --git a/src/main/kotlin/com/github/jengelman/gradle/plugins/shadow/relocation/SimpleRelocator.kt b/src/main/kotlin/com/github/jengelman/gradle/plugins/shadow/relocation/SimpleRelocator.kt
index feb00238..74d65b0f 100644
--- a/src/main/kotlin/com/github/jengelman/gradle/plugins/shadow/relocation/SimpleRelocator.kt
+++ b/src/main/kotlin/com/github/jengelman/gradle/plugins/shadow/relocation/SimpleRelocator.kt
@@ -26,6 +26,8 @@ public open class SimpleRelocator @JvmOverloads constructor(
   private val pathPattern: String
   private val shadedPattern: String
   private val shadedPathPattern: String
+  private val sourcePackageExcludes = mutableSetOf<String>()
+  private val sourcePathExcludes = mutableSetOf<String>()
 
   @get:Input
   public val includes: SetProperty<String> = objectFactory.setProperty(String::class.java)
@@ -57,6 +59,32 @@ public open class SimpleRelocator @JvmOverloads constructor(
     }
     this.includes.addAll(normalizePatterns(includes))
     this.excludes.addAll(normalizePatterns(excludes))
+
+    // Don't replace all dots to slashes, otherwise /META-INF/maven/${groupId} can't be matched.
+    if (!includes.isNullOrEmpty()) {
+      this.includes.addAll(includes)
+    }
+    if (!excludes.isNullOrEmpty()) {
+      this.excludes.addAll(excludes)
+    }
+
+    if (!rawString) {
+      // Create exclude pattern sets for sources
+      for (exclude in this.excludes.get()) {
+        // Excludes should be subpackages of the global pattern
+        if (exclude.startsWith(this.pattern)) {
+          sourcePackageExcludes.add(
+            exclude.substring(this.pattern.length).replaceFirst("[.][*]$".toRegex(), ""),
+          )
+        }
+        // Excludes should be subpackages of the global pattern
+        if (exclude.startsWith(pathPattern)) {
+          sourcePathExcludes.add(
+            exclude.substring(pathPattern.length).replaceFirst("/[*]$".toRegex(), ""),
+          )
+        }
+      }
+    }
   }
 
   public open fun include(pattern: String): SimpleRelocator = apply {
@@ -71,7 +99,7 @@ public open class SimpleRelocator @JvmOverloads constructor(
     if (rawString) return Pattern.compile(pathPattern).matcher(path).find()
     // If string is too short - no need to perform expensive string operations
     if (path.length < pathPattern.length) return false
-    val adjustedPath = if (path.endsWith(".class")) {
+    var adjustedPath = if (path.endsWith(".class")) {
       // Safeguard against strings containing only ".class"
       if (path.length == 6) return false
       path.dropLast(6)
@@ -80,8 +108,8 @@ public open class SimpleRelocator @JvmOverloads constructor(
     }
     // Allow for annoying option of an extra / on the front of a path. See MSHADE-119;
     // comes from getClass().getResource("/a/b/c.properties").
-    val startIndex = if (adjustedPath.startsWith("/")) 1 else 0
-    return isIncluded(adjustedPath) && !isExcluded(adjustedPath) && adjustedPath.startsWith(pathPattern, startIndex)
+    adjustedPath = adjustedPath.removePrefix("/")
+    return isIncluded(adjustedPath) && !isExcluded(adjustedPath) && adjustedPath.startsWith(pathPattern)
   }
 
   override fun canRelocateClass(className: String): Boolean {
@@ -100,15 +128,17 @@ public open class SimpleRelocator @JvmOverloads constructor(
 
   override fun relocateClass(context: RelocateClassContext): String {
     context.stats.relocate(pathPattern, shadedPathPattern)
-    return context.className.replaceFirst(pattern, shadedPattern)
+    val clazz = context.className
+    return if (rawString) clazz else clazz.replaceFirst(pattern.toRegex(), shadedPattern)
   }
 
+  /**
+   * We don't call this function now, so we don't have to expose [sourcePackageExcludes] and [sourcePathExcludes] as inputs.
+   */
   override fun applyToSourceContent(sourceContent: String): String {
-    return if (rawString) {
-      sourceContent
-    } else {
-      sourceContent.replace("\\b$pattern".toRegex(), shadedPattern)
-    }
+    if (rawString) return sourceContent
+    val content = shadeSourceWithExcludes(sourceContent, pattern, shadedPattern, sourcePackageExcludes)
+    return shadeSourceWithExcludes(content, pathPattern, shadedPathPattern, sourcePathExcludes)
   }
 
   private fun isIncluded(path: String): Boolean {
@@ -120,6 +150,28 @@ public open class SimpleRelocator @JvmOverloads constructor(
   }
 
   private companion object {
+    /**
+     * Match dot, slash or space at end of string
+     */
+    val RX_ENDS_WITH_DOT_SLASH_SPACE: Pattern = Pattern.compile("[./ ]$")
+
+    /**
+     * Match
+     *  - certain Java keywords + space
+     *  - beginning of Javadoc link + optional line breaks and continuations with '*'
+     *  - (opening curly brace / opening parenthesis / comma / equals / semicolon) + space
+     *  - (closing curly brace / closing multi-line comment) + space
+     *
+     * at end of string
+     */
+    val RX_ENDS_WITH_JAVA_KEYWORD: Pattern = Pattern.compile(
+      "\\b(import|package|public|protected|private|static|final|synchronized|abstract|volatile|extends|implements|throws) $" +
+        "|" +
+        "\\{@link( \\*)* $" +
+        "|" +
+        "([{}(=;,]|\\*/) $",
+    )
+
     fun normalizePatterns(patterns: Collection<String>?) = buildSet {
       patterns ?: return@buildSet
       for (pattern in patterns) {
@@ -131,12 +183,54 @@ public open class SimpleRelocator @JvmOverloads constructor(
 
         val classPattern = pattern.replace('.', '/')
         add(classPattern)
-
-        if (classPattern.endsWith("/*")) {
+        // Actually, class patterns should just use 'foo.bar.*' ending with a single asterisk, but some users
+        // mistake them for path patterns like 'my/path/**', so let us be a bit more lenient here.
+        if (classPattern.endsWith("/*") || classPattern.endsWith("/**")) {
           val packagePattern = classPattern.substring(0, classPattern.lastIndexOf('/'))
           add(packagePattern)
         }
       }
     }
+
+    fun shadeSourceWithExcludes(
+      sourceContent: String,
+      patternFrom: String,
+      patternTo: String,
+      excludedPatterns: Set<String>,
+    ): String {
+      // Usually shading makes package names a bit longer, so make buffer 10% bigger than original source
+      val shadedSourceContent = StringBuilder(sourceContent.length * 11 / 10)
+      var isFirstSnippet = true
+      // Make sure that search pattern starts at word boundary and that we look for literal ".", not regex jokers
+      val snippets = sourceContent.split(("\\b" + patternFrom.replace(".", "[.]") + "\\b").toRegex())
+        .dropLastWhile { it.isEmpty() }.toTypedArray()
+      var i = 0
+      val snippetsLength = snippets.size
+      while (i < snippetsLength) {
+        val snippet = snippets[i]
+        val previousSnippet = if (isFirstSnippet) "" else snippets[i - 1]
+        var doExclude = false
+        for (excludedPattern in excludedPatterns) {
+          if (snippet.startsWith(excludedPattern)) {
+            doExclude = true
+            break
+          }
+        }
+        if (isFirstSnippet) {
+          shadedSourceContent.append(snippet)
+          isFirstSnippet = false
+        } else {
+          val previousSnippetOneLine = previousSnippet.replace("\\s+".toRegex(), " ")
+          val afterDotSlashSpace = RX_ENDS_WITH_DOT_SLASH_SPACE.matcher(previousSnippetOneLine).find()
+          val afterJavaKeyWord = RX_ENDS_WITH_JAVA_KEYWORD.matcher(previousSnippetOneLine).find()
+          val shouldExclude = doExclude || afterDotSlashSpace && !afterJavaKeyWord
+          shadedSourceContent
+            .append(if (shouldExclude) patternFrom else patternTo)
+            .append(snippet)
+        }
+        i++
+      }
+      return shadedSourceContent.toString()
+    }
   }
 }


Test patch:
diff --git a/src/test/kotlin/com/github/jengelman/gradle/plugins/shadow/relocation/SimpleRelocatorTest.kt b/src/test/kotlin/com/github/jengelman/gradle/plugins/shadow/relocation/SimpleRelocatorTest.kt
index 4b13cd5b..0e9861b3 100644
--- a/src/test/kotlin/com/github/jengelman/gradle/plugins/shadow/relocation/SimpleRelocatorTest.kt
+++ b/src/test/kotlin/com/github/jengelman/gradle/plugins/shadow/relocation/SimpleRelocatorTest.kt
@@ -134,7 +134,7 @@ class SimpleRelocatorTest {
   @Test
   fun testCanRelocateRawString() {
     var relocator = SimpleRelocator("org/foo", rawString = true)
-    assertThat(relocator.canRelocatePath("(I)org/foo/bar/Class")).isTrue()
+    assertThat(relocator.canRelocatePath("(I)org/foo/bar/Class;")).isTrue()
 
     relocator = SimpleRelocator("^META-INF/org.foo.xml\$", rawString = true)
     assertThat(relocator.canRelocatePath("META-INF/org.foo.xml")).isTrue()
@@ -147,6 +147,32 @@ class SimpleRelocatorTest {
       .isEqualTo("/org/apache/momentum/mass.properties")
   }
 
+  @Test
+  fun testCanRelocateAbsClassPathWithExcludes() {
+    val relocator = SimpleRelocator(
+      "org/apache/velocity",
+      "org/apache/momentum",
+      excludes = listOf("org/apache/velocity/excluded/*"),
+    )
+    assertThat(relocator.canRelocatePath("/org/apache/velocity/mass.properties")).isTrue()
+    assertThat(relocator.canRelocatePath("org/apache/velocity/mass.properties")).isTrue()
+    assertThat(relocator.canRelocatePath("/org/apache/velocity/excluded/mass.properties")).isFalse()
+    assertThat(relocator.canRelocatePath("org/apache/velocity/excluded/mass.properties")).isFalse()
+  }
+
+  @Test
+  fun testCanRelocateAbsClassPathWithIncludes() {
+    val relocator = SimpleRelocator(
+      "org/apache/velocity",
+      "org/apache/momentum",
+      includes = listOf("org/apache/velocity/included/*"),
+    )
+    assertThat(relocator.canRelocatePath("/org/apache/velocity/mass.properties")).isFalse()
+    assertThat(relocator.canRelocatePath("org/apache/velocity/mass.properties")).isFalse()
+    assertThat(relocator.canRelocatePath("/org/apache/velocity/included/mass.properties")).isTrue()
+    assertThat(relocator.canRelocatePath("org/apache/velocity/included/mass.properties")).isTrue()
+  }
+
   @Test
   fun testRelocatePath() {
     var relocator = SimpleRelocator("org.foo")
@@ -172,15 +198,138 @@ class SimpleRelocatorTest {
   @Test
   fun testRelocateRawString() {
     var relocator = SimpleRelocator("Lorg/foo", "Lhidden/org/foo", rawString = true)
-    assertThat(relocator.relocatePath("(I)Lorg/foo/bar/Class"))
-      .isEqualTo("(I)Lhidden/org/foo/bar/Class")
+    assertThat(relocator.relocatePath("(I)Lorg/foo/bar/Class;"))
+      .isEqualTo("(I)Lhidden/org/foo/bar/Class;")
 
     relocator = SimpleRelocator("^META-INF/org.foo.xml\$", "META-INF/hidden.org.foo.xml", rawString = true)
     assertThat(relocator.relocatePath("META-INF/org.foo.xml"))
       .isEqualTo("META-INF/hidden.org.foo.xml")
   }
 
+  @Test
+  fun testRelocateMavenFiles() {
+    val relocator = SimpleRelocator(
+      "META-INF/maven",
+      "META-INF/shade/maven",
+      excludes = listOf("META-INF/maven/com.foo.bar/artifactId/pom.*"),
+    )
+    assertThat(relocator.canRelocatePath("META-INF/maven/com.foo.bar/artifactId/pom.properties")).isFalse()
+    assertThat(relocator.canRelocatePath("META-INF/maven/com.foo.bar/artifactId/pom.xml")).isFalse()
+    assertThat(relocator.canRelocatePath("META-INF/maven/com/foo/bar/artifactId/pom.properties")).isTrue()
+    assertThat(relocator.canRelocatePath("META-INF/maven/com/foo/bar/artifactId/pom.xml")).isTrue()
+    assertThat(relocator.canRelocatePath("META-INF/maven/com-foo-bar/artifactId/pom.properties")).isTrue()
+    assertThat(relocator.canRelocatePath("META-INF/maven/com-foo-bar/artifactId/pom.xml")).isTrue()
+  }
+
+  @Test
+  fun testRelocateSourceWithExcludesRaw() {
+    val relocator = SimpleRelocator(
+      "org.apache.maven",
+      "com.acme.maven",
+      listOf("foo.bar", "zot.baz"),
+      listOf("irrelevant.exclude", "org.apache.maven.exclude1", "org.apache.maven.sub.exclude2"),
+      true,
+    )
+    assertThat(relocator.applyToSourceContent(sourceFile)).isEqualTo(sourceFile)
+  }
+
+  @Test
+  fun testRelocateSourceWithExcludes() {
+    // Main relocator with in-/excludes
+    val relocator = SimpleRelocator(
+      "org.apache.maven",
+      "com.acme.maven",
+      listOf("foo.bar", "zot.baz"),
+      listOf("irrelevant.exclude", "org.apache.maven.exclude1", "org.apache.maven.sub.exclude2"),
+    )
+    // Make sure not to replace variables 'io' and 'ioInput', package 'java.io'
+    val ioRelocator = SimpleRelocator("io", "shaded.io")
+    // Check corner case which was not working in PR #100
+    val asmRelocator = SimpleRelocator("org.objectweb.asm", "aj.org.objectweb.asm")
+    // Make sure not to replace 'foo' package by path-like 'shaded/foo'
+    val fooRelocator = SimpleRelocator("foo", "shaded.foo", excludes = listOf("foo.bar"))
+    assertThat(
+      fooRelocator.applyToSourceContent(
+        asmRelocator.applyToSourceContent(
+          ioRelocator.applyToSourceContent(relocator.applyToSourceContent(sourceFile)),
+        ),
+      ),
+    ).isEqualTo(relocatedFile)
+  }
+
   private companion object {
+    val sourceFile = """
+      package org.apache.maven.hello;
+      package org.objectweb.asm;
+
+      import foo.bar.Bar;
+      import zot.baz.Baz;
+      import org.apache.maven.exclude1.Ex1;
+      import org.apache.maven.exclude1.a.b.Ex1AB;
+      import org.apache.maven.sub.exclude2.Ex2;
+      import org.apache.maven.sub.exclude2.c.d.Ex2CD;
+      import org.apache.maven.In;
+      import org.apache.maven.e.InE;
+      import org.apache.maven.f.g.InFG;
+      import java.io.IOException;
+
+      /**
+       * Also check out {@link org.apache.maven.hello.OtherClass} and {@link
+       * org.apache.maven.hello.YetAnotherClass}
+       */
+      public class MyClass {
+        private org.apache.maven.exclude1.x.X myX;
+        private org.apache.maven.h.H h;
+        private String ioInput;
+
+        /** Javadoc, followed by default visibility method with fully qualified return type */
+        org.apache.maven.MyReturnType doSomething( org.apache.maven.Bar bar, org.objectweb.asm.sub.Something something) {
+          org.apache.maven.Bar bar;
+          org.objectweb.asm.sub.Something something;
+          String io, val;
+          String noRelocation = "NoWordBoundaryXXXorg.apache.maven.In";
+          String relocationPackage = "org.apache.maven.In";
+          String relocationPath = "org/apache/maven/In";
+        }
+      }
+    """.trimIndent()
+
+    val relocatedFile = """
+      package com.acme.maven.hello;
+      package aj.org.objectweb.asm;
+
+      import foo.bar.Bar;
+      import zot.baz.Baz;
+      import org.apache.maven.exclude1.Ex1;
+      import org.apache.maven.exclude1.a.b.Ex1AB;
+      import org.apache.maven.sub.exclude2.Ex2;
+      import org.apache.maven.sub.exclude2.c.d.Ex2CD;
+      import com.acme.maven.In;
+      import com.acme.maven.e.InE;
+      import com.acme.maven.f.g.InFG;
+      import java.io.IOException;
+
+      /**
+       * Also check out {@link com.acme.maven.hello.OtherClass} and {@link
+       * com.acme.maven.hello.YetAnotherClass}
+       */
+      public class MyClass {
+        private org.apache.maven.exclude1.x.X myX;
+        private com.acme.maven.h.H h;
+        private String ioInput;
+
+        /** Javadoc, followed by default visibility method with fully qualified return type */
+        com.acme.maven.MyReturnType doSomething( com.acme.maven.Bar bar, aj.org.objectweb.asm.sub.Something something) {
+          com.acme.maven.Bar bar;
+          aj.org.objectweb.asm.sub.Something something;
+          String io, val;
+          String noRelocation = "NoWordBoundaryXXXorg.apache.maven.In";
+          String relocationPackage = "com.acme.maven.In";
+          String relocationPath = "com/acme/maven/In";
+        }
+      }
+    """.trimIndent()
+
     fun SimpleRelocator.relocatePath(path: String): String {
       return relocatePath(RelocatePathContext(path))
     }
diff --git a/src/test/kotlin/com/github/jengelman/gradle/plugins/shadow/util/Utils.kt b/src/test/kotlin/com/github/jengelman/gradle/plugins/shadow/util/Utils.kt
index 5f8b3115..8d8d7c01 100644
--- a/src/test/kotlin/com/github/jengelman/gradle/plugins/shadow/util/Utils.kt
+++ b/src/test/kotlin/com/github/jengelman/gradle/plugins/shadow/util/Utils.kt
@@ -10,8 +10,8 @@ val testObjectFactory: ObjectFactory = ProjectBuilder.builder().build().objects
 fun SimpleRelocator(
   pattern: String? = null,
   shadedPattern: String? = null,
-  includes: List<String>? = null,
-  excludes: List<String>? = null,
+  includes: List<String> = emptyList(),
+  excludes: List<String> = emptyList(),
   rawString: Boolean = false,
 ): SimpleRelocator = SimpleRelocator(
   objectFactory = testObjectFactory,