From 8f300853dc5f33120d9872ea0e4c9e00318446b1 Mon Sep 17 00:00:00 2001 From: Kevin Deldycke Date: Fri, 8 May 2026 15:36:45 +0200 Subject: [PATCH] Streamline option flag management --- CHANGES.md | 5 + src/click/core.py | 389 +++++++++++++++++++++++++++------------------- 2 files changed, 234 insertions(+), 160 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index e03b88ed9..c808c22d1 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -27,6 +27,11 @@ Unreleased {issue}`2819` {pr}`3678` - `unstyle` and the ANSI handling behind help-text wrapping now strip the full CSI escape-sequence grammar. +- Streamline `Option` flag handling: the flag-kind, type, lazy-default and + validation steps in `Option.__init__` move into focused helpers, and + `flag_value` and `default` keep their unset sentinel at construction + (resolved lazily on read) so `is UNSET` reliably tells a user-supplied value + from an auto-derived one. Behavior is unchanged. {pr}`3641` ## Version 8.4.2 diff --git a/src/click/core.py b/src/click/core.py index cc8fb47d8..b08f0437e 100644 --- a/src/click/core.py +++ b/src/click/core.py @@ -906,9 +906,8 @@ def invoke( else: ctx = self - with augment_usage_errors(self): - with ctx: - return callback(*args, **kwargs) + with augment_usage_errors(self), ctx: + return callback(*args, **kwargs) def forward(self, cmd: Command, /, *args: t.Any, **kwargs: t.Any) -> t.Any: """Similar to :meth:`invoke` but fills in default keyword @@ -1690,7 +1689,7 @@ class Group(Command): #: custom groups. #: #: .. versionadded:: 8.0 - group_class: type[Group] | type[type] | None = None + group_class: type[Group | type] | None = None # Literal[type] isn't valid, so use Type[type] commands: cabc.MutableMapping[str, Command] @@ -2364,6 +2363,14 @@ def __init__( f"{self.param_type_name} cannot be required." ) + @staticmethod + def _hide_unset(value: t.Any) -> t.Any: + """Present the internal :data:`UNSET` sentinel as ``None`` at a boundary that + exposes a parameter's value (introspection, prompts), keeping the sentinel an + implementation detail. + """ + return None if value is UNSET else value + def to_info_dict(self) -> dict[str, t.Any]: """Gather information that could be useful for a tool generating user-facing documentation. @@ -2385,10 +2392,7 @@ def to_info_dict(self) -> dict[str, t.Any]: "required": self.required, "nargs": self.nargs, "multiple": self.multiple, - # We explicitly hide the :attr:`UNSET` value to the user, as we choose to - # make it an implementation detail. And because ``to_info_dict`` has been - # designed for documentation purposes, we return ``None`` instead. - "default": self.default if self.default is not UNSET else None, + "default": self._hide_unset(self.default), "envvar": self.envvar, } @@ -2926,8 +2930,9 @@ class Option(Parameter): _flag_needs_value: bool is_flag: bool - is_bool_flag: bool flag_value: t.Any + type: types.ParamType[t.Any] + default: t.Any | t.Callable[[], t.Any] | None count: bool allow_from_autoenv: bool @@ -2964,6 +2969,8 @@ def __init__( param_decls, type=type, multiple=multiple, deprecated=deprecated, **attrs ) + # Phase 1: prompt-related attributes. ``_infer_flag_kind`` reads ``self.prompt`` + # and ``self.prompt_required`` so this must run first. if prompt is True: if not self.name: raise TypeError("'name' is required with 'prompt=True'.") @@ -2984,137 +2991,195 @@ def __init__( self.hide_input = hide_input self.hidden = hidden - # The _flag_needs_value property tells the parser that this option is a flag - # that cannot be used standalone and needs a value. With this information, the - # parser can determine whether to consider the next user-provided argument in - # the CLI as a value for this flag or as a new option. - # If prompt is enabled but not required, then it opens the possibility for the - # option to gets its value from the user. - self._flag_needs_value = self.prompt is not None and not self.prompt_required + # Phase 2: flag-kind inference. + self.is_flag, self._flag_needs_value = self._infer_flag_kind( + is_flag, flag_value + ) + + # Phase 3: type inference. Override the type set by :meth:`Parameter.__init__` + # when this option is a flag or a count. + self.type = self._pick_type(type, flag_value, count, self.is_flag) + + # Phase 4: store the raw ``flag_value`` and ``count`` settings. + # ``self.flag_value`` and ``self.default`` deliberately keep the :data:`UNSET` + # sentinel when the user didn't pass them. Auto-derived values are resolved + # lazily in :meth:`_resolve_lazy_default` and :attr:`flag_activation_value`. + # Keeping the raw values means ``is UNSET`` reliably answers "did the user pass + # this?": #3403 needs it for arbitration, and any future feature needing the + # same distinction can reuse it without reintroducing parallel "was it + # explicit?" tracking. + self.flag_value = flag_value + self.count = count + if count and self.default is UNSET: + self.default = 0 + + self.allow_from_autoenv = allow_from_autoenv + self.help = help + self.show_default = show_default + self.show_choices = show_choices + self.show_envvar = show_envvar + + # Phase 5: validate. Raises on illegal kwarg combinations. + self._validate(prompt, deprecated) + + @property + def is_bool_flag(self) -> bool: + """``True`` when this option is a flag with a boolean type. + + Derived from :attr:`is_flag` and :attr:`type`; computed on access so it cannot + drift if a subclass replaces :attr:`type` after construction. + """ + return self.is_flag and isinstance(self.type, types.BoolParamType) + + @property + def flag_activation_value(self) -> t.Any: + """Value the function receives when this flag is activated on the command line. + + Resolves a missing :attr:`flag_value` to ``True`` for actual flag options and + ``None`` otherwise. Used by the parser bridge (:meth:`add_to_parser`) and the + runtime (:meth:`consume_value`) so :attr:`flag_value` can keep the :data:`UNSET` + sentinel for "did the user pass one?" introspection. + """ + if self.flag_value is UNSET: + return True if self.is_flag else None + return self.flag_value + + def _infer_flag_kind( + self, is_flag: bool | None, flag_value: t.Any + ) -> tuple[bool, bool]: + """Resolve ``is_flag`` and the parser hint ``_flag_needs_value``. + + Returns ``(is_flag, flag_needs_value)``, where ``_flag_needs_value`` tells the + parser this option is a flag that cannot be used standalone and needs a value. + The parser uses it to decide whether to treat the next CLI token as the flag's + value or as a new option. If ``prompt`` is enabled with + ``prompt_required=False``, it opens the door for an interactive value, hence the + initial condition. Ref: https://github.com/pallets/click/issues/3084 + """ + needs_value = self.prompt is not None and not self.prompt_required - # Auto-detect if this is a flag or not. if is_flag is None: # Implicitly a flag because flag_value was set. if flag_value is not UNSET: - is_flag = True + return True, needs_value # Not a flag, but when used as a flag it shows a prompt. - elif self._flag_needs_value: - is_flag = False + if needs_value: + return False, needs_value # Implicitly a flag because secondary options names were given. - elif self.secondary_opts: - is_flag = True - - # The option is explicitly not a flag, but to determine whether or not it needs - # value, we need to check if `flag_value` or `default` was set. Either one is - # sufficient. - # Ref: https://github.com/pallets/click/issues/3084 - elif is_flag is False and not self._flag_needs_value: - self._flag_needs_value = flag_value is not UNSET or self.default is UNSET - - if is_flag: - # Set missing default for flags if not explicitly required or prompted. - if self.default is UNSET and not self.required and not self.prompt: - if multiple: - self.default = () - - # Auto-detect the type of the flag based on the flag_value. - if type is None: - # A flag without a flag_value is a boolean flag. - if flag_value is UNSET: - self.type: types.ParamType[t.Any] = types.BoolParamType() - # If the flag value is a boolean, use BoolParamType. - elif isinstance(flag_value, bool): - self.type = types.BoolParamType() - # Otherwise, guess the type from the flag value. - else: - guessed = types.convert_type(None, flag_value) - if ( - isinstance(guessed, types.StringParamType) - and not isinstance(flag_value, str) - and flag_value is not None - ): - # The flag_value type couldn't be auto-detected - # (not str, int, float, or bool). Since flag_value - # is a programmer-provided Python object, not CLI - # input, pass it through unchanged instead of - # stringifying it. - self.type = types.UNPROCESSED - else: - self.type = guessed - - self.is_flag = bool(is_flag) - self.is_bool_flag = self.is_flag and isinstance(self.type, types.BoolParamType) - self.flag_value = flag_value + if self.secondary_opts: + return True, needs_value + return False, needs_value - # Set boolean flag default to False if unset and not required. - if self.is_bool_flag: - if self.default is UNSET and not self.required: - self.default = False + if is_flag is False and not needs_value: + # Explicit ``is_flag=False`` with a flag-like value/default still makes the + # option flag-shaped to the parser. + needs_value = flag_value is not UNSET or self.default is UNSET - # The alignment of default to the flag_value is resolved lazily in - # get_default() to prevent callable flag_values (like classes) from - # being instantiated. Refs: - # https://github.com/pallets/click/issues/3121 - # https://github.com/pallets/click/issues/3024#issuecomment-3146199461 - # https://github.com/pallets/click/pull/3030/commits/06847da + return bool(is_flag), needs_value - # Set the default flag_value if it is not set. - if self.flag_value is UNSET: - if self.is_flag: - self.flag_value = True - else: - self.flag_value = None + def _pick_type( + self, + type_arg: t.Any, + flag_value: t.Any, + count: bool, + is_flag: bool, + ) -> types.ParamType[t.Any]: + """Pick the final :class:`ParamType` for this option. + + :meth:`Parameter.__init__` already stored ``self.type`` from the explicit + ``type`` argument or from ``default``. This method either returns that as-is, or + overrides it for flag and count options (which are inferred from ``flag_value`` + rather than ``default``). + """ + if type_arg is not None: + return self.type - # Counting. - self.count = count if count: - if type is None: - self.type = types.IntRange(min=0) - if self.default is UNSET: - self.default = 0 - - self.allow_from_autoenv = allow_from_autoenv - self.help = help - self.show_default = show_default - self.show_choices = show_choices - self.show_envvar = show_envvar + return types.IntRange(min=0) - if __debug__: - if deprecated and prompt: - raise ValueError("`deprecated` options cannot use `prompt`.") - - if self.nargs == -1: - raise TypeError("nargs=-1 is not supported for options.") + if not is_flag: + return self.type - if not self.is_bool_flag and self.secondary_opts: - raise TypeError("Secondary flag is not valid for non-boolean flag.") + # A flag without a flag_value is a boolean flag. + if flag_value is UNSET or isinstance(flag_value, bool): + return types.BoolParamType() - if self.is_bool_flag and self.hide_input and self.prompt is not None: - raise TypeError( - "'prompt' with 'hide_input' is not valid for boolean flag." - ) + guessed: types.ParamType[t.Any] = types.convert_type(None, flag_value) + if ( + isinstance(guessed, types.StringParamType) + and not isinstance(flag_value, str) + and flag_value is not None + ): + # The flag_value type couldn't be auto-detected (not str, int, float, or + # bool). Since flag_value is a programmer-provided Python object, not CLI + # input, pass it through unchanged instead of stringifying it. + return types.UNPROCESSED + return guessed + + def _resolve_lazy_default(self, value: t.Any) -> t.Any: + """Apply lazy auto-derivations to a default-style value. + + Shared between :meth:`get_default` (the runtime path) and :meth:`to_info_dict` + (the introspection path) so the two views cannot drift apart. Callables are + *not* invoked here: that is :meth:`get_default`'s responsibility. Rules: + + * ``UNSET`` resolves to ``False`` for a non-required boolean flag, and to ``()`` + for a non-required, non-prompted multi flag. + * ``True`` resolves to :attr:`flag_value` for a non-boolean flag (the "activate + this flag by default" shorthand). Boolean flags keep ``True`` as a literal. + """ + if value is UNSET and self.is_flag: + if self.multiple and not self.required and not self.prompt: + return () + if self.is_bool_flag and not self.required: + return False + if value is True and self.is_flag and not self.is_bool_flag: + # Use ``flag_activation_value`` so an unset ``flag_value`` resolves to + # ``True`` (the bool-flag activation value) rather than leaking the + # :data:`UNSET` sentinel. + return self.flag_activation_value + return value - if self.count: - if self.multiple: - raise TypeError("'count' is not valid with 'multiple'.") + def _validate(self, prompt: bool | str, deprecated: bool | str) -> None: + """Raise :class:`TypeError` / :class:`ValueError` on illegal kwarg combinations. - if self.is_flag: - raise TypeError("'count' is not valid with 'is_flag'.") + Called once, after every other attribute has been assigned, so each check can + read the final state. + """ + if not __debug__: + return + if deprecated and prompt: + raise ValueError("`deprecated` options cannot use `prompt`.") + if self.nargs == -1: + raise TypeError("nargs=-1 is not supported for options.") + if not self.is_bool_flag and self.secondary_opts: + raise TypeError("Secondary flag is not valid for non-boolean flag.") + if self.is_bool_flag and self.hide_input and self.prompt is not None: + raise TypeError("'prompt' with 'hide_input' is not valid for boolean flag.") + if self.count: + if self.multiple: + raise TypeError("'count' is not valid with 'multiple'.") + if self.is_flag: + raise TypeError("'count' is not valid with 'is_flag'.") def to_info_dict(self) -> dict[str, t.Any]: """ + .. versionchanged:: 8.5.0 + ``default`` and ``flag_value`` reflect the auto-derived values (``False`` + for unset boolean-flag defaults, ``True`` for unset boolean-flag activation + values, etc.) when no explicit value was passed, matching what the function + would receive at call time. + .. versionchanged:: 8.3.0 Returns ``None`` for the :attr:`flag_value` if it was not set. """ info_dict = super().to_info_dict() info_dict.update( + default=self._hide_unset(self._resolve_lazy_default(self.default)), help=self.help, prompt=self.prompt, is_flag=self.is_flag, - # We explicitly hide the :attr:`UNSET` value to the user, as we choose to - # make it an implementation detail. And because ``to_info_dict`` has been - # designed for documentation purposes, we return ``None`` instead. - flag_value=self.flag_value if self.flag_value is not UNSET else None, + flag_value=self.flag_activation_value, count=self.count, hidden=self.hidden, ) @@ -3125,32 +3190,33 @@ def get_default( ) -> t.Any | t.Callable[[], t.Any] | None: """Return the default value for this option. - For non-boolean flag options, ``default=True`` is treated as a sentinel - meaning "activate this flag by default" and is resolved to - :attr:`flag_value`. For example, with ``--upper/--lower`` feature - switches where ``flag_value="upper"`` and ``default=True``, the default - resolves to ``"upper"``. + Several auto-derived defaults are resolved lazily here rather than eagerly in + :meth:`__init__`. This keeps :attr:`default` raw at construction time, so + ``self.default is UNSET`` reliably answers "did the user pass a default?" for + feature-switch-group arbitration and for anything else that needs to distinguish + "absent" from a chosen value. The resolution rules live in + :meth:`_resolve_lazy_default`. - .. caution:: - This substitution only applies to non-boolean flags - (:attr:`is_bool_flag` is ``False``). For boolean flags, ``True`` is - a legitimate Python value and ``default=True`` is returned as-is. + .. versionchanged:: 8.5.0 + ``UNSET`` defaults for boolean and multi flags are now resolved here instead + of being coerced in :meth:`__init__`. Reading :attr:`default` directly + returns the user-supplied value (or ``UNSET`` if none was passed) rather + than the auto-derived one. .. versionchanged:: 8.3.3 - ``default=True`` is no longer substituted with ``flag_value`` for - boolean flags, fixing negative boolean flags like + ``default=True`` is no longer substituted with ``flag_value`` for boolean + flags, fixing negative boolean flags like ``flag_value=False, default=True``. """ - value = super().get_default(ctx, call=False) - - # Resolve default=True to flag_value lazily (here instead of - # __init__) to prevent callable flag_values (like classes) from - # being instantiated by the callable check below. - if value is True and self.is_flag and not self.is_bool_flag: - value = self.flag_value - elif call and callable(value): + raw = super().get_default(ctx, call=False) + value = self._resolve_lazy_default(raw) + # Only invoke the value as a callable when the lazy resolver passed it through + # unchanged. If the resolver substituted ``True`` -> :attr:`flag_value`, the + # result is the programmer-supplied ``flag_value`` (often a class or enum), + # which must NOT be instantiated here. See + # https://github.com/pallets/click/issues/3121. + if value is raw and call and callable(value): value = value() - return value def get_error_hint(self, ctx: Context | None) -> str: @@ -3248,7 +3314,10 @@ def add_to_parser(self, parser: _OptionParser, ctx: Context) -> None: opts=self.opts, dest=self.name, action=action, - const=self.flag_value, + # ``flag_activation_value`` resolves UNSET to the right parser-store + # constant (``True`` for bool flags) so the raw :attr:`flag_value` + # can keep the sentinel. + const=self.flag_activation_value, ) else: parser.add_option( @@ -3429,7 +3498,7 @@ def prompt_for_value(self, ctx: Context) -> t.Any: self.prompt, # Use ``None`` to inform the prompt() function to reiterate until a valid # value is provided by the user if we have no default. - default=None if default is UNSET else default, + default=self._hide_unset(default), type=self.type, hide_input=self.hide_input, show_choices=self.show_choices, @@ -3485,13 +3554,19 @@ def value_from_envvar(self, ctx: Context) -> t.Any: # flag, its envvar value still needs to be analyzed to determine if the flag is # activated or not. if self.is_flag and not self.is_bool_flag: - # If the flag_value is set and match the envvar value, return it - # directly. - if self.flag_value is not UNSET and rv == self.flag_value: + # An exact match against ``flag_value`` (a non-bool flag always has one + # explicitly set) returns it directly. Otherwise the value is analyzed as a + # boolean: a truthy reading activates the flag and the function receives + # ``flag_value``; a falsy reading produces ``False``; an unrecognized + # reading falls through as ``None``. Folding the substitution here means + # :meth:`consume_value` no longer has to repeat it in a source-checked + # branch. + if rv == self.flag_value: return self.flag_value - # Analyze the envvar value as a boolean to know if the flag is - # activated or not. - return types.BoolParamType.str_to_bool(rv) + parsed = types.BoolParamType.str_to_bool(rv) + if parsed is None: + return None + return self.flag_value if parsed else False # Split the envvar value if it is allowed to be repeated. value_depth = (self.nargs != 1) + bool(self.multiple) @@ -3518,31 +3593,22 @@ def consume_value( """ value, source = super().consume_value(ctx, opts) - # The parser will emit a sentinel value if the option is allowed to as a flag - # without a value. + # The parser emits a sentinel when a flag is allowed to be used without a value. + # Resolve it to a prompt or to the activation value depending on the option's + # configuration. if value is FLAG_NEEDS_VALUE: - # If the option allows for a prompt, we start an interaction with the user. + # If the option allows for a prompt, start an interaction with the user. if self.prompt is not None and not ctx.resilient_parsing: value = self.prompt_for_value(ctx) source = ParameterSource.PROMPT - # Else the flag takes its flag_value as value. + # Else the flag takes its activation value (resolves UNSET). else: - value = self.flag_value + value = self.flag_activation_value source = ParameterSource.COMMANDLINE - # A flag which is activated always returns the flag value, unless the value - # comes from the explicitly sets default. - elif ( - self.is_flag - and value is True - and not self.is_bool_flag - and source < ParameterSource.DEFAULT_MAP - ): - value = self.flag_value - - # Re-interpret a multiple option which has been sent as-is by the parser. - # Here we replace each occurrence of value-less flags (marked by the - # FLAG_NEEDS_VALUE sentinel) with the flag_value. + # Re-interpret a multiple option that the parser sent through as a list still + # containing the FLAG_NEEDS_VALUE sentinel, replacing each occurrence with the + # activation value. elif ( self.multiple and value is not UNSET @@ -3550,7 +3616,10 @@ def consume_value( and source < ParameterSource.DEFAULT_MAP and any(v is FLAG_NEEDS_VALUE for v in value) ): - value = [self.flag_value if v is FLAG_NEEDS_VALUE else v for v in value] + value = [ + self.flag_activation_value if v is FLAG_NEEDS_VALUE else v + for v in value + ] source = ParameterSource.COMMANDLINE # The value wasn't set, or used the param's default, prompt for one to the user