Revert changes in SimpleRelocator (#1052)

* Revert "Migrate SimpleRelocator to using lazy properties (#1047)"

This reverts commit 59676b7

* Still rename isRawString to rawString

* Optimize normalizePatterns

* Mark pattern and shadedPattern non-null

* Inline getters

* Note this change

* Revert "Inline getters"

This reverts commit b05bc7d.

api/shadow.api

@@ -181,22 +181,22 @@ public final class com/github/jengelman/gradle/plugins/shadow/relocation/Relocat
 }
 
 public class com/github/jengelman/gradle/plugins/shadow/relocation/SimpleRelocator : com/github/jengelman/gradle/plugins/shadow/relocation/Relocator {
-	public fun <init> (Lorg/gradle/api/model/ObjectFactory;Ljava/lang/String;Ljava/lang/String;)V
-	public fun <init> (Lorg/gradle/api/model/ObjectFactory;Ljava/lang/String;Ljava/lang/String;Ljava/util/List;)V
-	public fun <init> (Lorg/gradle/api/model/ObjectFactory;Ljava/lang/String;Ljava/lang/String;Ljava/util/List;Ljava/util/List;)V
-	public fun <init> (Lorg/gradle/api/model/ObjectFactory;Ljava/lang/String;Ljava/lang/String;Ljava/util/List;Ljava/util/List;Z)V
-	public synthetic fun <init> (Lorg/gradle/api/model/ObjectFactory;Ljava/lang/String;Ljava/lang/String;Ljava/util/List;Ljava/util/List;ZILkotlin/jvm/internal/DefaultConstructorMarker;)V
+	public fun <init> (Ljava/lang/String;Ljava/lang/String;)V
+	public fun <init> (Ljava/lang/String;Ljava/lang/String;Ljava/util/List;)V
+	public fun <init> (Ljava/lang/String;Ljava/lang/String;Ljava/util/List;Ljava/util/List;)V
+	public fun <init> (Ljava/lang/String;Ljava/lang/String;Ljava/util/List;Ljava/util/List;Z)V
+	public synthetic fun <init> (Ljava/lang/String;Ljava/lang/String;Ljava/util/List;Ljava/util/List;ZILkotlin/jvm/internal/DefaultConstructorMarker;)V
 	public fun applyToSourceContent (Ljava/lang/String;)Ljava/lang/String;
 	public fun canRelocateClass (Ljava/lang/String;)Z
 	public fun canRelocatePath (Ljava/lang/String;)Z
 	public fun exclude (Ljava/lang/String;)Lcom/github/jengelman/gradle/plugins/shadow/relocation/SimpleRelocator;
-	public fun getExcludes ()Lorg/gradle/api/provider/SetProperty;
-	public fun getIncludes ()Lorg/gradle/api/provider/SetProperty;
-	public fun getPathPattern ()Lorg/gradle/api/provider/Property;
-	public fun getPattern ()Lorg/gradle/api/provider/Property;
-	public fun getRawString ()Lorg/gradle/api/provider/Property;
-	public fun getShadedPathPattern ()Lorg/gradle/api/provider/Property;
-	public fun getShadedPattern ()Lorg/gradle/api/provider/Property;
+	public fun getExcludes ()Ljava/util/Set;
+	public fun getIncludes ()Ljava/util/Set;
+	public fun getPathPattern ()Ljava/lang/String;
+	public fun getPattern ()Ljava/lang/String;
+	public fun getRawString ()Z
+	public fun getShadedPathPattern ()Ljava/lang/String;
+	public fun getShadedPattern ()Ljava/lang/String;
 	public fun include (Ljava/lang/String;)Lcom/github/jengelman/gradle/plugins/shadow/relocation/SimpleRelocator;
 	public fun relocateClass (Lcom/github/jengelman/gradle/plugins/shadow/relocation/RelocateClassContext;)Ljava/lang/String;
 	public fun relocatePath (Lcom/github/jengelman/gradle/plugins/shadow/relocation/RelocatePathContext;)Ljava/lang/String;

src/docs/changes/README.md

@@ -3,6 +3,11 @@
 
 ## [Unreleased]
 
+**Fixed**
+
+- Revert "Migrate SimpleRelocator to using lazy properties" ([#1052](https://github.com/GradleUp/shadow/pull/1052))  
+  This fixes the relocation not working in `v9.0.0-beta1`.
+
 
 ## [v9.0.0-beta1] (2024-11-27)
 

src/main/kotlin/com/github/jengelman/gradle/plugins/shadow/relocation/SimpleRelocator.kt

@@ -1,11 +1,7 @@
 package com.github.jengelman.gradle.plugins.shadow.relocation
 
-import com.github.jengelman.gradle.plugins.shadow.internal.property
 import java.util.regex.Pattern
 import org.codehaus.plexus.util.SelectorUtils
-import org.gradle.api.model.ObjectFactory
-import org.gradle.api.provider.Property
-import org.gradle.api.provider.SetProperty
 import org.gradle.api.tasks.Input
 import org.gradle.api.tasks.Optional
 
@@ -16,77 +12,82 @@ import org.gradle.api.tasks.Optional
  * @author Mauro Talevi
  * @author John Engelman
  */
-@Suppress("LeakingThis")
 @CacheableRelocator
 public open class SimpleRelocator @JvmOverloads constructor(
-  objectFactory: ObjectFactory,
   pattern: String?,
   shadedPattern: String?,
   includes: List<String>? = null,
   excludes: List<String>? = null,
-  rawString: Boolean = false,
+  private val _rawString: Boolean = false,
 ) : Relocator {
+  private val _pattern: String
+  private val _pathPattern: String
+  private val _shadedPattern: String
+  private val _shadedPathPattern: String
+  private val _includes = mutableSetOf<String>()
+  private val _excludes = mutableSetOf<String>()
+
+  init {
+    if (_rawString) {
+      _pathPattern = pattern.orEmpty()
+      _shadedPathPattern = shadedPattern.orEmpty()
+      _pattern = "" // not used for raw string relocator
+      _shadedPattern = "" // not used for raw string relocator
+    } else {
+      if (pattern == null) {
+        _pattern = ""
+        _pathPattern = ""
+      } else {
+        _pattern = pattern.replace('/', '.')
+        _pathPattern = pattern.replace('.', '/')
+      }
+      if (shadedPattern == null) {
+        _shadedPattern = "hidden.${_pattern}"
+        _shadedPathPattern = "hidden/$_pathPattern"
+      } else {
+        _shadedPattern = shadedPattern.replace('/', '.')
+        _shadedPathPattern = shadedPattern.replace('.', '/')
+      }
+    }
+    _includes += normalizePatterns(includes)
+    _excludes += normalizePatterns(excludes)
+  }
 
   @get:Input
   @get:Optional
-  public open val pattern: Property<String> = objectFactory.property()
+  public open val pattern: String get() = _pattern
 
   @get:Input
-  public open val pathPattern: Property<String> = objectFactory.property()
+  public open val pathPattern: String get() = _pathPattern
 
   @get:Input
   @get:Optional
-  public open val shadedPattern: Property<String> = objectFactory.property()
+  public open val shadedPattern: String get() = _shadedPattern
 
   @get:Input
-  public open val shadedPathPattern: Property<String> = objectFactory.property()
+  public open val shadedPathPattern: String get() = _shadedPathPattern
 
   @get:Input
-  public open val rawString: Property<Boolean> = objectFactory.property(rawString)
+  public open val rawString: Boolean get() = _rawString
 
   @get:Input
-  public open val includes: SetProperty<String> = objectFactory.setProperty(String::class.java)
+  public open val includes: Set<String> get() = _includes
 
   @get:Input
-  public open val excludes: SetProperty<String> = objectFactory.setProperty(String::class.java)
-
-  init {
-    if (rawString) {
-      pathPattern.set(pattern.orEmpty())
-      shadedPathPattern.set(shadedPattern.orEmpty())
-      // Don't need to assign pattern and shadedPattern for raw string relocator
-    } else {
-      if (pattern == null) {
-        this.pattern.set("")
-        this.pathPattern.set("")
-      } else {
-        this.pattern.set(pattern.replace('/', '.'))
-        this.pathPattern.set(pattern.replace('.', '/'))
-      }
-      if (shadedPattern == null) {
-        this.shadedPattern.set(this.pattern.map { "hidden.$it" })
-        this.shadedPathPattern.set(this.pathPattern.map { "hidden/$it" })
-      } else {
-        this.shadedPattern.set(shadedPattern.replace('/', '.'))
-        this.shadedPathPattern.set(shadedPattern.replace('.', '/'))
-      }
-    }
-    this.includes.addAll(normalizePatterns(includes))
-    this.excludes.addAll(normalizePatterns(excludes))
-  }
+  public open val excludes: Set<String> get() = _excludes
 
   public open fun include(pattern: String): SimpleRelocator = apply {
-    includes.addAll(normalizePatterns(listOf(pattern)))
+    _includes += normalizePatterns(listOf(pattern))
   }
 
   public open fun exclude(pattern: String): SimpleRelocator = apply {
-    excludes.addAll(normalizePatterns(listOf(pattern)))
+    _excludes += normalizePatterns(listOf(pattern))
   }
 
   override fun canRelocatePath(path: String): Boolean {
-    if (rawString.get()) return Pattern.compile(pathPattern.get()).matcher(path).find()
+    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.get().length) return false
+    if (path.length < _pathPattern.length) return false
     val adjustedPath = if (path.endsWith(".class")) {
       // Safeguard against strings containing only ".class"
       if (path.length == 6) return false
@@ -96,60 +97,63 @@ 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
-    val pathStartsWithPattern = adjustedPath.startsWith(pathPattern.get(), startIndex)
+    val pathStartsWithPattern = adjustedPath.startsWith(_pathPattern, startIndex)
     return pathStartsWithPattern && isIncluded(adjustedPath) && !isExcluded(adjustedPath)
   }
 
   override fun canRelocateClass(className: String): Boolean {
-    return !rawString.get() && !className.contains('/') && canRelocatePath(className.replace('.', '/'))
+    return !_rawString && !className.contains('/') && canRelocatePath(className.replace('.', '/'))
   }
 
   override fun relocatePath(context: RelocatePathContext): String {
     val path = context.path
-    context.stats.relocate(pathPattern.get(), shadedPathPattern.get())
-    return if (rawString.get()) {
-      path.replace(pathPattern.get().toRegex(), shadedPathPattern.get())
+    context.stats.relocate(_pathPattern, _shadedPathPattern)
+    return if (_rawString) {
+      path.replace(_pathPattern.toRegex(), _shadedPathPattern)
     } else {
-      path.replaceFirst(pathPattern.get(), shadedPathPattern.get())
+      path.replaceFirst(_pathPattern, _shadedPathPattern)
     }
   }
 
   override fun relocateClass(context: RelocateClassContext): String {
-    context.stats.relocate(pathPattern.get(), shadedPathPattern.get())
-    return context.className.replaceFirst(pattern.orNull.orEmpty(), shadedPattern.orNull.orEmpty())
+    context.stats.relocate(_pathPattern, _shadedPathPattern)
+    return context.className.replaceFirst(_pattern, _shadedPattern)
   }
 
   override fun applyToSourceContent(sourceContent: String): String {
-    return if (rawString.get()) {
+    return if (_rawString) {
       sourceContent
     } else {
-      sourceContent.replace("\\b${pattern.orNull}".toRegex(), shadedPattern.orNull.orEmpty())
-    }
-  }
-
-  private fun normalizePatterns(patterns: Collection<String>?) = buildSet {
-    for (pattern in patterns.orEmpty()) {
-      // Regex patterns don't need to be normalized and stay as is
-      if (pattern.startsWith(SelectorUtils.REGEX_HANDLER_PREFIX)) {
-        add(pattern)
-        continue
-      }
-
-      val classPattern = pattern.replace('.', '/')
-      add(classPattern)
-
-      if (classPattern.endsWith("/*")) {
-        val packagePattern = classPattern.substring(0, classPattern.lastIndexOf('/'))
-        add(packagePattern)
-      }
+      sourceContent.replace("\\b$_pattern".toRegex(), _shadedPattern)
     }
   }
 
   private fun isIncluded(path: String): Boolean {
-    return includes.get().isEmpty() || includes.get().any { SelectorUtils.matchPath(it, path, "/", true) }
+    return _includes.isEmpty() || _includes.any { SelectorUtils.matchPath(it, path, "/", true) }
   }
 
   private fun isExcluded(path: String): Boolean {
-    return excludes.get().any { SelectorUtils.matchPath(it, path, "/", true) }
+    return _excludes.any { SelectorUtils.matchPath(it, path, "/", true) }
+  }
+
+  private companion object {
+    fun normalizePatterns(patterns: Collection<String>?) = buildSet {
+      patterns ?: return@buildSet
+      for (pattern in patterns) {
+        // Regex patterns don't need to be normalized and stay as is
+        if (pattern.startsWith(SelectorUtils.REGEX_HANDLER_PREFIX)) {
+          add(pattern)
+          continue
+        }
+
+        val classPattern = pattern.replace('.', '/')
+        add(classPattern)
+
+        if (classPattern.endsWith("/*")) {
+          val packagePattern = classPattern.substring(0, classPattern.lastIndexOf('/'))
+          add(packagePattern)
+        }
+      }
+    }
   }
 }

src/main/kotlin/com/github/jengelman/gradle/plugins/shadow/tasks/ShadowJar.kt

@@ -213,7 +213,7 @@ public abstract class ShadowJar :
     destination: String,
     action: Action<SimpleRelocator>?,
   ): ShadowJar = apply {
-    val relocator = SimpleRelocator(objectFactory, pattern, destination)
+    val relocator = SimpleRelocator(pattern, destination)
     addRelocator(relocator, action)
   }
 
@@ -289,7 +289,7 @@ public abstract class ShadowJar :
             jarFile.entries().toList()
               .filter { it.name.endsWith(".class") && it.name != "module-info.class" }
               .map { it.name.substringBeforeLast('/').replace('/', '.') }
-              .map { SimpleRelocator(objectFactory, it, "$prefix.$it") }
+              .map { SimpleRelocator(it, "$prefix.$it") }
           }
         }
       }

src/test/groovy/com/github/jengelman/gradle/plugins/shadow/relocation/SimpleRelocatorParameterTest.groovy

@@ -19,7 +19,6 @@
 
 package com.github.jengelman.gradle.plugins.shadow.relocation
 
-import org.gradle.testfixtures.ProjectBuilder
 import org.junit.jupiter.api.Test
 
 import static org.junit.jupiter.api.Assertions.fail
@@ -32,7 +31,6 @@ import static org.junit.jupiter.api.Assertions.fail
  * @author John Engelman
  */
 class SimpleRelocatorParameterTest {
-    private static final def objectFactory = ProjectBuilder.builder().build().objects
 
     @Test
     void testThatNullPatternInConstructorShouldNotThrowNullPointerException() {
@@ -46,7 +44,7 @@ class SimpleRelocatorParameterTest {
 
     private static void constructThenFailOnNullPointerException(String pattern, String shadedPattern) {
         try {
-            new SimpleRelocator(objectFactory, pattern, shadedPattern)
+            new SimpleRelocator(pattern, shadedPattern)
         }
         catch (NullPointerException ignored) {
             fail("Constructor should not throw null pointer exceptions")