chore: more edge case clean up

Author: KelvinChung2000

cmd2/annotated.py

@@ -7,7 +7,10 @@
 finer control is needed.
 
 Basic usage -- parameters without defaults become positional arguments,
-parameters with defaults become ``--option`` flags::
+parameters with defaults become ``--option`` flags.  Keyword-only
+parameters (after ``*``) always become options; without a default they
+are required.  The parameter name ``dest`` is reserved and cannot be
+used::
 
     class MyApp(cmd2.Cmd):
         @cmd2.with_annotated
@@ -55,6 +58,10 @@ def do_paint(
 - ``tuple[int, str, float]`` -- mixed element types are not currently supported
   because argparse can only apply a single ``type=`` converter per argument
 
+When combining ``Annotated`` with ``Optional``, the union must go
+*inside*: ``Annotated[T | None, meta]``.  Writing
+``Annotated[T, meta] | None`` is ambiguous and raises ``TypeError``.
+
 Note: ``Path`` and ``Enum`` types also get automatic tab completion via
 ``ArgparseCompleter`` type inference. This works for both ``@with_annotated``
 and ``@with_argparser`` -- see the ``argparse_completer`` module.
@@ -301,9 +308,9 @@ def _make_collection_resolver(collection_type: type) -> Callable[..., dict[str,
     """Create a resolver for single-arg collections (list[T], set[T])."""
 
     def _resolve(_tp: Any, args: tuple[Any, ...], *, has_default: bool = False, **_ctx: Any) -> dict[str, Any]:
+        nargs = '*' if has_default else '+'
         if len(args) == 0:
             # Bare list/tuple without type args -- treat as list[str]/set[str]
-            nargs = '*' if has_default else '+'
             return {
                 'is_collection': True,
                 'nargs': nargs,
@@ -314,7 +321,6 @@ def _resolve(_tp: Any, args: tuple[Any, ...], *, has_default: bool = False, **_c
         if len(args) != 1:
             return {}  # pragma: no cover
         element_type, inner = _resolve_element(args[0])
-        nargs = '*' if has_default else '+'
         return {
             **inner,
             'is_collection': True,
@@ -331,14 +337,13 @@ def _resolve_tuple(_tp: Any, args: tuple[Any, ...], *, has_default: bool = False
     """Resolve tuple[T, ...] and tuple[T1, T2, ...]."""
     cast_kwargs = {'action': _CollectionCastingAction, 'container_factory': tuple}
 
+    nargs = '*' if has_default else '+'
     if not args:
         # Bare tuple without type args -- treat as tuple[str, ...]
-        nargs = '*' if has_default else '+'
         return {'is_collection': True, 'nargs': nargs, 'base_type': str, **cast_kwargs}
 
     if len(args) == 2 and args[1] is Ellipsis:
         element_type, inner = _resolve_element(args[0])
-        nargs = '*' if has_default else '+'
         return {**inner, 'is_collection': True, 'nargs': nargs, 'base_type': element_type, **cast_kwargs}
 
     if Ellipsis not in args:
@@ -424,6 +429,26 @@ def _resolve_type(
     return tp, {}
 
 
+def _unwrap_optional(tp: type) -> tuple[type, bool]:
+    """If *tp* is ``T | None``, return ``(T, True)``.  Otherwise ``(tp, False)``.
+
+    Raises ``TypeError`` for ambiguous unions like ``str | int`` or ``str | int | None``.
+    """
+    origin = get_origin(tp)
+    if origin is Union or origin is types.UnionType:  # type: ignore[comparison-overlap]
+        all_args = get_args(tp)
+        non_none = [a for a in all_args if a is not type(None)]
+        has_none = len(non_none) < len(all_args)
+        if len(non_none) == 1:
+            if has_none:
+                return non_none[0], True
+            # Single-element union without None shouldn't happen, pass through
+            return non_none[0], False  # pragma: no cover
+        type_names = ' | '.join(a.__name__ if hasattr(a, '__name__') else str(a) for a in non_none)
+        raise TypeError(f"Union type {type_names} is ambiguous for auto-resolution. ")
+    return tp, False
+
+
 # ---------------------------------------------------------------------------
 # Annotation resolution
 # ---------------------------------------------------------------------------
@@ -437,13 +462,27 @@ def _resolve_annotation(
 ) -> tuple[dict[str, Any], ArgMetadata, bool, bool]:
     """Decompose a type annotation into ``(type_kwargs, metadata, is_positional, is_bool_flag)``.
 
-    Peels in order: Annotated → Optional → type resolution.
+    Peels ``Annotated`` then ``Optional``.  The only supported way to combine
+    ``Annotated`` with ``Optional`` is ``Annotated[T | None, meta]``.
+    Writing ``Annotated[T, meta] | None`` is ambiguous and raises ``TypeError``.
     """
     tp = annotation
     metadata: ArgMetadata = None
     is_optional = False
 
-    # 1. Annotated
+    # 1. If top level is Optional wrapping Annotated, only allow
+    #    Annotated[T | None, meta] form (inner type must contain None).
+    tp, unwrapped = _unwrap_optional(tp)
+    if unwrapped:
+        is_optional = True
+        if get_origin(tp) is Annotated:  # type: ignore[comparison-overlap]
+            inner_tp = get_args(tp)[0]
+            inner_origin = get_origin(inner_tp)
+            inner_is_union = inner_origin is Union or inner_origin is types.UnionType  # type: ignore[comparison-overlap]
+            if not (inner_is_union and type(None) in get_args(inner_tp)):
+                raise TypeError("Annotated[T, meta] | None is ambiguous. Use Annotated[T | None, meta] instead.")
+
+    # 2. Peel Annotated to extract metadata
     if get_origin(tp) is Annotated:  # type: ignore[comparison-overlap]
         args = get_args(tp)
         tp = args[0]
@@ -452,18 +491,10 @@ def _resolve_annotation(
                 metadata = meta
                 break
 
-    # 2. Optional / T | None
-    origin = get_origin(tp)
-    if origin is Union or origin is types.UnionType:  # type: ignore[comparison-overlap]
-        args = [a for a in get_args(tp) if a is not type(None)]  # type: ignore[assignment]
-        if len(args) == 1:
-            tp = args[0]
-            is_optional = True
-        else:
-            # len > 1: ambiguous union (e.g. str | int)
-            # len == 0: all-None union -- unreachable via normal typing but handle defensively
-            type_names = ' | '.join(a.__name__ if hasattr(a, '__name__') else str(a) for a in args)
-            raise TypeError(f"Union type {type_names} is ambiguous for auto-resolution. ")
+    # 3. Peel inner Optional (from Annotated[T | None, meta])
+    tp, inner_unwrapped = _unwrap_optional(tp)
+    if inner_unwrapped:
+        is_optional = True
 
     is_positional = isinstance(metadata, Argument) or (
         not isinstance(metadata, Option) and not has_default and not is_optional
@@ -548,12 +579,20 @@ def build_parser_from_function(func: Callable[..., Any]) -> argparse.ArgumentPar
         has_default = param.default is not inspect.Parameter.empty
         default = param.default if has_default else None
 
+        # Keyword-only params (after *) are always options, never positional
+        is_kw_only = param.kind == inspect.Parameter.KEYWORD_ONLY
+
         kwargs, metadata, positional, _is_bool_flag = _resolve_annotation(
             annotation,
-            has_default=has_default,
+            has_default=has_default or is_kw_only,
             default=default,
         )
 
+        # Keyword-only without default becomes a required option
+        if is_kw_only and not has_default:
+            positional = False
+            kwargs['required'] = True
+
         if positional:
             parser.add_argument(name, **kwargs)
         else:

cmd2/decorators.py

@@ -383,6 +383,9 @@ def decorator(func: Callable[..., Any]) -> Callable[..., Any]:
 
         command_name = func.__name__[len(constants.COMMAND_FUNC_PREFIX) :]
 
+        # Cache signature introspection at decoration time, not per-invocation
+        _accepted_params = set(inspect.signature(func).parameters.keys()) - {'self'}
+
         @functools.wraps(func)
         def cmd_wrapper(*args: Any, **kwargs: Any) -> bool | None:
             cmd2_app, statement_arg = _parse_positionals(args)
@@ -412,8 +415,6 @@ def cmd_wrapper(*args: Any, **kwargs: Any) -> bool | None:
                 raise Cmd2ArgparseError from exc
 
             # Unpack Namespace into function kwargs, filtering cmd2 internals.
-            # Note: "dest" keys could conflict with annotation names -- may need validation.
-            # Note: "required" handling when mixing with Options metadata needs verification.
             func_kwargs: dict[str, Any] = {}
             for key, value in vars(ns).items():
                 if key.startswith('cmd2_') or key == constants.NS_ATTR_SUBCMD_HANDLER:
@@ -431,9 +432,7 @@ def cmd_wrapper(*args: Any, **kwargs: Any) -> bool | None:
                 func_kwargs['_unknown'] = unknown
 
             # Only pass kwargs the function actually accepts
-            sig = inspect.signature(func)
-            accepted = set(sig.parameters.keys()) - {'self'}
-            filtered_kwargs = {k: v for k, v in func_kwargs.items() if k in accepted}
+            filtered_kwargs = {k: v for k, v in func_kwargs.items() if k in _accepted_params}
 
             # Explicit kwargs provided by direct wrapper invocation should override
             # parser-derived values.

docs/features/argument_processing.md

@@ -64,7 +64,9 @@ required.
 ### Basic usage
 
 Parameters without defaults become positional arguments. Parameters with defaults become `--option`
-flags. The function receives typed keyword arguments directly instead of an `argparse.Namespace`.
+flags. Keyword-only parameters (after `*`) always become options regardless of whether they have a
+default -- without a default they become required options. The function receives typed keyword
+arguments directly instead of an `argparse.Namespace`.
 
 ```py
 from cmd2 import with_annotated
@@ -96,6 +98,7 @@ The decorator converts Python type annotations into `add_argument()` calls:
 | `decimal.Decimal`                      | `type=decimal.Decimal`                              |
 | `Literal[...]`                         | `type=literal-converter`, `choices` from values     |
 | `list[T]` / `set[T]` / `tuple[T, ...]` | `nargs='+'` (or `'*'` if it has a default)          |
+| `list` / `tuple` (bare, no type arg)   | `nargs='+'` with `str` elements                     |
 | `tuple[T, T]` (fixed arity, same type) | `nargs=N` with `type=T`                             |
 | `T \| None`                            | unwrapped to `T`, treated as optional               |
 
@@ -106,6 +109,33 @@ function as:
 - `set[T]` as `set`
 - `tuple[T, ...]` as `tuple`
 
+### Keyword-only parameters
+
+Parameters after `*` in the function signature always become `--option` flags, even without a
+default value. Without a default, they become required options:
+
+```py
+@with_annotated
+def do_search(self, query: str, *, limit: int, verbose: bool = False):
+    # query   → positional (no default)
+    # limit   → --limit (required, keyword-only without default)
+    # verbose → --verbose / --no-verbose (keyword-only with default)
+    ...
+```
+
+### Defaults
+
+Default values are stored as-is on the argparse action and are not passed through the type
+converter. This means a default of `Color.blue` stays as the enum member, and `Path("/tmp")` stays
+as a `Path` object. If you provide a default of the wrong type (e.g. `count: int = "1"`), argparse
+will use the string `"1"` as the default without converting it.
+
+### Reserved parameter names
+
+The parameter name `dest` cannot be used with `@with_annotated` because this creates ambiguity in is
+the `dest` should be used or is the annotated variable name should be used. Using it raises
+`ValueError` at decoration time.
+
 ### Unsupported type patterns
 
 The following type patterns raise `TypeError` at decoration time:
@@ -116,6 +146,15 @@ The following type patterns raise `TypeError` at decoration time:
 - **Mixed-type tuples** like `tuple[int, str, float]` -- not currently supported because argparse
   can only apply a single `type=` converter per argument. Use `tuple[T, T]` (same type for all
   elements) or `tuple[T, ...]` instead.
+- **`Annotated[T, meta] | None`** -- ambiguous. Place the union inside `Annotated` instead:
+
+```py
+# Correct: union INSIDE Annotated
+name: Annotated[str | None, Option("--name")] = None
+
+# Wrong: raises TypeError
+name: Annotated[str, Option("--name")] | None = None  # don't do this
+```
 
 ### Annotated metadata