Migrate SimpleRelocator to using lazy properties (#1047)

* Migrate SimpleRelocator

* Rename isRawString to rawString

* Fix

* Add SimpleRelocatorParent

* Don't need to assign pattern and shadedPattern for raw string relocator

* Revert "Add SimpleRelocatorParent"

This reverts commit 11f67a3.

* Set rawString explicitly

* Note this change

* Suppress LeakingThis

* Tweak kdoc

* Revert "Set rawString explicitly"

This reverts commit 1c5e644.

api/shadow.api

@@ -181,23 +181,23 @@ 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> (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 <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 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 ()Ljava/util/Set;
-	public fun getIncludes ()Ljava/util/Set;
-	public fun getPathPattern ()Ljava/lang/String;
-	public fun getPattern ()Ljava/lang/String;
-	public fun getShadedPathPattern ()Ljava/lang/String;
-	public fun getShadedPattern ()Ljava/lang/String;
+	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 include (Ljava/lang/String;)Lcom/github/jengelman/gradle/plugins/shadow/relocation/SimpleRelocator;
-	public fun isRawString ()Z
 	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

@@ -12,6 +12,7 @@
 - **BREAKING CHANGE:** Rewrite this plugin in Kotlin. ([#1012](https://github.com/GradleUp/shadow/pull/1012))
 - **BREAKING CHANGE:** Migrate `Transformer`s to using lazy properties. ([#1036](https://github.com/GradleUp/shadow/pull/1036))  
 - **BREAKING CHANGE:** Migrate `ShadowJar` to using lazy properties. ([#1044](https://github.com/GradleUp/shadow/pull/1044))  
+- **BREAKING CHANGE:** Migrate `SimpleRelocator` to using lazy properties. ([#1047](https://github.com/GradleUp/shadow/pull/1047))  
   `isEnableRelocation` is deprecated, use `enableRelocation` instead.
 - **BREAKING CHANGE:** Resolve `Configuration` directly in `DependencyFilter`. ([#1045](https://github.com/GradleUp/shadow/pull/1045))
 

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

@@ -1,93 +1,92 @@
 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
 
 /**
- * Modified from `org.apache.maven.plugins.shade.relocation.SimpleRelocator.java`
+ * Modified from [org.apache.maven.plugins.shade.relocation.SimpleRelocator.java](https://github.com/apache/maven-shade-plugin/blob/master/src/main/java/org/apache/maven/plugins/shade/relocation/SimpleRelocator.java).
  *
  * @author Jason van Zyl
  * @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,
-  private val _isRawString: Boolean = false,
+  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 (_isRawString) {
-      _pathPattern = pattern.orEmpty()
-      _shadedPathPattern = shadedPattern.orEmpty()
-      _pattern = null // not used for raw string relocator
-      _shadedPattern = null // 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: String? get() = _pattern
+  public open val pattern: Property<String> = objectFactory.property()
 
   @get:Input
-  public open val pathPattern: String get() = _pathPattern
+  public open val pathPattern: Property<String> = objectFactory.property()
 
   @get:Input
   @get:Optional
-  public open val shadedPattern: String? get() = _shadedPattern
+  public open val shadedPattern: Property<String> = objectFactory.property()
 
   @get:Input
-  public open val shadedPathPattern: String get() = _shadedPathPattern
+  public open val shadedPathPattern: Property<String> = objectFactory.property()
 
   @get:Input
-  public open val isRawString: Boolean get() = _isRawString
+  public open val rawString: Property<Boolean> = objectFactory.property(rawString)
 
   @get:Input
-  public open val includes: Set<String> get() = _includes
+  public open val includes: SetProperty<String> = objectFactory.setProperty(String::class.java)
 
   @get:Input
-  public open val excludes: Set<String> get() = _excludes
+  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 fun include(pattern: String): SimpleRelocator = apply {
-    _includes += normalizePatterns(listOf(pattern))
+    includes.addAll(normalizePatterns(listOf(pattern)))
   }
 
   public open fun exclude(pattern: String): SimpleRelocator = apply {
-    _excludes += normalizePatterns(listOf(pattern))
+    excludes.addAll(normalizePatterns(listOf(pattern)))
   }
 
   override fun canRelocatePath(path: String): Boolean {
-    if (_isRawString) return Pattern.compile(_pathPattern).matcher(path).find()
+    if (rawString.get()) return Pattern.compile(pathPattern.get()).matcher(path).find()
     // If string is too short - no need to perform expensive string operations
-    if (path.length < _pathPattern.length) return false
+    if (path.length < pathPattern.get().length) return false
     val adjustedPath = if (path.endsWith(".class")) {
       // Safeguard against strings containing only ".class"
       if (path.length == 6) return false
@@ -97,34 +96,34 @@ 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, startIndex)
+    val pathStartsWithPattern = adjustedPath.startsWith(pathPattern.get(), startIndex)
     return pathStartsWithPattern && isIncluded(adjustedPath) && !isExcluded(adjustedPath)
   }
 
   override fun canRelocateClass(className: String): Boolean {
-    return !_isRawString && !className.contains('/') && canRelocatePath(className.replace('.', '/'))
+    return !rawString.get() && !className.contains('/') && canRelocatePath(className.replace('.', '/'))
   }
 
   override fun relocatePath(context: RelocatePathContext): String {
     val path = context.path
-    context.stats.relocate(_pathPattern, _shadedPathPattern)
-    return if (_isRawString) {
-      path.replace(_pathPattern.toRegex(), _shadedPathPattern)
+    context.stats.relocate(pathPattern.get(), shadedPathPattern.get())
+    return if (rawString.get()) {
+      path.replace(pathPattern.get().toRegex(), shadedPathPattern.get())
     } else {
-      path.replaceFirst(_pathPattern, _shadedPathPattern)
+      path.replaceFirst(pathPattern.get(), shadedPathPattern.get())
     }
   }
 
   override fun relocateClass(context: RelocateClassContext): String {
-    context.stats.relocate(_pathPattern, _shadedPathPattern)
-    return context.className.replaceFirst(_pattern.orEmpty(), _shadedPattern.orEmpty())
+    context.stats.relocate(pathPattern.get(), shadedPathPattern.get())
+    return context.className.replaceFirst(pattern.orNull.orEmpty(), shadedPattern.orNull.orEmpty())
   }
 
   override fun applyToSourceContent(sourceContent: String): String {
-    return if (_isRawString) {
+    return if (rawString.get()) {
       sourceContent
     } else {
-      sourceContent.replace("\\b$_pattern".toRegex(), _shadedPattern.orEmpty())
+      sourceContent.replace("\\b${pattern.orNull}".toRegex(), shadedPattern.orNull.orEmpty())
     }
   }
 
@@ -147,10 +146,10 @@ public open class SimpleRelocator @JvmOverloads constructor(
   }
 
   private fun isIncluded(path: String): Boolean {
-    return _includes.isEmpty() || _includes.any { SelectorUtils.matchPath(it, path, "/", true) }
+    return includes.get().isEmpty() || includes.get().any { SelectorUtils.matchPath(it, path, "/", true) }
   }
 
   private fun isExcluded(path: String): Boolean {
-    return _excludes.any { SelectorUtils.matchPath(it, path, "/", true) }
+    return excludes.get().any { SelectorUtils.matchPath(it, path, "/", true) }
   }
 }

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(pattern, destination)
+    val relocator = SimpleRelocator(objectFactory, 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(it, "$prefix.$it") }
+              .map { SimpleRelocator(objectFactory, it, "$prefix.$it") }
           }
         }
       }

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

@@ -19,6 +19,7 @@
 
 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
@@ -31,6 +32,7 @@ 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() {
@@ -44,7 +46,7 @@ class SimpleRelocatorParameterTest {
 
     private static void constructThenFailOnNullPointerException(String pattern, String shadedPattern) {
         try {
-            new SimpleRelocator(pattern, shadedPattern)
+            new SimpleRelocator(objectFactory, pattern, shadedPattern)
         }
         catch (NullPointerException ignored) {
             fail("Constructor should not throw null pointer exceptions")