From 1d8cc7fa9e9b3ca64dce0a5dab56c6d51c7fe218 Mon Sep 17 00:00:00 2001 From: Ben Woodworth Date: Sun, 12 Nov 2023 16:30:04 -0500 Subject: [PATCH] Immediately compute parameter arguments when declared This will make it possible to avoid re-calculating independent parameters, enable captured parameters be used only after an iteration completes, and iterate arguments closer to their declaration. See issue #11 for context. The code was minimally modified to change the behavior, and tests were changed to account for arguments iteration in declaration order instead of usage order (since that's now the order arguments are calculated and depend on each other). --- src/commonMain/kotlin/ParameterState.kt | 33 ++-- src/commonMain/kotlin/Parameterize.kt | 8 + src/commonMain/kotlin/ParameterizeState.kt | 38 ++-- src/commonTest/kotlin/ParameterStateSpec.kt | 178 +++++++++++------- .../ParameterizeConfigurationOnFailureSpec.kt | 18 ++ .../kotlin/ParameterizeScopeSpec.kt | 23 +-- src/commonTest/kotlin/ParameterizeSpec.kt | 94 +++++---- 7 files changed, 230 insertions(+), 162 deletions(-) diff --git a/src/commonMain/kotlin/ParameterState.kt b/src/commonMain/kotlin/ParameterState.kt index 0ae0a65..db82f7c 100644 --- a/src/commonMain/kotlin/ParameterState.kt +++ b/src/commonMain/kotlin/ParameterState.kt @@ -77,15 +77,14 @@ internal class ParameterState<@Suppress("unused") out T> internal constructor() /** - * Returns a string representation of the current argument, or a "not initialized" message. - * - * Useful while debugging, e.g. inline hints that show property values: - * ``` - * val letter by parameter { 'a'..'z' } //letter$delegate: b - * ``` + * Returns a string representation of the current argument, or a "not declared" message. */ override fun toString(): String = - argument.toString() + if (property == null) { + "Parameter not declared yet." + } else { + argument.toString() + } /** * Set up the delegate for a parameter [property] with the given [arguments]. @@ -95,11 +94,13 @@ internal class ParameterState<@Suppress("unused") out T> internal constructor() * The new [arguments] will be ignored in favor of re-using the existing arguments, under the assumption that they're equal. * * @throws ParameterizeException if already declared for a different [property]. + * @throws ParameterizeContinue if [arguments] is empty. */ internal fun declare(property: KProperty, arguments: Iterable) { val declaredProperty = this.property if (declaredProperty == null) { + initialize(arguments) // Before any state gets changed, in case arguments is empty this.property = property this.arguments = arguments } else if (!property.equalsProperty(declaredProperty)) { @@ -109,6 +110,8 @@ internal class ParameterState<@Suppress("unused") out T> internal constructor() /** * Initialize and return the argument. + * + * @throws ParameterizeContinue if [arguments] is empty. */ private fun initialize(arguments: Iterable): T { val iterator = arguments.iterator() @@ -184,14 +187,18 @@ internal class ParameterState<@Suppress("unused") out T> internal constructor() } /** - * Returns the property and argument if initialized, or `null` otherwise. + * Returns the property and argument. + * + * @throws IllegalStateException if this parameter is not declared. */ - internal fun getFailureArgumentOrNull(): ParameterizeFailure.Argument<*>? { - val argument = argument - if (argument === Uninitialized) return null - + internal fun getFailureArgument(): ParameterizeFailure.Argument<*> { val property = checkNotNull(property) { - "Parameter argument is initialized, but ${::property.name} is null" + "Cannot get failure argument before parameter has been declared" + } + + val argument = argument + check (argument !== Uninitialized) { + "Parameter delegate is declared with ${property.name}, but ${::argument.name} is uninitialized" } return ParameterizeFailure.Argument(property, argument) diff --git a/src/commonMain/kotlin/Parameterize.kt b/src/commonMain/kotlin/Parameterize.kt index 1053f36..73c4a47 100644 --- a/src/commonMain/kotlin/Parameterize.kt +++ b/src/commonMain/kotlin/Parameterize.kt @@ -184,6 +184,14 @@ public class ParameterizeScope internal constructor( public class ParameterDelegate internal constructor( internal val parameterState: ParameterState ) { + /** + * Returns a string representation of the current argument. + * + * Useful while debugging, e.g. inline hints that show property values: + * ``` + * val letter by parameter { 'a'..'z' } //letter$delegate: b + * ``` + */ override fun toString(): String = parameterState.toString() } diff --git a/src/commonMain/kotlin/ParameterizeState.kt b/src/commonMain/kotlin/ParameterizeState.kt index a778a80..94573b5 100644 --- a/src/commonMain/kotlin/ParameterizeState.kt +++ b/src/commonMain/kotlin/ParameterizeState.kt @@ -45,7 +45,7 @@ internal class ParameterizeState { if (it != null) throw ParameterizeException("Nesting parameters is not currently supported: `${property.name}` was declared within `${it.name}`'s arguments") } - val parameterIndex = parameterCount++ + val parameterIndex = parameterCount val parameter = if (parameterIndex in parameters.indices) { parameters[parameterIndex] @@ -54,7 +54,10 @@ internal class ParameterizeState { .also { parameters += it } } - parameter.parameterState.declare(property, arguments) + property.trackNestedUsage { + parameter.parameterState.declare(property, arguments) + parameterCount++ // After declaring, since the parameter shouldn't count if declare throws + } return parameter } @@ -73,10 +76,7 @@ internal class ParameterizeState { fun getParameterArgument(parameter: ParameterDelegate<*>, property: KProperty): T { val isFirstUse = !parameter.parameterState.hasBeenUsed - return property - .trackNestedUsage { - parameter.parameterState.getArgument(property) - } + return parameter.parameterState.getArgument(property) .also { if (isFirstUse) trackUsedParameter(parameter) } @@ -91,21 +91,20 @@ internal class ParameterizeState { } /** - * Iterate the last parameter (by the order they're first used) that has a - * next argument, and reset all parameters that were first used after it - * (since they may depend on the now changed value, and may be computed - * differently). + * Iterate the last parameter that has a next argument (in order of when their arguments were calculated), and reset + * all parameters that were first used after it (since they may depend on the now changed value, and may be computed + * differently now that a previous argument changed). * * Returns `true` if the arguments are at a new permutation. */ private fun nextArgumentPermutationOrFalse(): Boolean { var iterated = false - val usedParameterIterator = parametersUsed - .listIterator(parametersUsed.lastIndex + 1) + val declaredParameterIterator = parameters + .listIterator(parameterCount) - while (usedParameterIterator.hasPrevious()) { - val parameter = usedParameterIterator.previous() + while (declaredParameterIterator.hasPrevious()) { + val parameter = declaredParameterIterator.previous() if (!parameter.parameterState.isLastArgument) { parameter.parameterState.nextArgument() @@ -113,18 +112,9 @@ internal class ParameterizeState { break } - usedParameterIterator.remove() parameter.parameterState.reset() } - for (i in parameterCountAfterAllUsed..> = parameters.take(parameterCount) .filter { it.parameterState.hasBeenUsed } - .mapNotNull { it.parameterState.getFailureArgumentOrNull() } + .map { it.parameterState.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 25cefb3..10afa40 100644 --- a/src/commonTest/kotlin/ParameterStateSpec.kt +++ b/src/commonTest/kotlin/ParameterStateSpec.kt @@ -3,6 +3,10 @@ package com.benwoodworth.parameterize import kotlin.test.* class ParameterStateSpec { + private val getArgumentBeforeDeclaredMessage = "Cannot get argument before parameter has been declared" + private val getFailureArgumentBeforeDeclaredMessage = + "Cannot get failure argument before parameter has been declared" + private val property: String get() = error("${::property.name} is not meant to be used") private val differentProperty: String get() = error("${::differentProperty.name} is not meant to be used") @@ -14,113 +18,155 @@ class ParameterStateSpec { } - @Test - fun has_been_used_should_initially_be_false() { - assertFalse(parameter.hasBeenUsed) - } - - @Test - fun declare_should_not_immediately_get_an_argument() { - val arguments = Iterable { - fail("Arguments should not be iterated") + private fun assertUndeclared(parameter: ParameterState<*>) { + val failure = assertFailsWith { + parameter.getArgument(::property) } - parameter.declare(::property, arguments) + assertEquals(getArgumentBeforeDeclaredMessage, failure.message, "message") } @Test - fun getting_argument_before_declared_should_throw_IllegalStateException() { - val failure = assertFailsWith { - parameter.getArgument(::property) - } + fun string_representation_when_not_declared_should_match_message_from_lazy() { + val messageFromLazy = lazy { error("unused") }.toString() + + val replacements = listOf( + "Lazy value" to "Parameter", + "initialized" to "declared" + ) + + val expected = replacements + .onEach { (old) -> + check(old in messageFromLazy) { "'$old' in '$messageFromLazy'"} + } + .fold(messageFromLazy) { result, (old, new) -> + result.replace(old, new) + } - assertEquals("Cannot get argument before parameter has been declared", failure.message) + assertEquals(expected, parameter.toString()) } @Test - fun getting_argument_with_the_wrong_property_should_throw_ParameterizeException() { - parameter.declare(::property, emptyList()) + fun string_representation_when_initialized_should_equal_that_of_the_current_argument() { + val argument = "argument" - val exception = assertFailsWith { - parameter.getArgument(::differentProperty) - } + parameter.declare(::property, listOf(argument)) - assertEquals( - "Cannot use parameter delegate with `differentProperty`. Already declared for `property`.", - exception.message - ) + assertSame(argument, parameter.toString()) } @Test - fun getting_argument_with_no_arguments_should_throw_ParameterizeContinue() { - parameter.declare(::property, emptyList()) + fun has_been_used_should_initially_be_false() { + assertFalse(parameter.hasBeenUsed) + } + @Test + fun declaring_with_no_arguments_should_throw_ParameterizeContinue() { assertFailsWith { - parameter.getArgument(::property) + parameter.declare(::property, emptyList()) } } @Test - fun getting_argument_with_no_arguments_should_not_change_has_been_used() { - parameter.declare(::property, emptyList()) - val expected = parameter.hasBeenUsed - + fun declaring_with_no_arguments_should_leave_parameter_undeclared() { runCatching { - parameter.getArgument(::property) + parameter.declare(::property, emptyList()) } - assertEquals(expected, parameter.hasBeenUsed) + assertUndeclared(parameter) } @Test - fun getting_argument_should_initially_return_the_first_argument() { - parameter.declare(::property, listOf("first", "second")) + fun declare_should_immediately_get_the_first_argument() { + var gotFirstArgument = false - assertEquals("first", parameter.getArgument(::property)) + val arguments = Iterable { + gotFirstArgument = true + listOf(Unit).iterator() + } + + parameter.declare(::property, arguments) + assertTrue(gotFirstArgument, "gotFirstArgument") } @Test - fun getting_argument_should_set_has_been_used_to_true() { - parameter.declare(::property, listOf("first", "second")) - parameter.getArgument(::property) + fun declare_should_not_immediately_get_the_second_argument() { + class AssertingIterator : Iterator { + var nextArgument = 1 - assertTrue(parameter.hasBeenUsed) + override fun hasNext(): Boolean = + nextArgument <= 2 + + override fun next(): String { + assertNotEquals(2, nextArgument, "should not get argument 2") + + return "argument $nextArgument" + .also { nextArgument++ } + } + } + + parameter.declare(::property, Iterable(::AssertingIterator)) } @Test - fun getting_argument_with_one_argument_should_set_is_last_argument_to_true() { + fun declare_with_one_argument_should_set_is_last_argument_to_true() { parameter.declare(::property, listOf("first")) - parameter.getArgument(::property) assertTrue(parameter.isLastArgument) } @Test - fun getting_argument_with_more_than_one_argument_should_set_is_last_argument_to_false() { + fun declare_with_more_than_one_argument_should_set_is_last_argument_to_false() { parameter.declare(::property, listOf("first", "second")) - parameter.getArgument(::property) assertFalse(parameter.isLastArgument) } @Test - fun next_before_declare_should_throw_IllegalStateException() { + fun getting_argument_before_declared_should_throw_IllegalStateException() { val failure = assertFailsWith { - parameter.nextArgument() + parameter.getArgument(::property) } - assertEquals("Cannot iterate arguments before parameter has been declared", failure.message) + assertEquals(getArgumentBeforeDeclaredMessage, failure.message, "message") } @Test - fun next_before_initialized_should_throw_IllegalStateException() { - parameter.declare(::property, emptyList()) + fun getting_argument_with_the_wrong_property_should_throw_ParameterizeException() { + parameter.declare(::property, listOf(Unit)) + + val exception = assertFailsWith { + parameter.getArgument(::differentProperty) + } + assertEquals( + "Cannot use parameter delegate with `differentProperty`. Already declared for `property`.", + exception.message + ) + } + + @Test + fun getting_argument_should_initially_return_the_first_argument() { + parameter.declare(::property, listOf("first", "second")) + + assertEquals("first", parameter.getArgument(::property)) + } + + @Test + fun getting_argument_should_set_has_been_used_to_true() { + parameter.declare(::property, listOf("first", "second")) + parameter.getArgument(::property) + + assertTrue(parameter.hasBeenUsed) + } + + @Test + fun next_before_declare_should_throw_IllegalStateException() { val failure = assertFailsWith { parameter.nextArgument() } - assertEquals("Cannot iterate arguments before parameter argument has been initialized", failure.message) + assertEquals("Cannot iterate arguments before parameter has been declared", failure.message) } @Test @@ -186,6 +232,8 @@ class ParameterStateSpec { fail("Re-declaring should keep the old arguments") } parameter.declare(::property, newArguments) + + assertEquals("a", parameter.getArgument(::property)) } @Test @@ -200,10 +248,10 @@ class ParameterStateSpec { @Test fun redeclare_with_different_parameter_should_throw_ParameterizeException() { - parameter.declare(::property, emptyList()) + parameter.declare(::property, listOf(Unit)) assertFailsWith { - parameter.declare(::differentProperty, emptyList()) + parameter.declare(::differentProperty, listOf(Unit)) } } @@ -229,25 +277,20 @@ class ParameterStateSpec { } @Test - fun is_last_argument_before_initialized_should_throw() { - val failure1 = assertFailsWith { - parameter.isLastArgument - } - assertEquals("Argument has not been initialized", failure1.message) - - parameter.declare(::property, listOf("a")) - val failure2 = assertFailsWith { + fun is_last_argument_before_declared_should_throw() { + val failure = assertFailsWith { parameter.isLastArgument } - assertEquals("Argument has not been initialized", failure2.message) + assertEquals("Argument has not been initialized", failure.message) } @Test - fun get_failure_argument_when_not_initialized_should_be_null() { - assertNull(parameter.getFailureArgumentOrNull()) + fun get_failure_argument_when_not_declared_should_throw_IllegalStateException() { + val failure = assertFailsWith { + parameter.getFailureArgument() + } - parameter.declare(::property, emptyList()) - assertNull(parameter.getFailureArgumentOrNull()) + assertEquals(getFailureArgumentBeforeDeclaredMessage, failure.message, "message") } @Test @@ -256,10 +299,7 @@ class ParameterStateSpec { parameter.declare(::property, listOf(expectedArgument)) parameter.getArgument(::property) - val propertyArgument = parameter.getFailureArgumentOrNull() - assertNotNull(propertyArgument) - - val (property, argument) = propertyArgument + val (property, argument) = parameter.getFailureArgument() assertTrue(property.equalsProperty(::property)) assertSame(expectedArgument, argument) } diff --git a/src/commonTest/kotlin/ParameterizeConfigurationOnFailureSpec.kt b/src/commonTest/kotlin/ParameterizeConfigurationOnFailureSpec.kt index 0409337..fa4a046 100644 --- a/src/commonTest/kotlin/ParameterizeConfigurationOnFailureSpec.kt +++ b/src/commonTest/kotlin/ParameterizeConfigurationOnFailureSpec.kt @@ -156,4 +156,22 @@ class ParameterizeConfigurationOnFailureSpec : ParameterizeContext { fail() } } + + @Test + fun failure_arguments_should_only_include_used_parameters() = parameterize( + onFailure = { + val actualArguments = arguments.map { it.parameter.name } + assertEquals(listOf("used1", "used2"), actualArguments) + } + ) { + val used1 by parameterOf(Unit) + val unused1 by parameterOf(Unit) + val used2 by parameterOf(Unit) + val unused2 by parameterOf(Unit) + + useParameter(used1) + useParameter(used2) + + fail() + } } diff --git a/src/commonTest/kotlin/ParameterizeScopeSpec.kt b/src/commonTest/kotlin/ParameterizeScopeSpec.kt index a5c8aa4..574e8da 100644 --- a/src/commonTest/kotlin/ParameterizeScopeSpec.kt +++ b/src/commonTest/kotlin/ParameterizeScopeSpec.kt @@ -1,5 +1,3 @@ -@file:Suppress("IMPLICIT_NOTHING_TYPE_ARGUMENT_IN_RETURN_POSITION") - package com.benwoodworth.parameterize import com.benwoodworth.parameterize.ParameterizeScope.ParameterDelegate @@ -12,9 +10,9 @@ class ParameterizeScopeSpec { @Test fun string_representation_should_show_used_parameter_arguments_in_declaration_order() = parameterize { val a by parameterOf(1) - val unused1 by parameterOf() + val unused1 by parameterOf(Unit) val b by parameterOf(2) - val unused2 by parameterOf() + val unused2 by parameterOf(Unit) val c by parameterOf(3) // used in a different order @@ -25,23 +23,6 @@ class ParameterizeScopeSpec { assertEquals("${ParameterizeScope::class.simpleName}(a = $a, b = $b, c = $c)", this.toString()) } - @Test - fun parameter_delegate_string_representation_when_not_initialized_should_match_message_from_lazy() = parameterize { - lateinit var delegate: ParameterDelegate - - val parameter by PropertyDelegateProvider { thisRef: Nothing?, property -> - parameterOf() - .provideDelegate(thisRef, property) - .also { delegate = it } // intercept delegate - } - - val messageFromLazy = lazy { "unused" } - .toString() - .replace("Lazy value", "Parameter argument") - - assertEquals(messageFromLazy, delegate.toString()) - } - @Test fun parameter_delegate_string_representation_when_initialized_should_equal_that_of_the_current_argument() = parameterize { lateinit var delegate: ParameterDelegate diff --git a/src/commonTest/kotlin/ParameterizeSpec.kt b/src/commonTest/kotlin/ParameterizeSpec.kt index 53b3991..ad53124 100644 --- a/src/commonTest/kotlin/ParameterizeSpec.kt +++ b/src/commonTest/kotlin/ParameterizeSpec.kt @@ -4,7 +4,8 @@ package com.benwoodworth.parameterize import kotlin.test.Test import kotlin.test.assertEquals -import kotlin.test.assertFalse +import kotlin.test.assertTrue +import kotlin.test.fail class ParameterizeSpec { /** @@ -40,26 +41,56 @@ class ParameterizeSpec { } @Test - fun parameter_with_lazy_arguments_should_create_parameter_correctly() = parameterize { - val lazyParameter = parameter { 'a'..'z' } + fun parameter_arguments_iterator_should_be_computed_when_declared() = parameterize { + var computed = false - assertEquals(('a'..'z').toList(), lazyParameter.arguments.toList()) + val parameter by parameter(Iterable { + computed = true + listOf(Unit).iterator() + }) + + assertTrue(computed, "computed") } @Test - fun parameter_with_lazy_arguments_should_not_be_evaluated_before_used() = parameterize { - var evaluated = false + fun second_parameter_argument_should_not_be_computed_until_the_next_iteration() { + var finishedFirstIteration = false + + class AssertingIterator : Iterator { + var nextArgument = 1 + + override fun hasNext(): Boolean = + nextArgument <= 2 + + override fun next() { + if (nextArgument == 2) { + assertTrue(finishedFirstIteration, "finished first iteration before getting second argument") + } + nextArgument++ + } + } - parameter { - evaluated = true - emptyList() + parameterize { + val parameter by parameter(Iterable(::AssertingIterator)) + + finishedFirstIteration = true } + } + + @Test + fun parameter_with_lazy_arguments_should_create_parameter_correctly() = parameterize { + val lazyParameter = parameter { 'a'..'z' } - assertFalse(evaluated) + assertEquals(('a'..'z').toList(), lazyParameter.arguments.toList()) + } + + @Test + fun parameter_with_lazy_arguments_should_not_be_computed_before_declaring() = parameterize { + /*val undeclared by*/ parameter { fail("computed") } } @Test - fun parameter_with_lazy_arguments_should_only_be_evaluated_once() = parameterize { + fun parameter_with_lazy_arguments_should_only_be_computed_once() = parameterize { var evaluationCount = 0 val lazyParameter = parameter { @@ -149,8 +180,8 @@ class ParameterizeSpec { } @Test - fun unused_parameter_with_no_arguments_should_not_finish_iteration_early() = testParameterize( - listOf("finished") + fun unused_parameter_with_no_arguments_should_finish_iteration_early() = testParameterize( + listOf(null) ) { val unused by parameterOf() @@ -158,29 +189,22 @@ class ParameterizeSpec { } @Test - fun parameter_that_depends_on_a_later_one() = testParameterize( - listOf("a1", "a2", "a3", "b1", "b2", "b3") + fun parameters_that_are_used_out_of_order_should_iterate_in_declaration_order() = testParameterize( + listOf("a1", "a2", "b1", "b2") ) { - lateinit var laterValue: String - - val earlier by parameter { - listOf("${laterValue}1", "${laterValue}2", "${laterValue}3") - } + // Because they should iterate in the order their arguments are calculated, and that happens at declaration. - val later by parameterOf("a", "b") - laterValue = later + // Parameters that (potentially) depend on another parameter should iterate first, since if it were the other + // way around, and the depended on parameter iterates first, the dependent parameter's argument would be based + // on a now changed value and wouldn't be valid. - earlier - } + val first by parameterOf("a", "b") + val second by parameterOf(1, 2) - @Test - fun parameters_that_are_used_out_of_order() = testParameterize( - listOf("a1", "a2", "b1", "b2") - ) { - val first by parameterOf(1, 2) - val second by parameterOf("a", "b") + useParameter(second) + useParameter(first) - "$second$first" + "$first$second" } @Test @@ -189,16 +213,16 @@ class ParameterizeSpec { ) { val firstUsedParameter by parameterOf("a", "b") if (firstUsedParameter == "a") { - val unused1A by parameterOf() + val unused1A by parameterOf(Unit) } else { - val unused1B by parameterOf() + val unused1B by parameterOf(Unit) } val lastUsedParameter by parameterOf("1", "2") if (lastUsedParameter == "1") { - val unused2A by parameterOf() + val unused2A by parameterOf(Unit) } else { - val unused2B by parameterOf() + val unused2B by parameterOf(Unit) } "$firstUsedParameter$lastUsedParameter"