diff --git a/CHANGELOG.md b/CHANGELOG.md index 0273033eb..785ad5d8c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,16 @@ - Experimental features - `@with_annotated` now supports `frozenset[T]` collection parameters, alongside the existing `list[T]`, `set[T]`, and `tuple[T, ...]` collection types. + - `@with_annotated` mutually exclusive groups now accept a `title`/`description` to render the + group as a titled help section (argparse's one supported nesting, a mutex inside an argument + group), declared in one place with no paired `groups=` entry. + - `@with_annotated` now validates `groups` / `mutually_exclusive_groups` specs eagerly at + decoration time, so a misconfigured group (a member that names no parameter, a parameter + placed in two groups, a mutex group spanning or partially overlapping argument groups, a + titled section declared in two places, or `Group(required=True)` on a plain group) hard-fails + 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. ## 4.0.0 (June 5, 2026) diff --git a/cmd2/annotated.py b/cmd2/annotated.py index 63a27f392..dc9ca1ad7 100644 --- a/cmd2/annotated.py +++ b/cmd2/annotated.py @@ -127,7 +127,20 @@ def do_paint( leaking ``:param:`` directives; and ``prog`` is rejected with ``subcommand_to`` because cmd2 rewrites it from the parent command's hierarchy. Mutually exclusive groups accept ``Group(required=True)`` to require exactly one member; the same flag on a plain ``groups=`` entry -raises ``ValueError`` (argparse's ``add_argument_group`` has no ``required``). +raises ``ValueError`` (argparse's ``add_argument_group`` has no ``required``). Give a +``mutually_exclusive_groups`` entry a ``title``/``description`` to render it as a titled help section +(argparse's one supported nesting -- a mutex *inside* an argument group), and use +``Option(action='store_true')`` for any ``bool`` member so the mutex reads as ``[--foo | --bar]`` +instead of expanding to ``--no-*`` variants. To put non-mutex parameters in the same section, list +its members in a ``groups=`` entry instead and leave the title off the mutex; declaring the section in +both places, a mutex that sits only partly in a ``groups=`` entry, or one that spans two of them all +raise ``ValueError``. The other three nesting directions (an argument group in an argument group or +in a mutex, and a mutex in a mutex) are removed in argparse on Python 3.14 and cannot be expressed +here. These group-spec rules (and member references, double-assignment, and the ``required=True`` +rejection) are checked at decoration time from parameter names alone -- type hints are not resolved, +so forward-referenced annotations still decorate -- meaning a misconfigured group raises when the +class is defined rather than on first command use. The one group rule that needs the annotations +(a required member in a mutually exclusive group) fires when the parser is built. Unsupported patterns (raise ``TypeError``): @@ -896,8 +909,8 @@ def __init__( self.build_error: Exception | None = None # cross-argument facts, linked by _resolve_parameters once the whole list is built: self.has_following_positional = False - # 1-based indices of the groups=/mutually_exclusive_groups= this parameter belongs to: - self.argument_group_indices: list[int] = [] + # 1-based indices of the mutually_exclusive_groups= entries this parameter belongs to + # (spec-shaped rules live in _validate_group_specs; this fact feeds the required-member row): self.mutex_group_indices: list[int] = [] # Derive every output slot now; validation stays deferred to _check_constraints. self._apply() @@ -1662,20 +1675,6 @@ def _const_mismatches_type(a: _ArgparseArgument) -> bool: f"which creates a positional argument that conflicts with subcommand parsing." ), ), - ( - # Cross-config: a parameter assigned to two argument groups is ambiguous. The membership - # indices are linked by _resolve_parameters from the decorator's groups= before this runs. - lambda a: len(a.argument_group_indices) > 1, - lambda a: ValueError( - f"parameter {a.name!r} cannot be assigned to both argument " - f"group {a.argument_group_indices[0]} and argument group {a.argument_group_indices[1]}" - ), - ), - ( - # Cross-config: a parameter cannot belong to two mutually exclusive groups. - lambda a: len(a.mutex_group_indices) > 1, - lambda a: ValueError(f"parameter {a.name!r} cannot be assigned to multiple mutually exclusive groups"), - ), ( # Cross-config: a required member is incompatible with a mutex group -- only one member is # supplied, so the others arrive as None (violating its non-Optional type), and argparse forbids @@ -1709,21 +1708,21 @@ def _const_mismatches_type(a: _ArgparseArgument) -> bool: _SKIP_PARAMS = frozenset({constants.NS_ATTR_SUBCOMMAND_FUNC, constants.NS_ATTR_STATEMENT}) -def _link_group_membership( +def _link_mutex_group_membership( by_name: dict[str, _ArgparseArgument], - specs: tuple[Group, ...] | None, - select: Callable[[_ArgparseArgument], list[int]], + mutually_exclusive_groups: tuple[Group, ...] | None, ) -> None: - """Append each spec's 1-based index to the *select*-ed membership list of each member argument. + """Append each mutex group's 1-based index to its member arguments' ``mutex_group_indices``. - :func:`_resolve_parameters` validates member references via :meth:`Group._validate_members` - before calling this, so every member name resolves to a built argument. + This membership is the fact behind the required-member constraint row. Member references are + validated upstream by :func:`_validate_group_specs` before this runs, so every member name + resolves to a built argument. """ - if not specs: + if not mutually_exclusive_groups: return - for index, spec in enumerate(specs, start=1): + for index, spec in enumerate(mutually_exclusive_groups, start=1): for name in spec.members: - select(by_name[name]).append(index) + by_name[name].mutex_group_indices.append(index) def _resolve_parameters( @@ -1731,14 +1730,14 @@ def _resolve_parameters( *, skip_params: frozenset[str] = _SKIP_PARAMS, base_command: bool = False, - groups: tuple[Group, ...] | None = None, mutually_exclusive_groups: tuple[Group, ...] | None = None, ) -> list[_ArgparseArgument]: """Resolve a function signature into a list of argparse-argument builders. ``base_command`` marks each argument's context for the base-command :data:`_CONSTRAINTS` rows and - drives the function-level ``cmd2_subcommand_func`` check below. ``groups``/``mutually_exclusive_groups`` - are linked onto each argument as membership facts for the cross-config constraint rows. + drives the function-level ``cmd2_subcommand_func`` check below. ``mutually_exclusive_groups`` + membership is linked onto each argument as the fact behind the required-member constraint row; + the spec-shaped group rules live in :func:`_validate_group_specs`, which runs before this. """ sig = inspect.signature(func) # Function-level check (not a per-argument _CONSTRAINTS row): a base command dispatches through @@ -1807,14 +1806,7 @@ def _resolve_parameters( for arg in positionals[:-1]: # every positional except the last has a following positional arg.has_following_positional = True by_name = {arg.name: arg for arg in resolved} - # Reject group references to nonexistent parameters before the constraint table runs. - all_param_names = set(by_name) - for spec in groups or (): - spec._validate_members(all_param_names=all_param_names, group_type="groups") - for spec in mutually_exclusive_groups or (): - spec._validate_members(all_param_names=all_param_names, group_type="mutually_exclusive_groups") - _link_group_membership(by_name, groups, lambda a: a.argument_group_indices) - _link_group_membership(by_name, mutually_exclusive_groups, lambda a: a.mutex_group_indices) + _link_mutex_group_membership(by_name, mutually_exclusive_groups) for arg in resolved: arg._check_constraints() return resolved @@ -1872,6 +1864,80 @@ def _filtered_namespace_kwargs( return filtered +def _validate_group_specs( + func: Callable[..., Any], + *, + skip_params: frozenset[str], + groups: tuple[Group, ...] | None, + mutually_exclusive_groups: tuple[Group, ...] | None, +) -> None: + """Validate ``groups=`` / ``mutually_exclusive_groups=`` specs from parameter names alone. + + Runs at decoration time (from both the regular-command and subcommand decoration paths, and + again from :func:`build_parser_from_function` for direct callers), so a misconfigured group + hard-fails when the class is defined instead of on first command use, where cmd2's runtime + handler turns the error into a printed message. Reads only parameter names and the ``Group`` + specs -- never the type hints -- so forward-referenced annotations still decorate. The one + group rule that needs the annotations (a required member in a mutually exclusive group) stays + in :data:`_CONSTRAINTS` and fires when the parser is built. + """ + if not groups and not mutually_exclusive_groups: + return + params = list(inspect.signature(func).parameters)[1:] # skip self/cls by position + all_param_names = {name for name in params if name not in skip_params} + + group_entry_for: dict[str, int] = {} + for index, spec in enumerate(groups or (), start=1): + spec._validate_members(all_param_names=all_param_names, group_type="groups") + if spec.required: + raise ValueError( + "Group(required=True) is only valid in mutually_exclusive_groups; " + "argparse's add_argument_group has no 'required' flag" + ) + for name in spec.members: + previous = group_entry_for.get(name) + if previous == index: + raise ValueError(f"parameter {name!r} is listed more than once in argument group {index}") + if previous is not None: + raise ValueError( + f"parameter {name!r} cannot be assigned to both argument group {previous} and argument group {index}" + ) + group_entry_for[name] = index + + mutex_entry_for: dict[str, int] = {} + for index, spec in enumerate(mutually_exclusive_groups or (), start=1): + spec._validate_members(all_param_names=all_param_names, group_type="mutually_exclusive_groups") + for name in spec.members: + previous = mutex_entry_for.get(name) + if previous == index: + raise ValueError(f"parameter {name!r} is listed more than once in mutually exclusive group {index}") + if previous is not None: + raise ValueError(f"parameter {name!r} cannot be assigned to multiple mutually exclusive groups") + mutex_entry_for[name] = index + parent_entries = {group_entry_for[name] for name in spec.members if name in group_entry_for} + if len(parent_entries) > 1: + raise ValueError( + f"mutually exclusive group {index} spans parameters in different argument groups, " + "which argparse cannot represent cleanly" + ) + if parent_entries: + # Members already sit in a titled groups= entry, so the mutex nests there. A section + # declared on both sides is ambiguous, and nesting a mutex that only partly overlaps the + # group would pull the ungrouped members into that group's help section. + if spec.title is not None or spec.description is not None: + raise ValueError( + f"mutually exclusive group {index} sets title/description, but its members already " + "belong to a groups= entry; declare the titled section in one place only" + ) + ungrouped = [name for name in spec.members if name not in group_entry_for] + if ungrouped: + raise ValueError( + f"mutually exclusive group {index} mixes members in a titled argument group with " + f"members that are not ({ungrouped!r}); list all of its members in the same groups= " + "entry to nest the mutex inside that group, or none of them to keep it top-level" + ) + + def _build_argument_group_targets( parser: argparse.ArgumentParser, *, @@ -1879,9 +1945,9 @@ def _build_argument_group_targets( ) -> tuple[dict[str, _ArgumentTarget], dict[str, argparse._ArgumentGroup]]: """Build argument groups and return add_argument targets for their members. - Member references and double-assignment are validated upstream by :func:`_resolve_parameters` - (via :meth:`Group._validate_members`) and :data:`_CONSTRAINTS` (the ``argument_group_indices`` - fact), so construction can assign each member unconditionally. + The specs are validated upstream by :func:`_validate_group_specs` (member references, + double-assignment, ``required=True`` rejection), so construction can assign each member + unconditionally. """ target_for: dict[str, _ArgumentTarget] = {} argument_group_for: dict[str, argparse._ArgumentGroup] = {} @@ -1890,11 +1956,6 @@ def _build_argument_group_targets( return target_for, argument_group_for for spec in groups: - if spec.required: - raise ValueError( - "Group(required=True) is only valid in mutually_exclusive_groups; " - "argparse's add_argument_group has no 'required' flag" - ) group = parser.add_argument_group(title=spec.title, description=spec.description) for name in spec.members: argument_group_for[name] = group @@ -1912,27 +1973,29 @@ def _apply_mutex_group_targets( ) -> None: """Build mutually exclusive groups and update add_argument targets for their members. - Member references, double-assignment, and required-member rejections are validated upstream by - :func:`_resolve_parameters` and :data:`_CONSTRAINTS` (the ``mutex_group_indices`` fact); the - remaining check -- a mutex group spanning different argument groups -- stays here because its - subject is the group, not an argument. + The specs are validated upstream by :func:`_validate_group_specs` (member references, + double-assignment, and the group-shaped rules: spanning, partial overlap, a section declared in + two places) and :data:`_CONSTRAINTS` (the required-member rule), so construction only chooses + each mutex group's parent: the argument group all its members share, a new titled section when + the spec carries ``title``/``description``, or the parser itself. """ if not mutually_exclusive_groups: return - for index, spec in enumerate(mutually_exclusive_groups, start=1): - member_names = spec.members - - parent_groups = {argument_group_for[name] for name in member_names if name in argument_group_for} - if len(parent_groups) > 1: - raise ValueError( - f"mutually exclusive group {index} spans parameters in different argument groups, " - "which argparse cannot represent cleanly" - ) + for spec in mutually_exclusive_groups: + parent_groups = {argument_group_for[name] for name in spec.members if name in argument_group_for} + if parent_groups: + # All members sit in one titled groups= entry, so the mutex nests there. + mutex_parent: _ArgumentTarget = next(iter(parent_groups)) + elif spec.title is not None or spec.description is not None: + # title/description on the mutex create the titled section and nest the mutex inside it, + # so a titled exclusive group needs only its own declaration -- no paired groups= entry. + mutex_parent = parser.add_argument_group(title=spec.title, description=spec.description) + else: + mutex_parent = parser - mutex_parent: _ArgumentTarget = next(iter(parent_groups)) if parent_groups else parser mutex_group = mutex_parent.add_mutually_exclusive_group(required=spec.required) - for name in member_names: + for name in spec.members: target_for[name] = mutex_group @@ -1983,6 +2046,9 @@ def build_parser_from_function( """ from . import argparse_utils + # The decorator already ran this at decoration time; direct callers get the same checks here. + _validate_group_specs(func, skip_params=skip_params, groups=groups, mutually_exclusive_groups=mutually_exclusive_groups) + parser_cls = parser_class or argparse_utils.DEFAULT_ARGUMENT_PARSER if "description" not in parser_kwargs: auto_description = _docstring_first_paragraph(func.__doc__) @@ -1990,13 +2056,11 @@ def build_parser_from_function( parser_kwargs["description"] = auto_description parser = parser_cls(**parser_kwargs) - # _resolve_parameters validates each argument and the cross-argument/cross-config rules (e.g. a - # variable-arity positional must be last; double-assignment and required-mutex-member) once the - # whole list is built and the group memberships are linked. + # _resolve_parameters validates each argument and the cross-argument rules (e.g. a variable-arity + # positional must be last; a required member in a mutex group) once the whole list is built. resolved = _resolve_parameters( func, skip_params=skip_params, - groups=groups, mutually_exclusive_groups=mutually_exclusive_groups, ) @@ -2011,7 +2075,7 @@ def build_parser_from_function( f"signature is expected at invocation. Drop argument_default=argparse.SUPPRESS." ) - # Build the group lookup (member references already validated by _resolve_parameters). + # Build the group lookup (specs already validated by _validate_group_specs above). target_for, argument_group_for = _build_argument_group_targets(parser, groups=groups) _apply_mutex_group_targets( parser, @@ -2116,9 +2180,19 @@ def _build_subcommand_handler( """ subcmd_name = _derive_subcommand_name(func, subcommand_to) + # Validate the group specs eagerly (decoration time) so a misconfigured group hard-fails when + # the class is defined; the name-only checks never resolve type hints, so forward-referenced + # annotations still decorate and the parser build stays deferred. + _validate_group_specs( + func, + skip_params=_SKIP_PARAMS, + groups=options.groups, + mutually_exclusive_groups=options.mutually_exclusive_groups, + ) if base_command: # Validate eagerly (decoration time); the base-command rows in _CONSTRAINTS fire here. - _resolve_parameters(func, base_command=True) + # skip_params is spelled out so this call cannot silently diverge from the parser build below. + _resolve_parameters(func, skip_params=_SKIP_PARAMS, base_command=True) _accepted = set(list(inspect.signature(func).parameters.keys())[1:]) _leading_names, _var_positional_name = _var_positional_call_plan(func) @@ -2291,6 +2365,15 @@ def decorator(fn: Callable[..., Any]) -> Callable[..., Any]: command_name = fn.__name__[len(constants.COMMAND_FUNC_PREFIX) :] skip_params = _SKIP_PARAMS | ({"_unknown"} if with_unknown_args else frozenset()) + # Validate the group specs eagerly (decoration time) so a misconfigured group hard-fails when + # the class is defined; the name-only checks never resolve type hints, so forward-referenced + # annotations still decorate and the parser build stays deferred. + _validate_group_specs( + fn, + skip_params=skip_params, + groups=options.groups, + mutually_exclusive_groups=options.mutually_exclusive_groups, + ) if base_command: # Validate eagerly (decoration time); the base-command rows in _CONSTRAINTS fire here. _resolve_parameters(fn, skip_params=skip_params, base_command=True) diff --git a/docs/features/annotated.md b/docs/features/annotated.md index a55d81e53..a6777c626 100644 --- a/docs/features/annotated.md +++ b/docs/features/annotated.md @@ -350,8 +350,7 @@ def do_greet(self, name: str): # parser.description == "Greet someone by name." ``` -`mutually_exclusive_groups` also takes `Group` instances (their `title`/`description` are ignored, -since argparse mutually-exclusive groups have no header). Pass `Group(..., required=True)` to make +`mutually_exclusive_groups` also takes `Group` instances. Pass `Group(..., required=True)` to make the mutex group itself required -- argparse will then enforce that exactly one of its members must be supplied. `required=True` is rejected on a plain (non-mutex) `Group` because `add_argument_group` has no `required` flag. @@ -363,6 +362,39 @@ has no `required` flag. def do_run(self, verbose: bool = False, quiet: bool = False): ... ``` +Give a `mutually_exclusive_groups` `Group` a `title`/`description` to render it as a titled help +section -- argparse's one supported nesting, a mutex _inside_ an argument group. You declare it +once, with no paired `groups=` entry. Use `Option(action="store_true")` on each `bool` member so the +choice reads as `[--json | --csv]` instead of expanding to `--json`/`--no-json` and +`--csv`/`--no-csv`: + +```py +@with_annotated( + mutually_exclusive_groups=( + Group("json", "csv", title="output", description="how to write results"), + ), +) +def do_render( + self, + json: Annotated[bool, Option(action="store_true")] = False, + csv: Annotated[bool, Option(action="store_true")] = False, +): ... +``` + +To put non-mutex parameters in the same section, declare a `groups=` entry with all of them and +leave the title off the mutex; argparse nests the mutex inside that group. Declaring the section in +both places, a mutex that sits only partly in a `groups=` entry, or one that spans two of them, each +raises `ValueError`. The other three nestings (an argument group inside another group or a mutex, or +a mutex inside a mutex) are removed in argparse on Python 3.14 and cannot be expressed here. + +All of these group-spec rules -- member references, a parameter assigned to two groups, +`required=True` on a plain group, and the mutex nesting rules above -- are validated when the +decorator runs, so a misconfigured group raises `ValueError` at class definition time instead of on +first command use. The checks read only parameter names, never the type hints, so forward-referenced +annotations still decorate cleanly. The one group rule that depends on the annotations (a member of +a mutually exclusive group must be omittable -- have a default or be `T | None`) fires when the +parser is built. + `parents=` mirrors argparse's standard parents mechanism for sharing argument definitions across parsers. `argument_default=argparse.SUPPRESS` is not supported and raises `TypeError`. It removes an absent argument from the parsed namespace, but `@with_annotated` builds the call from the function diff --git a/examples/annotated_example.py b/examples/annotated_example.py index 94b65ea2e..899fdcf9a 100755 --- a/examples/annotated_example.py +++ b/examples/annotated_example.py @@ -486,23 +486,30 @@ def do_connect(self, host: str, port: int = 22, verbose: bool = False) -> None: self.poutput(f"{msg} (verbose)" if verbose else msg) # -- Mutually exclusive groups ------------------------------------------- - # Group instances passed to mutually_exclusive_groups make argparse reject - # combinations (title/description are ignored here). + # A plain (untitled) mutex rejects combinations of its members; required=True + # makes exactly one of them mandatory. @with_annotated( description="Export data in exactly one format.", - mutually_exclusive_groups=(Group("json", "csv"),), + mutually_exclusive_groups=(Group("json", "csv", required=True),), ) @cmd2.with_category(ANNOTATED_CATEGORY) - def do_export(self, name: str, json: bool = False, csv: bool = False) -> None: - """Export a dataset; --json and --csv are mutually exclusive. + def do_export( + self, + name: str, + json: Annotated[str | None, Option(help_text="write JSON to this path")] = None, + csv: Annotated[str | None, Option(help_text="write CSV to this path")] = None, + ) -> None: + """Export a dataset to exactly one of --json PATH or --csv PATH (exclusive, required). Try: - export sales --json - export sales --json --csv # rejected: not allowed together + export sales --json out.json + export sales # rejected: one of --json/--csv is required + export sales --json a --csv b # rejected: not allowed together """ - fmt = "json" if json else "csv" if csv else "text" - self.poutput(f"Exporting {name} as {fmt}") + target = json or csv + fmt = "json" if json else "csv" + self.poutput(f"Exporting {name} to {target} as {fmt}") # -- Custom formatter and parser classes --------------------------------- # A custom help formatter or Cmd2ArgumentParser subclass can be supplied. @@ -539,6 +546,32 @@ def do_echo(self, text: str) -> None: """ self.poutput(text) + # -- Mutually exclusive group as a titled section --------------------------- + # A title/description on the mutex group renders it as a titled help section + # and nests it there in one declaration -- no paired groups= entry needed. + # The format flags are store_true so the mutex stays a clean [--json | --csv] + # (a bool flag would expand to --json/--no-json and make the group 4-way). + + @with_annotated( + mutually_exclusive_groups=(Group("json", "csv", title="output", description="how to write results"),), + ) + @cmd2.with_category(ANNOTATED_CATEGORY) + def do_render( + self, + name: str = "report", + json: Annotated[bool, Option(action="store_true")] = False, + csv: Annotated[bool, Option(action="store_true")] = False, + ) -> None: + """Render output; --json/--csv are exclusive and listed under 'output' in help. + + Try: + help render + render --json + render + """ + fmt = "json" if json else "csv" if csv else "text" + self.poutput(f"Rendering {name} as {fmt}") + if __name__ == "__main__": app = AnnotatedExample() diff --git a/tests/test_annotated.py b/tests/test_annotated.py index 0b27dc8c9..9a5e4b98a 100644 --- a/tests/test_annotated.py +++ b/tests/test_annotated.py @@ -41,6 +41,7 @@ _make_literal_type, _normalize_annotation, _parse_bool, + _validate_group_specs, build_parser_from_function, with_annotated, ) @@ -1030,6 +1031,35 @@ def test_param_in_multiple_groups_raises(self) -> None: with pytest.raises(ValueError, match="cannot be assigned to both argument group"): build_parser_from_function(_func_grouped, groups=(Group("local"), Group("local", "remote"))) + @pytest.mark.filterwarnings("error::DeprecationWarning") + def test_mutex_group_nests_inside_argument_group(self) -> None: + """A mutex group whose members all sit in one Group(...) is created inside that argument group. + + This is the one nesting direction argparse supports: ``group.add_mutually_exclusive_group()``. + The Python 3.11 deprecations cover the other three directions (group-in-group, group-in-mutex, + mutex-in-mutex) -- group-in-group is a hard ``ValueError`` on current Python. The + ``filterwarnings`` marker turns any future argparse deprecation of this direction into a + test failure. + """ + parser = build_parser_from_function( + _func_grouped, + groups=(Group("local", "remote", "force", title="Location"),), + mutually_exclusive_groups=(Group("local", "remote"),), + ) + assert len(parser._mutually_exclusive_groups) == 1 + mutex = parser._mutually_exclusive_groups[0] + location = next(g for g in parser._action_groups if g.title == "Location") + # The mutex group's container is the titled argument group, so its members render under it. + assert mutex._container is location + mutex_dests = {action.dest for action in mutex._group_actions} + assert mutex_dests == {"local", "remote"} + # The non-mutex member still lands in the argument group. + assert {action.dest for action in location._group_actions} == {"local", "remote", "force"} + assert "Location" in parser.format_help() + # Exclusivity is enforced. + with pytest.raises(SystemExit): + parser.parse_args(["--local", "a", "--remote", "b"]) + def test_mutex_group_spanning_different_argument_groups_raises(self) -> None: with pytest.raises(ValueError, match="spans parameters in different argument groups"): build_parser_from_function( @@ -1038,6 +1068,41 @@ def test_mutex_group_spanning_different_argument_groups_raises(self) -> None: mutually_exclusive_groups=(Group("local", "remote"),), ) + def test_mutex_group_partially_in_argument_group_raises(self) -> None: + # ``local`` is in the titled group, ``remote`` is not: nesting the whole mutex inside the group + # would silently pull ``remote`` into its section, so all-or-none membership is required. + with pytest.raises(ValueError, match="mixes members in a titled argument group"): + build_parser_from_function( + _func_grouped, + groups=(Group("local", title="Location"),), + mutually_exclusive_groups=(Group("local", "remote"),), + ) + + def test_mutex_group_title_creates_titled_section(self) -> None: + # title/description on the mutex itself build the titled section and nest the mutex inside it, + # so no paired groups= entry is needed. + parser = build_parser_from_function( + _func_grouped, + mutually_exclusive_groups=(Group("local", "remote", title="Location", description="where"),), + ) + section = next(g for g in parser._action_groups if g.title == "Location") + assert section.description == "where" + assert len(parser._mutually_exclusive_groups) == 1 + mutex = parser._mutually_exclusive_groups[0] + assert mutex._container is section + assert {action.dest for action in mutex._group_actions} == {"local", "remote"} + with pytest.raises(SystemExit): + parser.parse_args(["--local", "a", "--remote", "b"]) + + def test_mutex_group_title_with_members_in_argument_group_raises(self) -> None: + # Declaring the titled section in both places (groups= and the mutex) is ambiguous. + with pytest.raises(ValueError, match="declare the titled section in one place only"): + build_parser_from_function( + _func_grouped, + groups=(Group("local", "remote", title="A"),), + mutually_exclusive_groups=(Group("local", "remote", title="B"),), + ) + def test_mutually_exclusive_group(self) -> None: """Mutually exclusive params cannot be used together.""" @@ -1314,8 +1379,7 @@ def test_build_argument_group_targets(self) -> None: assert target_for["dst"] is argument_group_for["dst"] def test_duplicate_argument_group_assignment_raises(self) -> None: - # Double-assignment is enforced by _CONSTRAINTS (the argument_group_indices membership fact), - # so it surfaces through the real build path rather than _build_argument_group_targets itself. + # Double-assignment is enforced by _validate_group_specs, which the build path runs first. def func(self, *, verbose: bool = False) -> None: ... with pytest.raises(ValueError, match="argument group 1 and argument group 2"): @@ -1336,25 +1400,136 @@ def test_apply_mutex_group_targets(self) -> None: assert isinstance(target_for["json"], argparse._MutuallyExclusiveGroup) def test_duplicate_mutex_group_assignment_raises(self) -> None: - # Double-assignment is enforced by _CONSTRAINTS (the mutex_group_indices membership fact). + # Double-assignment is enforced by _validate_group_specs, which the build path runs first. def func(self, *, verbose: bool = False) -> None: ... with pytest.raises(ValueError, match="multiple mutually exclusive groups"): build_parser_from_function(func, mutually_exclusive_groups=(Group("verbose"), Group("verbose"))) - def test_apply_mutex_group_targets_rejects_cross_group_members(self) -> None: - parser = argparse.ArgumentParser() - _target_for, argument_group_for = _build_argument_group_targets(parser, groups=(Group("src"), Group("dst"))) + def test_repeated_member_in_one_argument_group_raises(self) -> None: + # A name listed twice in a single Group is a distinct mistake from cross-group assignment + # and gets its own message (not the misleading "both group 1 and group 1"). + def func(self, *, verbose: bool = False) -> None: ... + + with pytest.raises(ValueError, match="listed more than once in argument group 1"): + build_parser_from_function(func, groups=(Group("verbose", "verbose"),)) + + def test_repeated_member_in_one_mutex_group_raises(self) -> None: + def func(self, *, verbose: bool = False) -> None: ... + + with pytest.raises(ValueError, match="listed more than once in mutually exclusive group 1"): + build_parser_from_function(func, mutually_exclusive_groups=(Group("verbose", "verbose"),)) + + def test_validate_group_specs_rejects_cross_group_members(self) -> None: + # The spec validator owns the group-shaped rules; construction trusts it. + def func(self, src: str = "a", dst: str = "b") -> None: ... with pytest.raises(ValueError, match="different argument groups"): - _apply_mutex_group_targets( - parser, - target_for={}, - argument_group_for=argument_group_for, + _validate_group_specs( + func, + skip_params=frozenset(), + groups=(Group("src"), Group("dst")), mutually_exclusive_groups=(Group("src", "dst"),), ) +class TestEagerGroupSpecValidation: + """Group specs hard-fail at decoration time (class definition), not on first command use. + + The decorator runs the name-only spec checks before deferring the parser build, so a + misconfigured group raises while the class body executes instead of surfacing as a swallowed + runtime error the first time the command runs. Type hints are never resolved by these checks, + so forward-referenced annotations still decorate (the parser build stays deferred for them). + """ + + def test_member_typo_fails_at_decoration(self) -> None: + def do_x(self, a: str = "v") -> None: ... + + with pytest.raises(ValueError, match="groups references nonexistent parameter 'typo'"): + with_annotated(groups=(Group("typo"),))(do_x) + + def test_mutex_member_typo_fails_at_decoration(self) -> None: + def do_x(self, a: str = "v") -> None: ... + + with pytest.raises(ValueError, match="mutually_exclusive_groups references nonexistent parameter 'typo'"): + with_annotated(mutually_exclusive_groups=(Group("typo"),))(do_x) + + def test_param_in_two_argument_groups_fails_at_decoration(self) -> None: + def do_x(self, a: str = "v") -> None: ... + + with pytest.raises(ValueError, match="argument group 1 and argument group 2"): + with_annotated(groups=(Group("a"), Group("a")))(do_x) + + def test_param_in_two_mutex_groups_fails_at_decoration(self) -> None: + def do_x(self, a: str = "v", b: str = "w") -> None: ... + + with pytest.raises(ValueError, match="multiple mutually exclusive groups"): + with_annotated(mutually_exclusive_groups=(Group("a", "b"), Group("a")))(do_x) + + def test_required_on_plain_group_fails_at_decoration(self) -> None: + def do_x(self, a: str = "v") -> None: ... + + with pytest.raises(ValueError, match="only valid in mutually_exclusive_groups"): + with_annotated(groups=(Group("a", required=True),))(do_x) + + def test_mutex_spanning_argument_groups_fails_at_decoration(self) -> None: + def do_x(self, a: str = "v", b: str = "w") -> None: ... + + with pytest.raises(ValueError, match="spans parameters in different argument groups"): + with_annotated(groups=(Group("a"), Group("b")), mutually_exclusive_groups=(Group("a", "b"),))(do_x) + + def test_mutex_partially_in_argument_group_fails_at_decoration(self) -> None: + def do_x(self, a: str = "v", b: str = "w") -> None: ... + + with pytest.raises(ValueError, match="mixes members in a titled argument group"): + with_annotated(groups=(Group("a", title="T"),), mutually_exclusive_groups=(Group("a", "b"),))(do_x) + + def test_titled_section_declared_twice_fails_at_decoration(self) -> None: + def do_x(self, a: str = "v", b: str = "w") -> None: ... + + with pytest.raises(ValueError, match="declare the titled section in one place only"): + with_annotated( + groups=(Group("a", "b", title="T"),), + mutually_exclusive_groups=(Group("a", "b", title="U"),), + )(do_x) + + def test_subcommand_group_typo_fails_at_decoration(self) -> None: + def team_add(self, a: str = "v") -> None: ... + + with pytest.raises(ValueError, match="nonexistent parameter 'typo'"): + with_annotated(subcommand_to="team", groups=(Group("typo"),))(team_add) + + def test_eager_validation_does_not_resolve_type_hints(self) -> None: + # The group error fires even though the annotation can never resolve: the checks read + # parameter names only, so the unresolvable hint is not touched. + def do_x(self, a: "NoSuchType" = None) -> None: ... # noqa: F821 + + with pytest.raises(ValueError, match="nonexistent parameter 'typo'"): + with_annotated(groups=(Group("typo"),))(do_x) + + def test_unresolvable_hints_with_valid_groups_decorate(self) -> None: + # Valid specs + an unresolvable annotation must decorate without raising; type resolution + # stays deferred to the parser build, preserving forward-reference support. + def do_x(self, a: "NoSuchType" = None) -> None: ... # noqa: F821 + + with_annotated(groups=(Group("a", title="T"),))(do_x) + + def test_valid_nested_config_decorates_cleanly(self) -> None: + def do_x( + self, + a: Annotated[bool, Option(action="store_true")] = False, + b: Annotated[bool, Option(action="store_true")] = False, + c: Annotated[bool, Option(action="store_true")] = False, + d: Annotated[bool, Option(action="store_true")] = False, + ) -> None: ... + + # A nested mutex plus a separately titled mutex must decorate without raising. + with_annotated( + groups=(Group("a", "b", title="T"),), + mutually_exclusive_groups=(Group("a", "b"), Group("c", "d", title="U")), + )(do_x) + + # --------------------------------------------------------------------------- # _resolve_annotation: positional vs option classification # ---------------------------------------------------------------------------