Support environment variables for options in a mutually exclusive gro…

…up (#385)

CHANGELOG.md

@@ -2,10 +2,11 @@
 
 ## Unreleased
 ### Changed
-- Updated Kotlin to 1.6.20
+- Updated Kotlin to 1.7.20
 
 ### Fixed
 - Support unicode in environment variable values on Native Windows. ([#362](https://github.com/ajalt/clikt/issues/362))
+- Support environment variables for options in a mutually exclusive options group. ([#384](https://github.com/ajalt/clikt/issues/384))
 
 ## 3.5.0
 ### Added

clikt/src/commonMain/kotlin/com/github/ajalt/clikt/parameters/groups/CoOccurringOptionGroup.kt

@@ -4,10 +4,7 @@ import com.github.ajalt.clikt.core.CliktCommand
 import com.github.ajalt.clikt.core.Context
 import com.github.ajalt.clikt.output.HelpFormatter
 import com.github.ajalt.clikt.parameters.internal.NullableLateinit
-import com.github.ajalt.clikt.parameters.options.FinalValue
-import com.github.ajalt.clikt.parameters.options.Option
-import com.github.ajalt.clikt.parameters.options.OptionWithValues
-import com.github.ajalt.clikt.parameters.options.getFinalValue
+import com.github.ajalt.clikt.parameters.options.*
 import com.github.ajalt.clikt.parsers.OptionParser
 import kotlin.properties.ReadOnlyProperty
 import kotlin.reflect.KProperty
@@ -37,8 +34,7 @@ class CoOccurringOptionGroup<GroupT : OptionGroup, OutT> internal constructor(
 
     override fun finalize(context: Context, invocationsByOption: Map<Option, List<OptionParser.Invocation>>) {
         occurred = invocationsByOption.isNotEmpty() || group.options.any {
-            // Also trigger the group if any of the options have values from envvars or value sources
-            it is OptionWithValues<*, *, *> && it.getFinalValue(context, emptyList(), it.envvar) !is FinalValue.Parsed
+            it.hasEnvvarOrSourcedValue(context, invocationsByOption[it] ?: emptyList())
         }
         if (occurred) group.finalize(context, invocationsByOption)
         value = transform(occurred, group, context)

clikt/src/commonMain/kotlin/com/github/ajalt/clikt/parameters/groups/MutuallyExclusiveOption.kt

@@ -41,9 +41,14 @@ class MutuallyExclusiveOptions<OptT : Any, OutT> internal constructor(
 
     override fun getValue(thisRef: CliktCommand, property: KProperty<*>): OutT = value
 
-    override fun finalize(context: Context, invocationsByOption: Map<Option, List<OptionParser.Invocation>>) {
+    override fun finalize(
+        context: Context,
+        invocationsByOption: Map<Option, List<OptionParser.Invocation>>,
+    ) {
         finalizeOptions(context, options, invocationsByOption)
-        val values = options.filter { it in invocationsByOption }.mapNotNull { it.value }
+        val values = options.filter {
+            it in invocationsByOption || it.hasEnvvarOrSourcedValue(context, emptyList())
+        }.mapNotNull { it.value }
         value = MutuallyExclusiveOptionTransformContext(context).transformAll(values)
     }
 
@@ -108,7 +113,11 @@ fun <T : Any> ParameterHolder.mutuallyExclusiveOptions(
     name: String? = null,
     help: String? = null,
 ): MutuallyExclusiveOptions<T, T?> {
-    return MutuallyExclusiveOptions(listOf(option1, option2) + options, name, help) { it.lastOrNull() }
+    return MutuallyExclusiveOptions(
+        listOf(option1, option2) + options,
+        name,
+        help
+    ) { it.lastOrNull() }
 }
 
 /**

clikt/src/commonMain/kotlin/com/github/ajalt/clikt/parameters/options/Option.kt

@@ -48,7 +48,8 @@ interface Option {
     /** Information about this option for the help output. */
     fun parameterHelp(context: Context): HelpFormatter.ParameterHelp.Option? = when {
         hidden -> null
-        else -> HelpFormatter.ParameterHelp.Option(names,
+        else -> HelpFormatter.ParameterHelp.Option(
+            names,
             secondaryNames,
             metavar(context),
             optionHelp,
@@ -86,7 +87,10 @@ interface OptionDelegate<T> : GroupableOption, ReadOnlyProperty<ParameterHolder,
     val value: T
 
     /** Implementations must call [ParameterHolder.registerOption] */
-    operator fun provideDelegate(thisRef: ParameterHolder, prop: KProperty<*>): ReadOnlyProperty<ParameterHolder, T>
+    operator fun provideDelegate(
+        thisRef: ParameterHolder,
+        prop: KProperty<*>,
+    ): ReadOnlyProperty<ParameterHolder, T>
 
     override fun getValue(thisRef: ParameterHolder, property: KProperty<*>): T = value
 }
@@ -157,12 +161,30 @@ internal fun Option.getFinalValue(
         context.readEnvvarBeforeValueSource -> {
             readEnvVar(context, envvar) ?: readValueSource(context)
         }
+
         else -> {
             readValueSource(context) ?: readEnvVar(context, envvar)
         }
     } ?: FinalValue.Parsed(emptyList())
 }
 
+// This is a pretty ugly hack: option groups need to enforce their contraints, including on options
+// from envvars/value sources, but not including default values. Unfortunately, we don't know
+// whether an option's value is from a default or envvar. So we do some ugly casts and read the
+// final value again to check for values from other sources.
+internal fun Option.hasEnvvarOrSourcedValue(
+    context: Context,
+    invocations: List<OptionParser.Invocation>,
+): Boolean {
+    val envvar = when (this) {
+        is OptionWithValues<*, *, *> -> envvar
+        is FlagOption<*> -> envvar
+        else -> null
+    }
+    val final = this.getFinalValue(context, invocations, envvar)
+    return final !is FinalValue.Parsed
+}
+
 private fun Option.readValueSource(context: Context): FinalValue? {
     return context.valueSource?.getValues(context, this)?.ifEmpty { null }
         ?.let { FinalValue.Sourced(it) }

clikt/src/commonTest/kotlin/com/github/ajalt/clikt/parameters/EnvvarOptionsTest.kt

@@ -1,15 +1,19 @@
 package com.github.ajalt.clikt.parameters
 
 import com.github.ajalt.clikt.core.CliktCommand
+import com.github.ajalt.clikt.core.MutuallyExclusiveGroupException
 import com.github.ajalt.clikt.core.context
 import com.github.ajalt.clikt.core.subcommands
 import com.github.ajalt.clikt.parameters.groups.OptionGroup
 import com.github.ajalt.clikt.parameters.groups.cooccurring
+import com.github.ajalt.clikt.parameters.groups.mutuallyExclusiveOptions
+import com.github.ajalt.clikt.parameters.groups.single
 import com.github.ajalt.clikt.parameters.options.*
 import com.github.ajalt.clikt.parameters.types.int
 import com.github.ajalt.clikt.testing.TestCommand
 import com.github.ajalt.clikt.testing.TestSource
 import com.github.ajalt.clikt.testing.parse
+import io.kotest.assertions.throwables.shouldThrow
 import io.kotest.data.blocking.forAll
 import io.kotest.data.row
 import io.kotest.matchers.shouldBe
@@ -180,7 +184,7 @@ class EnvvarOptionsTest {
     }
 
     @Test
-    @JsName("option_group_envvar")
+    @JsName("cooccurring_option_group_envvar")
     fun `cooccurring option group envvar`() = forAll(
         row("", "xx", "yy"),
         row("--x=z", "z", "yy"),
@@ -205,4 +209,24 @@ class EnvvarOptionsTest {
 
         C().withEnv().parse(argv)
     }
+
+    @Test
+    @JsName("mutually_exclusive_option_group_envvar")
+    fun `mutually exclusive option group envvar`() {
+        class C: TestCommand() {
+            val opt by mutuallyExclusiveOptions(
+                option("--foo", envvar = "FOO"),
+                option("--bar", envvar = "BAR"),
+            ).single()
+        }
+        C().withEnv().parse("--bar=x").opt shouldBe "x"
+
+        env["FOO"] = "y"
+        C().withEnv().parse("").opt shouldBe "y"
+
+        env["BAR"] = "z"
+        shouldThrow<MutuallyExclusiveGroupException> {
+            C().withEnv().parse("")
+        }
+    }
 }

clikt/src/commonTest/kotlin/com/github/ajalt/clikt/parameters/groups/OptionGroupsTest.kt

@@ -211,8 +211,8 @@ class OptionGroupsTest {
     @Test
     @JsName("mutually_exclusive_group_default_flag_single")
     fun `mutually exclusive group flag single`() = forAll(
-//        row("", false),
-//        row("--x", true),
+        row("", null),
+        row("--x", true),
         row("--y", true)
     ) { argv, eg ->
         class C : TestCommand() {