Fix get_parameter_source() during type conversion and eager callbacks

CHANGES.rst

@@ -5,11 +5,12 @@ Version 8.4.1
 
 Unreleased
 
+-   ``get_parameter_source()`` is available during eager callbacks and type
+    conversion again. :issue:`3458` :issue:`3484`
 -   Zsh completion scripts parse correctly on Windows. :issue:`3277`
 -   Shell completion of `Choice` `Enum` values produces a valid completion
     result. :issue:`3015`
 
-
 Version 8.4.0
 -------------
 

docs/advanced.md

@@ -144,8 +144,18 @@ it's good to know that the system works this way.
             click.echo(f"{url}: {fp.code}")
 
 In this case the callback returns the URL unchanged but also passes a
-second `fp` value to the callback. What's more recommended is to pass
-the information in a wrapper, however:
+second `fp` value to the callback.
+
+.. caution::
+
+   Writing to :attr:`~click.Context.params` directly bypasses Click's
+   per-parameter pipeline: :meth:`~click.Context.get_parameter_source` returns
+   ``None`` for it, the value is never run through any :class:`~click.ParamType`
+   conversion. And if the key collides with a declared parameter, then the
+   shared-name arbitration loses information about where the value came from.
+   Prefer the wrapper pattern below when any of those semantics matter.
+
+What's more recommended is to pass the information in a wrapper, however:
 
 .. click:example::
 

src/click/core.py

