Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Perform choices validation after parse/mapping time. #179

Merged
merged 1 commit into from
Nov 20, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@

## 0.25

### 0.25.1

- fix: Perform choices validation after parse/mapping time.

### 0.25.0

- feat: Add `Arg.propagate`.
Expand Down
2 changes: 1 addition & 1 deletion pyproject.toml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[project]
name = "cappa"
version = "0.25.0"
version = "0.25.1"
description = "Declarative CLI argument parser."

urls = {repository = "https://github.com/dancardin/cappa"}
Expand Down
15 changes: 13 additions & 2 deletions src/cappa/arg.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
from cappa.completion.completers import complete_choices
from cappa.completion.types import Completion
from cappa.env import Env
from cappa.parse import evaluate_parse, parse_value
from cappa.parse import Parser, evaluate_parse, parse_literal, parse_value
from cappa.type_view import Empty, EmptyType, TypeView
from cappa.typing import (
Doc,
Expand Down Expand Up @@ -582,7 +582,18 @@ def infer_parse(arg: Arg, type_view: TypeView) -> Callable:
else:
parse = parse_value(type_view)

return evaluate_parse(parse, type_view)
# This is the original arg choices, e.g. explicitly provided. Inferred choices
# need to be handled internally to the parsers.
if arg.choices:
if not isinstance(parse, Sequence):
parse = [parse]

literal_type = typing.Literal[tuple(arg.choices)] # type: ignore
parse = [*parse, parse_literal(literal_type)] # type: ignore

return evaluate_parse(
typing.cast(Union[Sequence[Parser], Parser], parse), type_view
)


def infer_help(arg: Arg, fallback_help: str | None) -> str | None:
Expand Down
4 changes: 2 additions & 2 deletions src/cappa/argparse.py
Original file line number Diff line number Diff line change
Expand Up @@ -250,8 +250,8 @@ def add_argument(
elif is_positional and not arg.required:
kwargs["nargs"] = "?"

if arg.choices:
kwargs["choices"] = arg.choices
# if arg.choices:
# kwargs["choices"] = arg.choices

deprecated_kwarg = add_deprecated_kwarg(arg)
kwargs.update(deprecated_kwarg)
Expand Down
27 changes: 23 additions & 4 deletions src/cappa/parse.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
from __future__ import annotations

import enum
import functools
import types
import typing
Expand Down Expand Up @@ -92,6 +93,9 @@ def parse_value(typ: MaybeTypeView) -> Parser[T]:
if type_view.is_none_type:
return parse_none

if type_view.is_subclass_of(enum.Enum):
return parse_enum(type_view)

if type_view.is_subclass_of(datetime):
return datetime.fromisoformat # type: ignore

Expand Down Expand Up @@ -130,14 +134,24 @@ def literal_mapper(value):
if raw_value == value:
return type_arg

options = ", ".join(f"'{t}'" for t in type_view.args)
raise ValueError(
f"Invalid choice: '{value}' (choose from literal values {options})"
)
raise choices_error(type_view.args, value)

return literal_mapper


def parse_enum(typ):
type_view = _as_type_view(typ)
choices = tuple(v.value for v in type_view.annotation)

def enum_mapper(value):
try:
return type_view.annotation(value)
except ValueError:
raise choices_error(choices, value)

return enum_mapper


def parse_list(typ: MaybeTypeView[list[T]]) -> Parser[list[T]]:
"""Create a value parser for a list of given type `of_type`."""
type_view = _as_type_view(typ)
Expand Down Expand Up @@ -278,3 +292,8 @@ def sequence_parsers(value):
return result

return sequence_parsers


def choices_error(choices, value):
options = ", ".join(f"{t!r}" for t in choices)
return ValueError(f"Invalid choice: '{value}' (choose from {options})")
10 changes: 1 addition & 9 deletions src/cappa/parser.py
Original file line number Diff line number Diff line change
Expand Up @@ -578,15 +578,6 @@ def consume_arg(
if orig_num_args == 1:
if result:
result = result[0]
if arg.choices and result not in arg.choices:
choices = ", ".join(f"'{c}'" for c in arg.choices)
raise BadArgumentError(
f"Invalid choice: '{result}' (choose from {choices})",
value=result,
command=parse_state.current_command,
arg=arg,
)

if parse_state.provide_completions and not parse_state.has_values():
if arg.completion:
completions: list[Completion] | list[FileCompletion] = (
Expand Down Expand Up @@ -645,6 +636,7 @@ def consume_arg(
ParseState: parse_state,
Arg: arg,
Value: Value(result),
typing.Any: result,
}
if option:
fulfilled_deps[RawOption] = option
Expand Down
15 changes: 15 additions & 0 deletions tests/arg/test_choices.py
Original file line number Diff line number Diff line change
Expand Up @@ -72,3 +72,18 @@ class ArgTest:

result = capsys.readouterr().out
assert "Valid options: one, two." not in result


@backends
def test_literal_parse_sequence(backend):
@dataclass
class LiteralParse:
log_level: Annotated[
str,
cappa.Arg(
short="-L", long=True, choices=["DEBUG"], parse=[str.upper, str.strip]
),
] = "DEBUG"

result = parse(LiteralParse, "--log-level", " debug ", backend=backend)
assert result == LiteralParse(log_level="DEBUG")
48 changes: 41 additions & 7 deletions tests/arg/test_literal.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,31 +11,52 @@
from tests.utils import backends, parse


@dataclass
class ArgTest:
name: Union[Literal["one"], Literal["two"], Literal["three"], Literal[4]]


@backends
def test_valid(backend):
@dataclass
class ArgTest:
name: Literal["one", "two"]

test = parse(ArgTest, "two", backend=backend)
assert test.name == "two"


@backends
def test_valid_int(backend):
@dataclass
class ArgTest:
name: Literal["one", 4]

test = parse(ArgTest, "4", backend=backend)
assert test.name == 4


@backends
def test_invalid(backend):
@dataclass
class ArgTest:
name: Literal["one", "two", "three", 4]

with pytest.raises(cappa.Exit) as e:
parse(ArgTest, "thename", backend=backend)

message = str(e.value.message).lower()
assert "invalid choice: 'thename' (choose from 'one', 'two', 'three', 4)" in message


@backends
def test_unioned_literals(backend):
@dataclass
class ArgTest:
name: Union[Literal["one"], Literal["two"], Literal["three"], Literal[4]]

with pytest.raises(cappa.Exit) as e:
parse(ArgTest, "thename", backend=backend)

message = str(e.value.message).lower()
assert (
"invalid choice: 'thename' (choose from 'one', 'two', 'three', '4')" in message
"invalid value for 'name': possible variants\n - literal['one']: invalid choice: 'thename' (choose from 'one')\n - literal['two']: invalid choice: 'thename' (choose from 'two')\n - literal['three']: invalid choice: 'thename' (choose from 'three')\n - literal[4]: invalid choice: 'thename' (choose from 4)"
== message
)


Expand All @@ -55,8 +76,21 @@ class Args:
err = textwrap.dedent(
"""\
Invalid value for '-f': Possible variants
- set[Literal['one', 'two']]: Invalid choice: 'three' (choose from literal values 'one', 'two')
- set[Literal['one', 'two']]: Invalid choice: 'three' (choose from 'one', 'two')
- list[int]: invalid literal for int() with base 10: 'three'
- <no value>"""
)
assert str(e.value.message).lower() == err.lower()


@backends
def test_literal_parse(backend):
@dataclass
class LiteralParse:
log_level: Annotated[
Literal["TRACE", "DEBUG", "INFO"],
cappa.Arg(short="-L", long=True, parse=str.upper),
] = "INFO"

result = parse(LiteralParse, "--log-level=debug", backend=backend)
assert result == LiteralParse(log_level="DEBUG")
2 changes: 1 addition & 1 deletion tests/arg/test_literal_intermixed_union.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ def test_invalid_string(backend):
err = textwrap.dedent(
"""\
Invalid value for 'name': Possible variants
- Literal['one']: Invalid choice: 'thename' (choose from literal values 'one')
- Literal['one']: Invalid choice: 'thename' (choose from 'one')
- int: invalid literal for int() with base 10: 'thename'"""
)
assert err in str(e.value.message)
4 changes: 1 addition & 3 deletions tests/arg/test_literal_mulitple_variant.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,4 @@ def test_literal(backend):
parse(ArgTest, "thename", backend=backend)

message = str(e.value.message).lower()
assert (
"invalid choice: 'thename' (choose from 'one', 'two', 'three', '4')" in message
)
assert "invalid choice: 'thename' (choose from 'one', 'two', 'three', 4)" in message
9 changes: 4 additions & 5 deletions tests/arg/test_literal_sequence.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
from __future__ import annotations

from dataclasses import dataclass
from typing import Union

import pytest
from typing_extensions import Annotated, Literal
Expand All @@ -13,15 +12,15 @@
@dataclass
class ArgTest:
list: Annotated[
list[Union[Literal["one"], Literal["two"], Literal["three"], Literal[4]]],
list[Literal["one", "two", "three", 4]],
cappa.Arg(short=True, default=[]),
]
tuple: Annotated[
tuple[Union[Literal["one"], Literal["two"], Literal["three"], Literal[4]], ...],
tuple[Literal["one", "two", "three", 4], ...],
cappa.Arg(short=True, default=()),
]
set: Annotated[
set[Union[Literal["one"], Literal["two"], Literal["three"], Literal[4]]],
set[Literal["one", "two", "three", 4]],
cappa.Arg(short=True, default=set()),
]

Expand Down Expand Up @@ -50,4 +49,4 @@ def test_invalid(backend):
parse(ArgTest, "-l", "one", "-l", "wat", backend=backend)

message = str(e.value.message).lower()
assert "invalid choice: 'wat' (choose from 'one', 'two', 'three', '4')" in message
assert "invalid choice: 'wat' (choose from 'one', 'two', 'three', 4)" in message
2 changes: 1 addition & 1 deletion uv.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading