Hide Sentinel.UNSET values as None in lookup_default() (#3224)

Author: Rowlando13

CHANGES.rst

@@ -4,6 +4,8 @@ Unreleased
 
 -   Fix handling of ``flag_value`` when ``is_flag=False`` to allow such options to be
     used without an explicit value. :issue:`3084`
+-   Hide ``Sentinel.UNSET`` values as ``None`` when using ``lookup_default()``.
+    :issue:`3136` :pr:`3199` :pr:`3202` :pr:`3209` :pr:`3212` :pr:`3224`
 
 Version 8.3.1
 --------------

src/click/core.py

@@ -706,14 +706,14 @@ def lookup_default(self, name: str, call: bool = True) -> t.Any | None:
             Added the ``call`` parameter.
         """
         if self.default_map is not None:
-            value = self.default_map.get(name, UNSET)
+            value = self.default_map.get(name)
 
             if call and callable(value):
                 return value()
 
             return value
 
-        return UNSET
+        return None
 
     def fail(self, message: str) -> t.NoReturn:
         """Aborts the execution of the program with a specific error
@@ -2278,9 +2278,12 @@ def get_default(
         .. versionchanged:: 8.0
             Added the ``call`` parameter.
         """
-        value = ctx.lookup_default(self.name, call=False)  # type: ignore
+        name = self.name
+        value = ctx.lookup_default(name, call=False) if name is not None else None
 
-        if value is UNSET:
+        if value is None and not (
+            ctx.default_map is not None and name is not None and name in ctx.default_map
+        ):
             value = self.default
 
         if call and callable(value):
@@ -2321,8 +2324,10 @@ def consume_value(
                 source = ParameterSource.ENVIRONMENT
 
         if value is UNSET:
-            default_map_value = ctx.lookup_default(self.name)  # type: ignore
-            if default_map_value is not UNSET:
+            default_map_value = ctx.lookup_default(self.name)  # type: ignore[arg-type]
+            if default_map_value is not None or (
+                ctx.default_map is not None and self.name in ctx.default_map
+            ):
                 value = default_map_value
                 source = ParameterSource.DEFAULT_MAP
 

tests/test_defaults.py

@@ -160,3 +160,152 @@ def prefers_red(color):
     assert "red" in result.output
     result = runner.invoke(prefers_red, ["--green"])
     assert "green" in result.output
+
+
+@pytest.mark.parametrize(
+    ("default_map", "key", "expected"),
+    [
+        # Key present in default_map.
+        ({"email": "a@b.com"}, "email", "a@b.com"),
+        # Key missing from default_map.
+        ({"email": "a@b.com"}, "nonexistent", None),
+        # No default_map at all / empty default_map.
+        (None, "anything", None),
+        ({}, "anything", None),
+        # Falsy values are returned as-is.
+        ({"key": None}, "key", None),
+        ({"key": 0}, "key", 0),
+        ({"key": ""}, "key", ""),
+        ({"key": False}, "key", False),
+    ],
+)
+def test_lookup_default_returns_hides_sentinel(default_map, key, expected):
+    """``lookup_default()`` should return ``None`` for missing keys, not :attr:`UNSET`.
+
+    Regression test for https://github.com/pallets/click/issues/3145.
+    """
+    cmd = click.Command("test")
+    ctx = click.Context(cmd)
+    if default_map is not None:
+        ctx.default_map = default_map
+    assert ctx.lookup_default(key) == expected
+
+
+def test_lookup_default_callable_in_default_map(runner):
+    """A callable in ``default_map`` is invoked with ``call=True``
+    (the default) and returned as-is with ``call=False``.
+
+    Click uses both paths internally:
+    - ``get_default()`` passes ``call=False``,
+    - ``resolve_ctx()`` passes ``call=True``.
+    """
+    factory = lambda: "lazy-value"  # noqa: E731
+
+    # Unit-level: call=True invokes, call=False returns as-is.
+    cmd = click.Command("test")
+    ctx = click.Context(cmd)
+    ctx.default_map = {"name": factory}
+    assert ctx.lookup_default("name", call=True) == "lazy-value"
+    assert ctx.lookup_default("name", call=False) is factory
+
+    # Integration: the callable is invoked during value resolution.
+    @click.command()
+    @click.option("--name", default="original", show_default=True)
+    @click.pass_context
+    def cli(ctx, name):
+        click.echo(f"name={name!r}")
+
+    result = runner.invoke(cli, [], default_map={"name": factory})
+    assert not result.exception
+    assert "name='lazy-value'" in result.output
+
+    # Help rendering gets the callable via call=False, so it
+    # shows "(dynamic)" rather than invoking it.
+    result = runner.invoke(cli, ["--help"], default_map={"name": factory})
+    assert not result.exception
+    assert "(dynamic)" in result.output
+
+
+@pytest.mark.parametrize(
+    ("args", "default_map", "expected_value", "expected_source"),
+    [
+        # CLI arg wins over everything.
+        (["--name", "cli"], {"name": "mapped"}, "cli", "COMMANDLINE"),
+        # default_map overrides parameter default.
+        ([], {"name": "mapped"}, "mapped", "DEFAULT_MAP"),
+        # Explicit None in default_map still counts as DEFAULT_MAP.
+        ([], {"name": None}, None, "DEFAULT_MAP"),
+        # Falsy values in default_map are not confused with missing keys.
+        ([], {"name": ""}, "", "DEFAULT_MAP"),
+        ([], {"name": 0}, "0", "DEFAULT_MAP"),
+        # No default_map falls back to parameter default.
+        ([], None, "original", "DEFAULT"),
+    ],
+)
+def test_default_map_source(runner, args, default_map, expected_value, expected_source):
+    """``get_parameter_source()`` reports the correct origin for a parameter
+    value across the resolution chain: CLI > default_map > parameter default.
+    """
+
+    @click.command()
+    @click.option("--name", default="original")
+    @click.pass_context
+    def cli(ctx, name):
+        source = ctx.get_parameter_source("name")
+        click.echo(f"name={name!r} source={source.name}")
+
+    kwargs = {}
+    if default_map is not None:
+        kwargs["default_map"] = default_map
+    result = runner.invoke(cli, args, **kwargs)
+    assert not result.exception
+    assert f"name={expected_value!r}" in result.output
+    assert f"source={expected_source}" in result.output
+
+
+def test_lookup_default_override_respected(runner):
+    """A subclass override of ``lookup_default()`` should be called by Click
+    internals, not bypassed by a private method.
+
+    Reproduce exactly https://github.com/pallets/click/issues/3145 in which a
+    subclass that falls back to prefix-based lookup when the parent returns
+    ``None``.
+
+    Previous attempts in https://github.com/pallets/click/pr/3199 were entirely
+    bypassing the user's overridded method.
+    """
+
+    class CustomContext(click.Context):
+        def lookup_default(self, name, call=True):
+            default = super().lookup_default(name, call=call)
+
+            if default is not None:
+                return default
+
+            # Prefix-based fallback: look up "app" sub-dict for "app_email".
+            prefix = name.split("_", 1)[0]
+            group = getattr(self, "default_map", None) or {}
+            sub = group.get(prefix)
+            if isinstance(sub, dict):
+                return sub.get(name)
+            return default
+
+    @click.command("get-views")
+    @click.option("--app-email", default="original", show_default=True)
+    @click.pass_context
+    def cli(ctx, app_email):
+        click.echo(f"app_email={app_email!r}")
+
+    cli.context_class = CustomContext
+    default_map = {"app": {"app_email": "prefix@example.com"}}
+
+    # resolve_ctx path: the override provides the runtime value.
+    result = runner.invoke(cli, [], default_map=default_map)
+    assert not result.exception
+    assert "app_email='prefix@example.com'" in result.output
+
+    # get_default path: the override is also used when
+    # rendering --help with show_default=True.
+    result = runner.invoke(cli, ["--help"], default_map=default_map)
+    assert not result.exception
+    assert "prefix@example.com" in result.output