From 547d9a4b7f4e729235c773a69e463b562f853012 Mon Sep 17 00:00:00 2001 From: Yuri Shevtsov Date: Tue, 19 May 2026 19:43:26 -0400 Subject: [PATCH] Fix get_parameter_source() during type conversion and eager callbacks Record parameter source on the context immediately after consume_value(), before process_value() runs. In 8.4.0, set_parameter_source() was deferred until after type conversion and flag-group arbitration (0f71fe7, #3403), so get_parameter_source() returned None inside ParamType.convert() and eager callbacks (#3458). When several options share one parameter name, the losing option still sets a provisional source before process_value(); restore the previous source if arbitration rejects it so the winner's origin is not replaced unintentionally. --- CHANGES.rst | 2 + src/click/core.py | 14 +++-- tests/test_defaults.py | 123 +++++++++++++++++++++++++++++++++++++++++ tests/test_options.py | 62 +++++++++++++++++++++ 4 files changed, 196 insertions(+), 5 deletions(-) diff --git a/CHANGES.rst b/CHANGES.rst index 7488c0c2fc..31f90cfe0a 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -11,6 +11,8 @@ Version 8.4.1 Unreleased +- ``get_parameter_source()`` is available during eager callbacks and type + conversion again. :issue:`3458` - Zsh completion scripts parse correctly on Windows. :issue:`3277` - Shell completion of `Choice` `Enum` values produces a valid completion result. :issue:`3015` diff --git a/src/click/core.py b/src/click/core.py index 3544c03b3b..3944bb4403 100644 --- a/src/click/core.py +++ b/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,13 @@ 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: keep the provisional source recorded before process_value. return value, args diff --git a/tests/test_defaults.py b/tests/test_defaults.py index 49f0ac6ed0..88b3e01d8f 100644 --- a/tests/test_defaults.py +++ b/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,126 @@ 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_lookup_default_override_respected(runner): """A subclass override of ``lookup_default()`` should be called by Click internals, not bypassed by a private method. diff --git a/tests/test_options.py b/tests/test_options.py index 25e649d045..44cb58f6df 100644 --- a/tests/test_options.py +++ b/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"), [