Support parameter usage after the iteration completes

Previously the same delegate instance was shared between all iterations, and its argument points to the value in the current iteration. This was fine if the parameter is only used in the current iteration, but if a parameter is captured in a lambda and used later, the argument will change for a future iteration's argument combination, and the value will not be the same as when it was captured. To fix this, the delegate is now unique to its iteration instead of being shared, and maintains an unchanging argument so that it can be captured and accessed later. See issue #10 for context.
BenWoodworth · Nov 17, 2023 · 6dc6ba5 · 6dc6ba5
1 parent 1ede2a9
commit 6dc6ba5
Show file tree

Hide file tree

Showing 7 changed files with 102 additions and 46 deletions.
diff --git a/src/commonMain/kotlin/ParameterState.kt b/src/commonMain/kotlin/ParameterState.kt
@@ -113,12 +113,14 @@ internal class ParameterState {
             throw ParameterizeException("Cannot use parameter delegate with `${property.name}`, since it was declared with `${declaredProperty.name}`.")
         }
 
-        hasBeenUsed = true
-
         @Suppress("UNCHECKED_CAST") // Argument is declared with property's arguments, so must be T
         return argument as T
     }
 
+    fun useArgument() {
+        hasBeenUsed = true
+    }
+
     /**
      * Iterates the parameter argument.
      *

diff --git a/src/commonMain/kotlin/Parameterize.kt b/src/commonMain/kotlin/Parameterize.kt
@@ -84,16 +84,19 @@ public fun ParameterizeContext.parameterize(
     }
 
     val state = ParameterizeState()
-    val scope = ParameterizeScope(state)
 
     while (state.startNextIteration()) {
+        val scope = ParameterizeScope(state)
+
         try {
             scope.block()
         } catch (_: ParameterizeContinue) {
         } catch (exception: ParameterizeException) {
             throw exception
         } catch (failure: Throwable) {
             state.handleFailure(onFailure, failure)
+        } finally {
+            scope.iterationCompleted = true
         }
     }
 
@@ -132,6 +135,8 @@ internal class ParameterizeException(override val message: String) : Exception(m
 public class ParameterizeScope internal constructor(
     private val state: ParameterizeState,
 ) {
+    internal var iterationCompleted: Boolean = false
+
     /** @suppress */
     override fun toString(): String =
         state.getFailureArguments().joinToString(
@@ -164,14 +169,20 @@ public class ParameterizeScope internal constructor(
         Parameter(arguments)
 
     /** @suppress */
-    public operator fun <T> Parameter<T>.provideDelegate(thisRef: Any?, property: KProperty<*>): ParameterDelegate<T> =
+    public operator fun <T> Parameter<T>.provideDelegate(thisRef: Any?, property: KProperty<*>): ParameterDelegate<T> {
+        if (iterationCompleted) {
+            throw ParameterizeException("Cannot declare parameter `${property.name}` after its iteration has completed")
+        }
+
         @Suppress("UNCHECKED_CAST")
-        state.declareParameter(property as KProperty<T>, arguments)
+        return state.declareParameter(this@ParameterizeScope, property as KProperty<T>, arguments)
+    }
 
     /** @suppress */
-    public operator fun <T> ParameterDelegate<T>.getValue(thisRef: Any?, property: KProperty<*>): T =
-        @Suppress("UNCHECKED_CAST")
-        parameterState.getArgument(property as KProperty<T>)
+    public operator fun <T> ParameterDelegate<T>.getValue(thisRef: Any?, property: KProperty<*>): T {
+        if (!iterationCompleted) parameterState.useArgument()
+        return argument
+    }
 
 
     /** @suppress */
@@ -181,8 +192,10 @@ public class ParameterizeScope internal constructor(
     )
 
     /** @suppress */
-    public class ParameterDelegate<@Suppress("unused") out T> internal constructor(
-        internal val parameterState: ParameterState
+    public class ParameterDelegate<out T> internal constructor(
+        internal val scope: ParameterizeScope,
+        internal val parameterState: ParameterState,
+        internal val argument: T
     ) {
         /**
          * Returns a string representation of the current argument.
@@ -193,7 +206,7 @@ public class ParameterizeScope internal constructor(
          * ```
          */
         override fun toString(): String =
-            parameterState.toString()
+            argument.toString()
     }
 }
 

diff --git a/src/commonMain/kotlin/ParameterizeState.kt b/src/commonMain/kotlin/ParameterizeState.kt
@@ -14,7 +14,7 @@ internal class ParameterizeState {
      * Parameter instances are re-used between iterations, so will never be removed.
      * The true number of parameters in the current iteration is maintained in [parameterCount].
      */
-    private val parameters = ArrayList<ParameterDelegate<Nothing>>()
+    private val parameters = ArrayList<ParameterState>()
     private var parameterBeingUsed: KProperty<*>? = null
     private var parameterCount = 0
 
@@ -37,7 +37,11 @@ internal class ParameterizeState {
         return shouldContinue
     }
 
-    fun <T> declareParameter(property: KProperty<T>, arguments: Iterable<T>): ParameterDelegate<Nothing> {
+    fun <T> declareParameter(
+        scope: ParameterizeScope,
+        property: KProperty<T>,
+        arguments: Iterable<T>
+    ): ParameterDelegate<T> {
         parameterBeingUsed?.let {
             throw ParameterizeException("Nesting parameters is not currently supported: `${property.name}` was declared within `${it.name}`'s arguments")
         }
@@ -47,16 +51,16 @@ internal class ParameterizeState {
         val parameter = if (parameterIndex in parameters.indices) {
             parameters[parameterIndex]
         } else {
-            ParameterDelegate<Nothing>(ParameterState())
+            ParameterState()
                 .also { parameters += it }
         }
 
         property.trackNestedUsage {
-            parameter.parameterState.declare(property, arguments)
+            parameter.declare(property, arguments)
             parameterCount++ // After declaring, since the parameter shouldn't count if declare throws
         }
 
-        return parameter
+        return ParameterDelegate(scope, parameter, parameter.getArgument(property))
     }
 
     private inline fun <T> KProperty<T>.trackNestedUsage(block: () -> T): T {
@@ -81,13 +85,13 @@ internal class ParameterizeState {
         var iterated = false
 
         for (parameter in parameters.subList(0, parameterCount).asReversed()) {
-            if (!parameter.parameterState.isLastArgument) {
-                parameter.parameterState.nextArgument()
+            if (!parameter.isLastArgument) {
+                parameter.nextArgument()
                 iterated = true
                 break
             }
 
-            parameter.parameterState.reset()
+            parameter.reset()
         }
 
         parameterCount = 0
@@ -100,8 +104,8 @@ internal class ParameterizeState {
      */
     fun getFailureArguments(): List<ParameterizeFailure.Argument<*>> =
         parameters.take(parameterCount)
-            .filter { it.parameterState.hasBeenUsed }
-            .map { it.parameterState.getFailureArgument() }
+            .filter { it.hasBeenUsed }
+            .map { it.getFailureArgument() }
 
     fun handleFailure(onFailure: OnFailureScope.(Throwable) -> Unit, failure: Throwable) {
         failureCount++

diff --git a/src/commonTest/kotlin/ParameterStateSpec.kt b/src/commonTest/kotlin/ParameterStateSpec.kt
@@ -153,9 +153,9 @@ class ParameterStateSpec {
     }
 
     @Test
-    fun getting_argument_should_set_has_been_used_to_true() {
+    fun use_argument_should_set_has_been_used_to_true() {
         parameter.declare(::property, listOf("first", "second"))
-        parameter.getArgument(::property)
+        parameter.useArgument()
 
         assertTrue(parameter.hasBeenUsed)
     }
@@ -258,7 +258,7 @@ class ParameterStateSpec {
     @Test
     fun redeclare_with_different_parameter_should_not_change_has_been_used() {
         parameter.declare(::property, listOf("a"))
-        parameter.getArgument(::property)
+        parameter.useArgument()
 
         runCatching {
             parameter.declare(::differentProperty, listOf("a"))

diff --git a/src/commonTest/kotlin/ParameterizeConfigurationOnFailureSpec.kt b/src/commonTest/kotlin/ParameterizeConfigurationOnFailureSpec.kt
@@ -192,4 +192,29 @@ class ParameterizeConfigurationOnFailureSpec : ParameterizeContext {
             fail()
         }
     }
+
+    @Test
+    fun failure_arguments_should_not_include_captured_parameters_from_previous_iterations() = parameterize(
+        onFailure = {
+            val parameters = arguments.map { it.parameter.name }
+
+            assertFalse(
+                "neverUsedDuringTheCurrentIteration" in parameters,
+                "neverUsedDuringTheCurrentIteration in $parameters"
+            )
+        }
+    ) {
+        val neverUsedDuringTheCurrentIteration by parameterOf(Unit)
+
+        @Suppress("UNUSED_EXPRESSION")
+        val usePreviousIterationParameter by parameterOf(
+            { }, // Don't use it the first iteration
+            { neverUsedDuringTheCurrentIteration }
+        )
+
+        // On the 2nd iteration, use the parameter captured from the 1st iteration
+        usePreviousIterationParameter()
+
+        fail()
+    }
 }
diff --git a/src/commonTest/kotlin/ParameterizeExceptionSpec.kt b/src/commonTest/kotlin/ParameterizeExceptionSpec.kt
@@ -1,7 +1,5 @@
 package com.benwoodworth.parameterize
 
-import com.benwoodworth.parameterize.ParameterizeScope.ParameterDelegate
-import kotlin.properties.PropertyDelegateProvider
 import kotlin.test.Test
 import kotlin.test.assertEquals
 import kotlin.test.assertFailsWith
@@ -27,26 +25,6 @@ class ParameterizeExceptionSpec {
         assertEquals(1, iterations, "Should not continue after exception")
     }
 
-    @Test
-    fun parameter_delegate_used_with_the_wrong_property() {
-        val exception = assertFailsWith<ParameterizeException> {
-            parameterize {
-                lateinit var interceptedDelegateFromA: ParameterDelegate<Int>
-
-                val a by PropertyDelegateProvider { thisRef: Any?, property ->
-                    parameterOf(1)
-                        .provideDelegate(thisRef, property)
-                        .also { interceptedDelegateFromA = it }
-                }
-
-                val b by interceptedDelegateFromA
-                useParameter(b)
-            }
-        }
-
-        assertEquals("Cannot use parameter delegate with `b`, since it was declared with `a`.", exception.message)
-    }
-
     @Test
     fun parameter_disappears_on_second_iteration_due_to_external_condition() {
         val exception = assertFailsWith<ParameterizeException> {
@@ -160,4 +138,21 @@ class ParameterizeExceptionSpec {
             exception.message
         )
     }
+
+    @Test
+    fun declaring_parameter_after_iteration_completed() {
+        var declareParameter = {}
+
+        parameterize {
+            declareParameter = {
+                val parameter by parameterOf(Unit)
+            }
+        }
+
+        val failure = assertFailsWith<ParameterizeException> {
+            declareParameter()
+        }
+
+        assertEquals("Cannot declare parameter `parameter` after its iteration has completed", failure.message)
+    }
 }
diff --git a/src/commonTest/kotlin/ParameterizeSpec.kt b/src/commonTest/kotlin/ParameterizeSpec.kt
@@ -233,4 +233,21 @@ class ParameterizeSpec {
 
         letterNumber
     }
+
+    @Test
+    fun captured_parameters_should_be_usable_after_the_iteration_completes() {
+        val capturedParameters = mutableListOf<() -> Int>()
+
+        parameterize {
+            val iteration by parameter(0..10)
+
+            capturedParameters += { iteration }
+        }
+
+        testAll(
+            (0..10).map { "iteration $it" to it }
+        ) { iteration ->
+            assertEquals(iteration, capturedParameters[iteration]())
+        }
+    }
 }