Skip to content

Commit fe2f919

Browse files
committed
Hide Sentinel.UNSET values as None in lookup_default()
Closes: #3145
1 parent cdab890 commit fe2f919

3 files changed

Lines changed: 161 additions & 4 deletions

File tree

CHANGES.rst

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@ Unreleased
44

55
- Fix handling of ``flag_value`` when ``is_flag=False`` to allow such options to be
66
used without an explicit value. :issue:`3084`
7+
- Hide ``Sentinel.UNSET`` values as ``None`` when using ``lookup_default()``.
8+
:issue:`3136` :pr:`3199` :pr:`3209` :pr:`3212`
79

810
Version 8.3.1
911
--------------

src/click/core.py

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -706,14 +706,14 @@ def lookup_default(self, name: str, call: bool = True) -> t.Any | None:
706706
Added the ``call`` parameter.
707707
"""
708708
if self.default_map is not None:
709-
value = self.default_map.get(name, UNSET)
709+
value = self.default_map.get(name)
710710

711711
if call and callable(value):
712712
return value()
713713

714714
return value
715715

716-
return UNSET
716+
return None
717717

718718
def fail(self, message: str) -> t.NoReturn:
719719
"""Aborts the execution of the program with a specific error
@@ -2280,7 +2280,9 @@ def get_default(
22802280
"""
22812281
value = ctx.lookup_default(self.name, call=False) # type: ignore
22822282

2283-
if value is UNSET:
2283+
if value is None and not (
2284+
ctx.default_map is not None and self.name in ctx.default_map # type: ignore[operator]
2285+
):
22842286
value = self.default
22852287

22862288
if call and callable(value):
@@ -2322,7 +2324,9 @@ def consume_value(
23222324

23232325
if value is UNSET:
23242326
default_map_value = ctx.lookup_default(self.name) # type: ignore
2325-
if default_map_value is not UNSET:
2327+
if default_map_value is not None or (
2328+
ctx.default_map is not None and self.name in ctx.default_map # type: ignore[operator]
2329+
):
23262330
value = default_map_value
23272331
source = ParameterSource.DEFAULT_MAP
23282332

tests/test_defaults.py

Lines changed: 151 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
import pytest
2+
13
import click
24

35

@@ -110,3 +112,152 @@ def prefers_red(color):
110112
assert "red" in result.output
111113
result = runner.invoke(prefers_red, ["--green"])
112114
assert "green" in result.output
115+
116+
117+
@pytest.mark.parametrize(
118+
("default_map", "key", "expected"),
119+
[
120+
# Key present in default_map.
121+
({"email": "a@b.com"}, "email", "a@b.com"),
122+
# Key missing from default_map.
123+
({"email": "a@b.com"}, "nonexistent", None),
124+
# No default_map at all / empty default_map.
125+
(None, "anything", None),
126+
({}, "anything", None),
127+
# Falsy values are returned as-is.
128+
({"key": None}, "key", None),
129+
({"key": 0}, "key", 0),
130+
({"key": ""}, "key", ""),
131+
({"key": False}, "key", False),
132+
],
133+
)
134+
def test_lookup_default_returns_hides_sentinel(default_map, key, expected):
135+
"""``lookup_default()`` should return ``None`` for missing keys, not :attr:`UNSET`.
136+
137+
Regression test for https://github.com/pallets/click/issues/3145.
138+
"""
139+
cmd = click.Command("test")
140+
ctx = click.Context(cmd)
141+
if default_map is not None:
142+
ctx.default_map = default_map
143+
assert ctx.lookup_default(key) == expected
144+
145+
146+
def test_lookup_default_callable_in_default_map(runner):
147+
"""A callable in ``default_map`` is invoked with ``call=True``
148+
(the default) and returned as-is with ``call=False``.
149+
150+
Click uses both paths internally:
151+
- ``get_default()`` passes ``call=False``,
152+
- ``resolve_ctx()`` passes ``call=True``.
153+
"""
154+
factory = lambda: "lazy-value" # noqa: E731
155+
156+
# Unit-level: call=True invokes, call=False returns as-is.
157+
cmd = click.Command("test")
158+
ctx = click.Context(cmd)
159+
ctx.default_map = {"name": factory}
160+
assert ctx.lookup_default("name", call=True) == "lazy-value"
161+
assert ctx.lookup_default("name", call=False) is factory
162+
163+
# Integration: the callable is invoked during value resolution.
164+
@click.command()
165+
@click.option("--name", default="original", show_default=True)
166+
@click.pass_context
167+
def cli(ctx, name):
168+
click.echo(f"name={name!r}")
169+
170+
result = runner.invoke(cli, [], default_map={"name": factory})
171+
assert not result.exception
172+
assert "name='lazy-value'" in result.output
173+
174+
# Help rendering gets the callable via call=False, so it
175+
# shows "(dynamic)" rather than invoking it.
176+
result = runner.invoke(cli, ["--help"], default_map={"name": factory})
177+
assert not result.exception
178+
assert "(dynamic)" in result.output
179+
180+
181+
@pytest.mark.parametrize(
182+
("args", "default_map", "expected_value", "expected_source"),
183+
[
184+
# CLI arg wins over everything.
185+
(["--name", "cli"], {"name": "mapped"}, "cli", "COMMANDLINE"),
186+
# default_map overrides parameter default.
187+
([], {"name": "mapped"}, "mapped", "DEFAULT_MAP"),
188+
# Explicit None in default_map still counts as DEFAULT_MAP.
189+
([], {"name": None}, None, "DEFAULT_MAP"),
190+
# Falsy values in default_map are not confused with missing keys.
191+
([], {"name": ""}, "", "DEFAULT_MAP"),
192+
([], {"name": 0}, "0", "DEFAULT_MAP"),
193+
# No default_map falls back to parameter default.
194+
([], None, "original", "DEFAULT"),
195+
],
196+
)
197+
def test_default_map_source(runner, args, default_map, expected_value, expected_source):
198+
"""``get_parameter_source()`` reports the correct origin for a parameter
199+
value across the resolution chain: CLI > default_map > parameter default.
200+
"""
201+
202+
@click.command()
203+
@click.option("--name", default="original")
204+
@click.pass_context
205+
def cli(ctx, name):
206+
source = ctx.get_parameter_source("name")
207+
click.echo(f"name={name!r} source={source.name}")
208+
209+
kwargs = {}
210+
if default_map is not None:
211+
kwargs["default_map"] = default_map
212+
result = runner.invoke(cli, args, **kwargs)
213+
assert not result.exception
214+
assert f"name={expected_value!r}" in result.output
215+
assert f"source={expected_source}" in result.output
216+
217+
218+
def test_lookup_default_override_respected(runner):
219+
"""A subclass override of ``lookup_default()`` should be called by Click
220+
internals, not bypassed by a private method.
221+
222+
Reproduce exactly https://github.com/pallets/click/issues/3145 in which a
223+
subclass that falls back to prefix-based lookup when the parent returns
224+
``None``.
225+
226+
Previous attempts in https://github.com/pallets/click/pr/3199 were entirely
227+
bypassing the user's overridded method.
228+
"""
229+
230+
class CustomContext(click.Context):
231+
def lookup_default(self, name, call=True):
232+
default = super().lookup_default(name, call=call)
233+
234+
if default is not None:
235+
return default
236+
237+
# Prefix-based fallback: look up "app" sub-dict for "app_email".
238+
prefix = name.split("_", 1)[0]
239+
group = getattr(self, "default_map", None) or {}
240+
sub = group.get(prefix)
241+
if isinstance(sub, dict):
242+
return sub.get(name)
243+
return default
244+
245+
@click.command("get-views")
246+
@click.option("--app-email", default="original", show_default=True)
247+
@click.pass_context
248+
def cli(ctx, app_email):
249+
click.echo(f"app_email={app_email!r}")
250+
251+
cli.context_class = CustomContext
252+
default_map = {"app": {"app_email": "prefix@example.com"}}
253+
254+
# resolve_ctx path: the override provides the runtime value.
255+
result = runner.invoke(cli, [], default_map=default_map)
256+
assert not result.exception
257+
assert "app_email='prefix@example.com'" in result.output
258+
259+
# get_default path: the override is also used when
260+
# rendering --help with show_default=True.
261+
result = runner.invoke(cli, ["--help"], default_map=default_map)
262+
assert not result.exception
263+
assert "prefix@example.com" in result.output

0 commit comments

Comments
 (0)