From 8cf8d62897aadd775ed6ede5f7800050b36b4eda Mon Sep 17 00:00:00 2001 From: Kelvin Chung Date: Mon, 15 Jun 2026 14:59:31 +0100 Subject: [PATCH 1/2] feat(annotated): allow honoring Enum _missing_ and support Enum | Enum --- CHANGELOG.md | 9 ++ cmd2/annotated.py | 134 +++++++++++++++++++++----- docs/features/annotated.md | 67 ++++++++++++- tests/test_annotated.py | 186 +++++++++++++++++++++++++++++++++++++ 4 files changed, 370 insertions(+), 26 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 785ad5d8c..10efe3c47 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,6 +23,15 @@ when the class is defined instead of being deferred to first command use where the error was swallowed. The checks read parameter names only, so forward-referenced annotations still decorate cleanly. + - `Argument`/`Option` accept a new `allow_unknown_entry` flag for `Enum` parameters. When set, + a command-line token matched by neither a member value nor name is routed through the enum's + own [`_missing_`](https://docs.python.org/3/library/enum.html#enum.Enum._missing_) hook, so an + enum can resolve aliases, alternate spellings, or special keywords. A token that `_missing_` + declines (returns `None`) is still rejected. + - `@with_annotated` now supports a union of `Enum` subclasses (e.g. `EnumA | EnumB`). Each + member keeps its own converter and a token resolves to the first member that accepts it, so + when two members share a representation the earlier one in the union wins. Unions containing a + `Literal` or any non-`Enum` member are still rejected as ambiguous. ## 4.0.0 (June 5, 2026) diff --git a/cmd2/annotated.py b/cmd2/annotated.py index dc9ca1ad7..44c946550 100644 --- a/cmd2/annotated.py +++ b/cmd2/annotated.py @@ -54,6 +54,9 @@ def do_paint( - positional ``bool`` -- parsed from ``true/false``, ``yes/no``, ``on/off``, ``1/0`` - ``pathlib.Path`` -- sets ``type=Path`` - ``enum.Enum`` subclass -- ``type=converter``, ``choices`` from member values +- a union of Enums (e.g. ``EnumA | EnumB``) -- each member keeps its own converter; a token resolves + to the first member that accepts it, and the merged ``choices`` are the concatenation of each + member's choices - ``decimal.Decimal`` -- sets ``type=Decimal`` - ``Literal[...]`` -- ``type=converter`` and ``choices`` from the literal values - ``list[T]`` / ``set[T]`` / ``frozenset[T]`` / ``tuple[T, ...]`` -- ``nargs='+'`` (or ``'*'`` with a default or ``| None``) @@ -150,7 +153,8 @@ class is defined rather than on first command use. The one group rule that need or any custom class), which would silently arrive as a plain string. Supported scalars are ``str``, ``int``, ``float``, ``bool``, ``decimal.Decimal``, ``pathlib.Path``, ``enum.Enum`` subclasses, and ``Literal[...]`` (``str``/``Any``/``object`` pass through raw) -- ``str | int`` -- a union of multiple non-None types is ambiguous +- ``str | int`` -- a union of multiple non-None types is ambiguous (unless every member is an + ``enum.Enum`` subclass, which resolves by trying each member's converter in turn) - ``tuple[int, str, float]`` -- mixed element types (argparse applies one ``type=`` per argument) - ``*args: tuple[T, ...]`` (or any collection element) -- the annotation is each value's type, so a collection element means a tuple-of-collections; annotate the element, e.g. ``*args: str`` @@ -193,6 +197,7 @@ class is defined rather than on first command use. The one group rule that need import enum import functools import inspect +import operator import types from collections.abc import ( Callable, @@ -324,6 +329,7 @@ def __init__( suppress_tab_hint: bool | None = None, const: Any = _UNSET, default: Any = _UNSET, + allow_unknown_entry: bool = False, **extra_kwargs: Any, ) -> None: """Initialise shared metadata fields. @@ -331,8 +337,10 @@ def __init__( ``const`` is the value stored on a present flag with no argument (``Option`` only: ``store_const``/``append_const``); ``_UNSET`` distinguishes "no const" from ``const=None``. ``default`` mirrors the signature default (``Option(default=v)`` == ``... = v``); supplying - both, or ``argparse.SUPPRESS``, is rejected. ``extra_kwargs`` forwards any other - ``add_argument`` parameter (incl. those from + both, or ``argparse.SUPPRESS``, is rejected. ``allow_unknown_entry`` only affects ``Enum`` + annotations: when set, a token matched by neither a member value nor name is routed through + the enum's ``_missing_`` hook (for aliases / special keywords) instead of being rejected + outright. ``extra_kwargs`` forwards any other ``add_argument`` parameter (incl. those from [`register_argparse_argument_parameter`][cmd2.argparse_utils.register_argparse_argument_parameter]) straight through. """ reserved = self._RESERVED_EXTRA_KWARGS & extra_kwargs.keys() @@ -360,6 +368,7 @@ def __init__( self.suppress_tab_hint = suppress_tab_hint self.const = const self.default = default + self.allow_unknown_entry = allow_unknown_entry self.extra_kwargs = extra_kwargs def to_kwargs(self) -> dict[str, Any]: @@ -489,6 +498,25 @@ def _parse_bool(value: str) -> bool: raise argparse.ArgumentTypeError(f"invalid boolean value: {value!r} (choose from: 1, 0, true, false, yes, no, on, off)") +def _choice_text(choice: Any) -> str: + """Command-line spelling of a choice (the ``CompletionItem`` text, else ``str``).""" + return choice.text if isinstance(choice, CompletionItem) else str(choice) + + +def _invalid_choice(value: str, choices: Iterable[Any]) -> argparse.ArgumentTypeError: + """Build the standard 'invalid choice' rejection, de-duplicating the listed choices.""" + valid = ", ".join(dict.fromkeys(_choice_text(c) for c in choices)) + return argparse.ArgumentTypeError(f"invalid choice: {value!r} (choose from {valid})") + + +def _dedupe_choices(choices: Iterable[Any]) -> list[Any]: + """Drop choices that share a command-line spelling, keeping the first occurrence.""" + by_text: dict[str, Any] = {} + for choice in choices: + by_text.setdefault(_choice_text(choice), choice) + return list(by_text.values()) + + def _make_literal_type(literal_values: list[Any]) -> Callable[[str], Any]: """Create an argparse converter for a Literal's exact values.""" value_map: dict[str, Any] = {} @@ -516,17 +544,20 @@ def _convert(value: str) -> Any: if type(v) is bool and v == bool_value: return bool_value - valid = ", ".join(str(v) for v in literal_values) - raise argparse.ArgumentTypeError(f"invalid choice: {value!r} (choose from {valid})") + raise _invalid_choice(value, literal_values) _convert.__name__ = "literal" return _convert -def _make_enum_type(enum_class: type[enum.Enum]) -> Callable[[str], enum.Enum]: +def _make_enum_type(enum_class: type[enum.Enum], *, allow_unknown_entry: bool = False) -> Callable[[str], enum.Enum]: """Create an argparse *type* converter for an Enum class. - Accepts both member *values* and member *names*. + Accepts both member *values* and member *names*. When ``allow_unknown_entry`` is set, a token + matched by neither is passed to the enum's own ``_missing_`` hook so it can resolve aliases, + alternate spellings, or special keywords; a token ``_missing_`` declines to claim (returns + ``None``) is still rejected. An enum that does not override ``_missing_`` inherits the default + (which returns ``None``), so the flag is simply inert for it. """ _value_map = {str(m.value): m for m in enum_class} @@ -536,9 +567,15 @@ def _convert(value: str) -> enum.Enum: return member try: return enum_class[value] - except KeyError as err: - valid = ", ".join(_value_map) - raise argparse.ArgumentTypeError(f"invalid choice: {value!r} (choose from {valid})") from err + except KeyError: + pass + if allow_unknown_entry: + # Call _missing_ directly so its return is honored and any error it raises propagates + # (rather than being masked as an "invalid choice"); a None return falls through below. + resolved = enum_class._missing_(value) + if isinstance(resolved, enum_class): + return resolved + raise _invalid_choice(value, _value_map) _convert.__name__ = enum_class.__name__ _convert._cmd2_enum_class = enum_class # type: ignore[attr-defined] @@ -594,9 +631,9 @@ def _resolve_bool(_tp: Any, _args: tuple[Any, ...], *, is_positional: bool = Fal return _TypeResult(converter=_parse_bool, choices=list(_BOOL_CHOICES)) -def _resolve_element(tp: Any) -> _TypeResult: +def _resolve_element(tp: Any, *, allow_unknown_entry: bool = False) -> _TypeResult: """Resolve a collection element type and reject nested collections.""" - inner = _resolve_base_type(tp, is_positional=True) + inner = _resolve_base_type(tp, is_positional=True, allow_unknown_entry=allow_unknown_entry) if inner.is_collection: raise TypeError("Nested collections are not supported") return inner @@ -605,7 +642,7 @@ def _resolve_element(tp: Any) -> _TypeResult: def _make_collection_resolver(collection_type: type) -> Callable[..., _TypeResult]: """Create a resolver for single-arg collections (list[T], set[T], frozenset[T]).""" - def _resolve(_tp: Any, args: tuple[Any, ...], **_ctx: Any) -> _TypeResult: + def _resolve(_tp: Any, args: tuple[Any, ...], *, allow_unknown_entry: bool = False, **_ctx: Any) -> _TypeResult: if len(args) == 0: # Bare list/set/frozenset without type args -- treat as list[str]/set[str]/frozenset[str]. return _TypeResult(is_collection=True, container_factory=collection_type) @@ -614,7 +651,7 @@ def _resolve(_tp: Any, args: tuple[Any, ...], **_ctx: Any) -> _TypeResult: f"{collection_type.__name__}[...] with {len(args)} type arguments is not supported; " f"use {collection_type.__name__}[T] with a single element type." ) - element = _resolve_element(args[0]) + element = _resolve_element(args[0], allow_unknown_entry=allow_unknown_entry) return _TypeResult( converter=element.converter, choices=element.choices, @@ -626,14 +663,14 @@ def _resolve(_tp: Any, args: tuple[Any, ...], **_ctx: Any) -> _TypeResult: return _resolve -def _resolve_tuple(_tp: Any, args: tuple[Any, ...], **_ctx: Any) -> _TypeResult: +def _resolve_tuple(_tp: Any, args: tuple[Any, ...], *, allow_unknown_entry: bool = False, **_ctx: Any) -> _TypeResult: """Resolve tuple[T, ...] (variable) and tuple[T, T] (fixed arity).""" if not args: # Bare tuple without type args -- treat as tuple[str, ...]. return _TypeResult(is_collection=True, container_factory=tuple) if len(args) == 2 and args[1] is Ellipsis: - element = _resolve_element(args[0]) + element = _resolve_element(args[0], allow_unknown_entry=allow_unknown_entry) return _TypeResult( converter=element.converter, choices=element.choices, @@ -651,7 +688,7 @@ def _resolve_tuple(_tp: Any, args: tuple[Any, ...], **_ctx: Any) -> _TypeResult: f"can only apply a single type= converter per argument. " f"Use tuple[T, T] (same type) or tuple[T, ...] instead." ) - element = _resolve_element(first) + element = _resolve_element(first, allow_unknown_entry=allow_unknown_entry) return _TypeResult( converter=element.converter, choices=element.choices, @@ -673,14 +710,54 @@ def _resolve_literal(_tp: Any, args: tuple[Any, ...], **_ctx: Any) -> _TypeResul return _TypeResult(converter=_make_literal_type(literal_values), choices=literal_values) -def _resolve_enum(tp: Any, _args: tuple[Any, ...], **_ctx: Any) -> _TypeResult: +def _resolve_enum(tp: Any, _args: tuple[Any, ...], *, allow_unknown_entry: bool = False, **_ctx: Any) -> _TypeResult: """Resolve Enum subclasses into converter + choices.""" return _TypeResult( - converter=_make_enum_type(tp), + converter=_make_enum_type(tp, allow_unknown_entry=allow_unknown_entry), choices=[CompletionItem(m, text=str(m.value), display_meta=m.name) for m in tp], ) +def _is_enum(tp: Any) -> bool: + """Whether *tp* is an ``enum.Enum`` subclass.""" + return isinstance(tp, type) and issubclass(tp, enum.Enum) + + +def _resolve_union(_tp: Any, args: tuple[Any, ...], *, allow_unknown_entry: bool = False, **_ctx: Any) -> _TypeResult: + """Resolve a union whose non-``None`` members are all Enums by trying each member's converter. + + Each member keeps its own converter, so member values, member names, and any ``_missing_`` + behavior (via ``allow_unknown_entry``) are preserved. A token is resolved by the first member + that accepts it, so when two members share a representation the earlier union member wins. A + union with any non-Enum member (including a ``Literal``) is rejected as ambiguous. + + A member converter signals "not mine, try the next member" by raising + ``argparse.ArgumentTypeError``; any other exception (e.g. a custom ``_missing_`` that *raises* + rather than returning ``None``) is a hard error and propagates, so order matters -- place a + strict/raising Enum after the members that should get first refusal. + """ + non_none = [a for a in args if a is not type(None)] + if not all(_is_enum(a) for a in non_none): + type_names = " | ".join(_type_name(a) for a in non_none) + raise TypeError(f"Union type {type_names} is ambiguous for auto-resolution.") + + parts = [_resolve_base_type(member, allow_unknown_entry=allow_unknown_entry) for member in non_none] + # Every part is an Enum (guarded above), so each has a converter; the None-filter keeps mypy happy. + converters = [part.converter for part in parts if part.converter is not None] + choices = _dedupe_choices(choice for part in parts for choice in (part.choices or [])) + + def _convert(value: str) -> Any: + for converter in converters: + try: + return converter(value) + except argparse.ArgumentTypeError: + continue # this member rejected the token; try the next one + raise _invalid_choice(value, choices) + + _convert.__name__ = "union" + return _TypeResult(converter=_convert, choices=choices) + + # -- Registry ----------------------------------------------------------------- _TYPE_TABLE: dict[Any, Callable[..., _TypeResult]] = { @@ -694,6 +771,8 @@ def _resolve_enum(tp: Any, _args: tuple[Any, ...], **_ctx: Any) -> _TypeResult: float: _make_simple_resolver(float), int: _make_simple_resolver(int), Literal: _resolve_literal, + Union: _resolve_union, + types.UnionType: _resolve_union, frozenset: _make_collection_resolver(frozenset), list: _make_collection_resolver(list), set: _make_collection_resolver(set), @@ -713,7 +792,7 @@ def _type_name(tp: Any) -> str: _PASSTHROUGH_TYPES = frozenset({str, object, Any, inspect.Parameter.empty}) -def _resolve_base_type(tp: Any, *, is_positional: bool = False) -> _TypeResult: +def _resolve_base_type(tp: Any, *, is_positional: bool = False, allow_unknown_entry: bool = False) -> _TypeResult: """Resolve a declared type into a :class:`_TypeResult` via the registry. Lookup order: ``get_origin(tp)`` -> ``tp`` -> ``issubclass`` fallback -> passthrough. @@ -730,7 +809,7 @@ def _resolve_base_type(tp: Any, *, is_positional: bool = False) -> _TypeResult: break if resolver is not None: - return resolver(tp, args, is_positional=is_positional) + return resolver(tp, args, is_positional=is_positional, allow_unknown_entry=allow_unknown_entry) if tp in _PASSTHROUGH_TYPES: return _TypeResult() raise TypeError( @@ -744,7 +823,9 @@ def _resolve_base_type(tp: Any, *, is_positional: bool = False) -> _TypeResult: def _unwrap_optional(tp: Any) -> tuple[Any, bool]: """If *tp* is ``T | None``, return ``(T, True)``. Otherwise ``(tp, False)``. - Raises ``TypeError`` for ambiguous unions like ``str | int`` or ``str | int | None``. + Only the ``None`` is stripped here. A multi-member union (with ``None`` removed) is handed back + intact for :func:`_resolve_union` to accept (all-Enum) or reject (ambiguous); that decision lives + there alone, so this helper never validates union members itself. """ origin = get_origin(tp) if origin is Union or origin is types.UnionType: # type: ignore[comparison-overlap] @@ -758,8 +839,8 @@ def _unwrap_optional(tp: Any) -> tuple[Any, bool]: f"Unexpected single-element Union without None: Union[{non_none[0]}]. " f"Use the type directly instead of wrapping in Union." ) - type_names = " | ".join(_type_name(a) for a in non_none) - raise TypeError(f"Union type {type_names} is ambiguous for auto-resolution.") + # Rebuild the union without its None member and let _resolve_union judge it. + return functools.reduce(operator.or_, non_none), has_none return tp, False @@ -1128,8 +1209,11 @@ def _apply_type(self) -> None: Rather than raise here -- which would let build order decide the message -- the error is captured so :data:`_CONSTRAINTS` can rank it against more specific rules and raise the winner. """ + allow_unknown_entry = self.metadata.allow_unknown_entry if self.metadata is not None else False try: - result = _resolve_base_type(self.inner_type, is_positional=self.is_positional) + result = _resolve_base_type( + self.inner_type, is_positional=self.is_positional, allow_unknown_entry=allow_unknown_entry + ) except TypeError as exc: self.build_error = exc return diff --git a/docs/features/annotated.md b/docs/features/annotated.md index a6777c626..d3b949720 100644 --- a/docs/features/annotated.md +++ b/docs/features/annotated.md @@ -85,6 +85,7 @@ The decorator converts Python type annotations into `add_argument()` calls: | positional `bool` | parsed from `true/false`, `yes/no`, `on/off`, `1/0` | | `Path` | `type=Path` | | `Enum` subclass | `type=converter`, `choices` from member values | +| `EnumA \| EnumB` (all members `Enum`) | first member to accept a token wins; `choices` merged | | `decimal.Decimal` | `type=decimal.Decimal` | | `Literal[...]` | `type=literal-converter`, `choices` from values | | `list[T]` / `set[T]` / `frozenset[T]` / `tuple[T, ...]` | `nargs='+'` (or `'*'` if it has a default or is `\| None`) | @@ -102,7 +103,8 @@ function as: Unsupported patterns raise `TypeError`, including: -- unions with multiple non-`None` members such as `str | int` +- unions with multiple non-`None` members such as `str | int`, unless every member is an `Enum` + subclass (e.g. `EnumA | EnumB`), which resolves by trying each member's converter in order - mixed-type tuples such as `tuple[int, str]` - `Annotated[T, meta] | None`; write `Annotated[T | None, meta]` instead - `Annotated[T, Argument(nargs=N)]` where `N` is `'*'`, `'+'`, or an integer `>= 1` and `T` is not a @@ -157,6 +159,69 @@ Both `Argument` and `Option` accept the same cmd2-specific fields as `add_argume `Option` additionally accepts `action`, `required`, and positional `*names` for custom flag strings (e.g. `Option("--color", "-c")`). +### Enum aliases and special keywords (`allow_unknown_entry`) + +By default an `Enum` parameter accepts only member values and member names. To let an enum also +accept aliases, alternate spellings, or special keywords, define the standard Python +[`_missing_`](https://docs.python.org/3/library/enum.html#enum.Enum._missing_) hook on the enum and +opt in with `allow_unknown_entry=True`: + +```py +import enum +from typing import Annotated +from cmd2.annotated import Argument, with_annotated + +class Color(enum.Enum): + red = "red" + green = "green" + blue = "blue" + + @classmethod + def _missing_(cls, value): + # map a special keyword onto a real member; return None to reject + return cls.red if str(value).lower() == "auto" else None + +class MyApp(cmd2.Cmd): + @with_annotated + def do_theme(self, choice: Annotated[Color, Argument(allow_unknown_entry=True)]) -> None: + self.poutput(f"theme set to {choice.value}") +``` + +`theme auto` now resolves through `_missing_` to `Color.red`, while `theme red` (value) and +`theme green` (name) keep working as before. A token that `_missing_` declines (returns `None`) is +still rejected with the usual "choose from ..." error. Any exception `_missing_` itself raises +propagates as-is (it is not masked as an "invalid choice"). The flag has no effect on non-`Enum` +annotations, and an `Enum` that does not override `_missing_` inherits the default (which returns +`None`), so the flag is simply inert for it. It also applies when the enum is a collection element +(e.g. `Annotated[list[Color], Argument(allow_unknown_entry=True)]`). + +Because `_missing_` aliases are dynamic, they are not added to the advertised `choices`, so they do +not appear in `--help` or tab-completion; the canonical member values remain the listed choice set. + +### Unions of Enums + +A parameter annotated as a union whose members are all `Enum` subclasses (e.g. `EnumA | EnumB`) is +accepted. Each member keeps its own converter, and a token is resolved by the **first member that +accepts it**: + +```py +@with_annotated +def do_pick(self, choice: Suit | Rank) -> None: + if isinstance(choice, Suit): + self.poutput(f"suit {choice.name}") + else: + self.poutput(f"rank {choice.name}") +``` + +Because resolution is first-match-wins, **order matters**: if a token is a valid value (or name) for +more than one member, the member listed first in the union wins, and the later member's identical +token becomes unreachable. `allow_unknown_entry` and each member's `_missing_` hook still apply per +member. Only `Enum` members are supported; a union containing a `Literal` or any non-`Enum` type is +still rejected as ambiguous. + +When the two value sets overlap, prefer [typed subcommands](#annotated-subcommands) (one `Enum` per +subcommand) so the choice is explicit and collision-free. + ### Actions When an `Option(action=...)` uses a zero-argument argparse action that takes no value from the diff --git a/tests/test_annotated.py b/tests/test_annotated.py index 9a5e4b98a..feaa72f40 100644 --- a/tests/test_annotated.py +++ b/tests/test_annotated.py @@ -3829,3 +3829,189 @@ class Color(enum.Enum): p = build_parser_from_function(_make_func(Color, name="c")) assert p.parse_args(["r"]).c is Color.RED assert p.parse_args(["RED"]).c is Color.RED + + +class _AliasColor(enum.Enum): + """An enum that maps extra command-line spellings to members via the standard ``_missing_`` hook.""" + + RED = "red" + GREEN = "green" + BLUE = "blue" + + @classmethod + def _missing_(cls, value: object) -> "_AliasColor | None": + return {"auto": cls.RED, "default": cls.RED}.get(str(value).lower()) + + +class TestAllowUnknownEntry: + """``allow_unknown_entry=True`` lets an Enum's ``_missing_`` hook resolve otherwise-unknown tokens.""" + + def test_missing_not_consulted_without_flag(self) -> None: + """By default the converter ignores ``_missing_`` -- an alias is rejected (current behavior).""" + parser = build_parser_from_function(_make_func(_AliasColor, name="c")) + with pytest.raises(SystemExit): + parser.parse_args(["auto"]) + + def test_flag_consults_missing(self) -> None: + """With the flag, an unknown token is resolved through the enum's ``_missing_`` hook.""" + parser = build_parser_from_function(_make_func(Annotated[_AliasColor, Argument(allow_unknown_entry=True)], name="c")) + assert parser.parse_args(["auto"]).c is _AliasColor.RED + assert parser.parse_args(["default"]).c is _AliasColor.RED + + def test_flag_still_accepts_canonical_value_and_name(self) -> None: + """Enabling the flag does not regress value or name lookup; ``_missing_`` is only a fallback.""" + parser = build_parser_from_function(_make_func(Annotated[_AliasColor, Argument(allow_unknown_entry=True)], name="c")) + assert parser.parse_args(["red"]).c is _AliasColor.RED # by value + assert parser.parse_args(["GREEN"]).c is _AliasColor.GREEN # by name + + def test_flag_preserves_intenum_value_bridge(self) -> None: + """An IntEnum value typed as a string still resolves with the flag on (str-bridge intact).""" + + class IntColor(enum.IntEnum): + red = 1 + green = 2 + + @classmethod + def _missing_(cls, value: object) -> "IntColor | None": + return cls.red if str(value).lower() == "auto" else None + + parser = build_parser_from_function(_make_func(Annotated[IntColor, Argument(allow_unknown_entry=True)], name="c")) + assert parser.parse_args(["1"]).c is IntColor.red # int value via str-bridge + assert parser.parse_args(["auto"]).c is IntColor.red # _missing_ + + def test_flag_unknown_without_missing_match_still_errors(self) -> None: + """A token neither matched nor rescued by ``_missing_`` is still rejected with the choices.""" + parser = build_parser_from_function(_make_func(Annotated[_AliasColor, Argument(allow_unknown_entry=True)], name="c")) + with pytest.raises(SystemExit): + parser.parse_args(["bogus"]) + + def test_flag_on_option(self) -> None: + """The flag also works on an ``Option`` (keyword) parameter.""" + parser = build_parser_from_function( + _make_func(Annotated[_AliasColor | None, Option("--c", allow_unknown_entry=True)], name="c", default=None) + ) + assert parser.parse_args(["--c", "auto"]).c is _AliasColor.RED + + def test_flag_on_collection_element(self) -> None: + """The flag propagates to an Enum used as a collection element.""" + parser = build_parser_from_function( + _make_func(Annotated[list[_AliasColor], Argument(allow_unknown_entry=True)], name="cs") + ) + assert parser.parse_args(["auto", "blue"]).cs == [_AliasColor.RED, _AliasColor.BLUE] + + def test_make_enum_type_flag_directly(self) -> None: + """Unit-level: ``_make_enum_type`` honors ``_missing_`` only when ``allow_unknown_entry`` is set.""" + without = _make_enum_type(_AliasColor) + with pytest.raises(argparse.ArgumentTypeError): + without("auto") + with_flag = _make_enum_type(_AliasColor, allow_unknown_entry=True) + assert with_flag("auto") is _AliasColor.RED + + def test_missing_exception_propagates_not_swallowed(self) -> None: + """A ``_missing_`` that raises is surfaced, not masked as an 'invalid choice' error.""" + + class Raises(enum.Enum): + a = "a" + + @classmethod + def _missing_(cls, value: object) -> "Raises | None": + raise ValueError("boom in _missing_") + + conv = _make_enum_type(Raises, allow_unknown_entry=True) + with pytest.raises(ValueError, match="boom in _missing_"): + conv("zzz") + + def test_flag_without_missing_handler_is_inert(self) -> None: + """An Enum with no ``_missing_`` override inherits the default (returns None), so the flag is inert.""" + + class NoHandler(enum.Enum): + a = "a" + + parser = build_parser_from_function(_make_func(Annotated[NoHandler, Argument(allow_unknown_entry=True)], name="c")) + assert parser.parse_args(["a"]).c is NoHandler.a # canonical value still works + with pytest.raises(SystemExit): + parser.parse_args(["zzz"]) # unknown token still rejected; the flag added nothing + + +class _OtherColor(enum.Enum): + """A third, disjoint Enum used to exercise multi-member unions.""" + + cyan = "cyan" + magenta = "magenta" + + +class TestEnumUnion: + """A union of Enums (``EnumA | EnumB``) resolves by trying each member's converter in order.""" + + @pytest.mark.parametrize( + ("annotation", "token", "expected"), + [ + pytest.param(_Color | _IntColor, "red", _Color.red, id="first-member-value"), + pytest.param(_Color | _IntColor, "2", _IntColor.green, id="second-member-value"), + pytest.param(_IntColor | _Color, "1", _IntColor.red, id="intenum-value"), + pytest.param(_IntColor | _Color, "green", _IntColor.green, id="member-name-first-wins"), + pytest.param(_Color | _PlainColor, "red", _Color.red, id="shared-repr-first-wins"), + pytest.param(_PlainColor | _Color, "red", _PlainColor.RED, id="shared-repr-order-flips"), + pytest.param(_Color | _IntColor | _OtherColor, "blue", _Color.blue, id="three-member-first"), + pytest.param(_Color | _IntColor | _OtherColor, "2", _IntColor.green, id="three-member-middle"), + pytest.param(_Color | _IntColor | _OtherColor, "cyan", _OtherColor.cyan, id="three-member-last"), + ], + ) + def test_resolution(self, annotation: Any, token: str, expected: enum.Enum) -> None: + """A token resolves to the first union member whose converter accepts it.""" + parser = build_parser_from_function(_make_func(annotation, name="c")) + assert parser.parse_args([token]).c is expected + + @pytest.mark.parametrize( + "annotation", + [ + pytest.param(_Color | Literal["auto"], id="literal-member"), + pytest.param(_Color | int, id="non-enum-member"), + pytest.param(str | int, id="plain-scalars"), + ], + ) + def test_out_of_scope_union_rejected(self, annotation: Any) -> None: + """Scope is Enum-only: a union with a Literal or non-Enum member is rejected as ambiguous.""" + with pytest.raises(TypeError, match="ambiguous"): + build_parser_from_function(_make_func(annotation, name="c")) + + def test_invalid_value_rejected(self) -> None: + """A token no member accepts is rejected at parse time.""" + parser = build_parser_from_function(_make_func(_Color | _IntColor, name="c")) + with pytest.raises(SystemExit): + parser.parse_args(["bogus"]) + + def test_optional_union_without_default_is_optional_positional(self) -> None: + """``A | B | None`` with no default is a positional ``nargs='?'`` yielding None when absent.""" + action = _get_param_action(_make_func(_Color | _IntColor | None, name="c")) + assert action.option_strings == [] # positional + assert action.nargs == "?" + parser = build_parser_from_function(_make_func(_Color | _IntColor | None, name="c")) + assert parser.parse_args([]).c is None + assert parser.parse_args(["red"]).c is _Color.red + + def test_optional_union_with_default_is_option(self) -> None: + """``A | B | None = None`` becomes a ``--flag`` option.""" + parser = build_parser_from_function(_make_func(_Color | _IntColor | None, name="c", default=None)) + assert parser.parse_args([]).c is None + assert parser.parse_args(["--c", "blue"]).c is _Color.blue + + def test_union_as_collection_element(self) -> None: + """A union works as a collection element type.""" + parser = build_parser_from_function(_make_func(list[_Color | _IntColor], name="cs")) + assert parser.parse_args(["red", "2"]).cs == [_Color.red, _IntColor.green] + + def test_allow_unknown_entry_threads_into_union_members(self) -> None: + """``allow_unknown_entry`` propagates to each member, so a member's ``_missing_`` still fires.""" + parser = build_parser_from_function( + _make_func(Annotated[_AliasColor | _IntColor, Argument(allow_unknown_entry=True)], name="c") + ) + assert parser.parse_args(["auto"]).c is _AliasColor.RED # _AliasColor._missing_ + assert parser.parse_args(["2"]).c is _IntColor.green + + def test_shared_representation_choices_are_deduped(self) -> None: + """When members share a text representation the merged choices show it once (not once per member).""" + # _Color and _PlainColor both spell their members "red"/"green"/"blue". + action = _get_param_action(_make_func(_Color | _PlainColor, name="c")) + texts = [c.text if isinstance(c, CompletionItem) else str(c) for c in action.choices] + assert texts == ["red", "green", "blue"] # deduped, order preserved (first member wins) From 4d10c4267640f8c04c647301d62e0067b09fe8b6 Mon Sep 17 00:00:00 2001 From: Kelvin Chung Date: Mon, 15 Jun 2026 21:07:05 +0100 Subject: [PATCH 2/2] chore: clean up --- CHANGELOG.md | 13 ++++++----- cmd2/annotated.py | 11 +++++---- docs/features/annotated.md | 6 +++-- tests/test_annotated.py | 46 ++++++++++++++++++++++++++++++++++++++ 4 files changed, 63 insertions(+), 13 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 10efe3c47..5bd13a6ea 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,15 +23,18 @@ when the class is defined instead of being deferred to first command use where the error was swallowed. The checks read parameter names only, so forward-referenced annotations still decorate cleanly. - - `Argument`/`Option` accept a new `allow_unknown_entry` flag for `Enum` parameters. When set, - a command-line token matched by neither a member value nor name is routed through the enum's - own [`_missing_`](https://docs.python.org/3/library/enum.html#enum.Enum._missing_) hook, so an + - `Argument`/`Option` accept a new `allow_unknown_entry` flag for `Enum` parameters. When set, a + command-line token matched by neither a member value nor name is routed through the enum's own + [`_missing_`](https://docs.python.org/3/library/enum.html#enum.Enum._missing_) hook, so an enum can resolve aliases, alternate spellings, or special keywords. A token that `_missing_` declines (returns `None`) is still rejected. - `@with_annotated` now supports a union of `Enum` subclasses (e.g. `EnumA | EnumB`). Each member keeps its own converter and a token resolves to the first member that accepts it, so - when two members share a representation the earlier one in the union wins. Unions containing a - `Literal` or any non-`Enum` member are still rejected as ambiguous. + when two members share a representation the earlier one in the union wins. A member whose + `_missing_` raises on a token declines it (the next member is still tried) rather than + aborting the union, and a merged "choose from ..." error is raised only when every member + declines. Unions containing a `Literal` or any non-`Enum` member are still rejected as + ambiguous. ## 4.0.0 (June 5, 2026) diff --git a/cmd2/annotated.py b/cmd2/annotated.py index 44c946550..d5697c1fb 100644 --- a/cmd2/annotated.py +++ b/cmd2/annotated.py @@ -731,10 +731,9 @@ def _resolve_union(_tp: Any, args: tuple[Any, ...], *, allow_unknown_entry: bool that accepts it, so when two members share a representation the earlier union member wins. A union with any non-Enum member (including a ``Literal``) is rejected as ambiguous. - A member converter signals "not mine, try the next member" by raising - ``argparse.ArgumentTypeError``; any other exception (e.g. a custom ``_missing_`` that *raises* - rather than returning ``None``) is a hard error and propagates, so order matters -- place a - strict/raising Enum after the members that should get first refusal. + A member declines a token by raising -- a clean ``ArgumentTypeError`` or anything a strict + ``_missing_`` raises -- and the next member is tried, so a raising member never pre-empts those + after it. Only when every member declines is the merged-choices rejection raised. """ non_none = [a for a in args if a is not type(None)] if not all(_is_enum(a) for a in non_none): @@ -750,8 +749,8 @@ def _convert(value: str) -> Any: for converter in converters: try: return converter(value) - except argparse.ArgumentTypeError: - continue # this member rejected the token; try the next one + except Exception: # noqa: BLE001, S112 - any raise means "not mine"; try the next member + continue raise _invalid_choice(value, choices) _convert.__name__ = "union" diff --git a/docs/features/annotated.md b/docs/features/annotated.md index d3b949720..c0355da0b 100644 --- a/docs/features/annotated.md +++ b/docs/features/annotated.md @@ -216,8 +216,10 @@ def do_pick(self, choice: Suit | Rank) -> None: Because resolution is first-match-wins, **order matters**: if a token is a valid value (or name) for more than one member, the member listed first in the union wins, and the later member's identical token becomes unreachable. `allow_unknown_entry` and each member's `_missing_` hook still apply per -member. Only `Enum` members are supported; a union containing a `Literal` or any non-`Enum` type is -still rejected as ambiguous. +member; a member whose `_missing_` _raises_ on a token (rather than returning `None`) simply +declines it, so the next member is still tried and the raise does not abort the whole union. Only +when every member declines is the usual "choose from ..." error raised. Only `Enum` members are +supported; a union containing a `Literal` or any non-`Enum` type is still rejected as ambiguous. When the two value sets overlap, prefer [typed subcommands](#annotated-subcommands) (one `Enum` per subcommand) so the choice is explicit and collision-free. diff --git a/tests/test_annotated.py b/tests/test_annotated.py index feaa72f40..011ee3a08 100644 --- a/tests/test_annotated.py +++ b/tests/test_annotated.py @@ -3940,6 +3940,16 @@ class _OtherColor(enum.Enum): magenta = "magenta" +class _StrictColor(enum.Enum): + """An Enum whose ``_missing_`` *raises* on an unknown token, as a strict enum typically does.""" + + crimson = "crimson" + + @classmethod + def _missing_(cls, value: object) -> "_StrictColor | None": + raise ValueError(f"{value!r} is not a valid _StrictColor") + + class TestEnumUnion: """A union of Enums (``EnumA | EnumB``) resolves by trying each member's converter in order.""" @@ -3968,6 +3978,9 @@ def test_resolution(self, annotation: Any, token: str, expected: enum.Enum) -> N pytest.param(_Color | Literal["auto"], id="literal-member"), pytest.param(_Color | int, id="non-enum-member"), pytest.param(str | int, id="plain-scalars"), + # `... | None` is stripped and the remaining union rebuilt, so the same rule applies. + pytest.param(_Color | int | None, id="non-enum-member-optional"), + pytest.param(_Color | Literal["auto"] | None, id="literal-member-optional"), ], ) def test_out_of_scope_union_rejected(self, annotation: Any) -> None: @@ -4015,3 +4028,36 @@ def test_shared_representation_choices_are_deduped(self) -> None: action = _get_param_action(_make_func(_Color | _PlainColor, name="c")) texts = [c.text if isinstance(c, CompletionItem) else str(c) for c in action.choices] assert texts == ["red", "green", "blue"] # deduped, order preserved (first member wins) + + def test_member_whose_missing_raises_does_not_preempt_later_members(self) -> None: + """A member whose ``_missing_`` *raises* declines the token; the union keeps trying later members. + + A strict Enum (one whose ``_missing_`` raises rather than returning ``None``) listed before a + member that accepts the token must not abort the whole union -- the raise means "not mine", + same as a clean rejection, so resolution falls through to the next member. + """ + parser = build_parser_from_function( + _make_func(Annotated[_StrictColor | _IntColor, Argument(allow_unknown_entry=True)], name="c") + ) + # _StrictColor._missing_("2") raises, but _IntColor accepts "2" -- the later member still wins. + assert parser.parse_args(["2"]).c is _IntColor.green + # The strict member still claims its own token first. + assert parser.parse_args(["crimson"]).c is _StrictColor.crimson + + def test_all_members_declining_reports_invalid_choice_not_member_error(self) -> None: + """When every member declines -- even one whose ``_missing_`` raises -- the union reports invalid choice. + + The deferred member error must not surface as-is; the user sees the standard merged-choices + rejection, raised only after all members have been tried. + """ + converter = _get_param_action( + _make_func(Annotated[_StrictColor | _IntColor, Argument(allow_unknown_entry=True)], name="c") + ).type + with pytest.raises(argparse.ArgumentTypeError, match="invalid choice"): + converter("bogus") + + def test_union_choices_preserve_display_meta(self) -> None: + """Merged union choices keep each member's ``display_meta`` (the per-member tab-completion hint).""" + action = _get_param_action(_make_func(_Color | _IntColor, name="c")) + meta = {c.text: c.display_meta for c in action.choices if isinstance(c, CompletionItem)} + assert meta == {"red": "red", "green": "green", "blue": "blue", "1": "red", "2": "green", "3": "blue"}