Skip to content

Commit 18d2919

Browse files
committed
Ensure that 'type_cast_value' never gets 'UNSET'
In order to ensure that user-defined `type_cast_value` hooks are never called with a value of `UNSET`, it is necessary to intercept `UNSET` values and, in the particular cases in which `UNSET` needs special handling, preempt any call to `type_cast_value()`. `process_value()` is updated to check before calling `type_cast_value` and to skip the call entirely if an `UNSET` value is detected. Passing a value of `None` would not have the same effect for `click.Option`, which previously had logic which is specific to the `value is UNSET` case, and which cannot be reliably detected if a `None` value is passed in. This means that `type_cast_value()` is no longer guaranteed to be called during argument processing, but when it is called, it's guaranteed not to see an `UNSET` value. New tests explore this only from the perspective of checking if `UNSET` is ever passed, not in terms of pinning down when/if the hook gets called. resolves #3069
1 parent 27aaed3 commit 18d2919

File tree

4 files changed

+102
-9
lines changed

4 files changed

+102
-9
lines changed

CHANGES.rst

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,8 @@ Unreleased
1111
the ``Context.invoke()`` method. :issue:`3066` :issue:`3065` :pr:`3068`
1212
- Fix conversion of ``Sentinel.UNSET`` happening too early, which caused incorrect
1313
behavior for multiple parameters using the same name. :issue:`3071` :pr:`3079`
14+
- When ``Sentinel.UNSET`` is found during parsing, it will skip calls to
15+
``type_cast_value``. :issue:`3069`
1416

1517
Version 8.3.0
1618
--------------

src/click/core.py

Lines changed: 27 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2338,7 +2338,7 @@ def type_cast_value(self, ctx: Context, value: t.Any) -> t.Any:
23382338
"""Convert and validate a value against the parameter's
23392339
:attr:`type`, :attr:`multiple`, and :attr:`nargs`.
23402340
"""
2341-
if value in (None, UNSET):
2341+
if value is None:
23422342
if self.multiple or self.nargs == -1:
23432343
return ()
23442344
else:
@@ -2421,7 +2421,16 @@ def process_value(self, ctx: Context, value: t.Any) -> t.Any:
24212421
24222422
:meta private:
24232423
"""
2424-
value = self.type_cast_value(ctx, value)
2424+
# shelter `type_cast_value` from ever seeing an `UNSET` value by handling the
2425+
# cases in which `UNSET` gets special treatment explicitly at this layer
2426+
#
2427+
# Refs:
2428+
# https://github.com/pallets/click/issues/3069
2429+
if value is UNSET:
2430+
if self.multiple or self.nargs == -1:
2431+
value = ()
2432+
else:
2433+
value = self.type_cast_value(ctx, value)
24252434

24262435
if self.required and self.value_is_missing(value):
24272436
raise MissingParameter(ctx=ctx, param=self)
@@ -3256,13 +3265,22 @@ def consume_value(
32563265

32573266
return value, source
32583267

3259-
def type_cast_value(self, ctx: Context, value: t.Any) -> t.Any:
3260-
if self.is_flag and not self.required:
3261-
if value is UNSET:
3262-
if self.is_bool_flag:
3263-
# If the flag is a boolean flag, we return False if it is not set.
3264-
value = False
3265-
return super().type_cast_value(ctx, value)
3268+
def process_value(self, ctx: Context, value: t.Any) -> t.Any:
3269+
# process_value has to be overridden on Options in order to capture
3270+
# `value == UNSET` cases before `type_cast_value()` gets called.
3271+
#
3272+
# Refs:
3273+
# https://github.com/pallets/click/issues/3069
3274+
if self.is_flag and not self.required and self.is_bool_flag and value is UNSET:
3275+
value = False
3276+
3277+
if self.callback is not None:
3278+
value = self.callback(ctx, self, value)
3279+
3280+
return value
3281+
3282+
# in the normal case, rely on Parameter.process_value
3283+
return super().process_value(ctx, value)
32663284

32673285

32683286
class Argument(Parameter):

tests/test_arguments.py

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -579,3 +579,40 @@ def cli(one, two):
579579

580580
with pytest.warns(UserWarning):
581581
runner.invoke(cli, [])
582+
583+
584+
@pytest.mark.parametrize(
585+
("argument_kwargs", "pass_argv"),
586+
(
587+
# there is a large potential parameter space to explore here
588+
# this is just a very small sample of it
589+
({}, ["myvalue"]),
590+
({"nargs": -1}, []),
591+
({"nargs": -1}, ["myvalue"]),
592+
({"default": None}, ["myvalue"]),
593+
({"required": False}, []),
594+
({"required": False}, ["myvalue"]),
595+
),
596+
)
597+
def test_argument_custom_class_can_override_type_cast_value_and_never_sees_unset(
598+
runner, argument_kwargs, pass_argv
599+
):
600+
"""
601+
Test that overriding type_cast_value is supported
602+
603+
In particular, the argument is never passed an UNSET sentinel value.
604+
"""
605+
606+
class CustomArgument(click.Argument):
607+
def type_cast_value(self, ctx, value):
608+
assert value is not UNSET
609+
return value
610+
611+
@click.command()
612+
@click.argument("myarg", **argument_kwargs, cls=CustomArgument)
613+
def cmd(myarg):
614+
click.echo("ok")
615+
616+
result = runner.invoke(cmd, pass_argv)
617+
assert not result.exception
618+
assert result.exit_code == 0

tests/test_options.py

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1012,6 +1012,42 @@ def cmd(testoption):
10121012
assert "you wont see me" not in result.output
10131013

10141014

1015+
@pytest.mark.parametrize(
1016+
("param_decl", "option_kwargs", "pass_argv"),
1017+
(
1018+
# there is a large potential parameter space to explore here
1019+
# this is just a very small sample of it
1020+
("--opt", {}, []),
1021+
("--opt", {"multiple": True}, []),
1022+
("--opt", {"is_flag": True}, []),
1023+
("--opt/--no-opt", {"is_flag": True, "default": None}, []),
1024+
("--req", {"is_flag": True, "required": True}, ["--req"]),
1025+
),
1026+
)
1027+
def test_option_custom_class_can_override_type_cast_value_and_never_sees_unset(
1028+
runner, param_decl, option_kwargs, pass_argv
1029+
):
1030+
"""
1031+
Test that overriding type_cast_value is supported
1032+
1033+
In particular, the option is never passed an UNSET sentinel value.
1034+
"""
1035+
1036+
class CustomOption(click.Option):
1037+
def type_cast_value(self, ctx, value):
1038+
assert value is not UNSET
1039+
return value
1040+
1041+
@click.command()
1042+
@click.option("myparam", param_decl, **option_kwargs, cls=CustomOption)
1043+
def cmd(myparam):
1044+
click.echo("ok")
1045+
1046+
result = runner.invoke(cmd, pass_argv)
1047+
assert not result.exception
1048+
assert result.exit_code == 0
1049+
1050+
10151051
def test_option_custom_class_reusable(runner):
10161052
"""Ensure we can reuse a custom class option. See Issue #926"""
10171053

0 commit comments

Comments
 (0)