@@ -2609,6 +2609,11 @@ def handle_parse_result(
         with augment_usage_errors(ctx, param=self):
             value, source = self.consume_value(ctx, opts)
 
+            # Record the source before processing so eager callbacks and type
+            # conversion can inspect it. Restored after arbitration if this
+            # option loses a feature-switch group.
+            ctx.set_parameter_source(self.name, source)
+
             # Display a deprecation warning if necessary.
             if (
                 self.deprecated
@@ -2654,14 +2659,16 @@ def handle_parse_result(
         )
 
         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)
+        elif existing_source is not None:
+            # Lost arbitration; restore the winning option's source.
+            ctx.set_parameter_source(self.name, existing_source)
+        # else: ctx.params[self.name] was populated by code that bypassed
+        # handle_parse_result (from another option's callback for example). Keep
+        # the provisional source recorded before process_value so downstream
+        # lookups don't return ``None``.
 
         return value, args
 

tests/test_defaults.py

@@ -1,8 +1,11 @@
+import os
+
 import pytest
 
 import click
 from click import UNPROCESSED
 from click._utils import UNSET
+from click.core import ParameterSource
 
 
 @pytest.mark.parametrize(
@@ -265,6 +268,153 @@ def cli(ctx, name):
     assert f"source={expected_source}" in result.output
 
 
+def test_parameter_source_during_paramtype_convert(runner):
+    """``get_parameter_source()`` is available during ``ParamType.convert``.
+
+    Uses the reproducer from https://github.com/pallets/click/issues/3458.
+    """
+
+    class Source(click.ParamType):
+        name = "source"
+
+        def convert(self, value, param, ctx):
+            return {
+                "value": value,
+                "source": ctx.get_parameter_source(param.name),
+            }
+
+    @click.command()
+    @click.option("--default", type=Source(), default="/tmp/file")
+    @click.option("--nodefault", type=Source())
+    def cli(default, nodefault):
+        click.echo(f"default: {default}")
+        click.echo(f"nodefault: {nodefault}")
+
+    result = runner.invoke(cli, [])
+    assert not result.exception
+    assert "default: {'value': '/tmp/file', 'source': " in result.output
+    assert "'source': None}" not in result.output.split("default:")[1].split("\n")[0]
+    assert (
+        result.output == "default: {'value': '/tmp/file', 'source': "
+        f"{ParameterSource.DEFAULT!r}}}\nnodefault: None\n"
+    )
+
+    result = runner.invoke(cli, ["--default", "cli", "--nodefault", "also"])
+    assert not result.exception
+    assert (
+        "default: {'value': 'cli', 'source': "
+        f"{ParameterSource.COMMANDLINE!r}}}" in result.output
+    )
+    assert (
+        "nodefault: {'value': 'also', 'source': "
+        f"{ParameterSource.COMMANDLINE!r}}}" in result.output
+    )
+
+
+def test_parameter_source_during_eager_callback(runner):
+    """``get_parameter_source()`` is available during eager callbacks.
+
+    Regression test for https://github.com/pallets/click/issues/3458.
+    """
+
+    def eager_cb(ctx, param, value):
+        source = ctx.get_parameter_source(param.name)
+        click.echo(f"callback source={source.name if source else None}")
+
+    @click.command()
+    @click.option(
+        "--flag/--no-flag",
+        default=False,
+        is_eager=True,
+        callback=eager_cb,
+        expose_value=False,
+    )
+    def cli():
+        source = click.get_current_context().get_parameter_source("flag")
+        click.echo(f"final source={source.name}")
+
+    result = runner.invoke(cli, [])
+    assert not result.exception
+    assert "callback source=DEFAULT" in result.output
+    assert "final source=DEFAULT" in result.output
+
+    result = runner.invoke(cli, ["--flag"])
+    assert not result.exception
+    assert "callback source=COMMANDLINE" in result.output
+    assert "final source=COMMANDLINE" in result.output
+
+
+def test_flask_debug_env_not_stomped_by_default_flag(runner, monkeypatch):
+    """Eager callback must not overwrite env when the flag used its default.
+
+    Covers the Flask ``_set_debug`` pattern (pallets/flask#6025). Regression test
+    for https://github.com/pallets/click/issues/3458.
+    """
+
+    monkeypatch.delenv("APP_DEBUG", raising=False)
+
+    def set_debug(ctx, param, value):
+        source = ctx.get_parameter_source(param.name)
+        if source is not None and source in (
+            ParameterSource.DEFAULT,
+            ParameterSource.DEFAULT_MAP,
+        ):
+            return None
+        os.environ["APP_DEBUG"] = "1" if value else "0"
+        return value
+
+    @click.command()
+    @click.option(
+        "--debug/--no-debug",
+        default=False,
+        is_eager=True,
+        expose_value=False,
+        callback=set_debug,
+    )
+    def cli():
+        click.echo(f"APP_DEBUG={os.environ.get('APP_DEBUG', '')}")
+
+    monkeypatch.setenv("APP_DEBUG", "1")
+    result = runner.invoke(cli, [])
+    assert result.exit_code == 0
+    assert result.output.strip() == "APP_DEBUG=1"
+
+    result = runner.invoke(cli, ["--debug"])
+    assert result.exit_code == 0
+    assert result.output.strip() == "APP_DEBUG=1"
+
+    result = runner.invoke(cli, ["--no-debug"])
+    assert result.exit_code == 0
+    assert result.output.strip() == "APP_DEBUG=0"
+
+
+def test_parameter_source_on_parse_result_bypass(runner):
+    """A losing option keeps its provisional source when ``ctx.params[name]``
+    is populated by code that bypassed ``handle_parse_result``.
+
+    This replicate the pattern documented in the "Parameter Modifications" section
+    of ``docs/advanced.md``. This test highlight the current behavior of
+    ``get_parameter_source()`` but is not intended as a contract enforcement.
+    """
+
+    def hijack(ctx, param, value):
+        ctx.params["target"] = "hijacked"
+        return value
+
+    @click.command()
+    @click.option("--hijacker", is_eager=True, callback=hijack, expose_value=False)
+    @click.option("--target", default="default_value")
+    @click.pass_context
+    def cli(ctx, target):
+        source = ctx.get_parameter_source("target")
+        click.echo(f"value={target} source={source.name if source else 'None'}")
+
+    result = runner.invoke(cli, ["--hijacker", "anything"])
+    assert result.exit_code == 0, result.output
+    assert "value=hijacked" in result.output
+    assert "source=DEFAULT" in result.output
+
+
 def test_lookup_default_override_respected(runner):
     """A subclass override of ``lookup_default()`` should be called by Click
     internals, not bypassed by a private method.

tests/test_options.py

@@ -2895,6 +2895,68 @@ def cli(enable_xyz):
     assert result.output == repr(expected)
 
 
+@pytest.mark.parametrize(
+    ("opts", "args", "invoke_kwargs", "expected_value", "expected_source"),
+    [
+        # https://github.com/pallets/click/issues/3458
+        pytest.param(
+            [
+                ("--without-xyz", {"flag_value": False}),
+                ("--with-xyz", {"flag_value": True, "default": True}),
+            ],
+            [],
+            {},
+            True,
+            "DEFAULT",
+            id="explicit-default-wins",
+        ),
+        pytest.param(
+            [
+                ("--without-xyz", {"flag_value": False}),
+                ("--with-xyz", {"flag_value": True, "default": True}),
+            ],
+            ["--without-xyz"],
+            {},
+            False,
+            "COMMANDLINE",
+            id="cmdline-wins",
+        ),
+        pytest.param(
+            [
+                ("--without-xyz", {"flag_value": False}),
+                ("--with-xyz", {"flag_value": True, "default": True}),
+            ],
+            ["--without-xyz"],
+            {"default_map": {"enable_xyz": True}},
+            False,
+            "COMMANDLINE",
+            id="loser-default-map-restores-winner-source",
+        ),
+    ],
+)
+def test_bool_flag_group_parameter_source(
+    runner, opts, args, invoke_kwargs, expected_value, expected_source
+):
+    """``get_parameter_source()`` stays correct for feature-switch groups.
+
+    Regression test for https://github.com/pallets/click/issues/3458.
+    """
+
+    @click.command()
+    @click.pass_context
+    def cli(ctx, enable_xyz):
+        source = ctx.get_parameter_source("enable_xyz")
+        click.echo(f"value={enable_xyz!r} source={source.name}")
+
+    for opt_name, opt_kwargs in opts:
+        cli = click.option(opt_name, "enable_xyz", **opt_kwargs)(cli)
+
+    result = runner.invoke(cli, args, **invoke_kwargs)
+    assert result.exit_code == 0, result.output
+    assert f"value={expected_value!r}" in result.output
+    assert f"source={expected_source}" in result.output
+
+
 @pytest.mark.parametrize(
     ("opts", "args", "expected"),
     [