From 6dc6ba59005685bb9c0e8b31b9bb570d880851c6 Mon Sep 17 00:00:00 2001 From: Ben Woodworth Date: Thu, 16 Nov 2023 14:24:48 -0500 Subject: [PATCH] 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. --- src/commonMain/kotlin/ParameterState.kt | 6 ++- src/commonMain/kotlin/Parameterize.kt | 31 ++++++++++----- src/commonMain/kotlin/ParameterizeState.kt | 24 +++++++----- src/commonTest/kotlin/ParameterStateSpec.kt | 6 +-- .../ParameterizeConfigurationOnFailureSpec.kt | 25 ++++++++++++ .../kotlin/ParameterizeExceptionSpec.kt | 39 ++++++++----------- src/commonTest/kotlin/ParameterizeSpec.kt | 17 ++++++++ 7 files changed, 102 insertions(+), 46 deletions(-) diff --git a/src/commonMain/kotlin/ParameterState.kt b/src/commonMain/kotlin/ParameterState.kt index bd9e912..93cd341 100644 --- 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 index 0e72752..394fd4c 100644 --- a/src/commonMain/kotlin/Parameterize.kt +++ b/src/commonMain/kotlin/Parameterize.kt @@ -84,9 +84,10 @@ public fun ParameterizeContext.parameterize( } val state = ParameterizeState() - val scope = ParameterizeScope(state) while (state.startNextIteration()) { + val scope = ParameterizeScope(state) + try { scope.block() } catch (_: ParameterizeContinue) { @@ -94,6 +95,8 @@ public fun ParameterizeContext.parameterize( 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 Parameter.provideDelegate(thisRef: Any?, property: KProperty<*>): ParameterDelegate = + public operator fun Parameter.provideDelegate(thisRef: Any?, property: KProperty<*>): ParameterDelegate { + if (iterationCompleted) { + throw ParameterizeException("Cannot declare parameter `${property.name}` after its iteration has completed") + } + @Suppress("UNCHECKED_CAST") - state.declareParameter(property as KProperty, arguments) + return state.declareParameter(this@ParameterizeScope, property as KProperty, arguments) + } /** @suppress */ - public operator fun ParameterDelegate.getValue(thisRef: Any?, property: KProperty<*>): T = - @Suppress("UNCHECKED_CAST") - parameterState.getArgument(property as KProperty) + public operator fun ParameterDelegate.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 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 index 1b42b66..cbd9a31 100644 --- 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>() + private val parameters = ArrayList() private var parameterBeingUsed: KProperty<*>? = null private var parameterCount = 0 @@ -37,7 +37,11 @@ internal class ParameterizeState { return shouldContinue } - fun declareParameter(property: KProperty, arguments: Iterable): ParameterDelegate { + fun declareParameter( + scope: ParameterizeScope, + property: KProperty, + arguments: Iterable + ): ParameterDelegate { 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(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 KProperty.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> = 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 index c354cde..2383d77 100644 --- 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 index 16f9eb5..aabfa75 100644 --- 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 index c178a24..0fb4189 100644 --- 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 { - parameterize { - lateinit var interceptedDelegateFromA: ParameterDelegate - - 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 { @@ -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 { + 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 index aefb6d2..fee39c6 100644 --- 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]()) + } + } }