Skip to content

Commit 4a35225

Browse files
authored
Reduce blast-radius of UNSET in default_map (#3240)
2 parents c7e1ba8 + c07bb93 commit 4a35225

3 files changed

Lines changed: 47 additions & 12 deletions

File tree

CHANGES.rst

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,9 @@ Unreleased
1313
``semver.Version``. :issue:`3298` :pr:`3299`
1414
- Fix pager test pollution under parallel execution by using pytest's
1515
``tmp_path`` fixture instead of a shared temporary file path. :pr:`3238`
16+
- Treat ``Sentinel.UNSET`` values in a ``default_map`` as absent, so they fall
17+
through to the next default source instead of being used as the value.
18+
:issue:`3224` :pr:`3240`
1619
- Patch ``pdb.Pdb`` in ``CliRunner`` isolation so ``pdb.set_trace()``,
1720
``breakpoint()``, and debuggers subclassing ``pdb.Pdb`` (ipdb, pdbpp) can
1821
interact with the real terminal instead of the captured I/O streams.

src/click/core.py

Lines changed: 24 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -698,6 +698,20 @@ def ensure_object(self, object_type: type[V]) -> V:
698698
self.obj = rv = object_type()
699699
return rv
700700

701+
def _default_map_has(self, name: str | None) -> bool:
702+
"""Check if :attr:`default_map` contains a real value for ``name``.
703+
704+
Returns ``False`` when the key is absent, the map is ``None``,
705+
``name`` is ``None``, or the stored value is the internal
706+
:data:`UNSET` sentinel.
707+
"""
708+
return (
709+
name is not None
710+
and self.default_map is not None
711+
and name in self.default_map
712+
and self.default_map[name] is not UNSET
713+
)
714+
701715
@t.overload
702716
def lookup_default(
703717
self, name: str, call: t.Literal[True] = True
@@ -718,15 +732,17 @@ def lookup_default(self, name: str, call: bool = True) -> t.Any | None:
718732
.. versionchanged:: 8.0
719733
Added the ``call`` parameter.
720734
"""
721-
if self.default_map is not None:
722-
value = self.default_map.get(name)
735+
if not self._default_map_has(name):
736+
return None
723737

724-
if call and callable(value):
725-
return value()
738+
# Assert to make the type checker happy.
739+
assert self.default_map is not None
740+
value = self.default_map[name]
726741

727-
return value
742+
if call and callable(value):
743+
return value()
728744

729-
return None
745+
return value
730746

731747
def fail(self, message: str) -> t.NoReturn:
732748
"""Aborts the execution of the program with a specific error
@@ -2294,9 +2310,7 @@ def get_default(
22942310
name = self.name
22952311
value = ctx.lookup_default(name, call=False) if name is not None else None
22962312

2297-
if value is None and not (
2298-
ctx.default_map is not None and name is not None and name in ctx.default_map
2299-
):
2313+
if value is None and not ctx._default_map_has(name):
23002314
value = self.default
23012315

23022316
if call and callable(value):
@@ -2338,9 +2352,7 @@ def consume_value(
23382352

23392353
if value is UNSET:
23402354
default_map_value = ctx.lookup_default(self.name) # type: ignore[arg-type]
2341-
if default_map_value is not None or (
2342-
ctx.default_map is not None and self.name in ctx.default_map
2343-
):
2355+
if default_map_value is not None or ctx._default_map_has(self.name):
23442356
value = default_map_value
23452357
source = ParameterSource.DEFAULT_MAP
23462358

tests/test_defaults.py

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
import click
44
from click import UNPROCESSED
5+
from click._utils import UNSET
56

67

78
@pytest.mark.parametrize(
@@ -354,3 +355,22 @@ def cli(value):
354355
result = runner.invoke(cli, args, **kwargs)
355356
assert result.exit_code == 0
356357
assert result.output == repr(expected)
358+
359+
360+
def test_unset_in_default_map(runner):
361+
"""An ``UNSET`` value in ``default_map`` should be treated as if
362+
the key is absent, and so fallback to the parameter's own default.
363+
364+
Refs: https://github.com/pallets/click/pull/3224#issuecomment-3968643305
365+
"""
366+
367+
@click.command(
368+
context_settings={"default_map": {"port": UNSET}},
369+
)
370+
@click.option("--port", default=8000)
371+
def cli(port):
372+
click.echo(f"port={port}")
373+
374+
result = runner.invoke(cli, [])
375+
assert result.exit_code == 0
376+
assert result.output.strip() == "port=8000"

0 commit comments

Comments
 (0)