modern_decode: read abstract sequence interfaces as several values

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):

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()

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}

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 = "<EOF>"\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: