Reconcile default value passing and default activation

Fix #3111
Reintroduce a commit pushed by mistake in 6c4a77b and reverted in bb7be1f
Refine tests introduced in 06847da for #3030
Document the interaction between `default` and `flag_value`

Author: kdeldycke

CHANGES.rst

@@ -21,6 +21,11 @@ Unreleased
     ``pytest-xdist`` to detect test pollution and race conditions. :pr:`3151`
 -   Add contributor documentation for running stress tests, randomized
     parallel tests, and Flask smoke tests. :pr:`3151` :pr:`3177`
+-   Fix ``default=True`` with boolean ``flag_value`` always returning the
+    ``flag_value`` instead of ``True``. The ``default=True`` to ``flag_value``
+    substitution now only applies to non-boolean flags, where ``True`` acts as a
+    sentinel meaning "activate this flag by default". For boolean flags,
+    ``default=True`` is returned as a literal value. :issue:`3111` :pr:`3239`
 
 Version 8.3.2
 -------------

docs/options.md

@@ -341,12 +341,19 @@ To have a flag pass a value to the decorated function set `flag_value`. This aut
     invoke(info)
 ```
 
-````{note}
-The `default` value is given to the decorated function as-is. So if you set `default=None`, the value passed to the function is the `None` Python value. Same for any other type.
+### How `default` and `flag_value` interact
 
-But there is a special case for flags. If a flag has a `flag_value`, then setting `default=True` is interpreted as *the flag should be activated by default*. So instead of the decorated function receiving the `True` Python value, it will receive the `flag_value`.
+The `default` value is given to the underlying function
+as-is. So if you set `default=None`, the function receives
+`None`. Same for any other type.
 
-Which means, in example above, this option:
+But there is a special case for **non-boolean** flags: if a
+flag has a non-boolean `flag_value` (like a string or a
+class), then `default=True` is interpreted as *the flag
+should be activated by default*. The function receives the
+`flag_value`, not the Python `True`.
+
+Which means, in the example above, this option:
 
 ```python
 @click.option('--upper', 'transformation', flag_value='upper', default=True)
@@ -358,9 +365,89 @@ is equivalent to:
 @click.option('--upper', 'transformation', flag_value='upper', default='upper')
 ```
 
-Because the two are equivalent, it is recommended to always set `default` to the actual value you want to pass and avoid using the special `True` case. This makes the code more explicit and predictable.
+Because the two are equivalent, it is recommended to always
+use the second form and set `default` to the actual value
+you want. This makes code more explicit and predictable.
+
+This special case does **not** apply to boolean flags (where
+`flag_value` is `True` or `False`). For boolean flags,
+`default=True` is the literal Python value `True`.
+
+The tables below show the value received by the function for
+each combination of `default`, `flag_value`, and whether
+the flag was passed on the command line.
+
+#### Boolean flags (`is_flag=True`, boolean `flag_value`)
+
+These are flags where `flag_value` is `True` or `False`.
+The `default` value is always passed through literally
+without any special substitution.
+
+| `default` | `flag_value` | Not passed | `--flag` passed |
+|-----------|--------------|------------|-----------------|
+| *(unset)* | *(unset)*    | `False`    | `True`          |
+| `True`    | *(unset)*    | `True`     | `True`          |
+| `False`   | *(unset)*    | `False`    | `True`          |
+| `None`    | *(unset)*    | `None`     | `True`          |
+| `True`    | `True`       | `True`     | `True`          |
+| `True`    | `False`      | `True`     | `False`         |
+| `False`   | `True`       | `False`    | `True`          |
+| `False`   | `False`      | `False`    | `False`         |
+| `None`    | `True`       | `None`     | `True`          |
+| `None`    | `False`      | `None`     | `False`         |
+
+````{tip}
+For a negative flag that defaults to off, prefer the
+explicit pair form `--with-xyz/--without-xyz` over the
+single-flag `flag_value=False, default=True`:
+
+```python
+@click.option('--with-xyz/--without-xyz', 'enable_xyz', default=True)
+```
 ````
 
+#### Boolean flag pairs (`--flag/--no-flag`)
+
+These use secondary option names to provide both an on and
+off switch. The `default` value is always literal.
+
+| `default` | Not passed | `--flag` | `--no-flag` |
+|-----------|------------|----------|-------------|
+| *(unset)* | `False`    | `True`   | `False`     |
+| `True`    | `True`     | `True`   | `False`     |
+| `False`   | `False`    | `True`   | `False`     |
+| `None`    | `None`     | `True`   | `False`     |
+
+#### Non-boolean feature switches (`flag_value` is a string, class, etc.)
+
+For these flags, `default=True` is a **special case**: it
+means "activate this flag by default" and resolves to the
+`flag_value`. All other `default` values are passed through
+literally.
+
+| `default`  | `flag_value` | Not passed  | `--flag` passed |
+|------------|--------------|-------------|-----------------|
+| *(unset)*  | `"upper"`    | `None`      | `"upper"`       |
+| `True`     | `"upper"`    | `"upper"`¹  | `"upper"`       |
+| `"lower"`  | `"upper"`    | `"lower"`   | `"upper"`       |
+| `None`     | `"upper"`    | `None`      | `"upper"`       |
+
+```{hint}
+¹: `default=True` is substituted with `flag_value`.
+```
+
+#### 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.
+
+| 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"` |
+
 ## Values from Environment Variables
 
 To pass in a value from a specific environment variable use `envvar`.

src/click/core.py

