Only expose includes and excludes as inputs in SimpleRelocator (#1079)

* Remove public properties

* Migrate includes and excludes to using ListProperty

* Note this change

api/shadow.api

@@ -185,24 +185,19 @@ 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> ()V
-	public fun <init> (Ljava/lang/String;)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 <init> (Lorg/gradle/api/model/ObjectFactory;)V
+	public fun <init> (Lorg/gradle/api/model/ObjectFactory;Ljava/lang/String;)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 getRawString ()Z
-	public fun getShadedPathPattern ()Ljava/lang/String;
-	public fun getShadedPattern ()Ljava/lang/String;
+	public final fun getExcludes ()Lorg/gradle/api/provider/ListProperty;
+	public final fun getIncludes ()Lorg/gradle/api/provider/ListProperty;
 	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,10 @@
 
 ## [Unreleased]
 
+**Changed**
+
+- **BREAKING CHANGE:** Some public getters are removed from `SimpleRelocator`, `includes` and `excludes` are exposed as `ListProperty`s. ([#1079](https://github.com/GradleUp/shadow/pull/1079))
+
 **Fixed**
 
 - Adjust property initializations and modifiers in `ShadowJar`. ([#1090](https://github.com/GradleUp/shadow/pull/1090))  

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

@@ -2,8 +2,9 @@ package com.github.jengelman.gradle.plugins.shadow.relocation
 
 import java.util.regex.Pattern
 import org.codehaus.plexus.util.SelectorUtils
+import org.gradle.api.model.ObjectFactory
+import org.gradle.api.provider.ListProperty
 import org.gradle.api.tasks.Input
-import org.gradle.api.tasks.Optional
 
 /**
  * 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).
@@ -14,80 +15,62 @@ import org.gradle.api.tasks.Optional
  */
 @CacheableRelocator
 public open class SimpleRelocator @JvmOverloads constructor(
+  objectFactory: ObjectFactory,
   pattern: String? = null,
   shadedPattern: String? = null,
   includes: List<String>? = null,
   excludes: List<String>? = null,
-  private val _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>()
+  private val pattern: String
+  private val pathPattern: String
+  private val shadedPattern: String
+  private val shadedPathPattern: String
+
+  @get:Input
+  public val includes: ListProperty<String> = objectFactory.listProperty(String::class.java)
+
+  @get:Input
+  public val excludes: ListProperty<String> = objectFactory.listProperty(String::class.java)
 
   init {
-    if (_rawString) {
-      _pathPattern = pattern.orEmpty()
-      _shadedPathPattern = shadedPattern.orEmpty()
-      _pattern = "" // not used for raw string relocator
-      _shadedPattern = "" // not used for raw string relocator
+    if (rawString) {
+      this.pathPattern = pattern.orEmpty()
+      this.shadedPathPattern = shadedPattern.orEmpty()
+      this.pattern = "" // not used for raw string relocator
+      this.shadedPattern = "" // not used for raw string relocator
     } else {
       if (pattern == null) {
-        _pattern = ""
-        _pathPattern = ""
+        this.pattern = ""
+        this.pathPattern = ""
       } else {
-        _pattern = pattern.replace('/', '.')
-        _pathPattern = pattern.replace('.', '/')
+        this.pattern = pattern.replace('/', '.')
+        this.pathPattern = pattern.replace('.', '/')
       }
       if (shadedPattern == null) {
-        _shadedPattern = "hidden.${_pattern}"
-        _shadedPathPattern = "hidden/$_pathPattern"
+        this.shadedPattern = "hidden.${this.pattern}"
+        this.shadedPathPattern = "hidden/${this.pathPattern}"
       } else {
-        _shadedPattern = shadedPattern.replace('/', '.')
-        _shadedPathPattern = shadedPattern.replace('.', '/')
+        this.shadedPattern = shadedPattern.replace('/', '.')
+        this.shadedPathPattern = shadedPattern.replace('.', '/')
       }
     }
-    _includes += normalizePatterns(includes)
-    _excludes += normalizePatterns(excludes)
+    this.includes.addAll(normalizePatterns(includes))
+    this.excludes.addAll(normalizePatterns(excludes))
   }
 
-  @get:Input
-  @get:Optional
-  public open val pattern: String get() = _pattern
-
-  @get:Input
-  public open val pathPattern: String get() = _pathPattern
-
-  @get:Input
-  @get:Optional
-  public open val shadedPattern: String get() = _shadedPattern
-
-  @get:Input
-  public open val shadedPathPattern: String get() = _shadedPathPattern
-
-  @get:Input
-  public open val rawString: Boolean get() = _rawString
-
-  @get:Input
-  public open val includes: Set<String> get() = _includes
-
-  @get:Input
-  public open val excludes: Set<String> get() = _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 (_rawString) return Pattern.compile(_pathPattern).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.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
@@ -97,43 +80,43 @@ 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, startIndex)
     return pathStartsWithPattern && isIncluded(adjustedPath) && !isExcluded(adjustedPath)
   }
 
   override fun canRelocateClass(className: String): Boolean {
-    return !_rawString && !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, _shadedPathPattern)
-    return if (_rawString) {
-      path.replace(_pathPattern.toRegex(), _shadedPathPattern)
+    context.stats.relocate(pathPattern, shadedPathPattern)
+    return if (rawString) {
+      path.replace(pathPattern.toRegex(), shadedPathPattern)
     } else {
-      path.replaceFirst(_pathPattern, _shadedPathPattern)
+      path.replaceFirst(pathPattern, shadedPathPattern)
     }
   }
 
   override fun relocateClass(context: RelocateClassContext): String {
-    context.stats.relocate(_pathPattern, _shadedPathPattern)
-    return context.className.replaceFirst(_pattern, _shadedPattern)
+    context.stats.relocate(pathPattern, shadedPathPattern)
+    return context.className.replaceFirst(pattern, shadedPattern)
   }
 
   override fun applyToSourceContent(sourceContent: String): String {
-    return if (_rawString) {
+    return if (rawString) {
       sourceContent
     } else {
-      sourceContent.replace("\\b$_pattern".toRegex(), _shadedPattern)
+      sourceContent.replace("\\b$pattern".toRegex(), shadedPattern)
     }
   }
 
   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) }
   }
 
   private companion object {

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

@@ -225,7 +225,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)
   }
 
@@ -301,7 +301,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/kotlin/com/github/jengelman/gradle/plugins/shadow/relocation/SimpleRelocatorTest.kt

@@ -4,6 +4,7 @@ import assertk.assertThat
 import assertk.assertions.isEqualTo
 import assertk.assertions.isFalse
 import assertk.assertions.isTrue
+import com.github.jengelman.gradle.plugins.shadow.util.SimpleRelocator
 import org.junit.jupiter.api.Test
 
 /**
@@ -132,10 +133,10 @@ class SimpleRelocatorTest {
 
   @Test
   fun testCanRelocateRawString() {
-    var relocator = SimpleRelocator("org/foo", _rawString = true)
+    var relocator = SimpleRelocator("org/foo", rawString = true)
     assertThat(relocator.canRelocatePath("(I)org/foo/bar/Class")).isTrue()
 
-    relocator = SimpleRelocator("^META-INF/org.foo.xml\$", _rawString = true)
+    relocator = SimpleRelocator("^META-INF/org.foo.xml\$", rawString = true)
     assertThat(relocator.canRelocatePath("META-INF/org.foo.xml")).isTrue()
   }
 
@@ -170,11 +171,11 @@ class SimpleRelocatorTest {
 
   @Test
   fun testRelocateRawString() {
-    var relocator = SimpleRelocator("Lorg/foo", "Lhidden/org/foo", _rawString = true)
+    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")
 
-    relocator = SimpleRelocator("^META-INF/org.foo.xml\$", "META-INF/hidden.org.foo.xml", _rawString = true)
+    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")
   }

src/test/kotlin/com/github/jengelman/gradle/plugins/shadow/transformers/Log4j2PluginsCacheFileTransformerTest.kt

@@ -4,6 +4,7 @@ import assertk.assertThat
 import assertk.assertions.isEqualTo
 import assertk.assertions.isTrue
 import com.github.jengelman.gradle.plugins.shadow.relocation.SimpleRelocator
+import com.github.jengelman.gradle.plugins.shadow.util.SimpleRelocator
 import java.io.File
 import java.net.URL
 import java.util.Collections

src/test/kotlin/com/github/jengelman/gradle/plugins/shadow/util/Utils.kt

@@ -1,6 +1,23 @@
 package com.github.jengelman.gradle.plugins.shadow.util
 
+import com.github.jengelman.gradle.plugins.shadow.relocation.SimpleRelocator
 import org.gradle.api.model.ObjectFactory
 import org.gradle.testfixtures.ProjectBuilder
 
 val testObjectFactory: ObjectFactory = ProjectBuilder.builder().build().objects
+
+@Suppress("TestFunctionName")
+fun SimpleRelocator(
+  pattern: String? = null,
+  shadedPattern: String? = null,
+  includes: List<String>? = null,
+  excludes: List<String>? = null,
+  rawString: Boolean = false,
+): SimpleRelocator = SimpleRelocator(
+  objectFactory = testObjectFactory,
+  pattern = pattern,
+  shadedPattern = shadedPattern,
+  includes = includes,
+  excludes = excludes,
+  rawString = rawString,
+)