From 2fa562af436ee8f626248c3534d834673068785b Mon Sep 17 00:00:00 2001 From: dhi13man Date: Thu, 12 Mar 2026 23:52:50 +0530 Subject: [PATCH] fix(core): ensure last-invoked option wins for shared dest names When multiple options target the same parameter name (dest), the parser stores only the last-written value. Previously, processing order caused earlier-invoked options to read the wrong value from opts, and the guard in handle_parse_result prevented the correct (last-invoked) option from overwriting ctx.params. Filter param_order to keep only the last invocation per dest name so overshadowed options process as uninvoked, falling through to defaults. The existing guard then allows the winning option's value through. Closes #2786 Co-Authored-By: Dhiman's Agentic Suite --- src/click/core.py | 13 +++++++++++++ tests/test_basic.py | 39 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 52 insertions(+) diff --git a/src/click/core.py b/src/click/core.py index 6adc65ccd6..9cf00b3d41 100644 --- a/src/click/core.py +++ b/src/click/core.py @@ -1223,6 +1223,19 @@ def parse_args(self, ctx: Context, args: list[str]) -> list[str]: parser = self.make_parser(ctx) opts, args, param_order = parser.parse_args(args=args) + # When multiple parameters target the same name (dest), the parser + # stores only the last-written value. To ensure the last-invoked + # parameter on the command line takes priority during processing, + # keep only the last invocation per name in param_order. Earlier + # invocations will process as uninvoked (falling through to + # defaults), and the existing guard in handle_parse_result + # prevents them from overwriting the winning value. + # Refs: https://github.com/pallets/click/issues/2786 + last_for_name: dict[str | None, Parameter] = {} + for param in param_order: + last_for_name[param.name] = param + param_order = [p for p in param_order if last_for_name.get(p.name) is p] + for param in iter_params_for_processing(param_order, self.get_params(ctx)): _, args = param.handle_parse_result(ctx, opts, args) diff --git a/tests/test_basic.py b/tests/test_basic.py index 125fac3c44..fa687da4e8 100644 --- a/tests/test_basic.py +++ b/tests/test_basic.py @@ -328,6 +328,45 @@ def cli(case): assert result.output == repr(expected) +@pytest.mark.parametrize( + ("args", "expected"), + ( + # --fetch alone: callback transforms sentinel to fetched value. + (["--fetch"], "foo"), + # --custom alone: value passed through as-is. + (["--custom", "bar"], "bar"), + # --custom first, --fetch last: last option wins, callback runs. + (["--custom", "bar", "--fetch"], "foo"), + # --fetch first, --custom last: last option wins. + (["--fetch", "--custom", "bar"], "bar"), + # Neither specified: default is None. + ([], None), + ), +) +def test_dual_option_callback_last_wins(runner, args, expected): + """When two options share a dest and one has a callback, the last + option specified on the command line should win. + + Regression test for https://github.com/pallets/click/issues/2786 + """ + SENTINEL = "$_fetch" + + def callback(ctx, param, value): + if value is SENTINEL: + return "foo" + return value + + @click.command() + @click.option("--custom", "custom") + @click.option("--fetch", "custom", flag_value=SENTINEL, callback=callback) + def cli(custom): + click.echo(repr(custom), nl=False) + + result = runner.invoke(cli, args) + assert result.exit_code == 0, result.output + assert result.output == repr(expected) + + def test_file_option(runner): @click.command() @click.option("--file", type=click.File("w"))