diff --git a/cw/grammar.py b/cw/grammar.py index f56b529..f0d4c2e 100644 --- a/cw/grammar.py +++ b/cw/grammar.py @@ -39,6 +39,7 @@ :class:`cw.base.ArghHelpFormatter` renders as ``repr(default)``. """ +import collections.abc import dataclasses import enum import inspect @@ -101,6 +102,46 @@ _NONE_TYPE = type(None) +#: Origins :func:`modern_decode` reads as "several values", beyond ``list``. +#: +#: ``argh`` recognises ``list`` and nothing else, so ``names: Sequence[str]`` +#: silently becomes one positional taking one token. That penalises exactly the +#: annotation style that prefers ``collections.abc`` interfaces over concrete +#: containers — and the failure is quiet: the parser builds, the command runs, +#: and the second argument is an "unrecognized argument". +#: +#: ``str`` and ``bytes`` are *not* reachable here and must never become so. Both +#: are registered ``Sequence``\ s, so an ``issubclass`` test would read every +#: ``name: str`` as variadic. Matching on ``typing.get_origin`` avoids that by +#: construction: ``get_origin(str)`` is ``None``. +_SEQUENCE_ORIGINS: tuple = ( + list, + set, + frozenset, + tuple, + collections.abc.Sequence, + collections.abc.MutableSequence, + collections.abc.Iterable, + collections.abc.Collection, + collections.abc.Set, + collections.abc.MutableSet, +) + +#: Bare, unsubscripted spellings of the same idea: ``x: Sequence`` rather than +#: ``x: Sequence[str]``. ``list`` is absent because :func:`argh_decode` already +#: covers it, and this table must not change what argh-compatible mode does. +_BARE_SEQUENCES: tuple = ( + set, + frozenset, + tuple, + collections.abc.Sequence, + collections.abc.MutableSequence, + collections.abc.Iterable, + collections.abc.Collection, + collections.abc.Set, + collections.abc.MutableSet, +) + class GrammarError(ValueError): """A signature and its overrides cannot be reconciled into a CLI. @@ -382,6 +423,14 @@ def modern_decode(param: inspect.Parameter, hint: Any) -> Mapping[str, Any]: >>> decoded['metavar'] '{RED}' + + Abstract sequence interfaces mean what they say, which argh-compatible mode + cannot do — argh recognises ``list`` and nothing else: + + >>> modern_decode(p, typing.Sequence[str]) == {'nargs': '*', 'type': str} + True + >>> argh_decode(p, typing.Sequence[str]) + {} """ hint = _unwrap_optional(hint) if isinstance(hint, type) and issubclass(hint, enum.Enum): @@ -396,9 +445,67 @@ def modern_decode(param: inspect.Parameter, hint: Any) -> Mapping[str, Any]: } if isinstance(hint, type) and issubclass(hint, pathlib.PurePath): return {"type": pathlib.Path} + sequence = _decode_sequence(hint) + if sequence is not None: + return sequence return argh_decode(param, hint) +def _decode_sequence(hint: Any) -> Optional[Dict[str, Any]]: + """``Sequence[str]`` and friends -> ``nargs``, or ``None`` to fall through. + + ``argh_decode`` covers ``list`` because argh does; this covers the abstract + interfaces and the other concrete containers, because a signature written + ``names: Sequence[str]`` means the same thing to a reader and currently + means something else to the parser. + + A **homogeneous fixed-length tuple** becomes a fixed ``nargs``, which is the + one case where the annotation carries a count: + + >>> _decode_sequence(tuple[int, int]) == {'nargs': 2, 'type': int} + True + + A heterogeneous one falls through to no inference, because ``add_argument`` + has a single ``type`` and there is no honest one to choose: + + >>> _decode_sequence(tuple[int, str]) is None + True + + The abstract interfaces, subscripted and bare: + + >>> _decode_sequence(typing.Sequence[str]) == {'nargs': '*', 'type': str} + True + >>> _decode_sequence(collections.abc.Iterable) == {'nargs': '*'} + True + + And the trap this must not fall into — ``str`` and ``bytes`` are registered + ``Sequence``\ s, so a subclass test would read every string parameter as + variadic: + + >>> _decode_sequence(str) is None + True + >>> _decode_sequence(bytes) is None + True + """ + if hint in _BARE_SEQUENCES: + return {"nargs": ZERO_OR_MORE} + origin = typing.get_origin(hint) + if origin is None or origin not in _SEQUENCE_ORIGINS: + return None + args = typing.get_args(hint) + if origin is tuple and args and Ellipsis not in args: + # A fixed-length tuple states its own count, but only a homogeneous one + # has a `type` argparse could apply to every position. + if len(set(args)) != 1 or args[0] not in BASIC_TYPES: + return None + return {"nargs": len(args), "type": args[0]} + guessed: Dict[str, Any] = {"nargs": ZERO_OR_MORE} + item = next((a for a in args if a is not Ellipsis), None) + if item in BASIC_TYPES: + guessed["type"] = item + return guessed + + def _unwrap_optional(hint: Any) -> Any: """``Optional[X]`` -> ``X``; anything else unchanged.""" if any(typing.get_origin(hint) is union for union in _UNION_TYPES): diff --git a/tests/test_cli.py b/tests/test_cli.py index 05548bc..7634ba0 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -773,9 +773,13 @@ def test_group_kwargs_help_is_inert_and_title_is_not(self): here deliberately; what changed is that the error message now says so. """ with_help = cw.mk_parser({}, prog="x") - add_commands(with_help, ARCHIVE, group_name="archive", group_kwargs={"help": "H"}) + add_commands( + with_help, ARCHIVE, group_name="archive", group_kwargs={"help": "H"} + ) assert "H" not in with_help.format_help() with_title = cw.mk_parser({}, prog="x") - add_commands(with_title, ARCHIVE, group_name="archive", group_kwargs={"title": "H"}) + add_commands( + with_title, ARCHIVE, group_name="archive", group_kwargs={"title": "H"} + ) assert "H" in with_title.format_help() diff --git a/tests/test_grammar.py b/tests/test_grammar.py index 4a0b756..f294177 100644 --- a/tests/test_grammar.py +++ b/tests/test_grammar.py @@ -11,6 +11,7 @@ import functools import inspect import pathlib +import collections.abc import typing import pytest @@ -334,6 +335,82 @@ def test_modern_decode_reads_an_enum_by_name_then_value(): decoded["type"]("green") +# -------------------------------------------------------------------------------------- +# Abstract sequence interfaces +# +# argh recognises `list` and nothing else, so `names: Sequence[str]` silently became one +# positional taking one token — the parser built, the command ran, and the second argument +# came back as an "unrecognized argument". That penalises exactly the annotation style +# that prefers `collections.abc` interfaces over concrete containers. + + +@pytest.mark.parametrize( + "hint", + [ + typing.Sequence[str], + typing.Iterable[str], + collections.abc.Sequence[str], + collections.abc.Iterable[str], + collections.abc.Collection[str], + tuple[str, ...], + set[str], + frozenset[str], + ], +) +def test_modern_decode_reads_abstract_sequences_as_several_values(hint): + assert modern_decode(PARAM, hint) == {"nargs": "*", "type": str} + + +@pytest.mark.parametrize( + "hint", + [typing.Sequence, collections.abc.Iterable, tuple, set, frozenset], +) +def test_modern_decode_reads_bare_sequences_too(hint): + """`x: Sequence` says the same thing as `x: Sequence[str]` about arity.""" + assert modern_decode(PARAM, hint) == {"nargs": "*"} + + +@pytest.mark.parametrize("hint", [str, bytes]) +def test_modern_decode_does_NOT_read_str_or_bytes_as_a_sequence(hint): + """The trap. Both are registered `Sequence`s, so an `issubclass` test would + read every string parameter as variadic — and every existing CLI would break + in a way whose error message points at the wrong thing. + + Matching on `typing.get_origin` avoids it by construction: `get_origin(str)` + is None. + """ + assert "nargs" not in modern_decode(PARAM, hint) + + +def test_modern_decode_gives_a_homogeneous_fixed_tuple_a_fixed_nargs(): + """The one case where the annotation carries a count.""" + assert modern_decode(PARAM, tuple[int, int]) == {"nargs": 2, "type": int} + + +def test_modern_decode_declines_a_heterogeneous_tuple(): + """`add_argument` has a single `type`, and there is no honest one to pick — + so no inference at all beats a wrong converter.""" + assert modern_decode(PARAM, tuple[int, str]) == {} + + +def test_modern_decode_unwraps_an_optional_sequence(): + assert modern_decode(PARAM, typing.Optional[typing.Sequence[int]]) == { + "nargs": "*", + "type": int, + } + + +def test_argh_decode_is_UNCHANGED_by_any_of_this(): + """`argh_decode` is argh's if-chain, if-branch for if-branch. Widening it + would break the compatibility contract that is its whole purpose — the new + behaviour lives in `modern_decode`, which is the seam for exactly this.""" + assert argh_decode(PARAM, typing.Sequence[str]) == {} + assert argh_decode(PARAM, collections.abc.Iterable[str]) == {} + assert argh_decode(PARAM, tuple[int, int]) == {} + # and what it DID cover still works + assert argh_decode(PARAM, list[int]) == {"nargs": "*", "type": int} + + def test_modern_decode_maps_pure_paths_to_path(): assert modern_decode(PARAM, pathlib.Path) == {"type": pathlib.Path} assert modern_decode(PARAM, pathlib.PurePosixPath) == {"type": pathlib.Path} diff --git a/tests/test_testing.py b/tests/test_testing.py index 805f7fa..984ae39 100644 --- a/tests/test_testing.py +++ b/tests/test_testing.py @@ -418,10 +418,10 @@ class TestPinnedStdin: """ READS_STDIN = ( - 'import sys\n' - 'try:\n' + "import sys\n" + "try:\n" ' line = input("prompt> ")\n' - 'except EOFError:\n' + "except EOFError:\n" ' line = ""\n' 'print("read:", line)\n' ) @@ -530,7 +530,10 @@ def test_only_the_program_s_own_exe_is_scrubbed(self): def test_the_program_need_not_be_the_command_s_first_word(self): command = ["/usr/bin/python", "/tmp/toy.exe"] - assert testing.scrub_exe_suffix("usage: toy.EXE [-h]", command) == "usage: toy [-h]" + assert ( + testing.scrub_exe_suffix("usage: toy.EXE [-h]", command) + == "usage: toy [-h]" + ) class TestExitStatus: