Dual-option value arbitrage: first vs. last vs. most explicit policy

CHANGES.rst

@@ -51,6 +51,14 @@ Unreleased
     commands. :issue:`3107` :pr:`3228`
 -   Add ``click.get_pager_file`` for file-like access to an output
     pager. :pr:`1572`
+-   Fix feature switch groups (several ``flag_value`` options sharing one
+    parameter name) silently dropping an explicit ``default`` when a sibling
+    option without an explicit default was declared first. Arbitration is now
+    source-aware: a more explicit :class:`ParameterSource` always wins, and
+    within ``ParameterSource.DEFAULT``, an option that received an explicit
+    ``default=`` keyword wins over a sibling whose default was auto-derived.
+    The 8.3.x first-wins fallback for remaining ties was reverted to the
+    pre-8.3.x last-wins fallback. :issue:`3403` :pr:`3404`
 
 Version 8.3.3
 -------------

docs/options.md

@@ -469,15 +469,131 @@ literally.
 
 #### Feature switch groups (multiple flags sharing one variable)
 
-When multiple `flag_value` options target the same parameter
-name, `default=True` on one of them marks it as the default
-choice.
+Several `flag_value` options can target the same parameter name to form a
+feature switch group. The user picks one flag on the command line, and the
+function receives the corresponding `flag_value`. When the user picks none,
+Click falls back to whichever option claims the slot under the arbitration
+rules described below.
+
+##### Non-boolean groups
+
+For non-boolean `flag_value` (strings, enum members, classes, ...), place
+`default=True` on the option that should win when no flag is passed. The
+substitution rule above resolves it to that option's `flag_value`. Any other
+explicit `default` is passed through literally.
 
 | Definition                                             | Not passed | `--upper` | `--lower` |
 |--------------------------------------------------------|------------|-----------|-----------|
 | `--upper` with `flag_value='upper'`, `default=True`    | `"upper"`  | `"upper"` | `"lower"` |
 | `--upper` with `flag_value='upper'`, `default='upper'` | `"upper"`  | `"upper"` | `"lower"` |
-| Both without `default`                                 | `None`     | `"upper"` | `"lower"` |
+| `--upper` with `flag_value='upper'`, `default=None`    | `None`     | `"upper"` | `"lower"` |
+| Neither option carries a `default`                     | `None`     | `"upper"` | `"lower"` |
+
+The third row is the three-state pattern: the function receives `None` when no
+flag is passed, distinguishable from either explicit choice.
+
+##### Boolean groups
+
+When `flag_value` is `True` or `False`, the substitution rule does not apply:
+`default=True` is the literal Python `True`. To make one flag in an
+enable/disable pair the default, set its `default=True` explicitly:
+
+```python
+@click.option("--without-xyz", "enable_xyz", flag_value=False)
+@click.option("--with-xyz", "enable_xyz", flag_value=True, default=True)
+```
+
+| Definition                                              | Not passed | `--with-xyz` | `--without-xyz` |
+|---------------------------------------------------------|------------|--------------|-----------------|
+| `--with-xyz` with `flag_value=True`, `default=True`     | `True`     | `True`       | `False`         |
+| `--without-xyz` with `flag_value=False`, `default=False`| `False`    | `True`       | `False`         |
+| `--with-xyz` with `flag_value=True`, `default=None`     | `None`     | `True`       | `False`         |
+| Neither option carries a `default`                      | `False`    | `True`       | `False`         |
+
+```{tip}
+For most enable/disable cases, the pair form `--with-xyz/--without-xyz` is
+shorter and equivalent. The multi-flag group form is useful when the on and off
+flags need distinct names without a shared stem, or when each flag needs its
+own help text.
+```
+
+##### Arbitration rules
+
+When several options in a group resolve their values simultaneously, only one
+wins the parameter slot. The full arbitration policy (source precedence,
+explicit-beats-auto tie-break, first-declared fallback) is enumerated under
+[Option value resolution](#option-value-resolution).
+
+## Option value resolution
+
+This section enumerates the rules Click applies when computing the value
+delivered to the decorated function for every option. Rules are listed in the
+order they fire during the parsing pipeline.
+
+### Type inference
+
+Without an explicit `type=`, Click infers the parameter type at construction:
+
+1. If `flag_value` is `True` or `False`, the type is {class}`BoolParamType`.
+2. If `flag_value` is an `int`, `float`, or `str`, the type is the matching
+   basic type.
+3. If `flag_value` is any other Python object (a class, an enum member, a
+   `frozenset`, ...), the type is {data}`UNPROCESSED` so the value passes
+   through unchanged.
+4. Otherwise, the type is inferred from `default` if set, falling back to
+   {class}`StringParamType` when neither hint is available.
+
+### `default` interpretation
+
+The literal value passed as `default=` is interpreted differently depending on
+whether the option is a flag and what `flag_value` it carries:
+
+1. `default=UNSET` (the absence sentinel) is treated as if `default` was not
+   passed at all. It does not count as "the user picked nothing", and it does
+   not count as an explicit default for arbitration purposes.
+2. For a bare boolean flag (no `flag_value`, or `flag_value` of `True` or
+   `False`), an unset `default` auto-derives to `False`.
+3. For a non-boolean flag with a `flag_value`, `default=True` is substituted
+   with `flag_value`. This is the "activate this flag by default" shorthand.
+   Any non-`True` `default` is passed through literally.
+4. For a boolean flag with `flag_value` set, `default=True` is the literal
+   Python `True`. The substitution from rule 3 does not apply.
+5. `default=None` is always a real explicit value, distinct from `UNSET`
+   absence.
+6. Any other `default` is delivered to the function unchanged after conversion
+   through the parameter's type.
+
+### Value sources
+
+Click resolves the value of every option from the following
+sources, in order of decreasing precedence:
+
+1. **command line input** ({attr}`ParameterSource.COMMANDLINE`),
+2. **environment variable** named in `envvar=` or derived from `auto_envvar_prefix`
+   ({attr}`ParameterSource.ENVIRONMENT`),
+3. **`default_map` entry** matching the parameter name on the active {class}`Context`
+   ({attr}`ParameterSource.DEFAULT_MAP`),
+4. **parameter default** ({attr}`ParameterSource.DEFAULT`).
+
+The first source that produces a value wins. Environment variables and
+`default_map` entries set to `Sentinel.UNSET` are skipped, so they fall through
+to the next source rather than supplying `UNSET` to the function.
+
+### Slot arbitration
+
+Several options can target the same `name` to form a feature switch group. When
+they do, only one option's value reaches the function. Arbitration applies
+these rules, in order:
+
+1. **By source.** Whichever option resolved its value from the most explicit
+   source wins, regardless of decorator order. Any command-line input beats any
+   default, an environment variable beats a `default_map` entry, and so on.
+2. **Within the default tier, explicit beats auto-derived.** An option that
+   received an explicit `default=` keyword wins over one whose default came
+   from `default` interpretation.
+3. **Otherwise, last declared wins.** When all options in the group resolved
+   from the same source and tier (all auto-derived defaults, or all explicit
+   defaults), the option declared last in the source code keeps the slot.
 
 ## Values from Environment Variables
 

src/click/core.py

@@ -454,6 +454,12 @@ def __init__(
         self._close_callbacks: list[t.Callable[[], t.Any]] = []
         self._depth = 0
         self._parameter_source: dict[str, ParameterSource] = {}
+        # Tracks whether the option that currently owns each parameter slot in
+        # :attr:`params` had its ``default`` set explicitly by the user. Used
+        # to tie-break feature-switch groups where multiple options share a
+        # parameter name and both fall back to their default value.
+        # Refs: https://github.com/pallets/click/issues/3403
+        self._param_default_explicit: dict[str, bool] = {}
         self._exit_stack = ExitStack()
 
     @property
@@ -2197,6 +2203,12 @@ def __init__(
         self.multiple = multiple
         self.expose_value = expose_value
         self.default: t.Any | t.Callable[[], t.Any] | None = default
+        # Whether the user passed ``default`` explicitly to the constructor.
+        # Captured before any auto-derived default (like ``False`` for boolean
+        # flags in :class:`Option`) replaces the :data:`UNSET` sentinel, so it
+        # remains ``False`` when the default was inferred rather than chosen.
+        # Refs: https://github.com/pallets/click/issues/3403
+        self._default_explicit: bool = default is not UNSET
         self.is_eager = is_eager
         self.metavar = metavar
         self.envvar = envvar
@@ -2583,11 +2595,17 @@ def handle_parse_result(
 
         :meta private:
         """
+        # Capture the slot's existing state before we mutate
+        # ``_parameter_source`` so the write decision below can compare our
+        # incoming source against the source of the option that already wrote
+        # the slot (if any).
+        existing_value = ctx.params.get(self.name, UNSET)
+        existing_source = ctx.get_parameter_source(self.name)
+        existing_default_explicit = ctx._param_default_explicit.get(self.name, False)
+
         with augment_usage_errors(ctx, param=self):
             value, source = self.consume_value(ctx, opts)
 
-            ctx.set_parameter_source(self.name, source)
-
             # Display a deprecation warning if necessary.
             if (
                 self.deprecated
@@ -2618,15 +2636,32 @@ def handle_parse_result(
                 # to UNSET, which will be interpreted as a missing value.
                 value = UNSET
 
-        # Add parameter's value to the context.
-        if (
-            self.expose_value
-            # We skip adding the value if it was previously set by another parameter
-            # targeting the same variable name. This prevents parameters competing for
-            # the same name to override each other.
-            and (self.name not in ctx.params or ctx.params[self.name] is UNSET)
-        ):
-            ctx.params[self.name] = value
+        # Arbitrate the slot when several parameters target the same variable
+        # name (feature-switch groups). See: https://github.com/pallets/click/issues/3403
+        slot_empty = existing_value is UNSET
+        more_explicit = existing_source is not None and source < existing_source
+        same_source = existing_source is not None and source == existing_source
+        auto_would_downgrade_explicit = (
+            same_source
+            and source == ParameterSource.DEFAULT
+            and existing_default_explicit
+            and not self._default_explicit
+        )
+        is_winner = (
+            slot_empty
+            or more_explicit
+            or (same_source and not auto_would_downgrade_explicit)
+        )
+
+        if is_winner:
+            ctx.set_parameter_source(self.name, source)
+            if self.expose_value:
+                ctx.params[self.name] = value
+                ctx._param_default_explicit[self.name] = self._default_explicit
+        elif existing_source is None:
+            # Nothing has claimed the slot yet. Record at least our source so downstream
+            # lookups don't return ``None``.
+            ctx.set_parameter_source(self.name, source)
 
         return value, args