From 546f2851f414b07413777ebcae89b2c21a685252 Mon Sep 17 00:00:00 2001 From: Kevin Deldycke Date: Sat, 21 Feb 2026 18:01:42 +0400 Subject: [PATCH] Fix callable ``flag_value`` being instantiated when used as a default Closes #3121 Refs: #3201 #3213 --- CHANGES.rst | 2 + src/click/core.py | 26 +++++-- tests/test_defaults.py | 45 +++++++++++ tests/test_options.py | 170 ++++++++++++++++++++++++++++++++++++++++- 4 files changed, 234 insertions(+), 9 deletions(-) diff --git a/CHANGES.rst b/CHANGES.rst index 234eaddb88..2e29b245d4 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -12,6 +12,8 @@ Unreleased logging interaction, multi-threaded safety, and sequential invocation isolation. Add high-iteration stress tests behind a ``stress`` marker with a dedicated CI job. :pr:`3139` +- Fix callable ``flag_value`` being instantiated when used as a default via + ``default=True``. :issue:`3121` :pr:`3201` :pr:`3213` :pr:`3225` Version 8.3.1 -------------- diff --git a/src/click/core.py b/src/click/core.py index 23d1dd5705..f0a624be3b 100644 --- a/src/click/core.py +++ b/src/click/core.py @@ -2819,14 +2819,12 @@ def __init__( if self.default is UNSET and not self.required: self.default = False - # Support the special case of aligning the default value with the flag_value - # for flags whose default is explicitly set to True. Note that as long as we - # have this condition, there is no way a flag can have a default set to True, - # and a flag_value set to something else. Refs: + # The alignement of default to the flag_value is resolved lazily in + # get_default() to prevent callable flag_values (like classes) from + # being instantiated. Refs: + # https://github.com/pallets/click/issues/3121 # https://github.com/pallets/click/issues/3024#issuecomment-3146199461 # https://github.com/pallets/click/pull/3030/commits/06847da - if self.default is True and self.flag_value is not UNSET: - self.default = self.flag_value # Set the default flag_value if it is not set. if self.flag_value is UNSET: @@ -2890,6 +2888,22 @@ def to_info_dict(self) -> dict[str, t.Any]: ) return info_dict + def get_default( + self, ctx: Context, call: bool = True + ) -> t.Any | t.Callable[[], t.Any] | None: + 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: + value = self.flag_value + elif call and callable(value): + value = value() + + return value + def get_error_hint(self, ctx: Context) -> str: result = super().get_error_hint(ctx) if self.show_envvar and self.envvar is not None: diff --git a/tests/test_defaults.py b/tests/test_defaults.py index 5ef20fe882..7dc74f0b7e 100644 --- a/tests/test_defaults.py +++ b/tests/test_defaults.py @@ -1,6 +1,7 @@ import pytest import click +from click import UNPROCESSED @pytest.mark.parametrize( @@ -309,3 +310,47 @@ def cli(ctx, app_email): result = runner.invoke(cli, ["--help"], default_map=default_map) assert not result.exception assert "prefix@example.com" in result.output + + +class _Marker: + """Dummy callable used as a flag_value in default tests.""" + + pass + + +@pytest.mark.parametrize( + ("default_map", "args", "expected"), + [ + # No default_map: auto-aligned default returns the class, not an instance. + (None, [], _Marker), + # CLI flag always returns the class. + (None, ["--opt"], _Marker), + # Static value in default_map overrides the auto-aligned flag_value. + ({"value": "from-map"}, [], "from-map"), + # Callable in default_map is still invoked (not suppressed by the fix). + ({"value": lambda: "lazy-map"}, [], "lazy-map"), + # None in default_map overrides the flag_value. + ({"value": None}, [], None), + # CLI arg wins over default_map. + ({"value": "from-map"}, ["--opt"], _Marker), + ], +) +def test_default_map_with_callable_flag_value(runner, default_map, args, expected): + """``default_map`` entries should override the auto-aligned callable ``flag_value``, + and callable entries in ``default_map`` should still be invoked. + + Verifies the fix for https://github.com/pallets/click/issues/3121 does not + break ``default_map`` precedence. + """ + + @click.command() + @click.option("--opt", "value", flag_value=_Marker, type=UNPROCESSED, default=True) + def cli(value): + click.echo(repr(value), nl=False) + + kwargs = {} + if default_map is not None: + kwargs["default_map"] = default_map + result = runner.invoke(cli, args, **kwargs) + assert result.exit_code == 0 + assert result.output == repr(expected) diff --git a/tests/test_options.py b/tests/test_options.py index b162a96de4..e335f3c1a0 100644 --- a/tests/test_options.py +++ b/tests/test_options.py @@ -290,7 +290,6 @@ def cmd(a): None, "Error: Invalid value for '-a': Value must be an iterable.", ), - # ( False, 2, @@ -2162,11 +2161,12 @@ def scan(pro): ), # Check that passing exotic flag values like classes is supported, but are # rendered to strings when the type is not specified. + # https://github.com/pallets/click/issues/3121 ( {"flag_value": Class1, "default": True}, {"flag_value": Class2}, [], - re.compile(r"''"), + "", ), ( {"flag_value": Class1, "default": True}, @@ -2184,11 +2184,12 @@ def scan(pro): ({"flag_value": Class1, "default": "True"}, {"flag_value": Class2}, [], "True"), ({"flag_value": Class1, "default": None}, {"flag_value": Class2}, [], None), # To get the classes as-is, we need to specify the type as UNPROCESSED. + # https://github.com/pallets/click/issues/3121 ( {"flag_value": Class1, "type": UNPROCESSED, "default": True}, {"flag_value": Class2, "type": UNPROCESSED}, [], - re.compile(r""), + Class1, ), ( {"flag_value": Class1, "type": UNPROCESSED, "default": True}, @@ -2269,6 +2270,169 @@ def cli(dual_option): assert result.output == repr(expected) +@pytest.mark.parametrize( + ("opt_params", "args", "expected"), + [ + # Class flag_value with default=True and UNPROCESSED: the class itself is + # returned, NOT an instance. Regression test for + # https://github.com/pallets/click/issues/3121 + ({"flag_value": Class1, "type": UNPROCESSED, "default": True}, [], Class1), + ( + {"flag_value": Class1, "type": UNPROCESSED, "default": True}, + ["--opt"], + Class1, + ), + # Without UNPROCESSED, the class is str()-ified by the default STRING type. + ({"flag_value": Class1, "default": True}, [], ""), + ( + {"flag_value": Class1, "default": True}, + ["--opt"], + "", + ), + # Explicit default=Class1 (not via default=True alignment): callable IS invoked, + # because the user explicitly set a callable as the default. + ( + {"flag_value": Class1, "type": UNPROCESSED, "default": Class1}, + [], + re.compile(r""), + ), + # Explicit default=Class2, different from flag_value=Class1. + ( + {"flag_value": Class1, "type": UNPROCESSED, "default": Class2}, + [], + re.compile(r""), + ), + # Non-callable flag_value with default=True: unaffected by the fix. + ({"flag_value": "upper", "default": True}, [], "upper"), + ({"flag_value": "upper", "default": True}, ["--opt"], "upper"), + ], +) +def test_callable_flag_value_not_instantiated(runner, opt_params, args, expected): + """A callable ``flag_value`` like a class, with ``default=True`` should not be + invoked when resolving the default. This is the single-option variant of + the regression reported in https://github.com/pallets/click/issues/3121. + """ + + @click.command() + @click.option("--opt", "value", **opt_params) + def cli(value): + click.echo(repr(value), nl=False) + + result = runner.invoke(cli, args) + assert result.exit_code == 0 + if isinstance(expected, re.Pattern): + assert re.match(expected, result.output) + else: + assert result.output == repr(expected) + + +def test_callable_flag_value_default_map(runner): + """A ``default_map`` entry should override the auto-aligned callable ``flag_value``. + + When ``default=True`` and ``flag_value=SomeClass``, the default is aligned to + ``SomeClass``. If ``default_map`` provides a different value (including a + callable), it should take precedence and callables from ``default_map`` should + still be invoked. + """ + + @click.command() + @click.option("--opt", "value", flag_value=Class1, type=UNPROCESSED, default=True) + def cli(value): + click.echo(repr(value), nl=False) + + # Static value in default_map overrides the flag_value default. + result = runner.invoke(cli, [], default_map={"value": "from-map"}) + assert result.output == repr("from-map") + + # Callable in default_map is still invoked (not suppressed by the fix). + result = runner.invoke(cli, [], default_map={"value": lambda: "lazy-map"}) + assert result.output == repr("lazy-map") + + # CLI arg still wins over everything. + result = runner.invoke(cli, ["--opt"]) + assert result.output == repr(Class1) + + +def test_callable_flag_value_show_default(runner): + """Help text with ``show_default=True`` should display the class name, not + instantiate it. + """ + + @click.command() + @click.option( + "--opt", + "value", + flag_value=Class1, + type=UNPROCESSED, + default=True, + show_default=True, + ) + def cli(value): + pass + + result = runner.invoke(cli, ["--help"]) + assert result.exit_code == 0 + assert "Class1" in result.output + assert "object at 0x" not in result.output + + +@pytest.mark.parametrize( + ("opt_params", "expected_default_attr", "expected_get_default"), + [ + # default=True with callable flag_value: the attribute stays True + # (not eagerly aligned), but get_default() resolves to the flag_value. + ( + {"flag_value": Class1, "type": UNPROCESSED, "default": True}, + True, + Class1, + ), + # default=True with non-callable flag_value: same lazy resolution. + ( + {"flag_value": "upper", "default": True}, + True, + "upper", + ), + # Explicit default (not True): attribute and get_default() agree. + ( + {"flag_value": Class1, "type": UNPROCESSED, "default": "custom"}, + "custom", + "custom", + ), + # No default: attribute and get_default() are both UNSET. + ( + {"flag_value": Class1, "type": UNPROCESSED}, + UNSET, + UNSET, + ), + ], +) +def test_callable_flag_value_get_default_override( + runner, opt_params, expected_default_attr, expected_get_default +): + """The ``default=True`` to ``flag_value`` alignment is resolved lazily in + ``get_default()`` rather than eagerly in ``__init__``. This means + ``option.default`` stays as ``True`` while ``get_default()`` returns the + ``flag_value``. + + A user subclass that reads ``self.default`` directly (bypassing + ``get_default()``) will see ``True`` instead of the ``flag_value``. + """ + + @click.command() + @click.option("--opt", "value", **opt_params) + def cli(value): + pass + + opt = cli.params[0] + + # The raw attribute reflects what the user wrote. + assert opt.default is expected_default_attr + + # get_default() resolves the alignment lazily. + ctx = click.Context(cli) + assert opt.get_default(ctx, call=True) is expected_get_default + + def test_custom_type_frozenset_flag_value(runner): """Check that frozenset is correctly handled as a type, a flag value and a default.