Merge pull request #119 from DanCardin/dc/prettier-union-errors

feat: Improve union error messages.

CHANGELOG.md

@@ -2,6 +2,10 @@
 
 ## 0.20
 
+### 0.20.1
+
+- feat: Improve union error message.
+
 ### 0.20.0
 
 - feat: Implement exclusive argument groups.

pyproject.toml

@@ -1,6 +1,6 @@
 [tool.poetry]
 name = "cappa"
-version = "0.20.0"
+version = "0.20.1"
 description = "Declarative CLI argument parser."
 
 repository = "https://github.com/dancardin/cappa"

src/cappa/annotation.py

@@ -7,7 +7,7 @@
 from typing_inspect import get_origin, is_literal_type
 
 from cappa.file_io import FileMode
-from cappa.typing import T, backend_type, is_none_type, is_subclass, is_union_type
+from cappa.typing import T, is_none_type, is_subclass, is_union_type
 
 __all__ = [
     "parse_value",
@@ -57,7 +57,7 @@ def parse_value(
         return origin
 
     if is_none_type(origin):
-        return parse_none()
+        return parse_none
 
     if is_subclass(origin, list):
         return parse_list(*type_args)
@@ -87,8 +87,9 @@ def literal_mapper(value):
             if raw_value == value:
                 return type_arg
 
+        options = ", ".join(f"'{t}'" for t in type_args)
         raise ValueError(
-            f"Invalid choice: '{value}' (choose from {', '.join(str(t) for t in type_args)})"
+            f"Invalid choice: '{value}' (choose from literal values {options})"
         )
 
     return literal_mapper
@@ -142,20 +143,27 @@ def parse_union(*type_args: type) -> typing.Callable[[typing.Any], typing.Any]:
     def type_priority_key(type_) -> int:
         return type_priority.get(type_, 1)
 
-    mappers: list[typing.Callable] = [
-        parse_value(t) for t in sorted(type_args, key=type_priority_key)
+    mappers: list[tuple[type, typing.Callable]] = [
+        (t, parse_value(t)) for t in sorted(type_args, key=type_priority_key)
     ]
 
     def union_mapper(value):
-        for mapper in mappers:
+        exceptions = []
+        for type_arg, mapper in mappers:
             try:
                 return mapper(value)
-            except (ValueError, TypeError):
-                pass
-
-        raise ValueError(
-            f"Could not parse '{value}' given options: {', '.join(backend_type(t) for t in type_args)}"
-        )
+            except (ValueError, TypeError) as e:
+                if mapper is parse_none:
+                    err = " - <no value>"
+                else:
+                    err = f" - {repr_type(type_arg)}: {e}"
+                exceptions.append(err)
+
+        # Perhaps we should be showing all failed mappings at some point. As-is,
+        # the preferred interpretation will be determined by order in the event
+        # of all mappings failing
+        reasons = "\n".join(exceptions)
+        raise ValueError(f"Possible variants\n{reasons}")
 
     return union_mapper
 
@@ -172,16 +180,12 @@ def optional_mapper(value) -> T | None:
     return optional_mapper
 
 
-def parse_none():
+def parse_none(value):
     """Create a value parser for None."""
+    if value is None:
+        return
 
-    def map_none(value):
-        if value is None:
-            return
-
-        raise ValueError(value)
-
-    return map_none
+    raise ValueError(value)
 
 
 def parse_file_io(
@@ -226,3 +230,10 @@ def detect_choices(origin: type, type_args: tuple[type, ...]) -> list[str] | Non
 
 def is_sequence_type(typ):
     return is_subclass(get_origin(typ) or typ, (typing.List, typing.Tuple, typing.Set))
+
+
+def repr_type(t):
+    if isinstance(t, type) and not typing.get_origin(t):
+        return str(t.__name__)
+
+    return str(t).replace("typing.", "")

src/cappa/command.py

@@ -227,7 +227,7 @@ def map_result(self, command: Command[T], prog: str, parsed_args) -> T:
                 except Exception as e:
                     exception_reason = str(e)
                     raise Exit(
-                        f"Invalid value for '{arg.names_str()}' with value '{value}': {exception_reason}",
+                        f"Invalid value for '{arg.names_str()}': {exception_reason}",
                         code=2,
                         prog=prog,
                     )

tests/arg/test_file_io.py

@@ -173,10 +173,8 @@ class Foo:
     with pytest.raises(cappa.Exit) as e:
         parse(Foo, "foo.py", backend=backend)
 
-    assert (
-        e.value.message
-        == "Invalid value for 'bar' with value 'foo.py': can't have text and binary mode at once"
-    )
+    assert str(e.value.message).startswith("Invalid value for 'bar'")
+    assert str(e.value.message).endswith("can't have text and binary mode at once")
 
 
 @backends

tests/arg/test_literal.py

@@ -1,10 +1,12 @@
 from __future__ import annotations
 
+import textwrap
 from dataclasses import dataclass
 from typing import Literal, Union
 
 import cappa
 import pytest
+from typing_extensions import Annotated
 
 from tests.utils import backends, parse
 
@@ -35,3 +37,23 @@ def test_invalid(backend):
     assert (
         "invalid choice: 'thename' (choose from 'one', 'two', 'three', '4')" in message
     )
+
+
+@backends
+def test_invalid_collection_of_literals(backend):
+    @dataclass
+    class Args:
+        foo: Annotated[set[Literal["one", "two"]] | None, cappa.Arg(short=True)] = None
+
+    with pytest.raises(cappa.Exit) as e:
+        parse(Args, "-f", "three", backend=backend)
+
+    assert e.value.code == 2
+
+    err = textwrap.dedent(
+        """\
+        Invalid value for '-f': Possible variants
+         - set[Literal['one', 'two']]: Invalid choice: 'three' (choose from literal values 'one', 'two')
+         - <no value>"""
+    )
+    assert str(e.value.message).lower() == err.lower()

tests/arg/test_literal_intermixed_union.py

@@ -1,5 +1,6 @@
 from __future__ import annotations
 
+import textwrap
 from dataclasses import dataclass
 from typing import Literal, Union
 
@@ -33,7 +34,10 @@ def test_invalid_string(backend):
 
     assert e.value.code == 2
 
-    assert e.value.message == (
-        "Invalid value for 'name' with value 'thename': "
-        "Could not parse 'thename' given options: <int>, one"
+    err = textwrap.dedent(
+        """\
+        Invalid value for 'name': Possible variants
+         - Literal['one']: Invalid choice: 'thename' (choose from literal values 'one')
+         - int: invalid literal for int() with base 10: 'thename'"""
     )
+    assert err in str(e.value.message)

tests/arg/test_mapping_failure.py

@@ -23,7 +23,7 @@ class ArgTest:
     assert e.value.code == 2
     assert (
         e.value.message
-        == "Invalid value for 'default' with value 'foo': invalid literal for int() with base 10: 'foo'"
+        == "Invalid value for 'default': invalid literal for int() with base 10: 'foo'"
     )
 
 

tests/arg/test_parse.py

@@ -74,5 +74,5 @@ class ArgTest:
     assert e.value.code == 2
     assert (
         e.value.message
-        == "Invalid value for 'numbers' with value 'one': could not convert string to float: 'one'"
+        == "Invalid value for 'numbers': could not convert string to float: 'one'"
     )