@@ -2891,13 +2891,32 @@ def to_info_dict(self) -> dict[str, t.Any]:
     def get_default(
         self, ctx: Context, call: bool = True
     ) -> t.Any | t.Callable[[], t.Any] | None:
+        """For non-boolean flag options, ``default=True`` is treated as a
+        sentinel meaning "activate this flag by default" and is resolved to
+        :attr:`flag_value`. This resolution is performed lazily here (rather
+        than eagerly in :meth:`__init__`) to prevent callable ``flag_value``
+        values (like classes) from being instantiated prematurely.
+
+        For example, with ``--upper/--lower`` feature switches where
+        ``flag_value="upper"`` and ``default=True``, the default resolves
+        to ``"upper"``.
+
+        .. caution::
+            This substitution only applies to **non-boolean** flags
+            (:attr:`is_bool_flag` is ``False``). For boolean flags, ``True`` is
+            not a sentinel but a legitimate Python value, so ``default=True`` is
+            returned as-is. Without this distinction, ``flag_value=False,
+            default=True`` would silently always return ``False``, regardless of
+            whether the flag was passed or not.
+
+        .. versionchanged:: 8.3.3
+            ``default=True`` is no longer substituted with ``flag_value`` for
+            boolean flags, fixing negative boolean flags like ``flag_value=False,
+            default=True``.
+        """
         value = super().get_default(ctx, call=False)
 
-        # Lazily resolve default=True to flag_value. Doing this here
-        # (instead of eagerly in __init__) prevents callable flag_values
-        # (like classes) from being instantiated by the callable check below.
-        # https://github.com/pallets/click/issues/3121
-        if value is True and self.is_flag:
+        if value is True and self.is_flag and not self.is_bool_flag:
             value = self.flag_value
         elif call and callable(value):
             value = value()

tests/test_options.py

@@ -1457,9 +1457,12 @@ def test_type_from_flag_value():
         ({"type": str, "flag_value": None}, [], None),
         ({"type": str, "flag_value": None}, ["--foo"], None),
         # Not passing --foo returns the default value as-is, in its Python type, then
-        # converted by the option type.
+        # converted by the option type. For boolean flags, default=True is a literal
+        # value, not a sentinel meaning "activate flag". So it is NOT substituted with
+        # flag_value. See: https://github.com/pallets/click/issues/3111
+        # https://github.com/pallets/click/pull/3239
         ({"type": bool, "default": True, "flag_value": True}, [], True),
-        ({"type": bool, "default": True, "flag_value": False}, [], False),
+        ({"type": bool, "default": True, "flag_value": False}, [], True),
         ({"type": bool, "default": False, "flag_value": True}, [], False),
         ({"type": bool, "default": False, "flag_value": False}, [], False),
         ({"type": bool, "default": None, "flag_value": True}, [], None),
@@ -2484,6 +2487,38 @@ def rcli(scm_ignore_files):
     assert result.exit_code == 0
 
 
+@pytest.mark.parametrize(
+    ("default", "args", "expected"),
+    [
+        # default=None: 3-state pattern (e.g. Flask --reload/--no-reload).
+        # https://github.com/pallets/click/issues/3024
+        (None, [], None),
+        (None, ["--flag"], True),
+        (None, ["--no-flag"], False),
+        # default=True: literal value, not substituted with flag_value.
+        # https://github.com/pallets/click/issues/3111
+        (True, [], True),
+        (True, ["--flag"], True),
+        (True, ["--no-flag"], False),
+    ],
+)
+def test_bool_flag_pair_default(runner, default, args, expected):
+    """Boolean flag pairs pass ``default`` through literally.
+
+    Ensures ``default=True`` is not replaced by ``flag_value`` for boolean
+    flags, and that ``default=None`` enables 3-state logic.
+    """
+
+    @click.command()
+    @click.option("--flag/--no-flag", default=default)
+    def cli(flag):
+        click.echo(repr(flag), nl=False)
+
+    result = runner.invoke(cli, args)
+    assert result.exit_code == 0
+    assert result.output == repr(expected)
+
+
 @pytest.mark.parametrize(
     ("flag_type", "args", "expect_output"),
     [

tests/test_termui.py

@@ -825,9 +825,13 @@ def cmd(arg1):
     ({"prompt": True, "default": True, "flag_value": True}, [], "[Y/n]", "", True),
     ({"prompt": True, "default": True, "flag_value": True}, [], "[Y/n]", "y", True),
     ({"prompt": True, "default": True, "flag_value": True}, [], "[Y/n]", "n", False),
-    ({"prompt": True, "default": True, "flag_value": False}, [], "[y/N]", "", False),
-    ({"prompt": True, "default": True, "flag_value": False}, [], "[y/N]", "y", True),
-    ({"prompt": True, "default": True, "flag_value": False}, [], "[y/N]", "n", False),
+    # For boolean flags, default=True is a literal value, not a sentinel meaning
+    # "activate flag", so the prompt shows [Y/n] with default=True. See:
+    # https://github.com/pallets/click/issues/3111
+    # https://github.com/pallets/click/pull/3239
+    ({"prompt": True, "default": True, "flag_value": False}, [], "[Y/n]", "", True),
+    ({"prompt": True, "default": True, "flag_value": False}, [], "[Y/n]", "y", True),
+    ({"prompt": True, "default": True, "flag_value": False}, [], "[Y/n]", "n", False),
     # default=False
     ({"prompt": True, "default": False, "flag_value": True}, [], "[y/N]", "", False),
     ({"prompt": True, "default": False, "flag_value": True}, [], "[y/N]", "y", True),