Skip to content

Commit 7cc0986

Browse files
chore: update implementation
1 parent 7515a03 commit 7cc0986

4 files changed

Lines changed: 245 additions & 133 deletions

File tree

CHANGELOG.md

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,21 @@
11
## 4.2.0 (TBD)
22

3+
- Enhancements
4+
- `@with_annotated` argument groups can now contain an `ArgumentBlock`'s arguments. A `Group`
5+
member names a command-line argument, and a block expands into one argument per field, so its
6+
fields are named: `Group("host", "port")`.
7+
- Breaking Changes
8+
- A `Group` member now names an argument rather than a parameter. The two differ only for an
9+
`ArgumentBlock` parameter, which is expanded away and has no argument of its own:
10+
`Group("conn")` now raises `ValueError` pointing at the block's fields. It previously produced
11+
an empty argument group in `groups=` and a `KeyError` in `mutually_exclusive_groups=`.
12+
- Whether a `Group` member exists is now validated when the parser is built rather than at
13+
decoration time, because a block's field names cannot be known without resolving its type
14+
hint, and resolving hints eagerly would break forward-referenced annotations. A typo in a
15+
member name still raises `ValueError`, but on first use of that command rather than at class
16+
definition. The spec-shape rules (a member listed twice, a member in two groups,
17+
`required=True` on a plain group, the mutex nesting rules) are unaffected and still hard-fail
18+
at decoration time.
319
- Bug Fixes
420
- Fixed `@with_annotated(base_command=True)` not listing its subcommands under the positional
521
arguments section of the parent command's `--help`, unlike `argparse` and

cmd2/annotated.py

Lines changed: 57 additions & 63 deletions
Original file line numberDiff line numberDiff line change
@@ -552,25 +552,20 @@ def __init__(
552552
) -> None:
553553
"""Initialise an argument group definition.
554554
555-
:param members: parameter names to place in the group (at least one)
555+
:param members: argument names to place in the group (at least one). A plain parameter's name is its
556+
argument name; for an ``ArgumentBlock`` name its fields.
556557
:param title: group title shown as a help section header
557558
:param description: group description shown under the title
558559
:param required: ``mutually_exclusive_groups`` only -- require exactly one member.
559560
On a ``groups=`` entry it raises ``ValueError``.
560561
"""
561562
if not members:
562-
raise ValueError("Group requires at least one member parameter name")
563+
raise ValueError("Group requires at least one member argument name")
563564
self.members = members
564565
self.title = title
565566
self.description = description
566567
self.required = required
567568

568-
def _validate_members(self, *, all_param_names: set[str], group_type: str) -> None:
569-
"""Validate that every referenced member parameter exists."""
570-
for name in self.members:
571-
if name not in all_param_names:
572-
raise ValueError(f"{group_type} references nonexistent parameter {name!r}")
573-
574569

575570
#: Metadata extracted from ``Annotated[T, meta]``, or ``None`` for plain types.
576571
ArgMetadata = Argument | Option | None
@@ -1126,7 +1121,6 @@ def __init__(
11261121
kind: inspect._ParameterKind,
11271122
is_base_command: bool,
11281123
is_block_field: bool = False,
1129-
block_param_name: str | None = None,
11301124
dest_override: str | None = None,
11311125
) -> None:
11321126
# signature-derived inputs (never emitted):
@@ -1135,7 +1129,6 @@ def __init__(
11351129
self.has_default = has_default
11361130
self.param_default = param_default # the function's own default, not the argparse `default` slot
11371131
self.is_block_field = is_block_field
1138-
self.block_param_name = block_param_name
11391132
self.dest_override = dest_override
11401133
self.is_kw_only = is_kw_only
11411134
self.is_variadic = is_variadic
@@ -2074,26 +2067,53 @@ def _shared_field_dest(dc_type: type, field_name: str) -> str:
20742067
return constants.cmd2_private_attr_name(f"shared_{id(dc_type):x}_{field_name}")
20752068

20762069

2070+
def _check_group_members_exist(
2071+
by_name: dict[str, _ArgparseArgument],
2072+
block_param_names: set[str],
2073+
func_qualname: str,
2074+
*specs: tuple[Group, ...] | None,
2075+
) -> None:
2076+
"""Reject a ``Group`` member that names no command-line argument, once every argument is built.
2077+
2078+
:func:`_validate_group_specs` cannot do this at decoration time: an ArgumentBlock's arguments are its
2079+
dataclass fields, which only resolving the block's type hint would reveal, and resolving hints there
2080+
would break forward-referenced annotations. So this is where a bad member name is finally caught.
2081+
2082+
Naming the block parameter itself is the likely mistake -- it reads natural but the parameter is expanded
2083+
away and has no argument -- so it gets its own message pointing at the fields to name instead.
2084+
"""
2085+
for spec_group in specs:
2086+
for spec in spec_group or ():
2087+
for name in spec.members:
2088+
if name in by_name:
2089+
continue
2090+
if name in block_param_names:
2091+
raise ValueError(
2092+
f"group in {func_qualname} references {name!r}, which is an ArgumentBlock parameter "
2093+
f"and not a command-line argument: a block is expanded into one argument per field, "
2094+
f"so name those fields instead."
2095+
)
2096+
raise ValueError(
2097+
f"group in {func_qualname} references {name!r}, which is not a command-line argument. "
2098+
f"Members name arguments: a plain parameter is its own argument, and an ArgumentBlock's "
2099+
f"arguments are its field names."
2100+
)
2101+
2102+
20772103
def _link_mutex_group_membership(
20782104
by_name: dict[str, _ArgparseArgument],
2079-
by_block_param_name: dict[str, list[_ArgparseArgument]],
20802105
mutually_exclusive_groups: tuple[Group, ...] | None,
20812106
) -> None:
20822107
"""Append each mutex group's 1-based index to its member arguments' ``mutex_group_indices``.
20832108
20842109
This membership is the fact behind the required-member constraint row. Member references are
2085-
validated upstream by :func:`_validate_group_specs` before this runs, so every member name
2086-
resolves to a built argument or block parameter.
2110+
resolved to built arguments upstream by :func:`_check_group_members_exist` before this runs.
20872111
"""
20882112
if not mutually_exclusive_groups:
20892113
return
20902114
for index, spec in enumerate(mutually_exclusive_groups, start=1):
20912115
for name in spec.members:
2092-
if name in by_block_param_name:
2093-
for arg in by_block_param_name[name]:
2094-
arg.mutex_group_indices.append(index)
2095-
else:
2096-
by_name[name].mutex_group_indices.append(index)
2116+
by_name[name].mutex_group_indices.append(index)
20972117

20982118

20992119
def _resolve_func_hints(func: Callable[..., Any], *, skip_params: frozenset[str] = _SKIP_PARAMS) -> dict[str, Any]:
@@ -2191,7 +2211,6 @@ def _expand_dataclass_block(
21912211
*,
21922212
func_qualname: str,
21932213
base_command: bool,
2194-
block_param_name: str,
21952214
shared: bool = False,
21962215
) -> list[_ArgparseArgument]:
21972216
"""Expand a dataclass block's ``init`` fields into flat ``_ArgparseArgument`` builders.
@@ -2255,7 +2274,6 @@ def _expand_dataclass_block(
22552274
kind=inspect.Parameter.POSITIONAL_OR_KEYWORD,
22562275
is_base_command=base_command,
22572276
is_block_field=True,
2258-
block_param_name=block_param_name,
22592277
dest_override=_shared_field_dest(dc_type, f.name) if shared else None,
22602278
)
22612279
)
@@ -2361,6 +2379,7 @@ def _resolve_parameters(
23612379
*,
23622380
skip_params: frozenset[str] = _SKIP_PARAMS,
23632381
base_command: bool = False,
2382+
groups: tuple[Group, ...] | None = None,
23642383
mutually_exclusive_groups: tuple[Group, ...] | None = None,
23652384
) -> tuple[list[_ArgparseArgument], set[type]]:
23662385
"""Resolve a function signature into argparse-argument builders and inheritable-block types.
@@ -2387,6 +2406,7 @@ def _resolve_parameters(
23872406
resolved: list[_ArgparseArgument] = []
23882407
inherited_field_names: set[str] = set()
23892408
base_args_types: set[type] = set()
2409+
block_param_names: set[str] = set()
23902410

23912411
# Skip the first parameter by position (self/cls for methods)
23922412
params = list(sig.parameters.items())
@@ -2407,6 +2427,7 @@ def _resolve_parameters(
24072427
f"cannot have a default value; remove the default."
24082428
)
24092429
inherited_field_names.update(_init_field_names(inherited))
2430+
block_param_names.add(name)
24102431
continue
24112432

24122433
# An ArgumentBlock-typed parameter is an argument block: expand its fields in place (flat) instead of
@@ -2422,13 +2443,10 @@ def _resolve_parameters(
24222443
# Expand first so its @dataclass-ness check runs before we read field names for the next guard.
24232444
# A cmd2_base_args block is shared down the chain: its fields use a type-qualified dest.
24242445
expanded = _expand_dataclass_block(
2425-
block_hint,
2426-
func_qualname=func.__qualname__,
2427-
base_command=base_command,
2428-
block_param_name=name,
2429-
shared=name == NS_ATTR_BASE_ARGS,
2446+
block_hint, func_qualname=func.__qualname__, base_command=base_command, shared=name == NS_ATTR_BASE_ARGS
24302447
)
24312448
_reject_field_shadowing_block_param(block_hint, name, func.__qualname__)
2449+
block_param_names.add(name)
24322450
resolved.extend(expanded)
24332451
continue
24342452
# The magic name requires a bare ArgumentBlock; a non-block annotation on it is a clear mistake.
@@ -2493,11 +2511,8 @@ def _resolve_parameters(
24932511
for arg in positionals[:-1]: # every positional except the last has a following positional
24942512
arg.has_following_positional = True
24952513
by_name = {arg.name: arg for arg in resolved}
2496-
by_block_param_name: dict[str, list[_ArgparseArgument]] = {}
2497-
for arg in resolved:
2498-
if arg.block_param_name is not None:
2499-
by_block_param_name.setdefault(arg.block_param_name, []).append(arg)
2500-
_link_mutex_group_membership(by_name, by_block_param_name, mutually_exclusive_groups)
2514+
_check_group_members_exist(by_name, block_param_names, func.__qualname__, groups, mutually_exclusive_groups)
2515+
_link_mutex_group_membership(by_name, mutually_exclusive_groups)
25012516
for arg in resolved:
25022517
arg._check_constraints()
25032518
return resolved, base_args_types
@@ -2556,30 +2571,22 @@ def _filtered_namespace_kwargs(
25562571

25572572

25582573
def _validate_group_specs(
2559-
func: Callable[..., Any],
2560-
*,
2561-
skip_params: frozenset[str],
25622574
groups: tuple[Group, ...] | None,
25632575
mutually_exclusive_groups: tuple[Group, ...] | None,
25642576
) -> None:
2565-
"""Validate ``groups=`` / ``mutually_exclusive_groups=`` specs from parameter names alone.
2566-
2567-
Runs at decoration time (from both the regular-command and subcommand decoration paths, and
2568-
again from :func:`build_parser_from_function` for direct callers), so a misconfigured group
2569-
hard-fails when the class is defined instead of on first command use, where cmd2's runtime
2570-
handler turns the error into a printed message. Reads only parameter names and the ``Group``
2571-
specs -- never the type hints -- so forward-referenced annotations still decorate. The one
2572-
group rule that needs the annotations (a required member in a mutually exclusive group) stays
2573-
in :data:`_CONSTRAINTS` and fires when the parser is built.
2577+
"""Validate the *shape* of ``groups=`` / ``mutually_exclusive_groups=`` specs.
2578+
2579+
Runs at decoration time (from both the regular-command and subcommand decoration paths, and again from
2580+
:func:`build_parser_from_function` for direct callers), so a misshapen group hard-fails when the class is
2581+
defined instead of on first command use, where cmd2's runtime handler turns the error into a printed
2582+
message. Reads only the ``Group`` specs -- never the signature or the type hints -- so
2583+
forward-referenced annotations still decorate.
25742584
"""
25752585
if not groups and not mutually_exclusive_groups:
25762586
return
2577-
params = list(inspect.signature(func).parameters)[1:] # skip self/cls by position
2578-
all_param_names = {name for name in params if name not in skip_params}
25792587

25802588
group_entry_for: dict[str, int] = {}
25812589
for index, spec in enumerate(groups or (), start=1):
2582-
spec._validate_members(all_param_names=all_param_names, group_type="groups")
25832590
if spec.required:
25842591
raise ValueError(
25852592
"Group(required=True) is only valid in mutually_exclusive_groups; "
@@ -2597,7 +2604,6 @@ def _validate_group_specs(
25972604

25982605
mutex_entry_for: dict[str, int] = {}
25992606
for index, spec in enumerate(mutually_exclusive_groups or (), start=1):
2600-
spec._validate_members(all_param_names=all_param_names, group_type="mutually_exclusive_groups")
26012607
for name in spec.members:
26022608
previous = mutex_entry_for.get(name)
26032609
if previous == index:
@@ -2738,7 +2744,7 @@ def build_parser_from_function(
27382744
from . import argparse_utils
27392745

27402746
# The decorator already ran this at decoration time; direct callers get the same checks here.
2741-
_validate_group_specs(func, skip_params=skip_params, groups=groups, mutually_exclusive_groups=mutually_exclusive_groups)
2747+
_validate_group_specs(groups, mutually_exclusive_groups)
27422748

27432749
parser_cls = parser_class or argparse_utils.DEFAULT_ARGUMENT_PARSER
27442750
if "description" not in parser_kwargs:
@@ -2752,6 +2758,7 @@ def build_parser_from_function(
27522758
resolved, base_args_types = _resolve_parameters(
27532759
func,
27542760
skip_params=skip_params,
2761+
groups=groups,
27552762
mutually_exclusive_groups=mutually_exclusive_groups,
27562763
)
27572764

@@ -2777,10 +2784,7 @@ def build_parser_from_function(
27772784

27782785
# Add each argument to its target (its group/mutex group if assigned, else the parser).
27792786
for arg in resolved:
2780-
target = target_for.get(arg.name)
2781-
if target is None and arg.block_param_name is not None:
2782-
target = target_for.get(arg.block_param_name)
2783-
arg.add_to(target if target is not None else parser)
2787+
arg.add_to(target_for.get(arg.name, parser))
27842788

27852789
# A cmd2_base_args block is inheritable: stamp a parse-time presence marker so a descendant's
27862790
# cmd2_parent_args can confirm an ancestor actually declared the block.
@@ -2884,12 +2888,7 @@ def _build_subcommand_handler(
28842888
# Validate the group specs eagerly (decoration time) so a misconfigured group hard-fails when
28852889
# the class is defined; the name-only checks never resolve type hints, so forward-referenced
28862890
# annotations still decorate and the parser build stays deferred.
2887-
_validate_group_specs(
2888-
func,
2889-
skip_params=_SKIP_PARAMS,
2890-
groups=options.groups,
2891-
mutually_exclusive_groups=options.mutually_exclusive_groups,
2892-
)
2891+
_validate_group_specs(options.groups, options.mutually_exclusive_groups)
28932892
if base_command:
28942893
# Validate eagerly (decoration time); the base-command rows in _CONSTRAINTS fire here.
28952894
# skip_params is spelled out so this call cannot silently diverge from the parser build below.
@@ -3073,12 +3072,7 @@ def decorator(fn: Callable[..., Any]) -> Callable[..., Any]:
30733072
# Validate the group specs eagerly (decoration time) so a misconfigured group hard-fails when
30743073
# the class is defined; the name-only checks never resolve type hints, so forward-referenced
30753074
# annotations still decorate and the parser build stays deferred.
3076-
_validate_group_specs(
3077-
fn,
3078-
skip_params=skip_params,
3079-
groups=options.groups,
3080-
mutually_exclusive_groups=options.mutually_exclusive_groups,
3081-
)
3075+
_validate_group_specs(options.groups, options.mutually_exclusive_groups)
30823076
if base_command:
30833077
# Validate eagerly (decoration time); the base-command rows in _CONSTRAINTS fire here.
30843078
_resolve_parameters(fn, skip_params=skip_params, base_command=True)

docs/features/annotated.md

Lines changed: 30 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -445,8 +445,8 @@ token to convert. Both raise `TypeError` at decoration time.
445445
- `help` -- help text for an annotated subcommand (only valid with `subcommand_to`)
446446
- `aliases` -- aliases for an annotated subcommand (only valid with `subcommand_to`)
447447
- `deprecated` -- mark the subcommand as deprecated in `--help` (only valid with `subcommand_to`)
448-
- `groups` -- `Group` instances assigning parameter names to argument groups
449-
- `mutually_exclusive_groups` -- `Group` instances of mutually exclusive parameters
448+
- `groups` -- `Group` instances assigning argument names to argument groups
449+
- `mutually_exclusive_groups` -- `Group` instances of mutually exclusive arguments
450450
- `parser_class` -- a custom parser class (defaults to the configured default)
451451
- `**parser_kwargs` -- every other `Cmd2ArgumentParser` constructor kwarg, forwarded through PEP 692
452452
[`Unpack[Cmd2ParserKwargs]`][cmd2.annotated.Cmd2ParserKwargs]. See
@@ -549,13 +549,35 @@ both places, a mutex that sits only partly in a `groups=` entry, or one that spa
549549
raises `ValueError`. The other three nestings (an argument group inside another group or a mutex, or
550550
a mutex inside a mutex) are removed in argparse on Python 3.14 and cannot be expressed here.
551551

552-
All of these group-spec rules -- member references, a parameter assigned to two groups,
552+
A `Group` member names a command-line **argument**. For a plain parameter the argument is the
553+
parameter, so the name is the same either way. An `ArgumentBlock` parameter is different: it is
554+
expanded into one argument per field (field name == argument name), so you name its fields rather
555+
than the block:
556+
557+
```python
558+
@dataclass
559+
class Conn(ArgumentBlock):
560+
host: Annotated[str, Option("--host")] = "localhost"
561+
port: Annotated[int, Option("--port")] = 8080
562+
563+
@with_annotated(groups=(Group("host", "port", title="connection"),))
564+
def do_connect(self, conn: Conn) -> None: ...
565+
```
566+
567+
Naming the block parameter itself (`Group("conn")`) raises `ValueError`: the parameter is expanded
568+
away and has no argument of its own. Naming a block's fields individually is also what lets you say
569+
which fields are alternatives -- put `Group("json", "csv")` in `mutually_exclusive_groups` and only
570+
those two exclude each other, rather than every field of the block they happen to live in.
571+
572+
The group-spec **shape** rules -- a member listed twice, a member assigned to two groups,
553573
`required=True` on a plain group, and the mutex nesting rules above -- are validated when the
554-
decorator runs, so a misconfigured group raises `ValueError` at class definition time instead of on
555-
first command use. The checks read only parameter names, never the type hints, so forward-referenced
556-
annotations still decorate cleanly. The one group rule that depends on the annotations (a member of
557-
a mutually exclusive group must be omittable -- have a default or be `T | None`) fires when the
558-
parser is built.
574+
decorator runs, so a misshapen group raises `ValueError` at class definition time instead of on
575+
first command use. Those checks read only the `Group` specs, never the signature or the type hints,
576+
so forward-referenced annotations still decorate cleanly.
577+
578+
The rules that depend on what a member _is_ fire when the parser is built, because a block's field
579+
names cannot be known without resolving its type hint: that every member names a real argument, and
580+
that a member of a mutually exclusive group is omittable (has a default or is `T | None`).
559581

560582
`parents=` mirrors argparse's standard parents mechanism for sharing argument definitions across
561583
parsers. `argument_default=argparse.SUPPRESS` is not supported and raises `TypeError`. It removes an

0 commit comments

Comments
 (0)