Skip to content

Commit 547d9a4

Browse files
committed
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.
1 parent dbf4d14 commit 547d9a4

4 files changed

Lines changed: 196 additions & 5 deletions

File tree

CHANGES.rst

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,8 @@ Version 8.4.1
1111

1212
Unreleased
1313

14+
- ``get_parameter_source()`` is available during eager callbacks and type
15+
conversion again. :issue:`3458`
1416
- Zsh completion scripts parse correctly on Windows. :issue:`3277`
1517
- Shell completion of `Choice` `Enum` values produces a valid completion
1618
result. :issue:`3015`

src/click/core.py

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2609,6 +2609,11 @@ def handle_parse_result(
26092609
with augment_usage_errors(ctx, param=self):
26102610
value, source = self.consume_value(ctx, opts)
26112611

2612+
# Record the source before processing so eager callbacks and type
2613+
# conversion can inspect it. Restored after arbitration if this
2614+
# option loses a feature-switch group.
2615+
ctx.set_parameter_source(self.name, source)
2616+
26122617
# Display a deprecation warning if necessary.
26132618
if (
26142619
self.deprecated
@@ -2654,14 +2659,13 @@ def handle_parse_result(
26542659
)
26552660

26562661
if is_winner:
2657-
ctx.set_parameter_source(self.name, source)
26582662
if self.expose_value:
26592663
ctx.params[self.name] = value
26602664
ctx._param_default_explicit[self.name] = self._default_explicit
2661-
elif existing_source is None:
2662-
# Nothing has claimed the slot yet. Record at least our source so downstream
2663-
# lookups don't return ``None``.
2664-
ctx.set_parameter_source(self.name, source)
2665+
elif existing_source is not None:
2666+
# Lost arbitration; restore the winning option's source.
2667+
ctx.set_parameter_source(self.name, existing_source)
2668+
# else: keep the provisional source recorded before process_value.
26652669

26662670
return value, args
26672671

tests/test_defaults.py

Lines changed: 123 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,11 @@
1+
import os
2+
13
import pytest
24

35
import click
46
from click import UNPROCESSED
57
from click._utils import UNSET
8+
from click.core import ParameterSource
69

710

811
@pytest.mark.parametrize(
@@ -265,6 +268,126 @@ def cli(ctx, name):
265268
assert f"source={expected_source}" in result.output
266269

267270

271+
def test_parameter_source_during_paramtype_convert(runner):
272+
"""``get_parameter_source()`` is available during ``ParamType.convert``.
273+
274+
Uses the reproducer from https://github.com/pallets/click/issues/3458.
275+
"""
276+
277+
class Source(click.ParamType):
278+
name = "source"
279+
280+
def convert(self, value, param, ctx):
281+
return {
282+
"value": value,
283+
"source": ctx.get_parameter_source(param.name),
284+
}
285+
286+
@click.command()
287+
@click.option("--default", type=Source(), default="/tmp/file")
288+
@click.option("--nodefault", type=Source())
289+
def cli(default, nodefault):
290+
click.echo(f"default: {default}")
291+
click.echo(f"nodefault: {nodefault}")
292+
293+
result = runner.invoke(cli, [])
294+
assert not result.exception
295+
assert "default: {'value': '/tmp/file', 'source': " in result.output
296+
assert "'source': None}" not in result.output.split("default:")[1].split("\n")[0]
297+
assert (
298+
result.output == "default: {'value': '/tmp/file', 'source': "
299+
f"{ParameterSource.DEFAULT!r}}}\nnodefault: None\n"
300+
)
301+
302+
result = runner.invoke(cli, ["--default", "cli", "--nodefault", "also"])
303+
assert not result.exception
304+
assert (
305+
"default: {'value': 'cli', 'source': "
306+
f"{ParameterSource.COMMANDLINE!r}}}" in result.output
307+
)
308+
assert (
309+
"nodefault: {'value': 'also', 'source': "
310+
f"{ParameterSource.COMMANDLINE!r}}}" in result.output
311+
)
312+
313+
314+
def test_parameter_source_during_eager_callback(runner):
315+
"""``get_parameter_source()`` is available during eager callbacks.
316+
317+
Regression test for https://github.com/pallets/click/issues/3458.
318+
"""
319+
320+
def eager_cb(ctx, param, value):
321+
source = ctx.get_parameter_source(param.name)
322+
click.echo(f"callback source={source.name if source else None}")
323+
324+
@click.command()
325+
@click.option(
326+
"--flag/--no-flag",
327+
default=False,
328+
is_eager=True,
329+
callback=eager_cb,
330+
expose_value=False,
331+
)
332+
def cli():
333+
source = click.get_current_context().get_parameter_source("flag")
334+
click.echo(f"final source={source.name}")
335+
336+
result = runner.invoke(cli, [])
337+
assert not result.exception
338+
assert "callback source=DEFAULT" in result.output
339+
assert "final source=DEFAULT" in result.output
340+
341+
result = runner.invoke(cli, ["--flag"])
342+
assert not result.exception
343+
assert "callback source=COMMANDLINE" in result.output
344+
assert "final source=COMMANDLINE" in result.output
345+
346+
347+
def test_flask_debug_env_not_stomped_by_default_flag(runner, monkeypatch):
348+
"""Eager callback must not overwrite env when the flag used its default.
349+
350+
Covers the Flask ``_set_debug`` pattern (pallets/flask#6025). Regression test
351+
for https://github.com/pallets/click/issues/3458.
352+
"""
353+
354+
monkeypatch.delenv("APP_DEBUG", raising=False)
355+
356+
def set_debug(ctx, param, value):
357+
source = ctx.get_parameter_source(param.name)
358+
if source is not None and source in (
359+
ParameterSource.DEFAULT,
360+
ParameterSource.DEFAULT_MAP,
361+
):
362+
return None
363+
os.environ["APP_DEBUG"] = "1" if value else "0"
364+
return value
365+
366+
@click.command()
367+
@click.option(
368+
"--debug/--no-debug",
369+
default=False,
370+
is_eager=True,
371+
expose_value=False,
372+
callback=set_debug,
373+
)
374+
def cli():
375+
click.echo(f"APP_DEBUG={os.environ.get('APP_DEBUG', '')}")
376+
377+
monkeypatch.setenv("APP_DEBUG", "1")
378+
result = runner.invoke(cli, [])
379+
assert result.exit_code == 0
380+
assert result.output.strip() == "APP_DEBUG=1"
381+
382+
result = runner.invoke(cli, ["--debug"])
383+
assert result.exit_code == 0
384+
assert result.output.strip() == "APP_DEBUG=1"
385+
386+
result = runner.invoke(cli, ["--no-debug"])
387+
assert result.exit_code == 0
388+
assert result.output.strip() == "APP_DEBUG=0"
389+
390+
268391
def test_lookup_default_override_respected(runner):
269392
"""A subclass override of ``lookup_default()`` should be called by Click
270393
internals, not bypassed by a private method.

tests/test_options.py

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2895,6 +2895,68 @@ def cli(enable_xyz):
28952895
assert result.output == repr(expected)
28962896

28972897

2898+
@pytest.mark.parametrize(
2899+
("opts", "args", "invoke_kwargs", "expected_value", "expected_source"),
2900+
[
2901+
# https://github.com/pallets/click/issues/3458
2902+
pytest.param(
2903+
[
2904+
("--without-xyz", {"flag_value": False}),
2905+
("--with-xyz", {"flag_value": True, "default": True}),
2906+
],
2907+
[],
2908+
{},
2909+
True,
2910+
"DEFAULT",
2911+
id="explicit-default-wins",
2912+
),
2913+
pytest.param(
2914+
[
2915+
("--without-xyz", {"flag_value": False}),
2916+
("--with-xyz", {"flag_value": True, "default": True}),
2917+
],
2918+
["--without-xyz"],
2919+
{},
2920+
False,
2921+
"COMMANDLINE",
2922+
id="cmdline-wins",
2923+
),
2924+
pytest.param(
2925+
[
2926+
("--without-xyz", {"flag_value": False}),
2927+
("--with-xyz", {"flag_value": True, "default": True}),
2928+
],
2929+
["--without-xyz"],
2930+
{"default_map": {"enable_xyz": True}},
2931+
False,
2932+
"COMMANDLINE",
2933+
id="loser-default-map-restores-winner-source",
2934+
),
2935+
],
2936+
)
2937+
def test_bool_flag_group_parameter_source(
2938+
runner, opts, args, invoke_kwargs, expected_value, expected_source
2939+
):
2940+
"""``get_parameter_source()`` stays correct for feature-switch groups.
2941+
2942+
Regression test for https://github.com/pallets/click/issues/3458.
2943+
"""
2944+
2945+
@click.command()
2946+
@click.pass_context
2947+
def cli(ctx, enable_xyz):
2948+
source = ctx.get_parameter_source("enable_xyz")
2949+
click.echo(f"value={enable_xyz!r} source={source.name}")
2950+
2951+
for opt_name, opt_kwargs in opts:
2952+
cli = click.option(opt_name, "enable_xyz", **opt_kwargs)(cli)
2953+
2954+
result = runner.invoke(cli, args, **invoke_kwargs)
2955+
assert result.exit_code == 0, result.output
2956+
assert f"value={expected_value!r}" in result.output
2957+
assert f"source={expected_source}" in result.output
2958+
2959+
28982960
@pytest.mark.parametrize(
28992961
("opts", "args", "expected"),
29002962
[

0 commit comments

Comments
 (0)