From 9f60a1956e08a377d17d4f5dbef1ed202365826e Mon Sep 17 00:00:00 2001 From: rafearnold Date: Sat, 18 Oct 2025 08:52:56 +0100 Subject: [PATCH 1/3] add location property to ValueSource.Invocation allowing more useful help messages upon transform failure. --- clikt/api/clikt.api | 15 ++++-- .../ajalt/clikt/parameters/options/Option.kt | 2 +- .../github/ajalt/clikt/sources/ValueSource.kt | 19 ++++++-- .../ajalt/clikt/sources/ValueSourceTest.kt | 46 +++++++++++++++++++ 4 files changed, 72 insertions(+), 10 deletions(-) create mode 100644 test/src/commonTest/kotlin/com/github/ajalt/clikt/sources/ValueSourceTest.kt diff --git a/clikt/api/clikt.api b/clikt/api/clikt.api index aed99261..cf0d09c3 100644 --- a/clikt/api/clikt.api +++ b/clikt/api/clikt.api @@ -1457,18 +1457,23 @@ public final class com/github/ajalt/clikt/sources/ValueSource$Companion { public final class com/github/ajalt/clikt/sources/ValueSource$Invocation { public static final field Companion Lcom/github/ajalt/clikt/sources/ValueSource$Invocation$Companion; - public fun (Ljava/util/List;)V + public fun (Ljava/util/List;Ljava/lang/String;)V + public synthetic fun (Ljava/util/List;Ljava/lang/String;ILkotlin/jvm/internal/DefaultConstructorMarker;)V public final fun component1 ()Ljava/util/List; - public final fun copy (Ljava/util/List;)Lcom/github/ajalt/clikt/sources/ValueSource$Invocation; - public static synthetic fun copy$default (Lcom/github/ajalt/clikt/sources/ValueSource$Invocation;Ljava/util/List;ILjava/lang/Object;)Lcom/github/ajalt/clikt/sources/ValueSource$Invocation; + public final fun component2 ()Ljava/lang/String; + public final fun copy (Ljava/util/List;Ljava/lang/String;)Lcom/github/ajalt/clikt/sources/ValueSource$Invocation; + public static synthetic fun copy$default (Lcom/github/ajalt/clikt/sources/ValueSource$Invocation;Ljava/util/List;Ljava/lang/String;ILjava/lang/Object;)Lcom/github/ajalt/clikt/sources/ValueSource$Invocation; public fun equals (Ljava/lang/Object;)Z + public final fun getLocation ()Ljava/lang/String; public final fun getValues ()Ljava/util/List; public fun hashCode ()I public fun toString ()Ljava/lang/String; } public final class com/github/ajalt/clikt/sources/ValueSource$Invocation$Companion { - public final fun just (Ljava/lang/Object;)Ljava/util/List; - public final fun value (Ljava/lang/Object;)Lcom/github/ajalt/clikt/sources/ValueSource$Invocation; + public final fun just (Ljava/lang/Object;Ljava/lang/String;)Ljava/util/List; + public static synthetic fun just$default (Lcom/github/ajalt/clikt/sources/ValueSource$Invocation$Companion;Ljava/lang/Object;Ljava/lang/String;ILjava/lang/Object;)Ljava/util/List; + public final fun value (Ljava/lang/Object;Ljava/lang/String;)Lcom/github/ajalt/clikt/sources/ValueSource$Invocation; + public static synthetic fun value$default (Lcom/github/ajalt/clikt/sources/ValueSource$Invocation$Companion;Ljava/lang/Object;Ljava/lang/String;ILjava/lang/Object;)Lcom/github/ajalt/clikt/sources/ValueSource$Invocation; } diff --git a/clikt/src/commonMain/kotlin/com/github/ajalt/clikt/parameters/options/Option.kt b/clikt/src/commonMain/kotlin/com/github/ajalt/clikt/parameters/options/Option.kt index bd341870..9d1c9926 100644 --- a/clikt/src/commonMain/kotlin/com/github/ajalt/clikt/parameters/options/Option.kt +++ b/clikt/src/commonMain/kotlin/com/github/ajalt/clikt/parameters/options/Option.kt @@ -174,7 +174,7 @@ internal fun Option.hasEnvvarOrSourcedValue( private fun Option.readValueSource(context: Context): List? { return context.valueSource?.getValues(context, this) - ?.map { OptionInvocation("", it.values) } + ?.map { OptionInvocation(it.location, it.values) } ?.ifEmpty { null } } diff --git a/clikt/src/commonMain/kotlin/com/github/ajalt/clikt/sources/ValueSource.kt b/clikt/src/commonMain/kotlin/com/github/ajalt/clikt/sources/ValueSource.kt index 42e1dd6a..aab96cf1 100644 --- a/clikt/src/commonMain/kotlin/com/github/ajalt/clikt/sources/ValueSource.kt +++ b/clikt/src/commonMain/kotlin/com/github/ajalt/clikt/sources/ValueSource.kt @@ -1,16 +1,27 @@ package com.github.ajalt.clikt.sources import com.github.ajalt.clikt.core.Context -import com.github.ajalt.clikt.parameters.options.* +import com.github.ajalt.clikt.parameters.options.Option +import com.github.ajalt.clikt.parameters.options.OptionWithValues +import com.github.ajalt.clikt.parameters.options.inferEnvvar +import com.github.ajalt.clikt.parameters.options.longestName +import com.github.ajalt.clikt.parameters.options.splitOptionPrefix +import com.github.ajalt.clikt.sources.ValueSource.Companion.name interface ValueSource { - data class Invocation(val values: List) { + /** + * @property location A pointer to where the invocation's values were retrieved from. Useful for indicating where + * a failure occurred in error help messages. + */ + data class Invocation(val values: List, val location: String = "") { companion object { /** Create a list of a single Invocation with a single value */ - fun just(value: Any?): List = listOf(value(value)) + fun just(value: Any?, location: String = ""): List = + listOf(value(value = value, location = location)) /** Create an Invocation with a single value */ - fun value(value: Any?): Invocation = Invocation(listOf(value.toString())) + fun value(value: Any?, location: String = ""): Invocation = + Invocation(values = listOf(value.toString()), location = location) } } diff --git a/test/src/commonTest/kotlin/com/github/ajalt/clikt/sources/ValueSourceTest.kt b/test/src/commonTest/kotlin/com/github/ajalt/clikt/sources/ValueSourceTest.kt new file mode 100644 index 00000000..43f753a3 --- /dev/null +++ b/test/src/commonTest/kotlin/com/github/ajalt/clikt/sources/ValueSourceTest.kt @@ -0,0 +1,46 @@ +package com.github.ajalt.clikt.sources + +import com.github.ajalt.clikt.core.BadParameterValue +import com.github.ajalt.clikt.core.Context +import com.github.ajalt.clikt.parameters.options.Option +import com.github.ajalt.clikt.parameters.options.option +import com.github.ajalt.clikt.parameters.types.int +import com.github.ajalt.clikt.testing.TestCommand +import com.github.ajalt.clikt.testing.formattedMessage +import com.github.ajalt.clikt.testing.parse +import io.kotest.assertions.throwables.shouldThrow +import io.kotest.matchers.shouldBe +import kotlin.test.Test + +class ValueSourceTest { + @Test + fun `parameter name can be provided to invocation`() { + class C : TestCommand() { + @Suppress("unused") + val theInteger by option("-i").int() + } + + val sourceWithoutParameterName = object : ValueSource { + override fun getValues( + context: Context, + option: Option + ): List = ValueSource.Invocation.just(value = "foo") + } + + val sourceWithParameterName = object : ValueSource { + override fun getValues( + context: Context, + option: Option + ): List = + ValueSource.Invocation.just(value = "foo", location = "value_source_option") + } + + shouldThrow { + C().apply { configureContext { valueSource = sourceWithoutParameterName } }.parse("") + }.formattedMessage shouldBe "invalid value: foo is not a valid integer" + + shouldThrow { + C().apply { configureContext { valueSource = sourceWithParameterName } }.parse("") + }.formattedMessage shouldBe "invalid value for value_source_option: foo is not a valid integer" + } +} \ No newline at end of file From 265307c75f616f65d0bc4d8dee665d0073e8af4e Mon Sep 17 00:00:00 2001 From: rafearnold Date: Sat, 18 Oct 2025 09:40:37 +0100 Subject: [PATCH 2/3] add location to MapValueSource invocations. --- .../ajalt/clikt/sources/MapValueSource.kt | 5 +-- .../ajalt/clikt/sources/MapValueSourceTest.kt | 32 +++++++++++++++++ .../sources/PropertiesValueSourceTest.kt | 35 +++++++++++++++++++ 3 files changed, 70 insertions(+), 2 deletions(-) diff --git a/clikt/src/commonMain/kotlin/com/github/ajalt/clikt/sources/MapValueSource.kt b/clikt/src/commonMain/kotlin/com/github/ajalt/clikt/sources/MapValueSource.kt index c2dba6db..a4b2e309 100644 --- a/clikt/src/commonMain/kotlin/com/github/ajalt/clikt/sources/MapValueSource.kt +++ b/clikt/src/commonMain/kotlin/com/github/ajalt/clikt/sources/MapValueSource.kt @@ -19,7 +19,8 @@ class MapValueSource( private val getKey: (Context, Option) -> String = ValueSource.getKey(joinSubcommands = "."), ) : ValueSource { override fun getValues(context: Context, option: Option): List { - return values[option.valueSourceKey ?: getKey(context, option)] - ?.let { ValueSource.Invocation.just(it) }.orEmpty() + val key = option.valueSourceKey ?: getKey(context, option) + return values[key] + ?.let { ValueSource.Invocation.just(value = it, location = key) }.orEmpty() } } diff --git a/test/src/commonTest/kotlin/com/github/ajalt/clikt/sources/MapValueSourceTest.kt b/test/src/commonTest/kotlin/com/github/ajalt/clikt/sources/MapValueSourceTest.kt index 2176dda2..c1f926ef 100644 --- a/test/src/commonTest/kotlin/com/github/ajalt/clikt/sources/MapValueSourceTest.kt +++ b/test/src/commonTest/kotlin/com/github/ajalt/clikt/sources/MapValueSourceTest.kt @@ -7,9 +7,11 @@ import com.github.ajalt.clikt.core.subcommands import com.github.ajalt.clikt.parameters.options.Option import com.github.ajalt.clikt.parameters.options.flag import com.github.ajalt.clikt.parameters.options.option +import com.github.ajalt.clikt.parameters.types.int import com.github.ajalt.clikt.sources.ValueSource.Invocation import com.github.ajalt.clikt.testing.TestCommand import com.github.ajalt.clikt.testing.defaultLocalization +import com.github.ajalt.clikt.testing.formattedMessage import com.github.ajalt.clikt.testing.parse import io.kotest.assertions.throwables.shouldThrow import io.kotest.data.blocking.forAll @@ -102,4 +104,34 @@ class MapValueSourceTest { }.message shouldBe defaultLocalization.invalidFlagValueInFile("") } + @Test + fun errorMessage() { + class C : TestCommand() { + @Suppress("unused") + val theInteger by option().int() + + @Suppress("unused") + val theFlag by option(valueSourceKey = "that flag").flag() + } + + shouldThrow { + val valueSource = MapValueSource(mapOf("the-integer" to "foo")) + C().apply { configureContext { this.valueSource = valueSource } }.parse("") + }.formattedMessage shouldBe "invalid value for the-integer: foo is not a valid integer" + + shouldThrow { + val valueSource = MapValueSource(mapOf("that flag" to "foo")) + C().apply { configureContext { this.valueSource = valueSource } }.parse("") + }.formattedMessage shouldBe "invalid value for that flag: foo is not a valid boolean" + + shouldThrow { + val valueSource = MapValueSource(mapOf("A_THE_INTEGER" to "foo"), getKey = ValueSource.envvarKey()) + C().apply { + configureContext { + this.valueSource = valueSource + this.autoEnvvarPrefix = "A" + } + }.parse("") + }.formattedMessage shouldBe "invalid value for A_THE_INTEGER: foo is not a valid integer" + } } diff --git a/test/src/jvmTest/kotlin/com/github/ajalt/clikt/sources/PropertiesValueSourceTest.kt b/test/src/jvmTest/kotlin/com/github/ajalt/clikt/sources/PropertiesValueSourceTest.kt index 71a75c20..2a2a03ed 100644 --- a/test/src/jvmTest/kotlin/com/github/ajalt/clikt/sources/PropertiesValueSourceTest.kt +++ b/test/src/jvmTest/kotlin/com/github/ajalt/clikt/sources/PropertiesValueSourceTest.kt @@ -1,5 +1,6 @@ package com.github.ajalt.clikt.sources +import com.github.ajalt.clikt.core.BadParameterValue import com.github.ajalt.clikt.core.InvalidFileFormat import com.github.ajalt.clikt.core.context import com.github.ajalt.clikt.core.subcommands @@ -10,6 +11,7 @@ import com.github.ajalt.clikt.parameters.types.double import com.github.ajalt.clikt.parameters.types.float import com.github.ajalt.clikt.parameters.types.int import com.github.ajalt.clikt.testing.TestCommand +import com.github.ajalt.clikt.testing.formattedMessage import com.github.ajalt.clikt.testing.parse import io.kotest.assertions.throwables.shouldThrow import io.kotest.data.blocking.forAll @@ -19,6 +21,7 @@ import org.junit.Rule import org.junit.Test import org.junit.rules.TemporaryFolder import java.io.File +import java.util.Properties class PropertiesValueSourceTest { @get:Rule @@ -174,4 +177,36 @@ class PropertiesValueSourceTest { C().parse("") } + + @Test + fun errorMessage() { + class C : TestCommand() { + @Suppress("unused") + val theInteger by option().int() + + @Suppress("unused") + val theFlag by option(valueSourceKey = "that flag").flag() + } + + shouldThrow { + val valueSource = PropertiesValueSource.from(Properties().apply { setProperty("the-integer", "foo") }) + C().apply { configureContext { this.valueSource = valueSource } }.parse("") + }.formattedMessage shouldBe "invalid value for the-integer: foo is not a valid integer" + + shouldThrow { + val valueSource = PropertiesValueSource.from(Properties().apply { setProperty("that flag", "foo") }) + C().apply { configureContext { this.valueSource = valueSource } }.parse("") + }.formattedMessage shouldBe "invalid value for that flag: foo is not a valid boolean" + + shouldThrow { + val properties = Properties().apply { setProperty("A_THE_INTEGER", "foo") } + val valueSource = PropertiesValueSource.from(properties, getKey = ValueSource.envvarKey()) + C().apply { + configureContext { + this.valueSource = valueSource + this.autoEnvvarPrefix = "A" + } + }.parse("") + }.formattedMessage shouldBe "invalid value for A_THE_INTEGER: foo is not a valid integer" + } } From bd9bd795fc79496279cc8eaae7ee49f9b5f97a41 Mon Sep 17 00:00:00 2001 From: rafearnold Date: Sat, 18 Oct 2025 10:08:57 +0100 Subject: [PATCH 3/3] add location to JsonValueSource invocations. to demonstrate more complex invocation locations. --- .../clikt/samples/json/JsonValueSource.kt | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/samples/json/src/main/kotlin/com/github/ajalt/clikt/samples/json/JsonValueSource.kt b/samples/json/src/main/kotlin/com/github/ajalt/clikt/samples/json/JsonValueSource.kt index 412be16c..cc06e0df 100644 --- a/samples/json/src/main/kotlin/com/github/ajalt/clikt/samples/json/JsonValueSource.kt +++ b/samples/json/src/main/kotlin/com/github/ajalt/clikt/samples/json/JsonValueSource.kt @@ -5,7 +5,11 @@ import com.github.ajalt.clikt.core.InvalidFileFormat import com.github.ajalt.clikt.parameters.options.Option import com.github.ajalt.clikt.sources.ValueSource import kotlinx.serialization.SerializationException -import kotlinx.serialization.json.* +import kotlinx.serialization.json.Json +import kotlinx.serialization.json.JsonArray +import kotlinx.serialization.json.JsonElement +import kotlinx.serialization.json.JsonObject +import kotlinx.serialization.json.jsonPrimitive import java.io.File /** @@ -13,6 +17,7 @@ import java.io.File */ class JsonValueSource( private val root: JsonObject, + private val referencePrefix: String, ) : ValueSource { override fun getValues(context: Context, option: Option): List { var cursor: JsonElement? = root @@ -25,21 +30,25 @@ class JsonValueSource( if (cursor == null) return emptyList() try { + val jsonReference = referencePrefix + buildJsonPointer(parts) // This implementation interprets a list as multiple invocations, but you could also // implement it as a single invocation with multiple values. if (cursor is JsonArray) return cursor.map { - ValueSource.Invocation.value(it.jsonPrimitive.content) + ValueSource.Invocation.value(value = it.jsonPrimitive.content, location = jsonReference) } - return ValueSource.Invocation.just(cursor.jsonPrimitive.content) + return ValueSource.Invocation.just(value = cursor.jsonPrimitive.content, location = jsonReference) } catch (e: IllegalArgumentException) { // This implementation skips invalid values, but you could handle them differently. return emptyList() } } + private fun buildJsonPointer(parts: List): String = + parts.joinToString(separator = "/", prefix = "/") { it.replace("~", "~0").replace("/", "~1") } + companion object { fun from(file: File, requireValid: Boolean = false): JsonValueSource { - if (!file.isFile) return JsonValueSource(JsonObject(emptyMap())) + if (!file.isFile) return JsonValueSource(JsonObject(emptyMap()), referencePrefix = "") val json = try { Json.parseToJsonElement(file.readText()) as? JsonObject @@ -50,7 +59,7 @@ class JsonValueSource( } JsonObject(emptyMap()) } - return JsonValueSource(json) + return JsonValueSource(json, referencePrefix = file.invariantSeparatorsPath + "#") } fun from(file: String, requireValid: Boolean = false): JsonValueSource {