From ebd9aee82ea08c76bdcdc0427c29f476c7f12b0d Mon Sep 17 00:00:00 2001 From: Gianluca Gippetto Date: Wed, 23 Jun 2021 00:55:53 +0200 Subject: [PATCH 1/4] Change ConstraintViolated signature - Make ctx required - Add argument constraint and params This change aims at making ConstraintViolated a complete source of information about the error, so that ErrorRephraser can be simplified taking just a single argument (the error itself). --- cloup/constraints/_conditional.py | 3 ++- cloup/constraints/_core.py | 16 +++++++++++----- cloup/constraints/exceptions.py | 20 ++++++++++++++++---- tests/constraints/test_constraints.py | 2 +- 4 files changed, 30 insertions(+), 11 deletions(-) diff --git a/cloup/constraints/_conditional.py b/cloup/constraints/_conditional.py index aa7a957..2b49715 100644 --- a/cloup/constraints/_conditional.py +++ b/cloup/constraints/_conditional.py @@ -73,7 +73,8 @@ def check_values(self, params: Sequence[Parameter], ctx: Context): except ConstraintViolated as err: desc = (condition.description(ctx) if condition_is_true else condition.negated_description(ctx)) - raise ConstraintViolated(f"when {desc}, {err}", ctx=ctx) + raise ConstraintViolated( + f"when {desc}, {err}", ctx=ctx, constraint=self, params=params) def __repr__(self) -> str: if self._else: diff --git a/cloup/constraints/_core.py b/cloup/constraints/_core.py index 36f792f..e93791e 100644 --- a/cloup/constraints/_core.py +++ b/cloup/constraints/_core.py @@ -224,7 +224,9 @@ def check_values(self, params: Sequence[Parameter], ctx: Context): return c.check_values(params, ctx) except ConstraintViolated: pass - raise ConstraintViolated.default(params, self.help(ctx), ctx=ctx) + raise ConstraintViolated.default( + self.help(ctx), ctx=ctx, constraint=self, params=params + ) def __or__(self, other) -> 'Or': if isinstance(other, Or): @@ -282,7 +284,8 @@ def check_values(self, params: Sequence[Parameter], ctx: Context): except ConstraintViolated: rephrased_error = self._get_rephrased_error(ctx, params) if rephrased_error: - raise ConstraintViolated(rephrased_error, ctx=ctx) + raise ConstraintViolated( + rephrased_error, ctx=ctx, constraint=self, params=params) raise def __repr__(self): @@ -341,6 +344,8 @@ def check_values(self, params: Sequence[Parameter], ctx: Context): many=f"the following parameters are required:\n" f"{format_param_list(unset_params)}"), ctx=ctx, + constraint=self, + params=params, ) @@ -370,7 +375,7 @@ def check_values(self, params: Sequence[Parameter], ctx: Context): raise ConstraintViolated( f"at least {n} of the following parameters must be set:\n" f"{format_param_list(params)}", - ctx=ctx + ctx=ctx, constraint=self, params=params, ) def __repr__(self): @@ -400,7 +405,7 @@ def check_values(self, params: Sequence[Parameter], ctx: Context): raise ConstraintViolated( f"no more than {n} of the following parameters can be set:\n" f"{format_param_list(params)}", - ctx=ctx, + ctx=ctx, constraint=self, params=params, ) def __repr__(self): @@ -427,7 +432,8 @@ def check_values(self, params: Sequence[Parameter], ctx: Context): zero='none of the following parameters must be set:\n', many=f'exactly {n} of the following parameters must be set:\n' ) + format_param_list(params) - raise ConstraintViolated(reason, ctx=ctx) + raise ConstraintViolated( + reason, ctx=ctx, constraint=self, params=params) class AcceptBetween(WrapperConstraint): diff --git a/cloup/constraints/exceptions.py b/cloup/constraints/exceptions.py index 9d76550..7829489 100644 --- a/cloup/constraints/exceptions.py +++ b/cloup/constraints/exceptions.py @@ -1,4 +1,4 @@ -from typing import Iterable, Optional, TYPE_CHECKING +from typing import Iterable, Sequence, TYPE_CHECKING import click from click import Context, Parameter @@ -18,16 +18,28 @@ def default_constraint_error(params: Iterable[Parameter], desc: str) -> str: class ConstraintViolated(click.UsageError): def __init__( - self, message: str, ctx: Optional[Context] = None + self, message: str, + ctx: Context, + constraint: 'Constraint', + params: Sequence[click.Parameter] ): super().__init__(message, ctx=ctx) + self.ctx = ctx + self.constraint = constraint + self.params = params @classmethod def default( - cls, params: Iterable[Parameter], desc: str, ctx: Optional[Context] = None + cls, + desc: str, + ctx: Context, + constraint: 'Constraint', + params: Sequence[Parameter], ) -> 'ConstraintViolated': return ConstraintViolated( - default_constraint_error(params, desc), ctx=ctx) + default_constraint_error(params, desc), + ctx=ctx, constraint=constraint, params=params, + ) class UnsatisfiableConstraint(Exception): diff --git a/tests/constraints/test_constraints.py b/tests/constraints/test_constraints.py index 47b5d9a..1bcb39d 100644 --- a/tests/constraints/test_constraints.py +++ b/tests/constraints/test_constraints.py @@ -51,7 +51,7 @@ def check_consistency(self, params: Sequence[Parameter]) -> None: def check_values(self, params: Sequence[Parameter], ctx: Context): self.check_values_calls.append(dict(params=params, ctx=ctx)) if not self.satisfied: - raise ConstraintViolated(self.error, ctx=ctx) + raise ConstraintViolated(self.error, ctx=ctx, constraint=self, params=params) class TestBaseConstraint: From 0bbdeb588ecd5c5fd5e831445242885ddbb4de97 Mon Sep 17 00:00:00 2001 From: Gianluca Gippetto Date: Wed, 23 Jun 2021 01:16:33 +0200 Subject: [PATCH 2/4] Simplify ErrorRephraser redefining it as `(ConstraintViolated) -> str` --- cloup/constraints/_core.py | 37 +++++++++++++++++++-------- tests/constraints/test_constraints.py | 16 +++++++++--- 2 files changed, 40 insertions(+), 13 deletions(-) diff --git a/cloup/constraints/_core.py b/cloup/constraints/_core.py index e93791e..cbc1a1a 100644 --- a/cloup/constraints/_core.py +++ b/cloup/constraints/_core.py @@ -19,7 +19,7 @@ Op = TypeVar('Op', bound='Operator') HelpRephraser = Callable[[Context, 'Constraint'], str] -ErrorRephraser = Callable[[Context, 'Constraint', Sequence[Parameter]], str] +ErrorRephraser = Callable[[ConstraintViolated], str] class Constraint(abc.ABC): @@ -146,6 +146,23 @@ def rephrased( help: Union[None, str, HelpRephraser] = None, error: Union[None, str, ErrorRephraser] = None, ) -> 'Rephraser': + """ + Overrides the help string and/or the error message of this constraint + wrapping it with a :class:`Rephraser`. + + :param help: + if provided, overrides the help string of this constraint. It can be + a string or a function ``(ctx: Context, constr: Constraint) -> str``. + :param error: + if provided, overrides the error message of this constraint. + It can be: + + - a template string for the ``format`` built-in function + - or a function ``(err: ConstraintViolated) -> str``; note that + a :class:`ConstraintViolated` error has fields for ``ctx``, + ``constraint`` and ``params``, so it's a complete description + of what happened. + """ return Rephraser(self, help=help, error=error) def hidden(self) -> 'Rephraser': @@ -238,8 +255,8 @@ class Rephraser(Constraint): """A Constraint decorator that can override the help and/or the error message of the wrapped constraint. - This is useful also for defining new constraints. - See also :class:`WrapperConstraint`. + .. seealso:: + :class:`WrapperConstraint`. """ def __init__( @@ -261,15 +278,15 @@ def help(self, ctx: Context) -> str: else: return self._help(ctx, self._constraint) - def _get_rephrased_error( - self, ctx: Context, params: Sequence[Parameter] - ) -> Optional[str]: + def _get_rephrased_error(self, err: ConstraintViolated) -> Optional[str]: if self._error is None: return None elif isinstance(self._error, str): - return self._error.format(param_list=format_param_list(params)) + return self._error.format( + param_list=format_param_list(err.params), + ) else: - return self._error(ctx, self._constraint, params) + return self._error(err) def check_consistency(self, params: Sequence[Parameter]) -> None: try: @@ -281,8 +298,8 @@ def check_consistency(self, params: Sequence[Parameter]) -> None: def check_values(self, params: Sequence[Parameter], ctx: Context): try: return self._constraint.check_values(params, ctx) - except ConstraintViolated: - rephrased_error = self._get_rephrased_error(ctx, params) + except ConstraintViolated as err: + rephrased_error = self._get_rephrased_error(err) if rephrased_error: raise ConstraintViolated( rephrased_error, ctx=ctx, constraint=self, params=params) diff --git a/tests/constraints/test_constraints.py b/tests/constraints/test_constraints.py index 1bcb39d..afb3282 100644 --- a/tests/constraints/test_constraints.py +++ b/tests/constraints/test_constraints.py @@ -307,11 +307,21 @@ def test_error_is_overridden_passing_function(self): params = make_options('abc') fake_ctx = make_fake_context(params) wrapped = FakeConstraint(satisfied=False) - get_error = Mock(return_value='rephrased error') - rephrased = Rephraser(wrapped, error=get_error) + error_rephraser_mock = Mock(return_value='rephrased error') + rephrased = Rephraser(wrapped, error=error_rephraser_mock) with pytest.raises(ConstraintViolated, match='rephrased error'): rephrased.check(params, ctx=fake_ctx) - get_error.assert_called_once_with(fake_ctx, wrapped, params) + + # Check the function is called with a single argument of type ConstraintViolated + error_rephraser_mock.assert_called_once() + args = error_rephraser_mock.call_args[0] + assert len(args) == 1 + error = args[0] + assert isinstance(error, ConstraintViolated) + # Check the error has all fields set + assert isinstance(error.ctx, Context) + assert isinstance(error.constraint, Constraint) + assert len(error.params) == 3 def test_check_consistency_raises_if_wrapped_constraint_raises(self): constraint = FakeConstraint(consistent=True) From 6f90bd9740d49a962118356a0a0b4718254d2bd2 Mon Sep 17 00:00:00 2001 From: Gianluca Gippetto Date: Wed, 23 Jun 2021 01:25:31 +0200 Subject: [PATCH 3/4] Add error template key: `{error}` --- cloup/constraints/_core.py | 1 + tests/constraints/test_constraints.py | 10 +++++++++- 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/cloup/constraints/_core.py b/cloup/constraints/_core.py index cbc1a1a..a513dcd 100644 --- a/cloup/constraints/_core.py +++ b/cloup/constraints/_core.py @@ -283,6 +283,7 @@ def _get_rephrased_error(self, err: ConstraintViolated) -> Optional[str]: return None elif isinstance(self._error, str): return self._error.format( + error=str(err), param_list=format_param_list(err.params), ) else: diff --git a/tests/constraints/test_constraints.py b/tests/constraints/test_constraints.py index afb3282..8be360b 100644 --- a/tests/constraints/test_constraints.py +++ b/tests/constraints/test_constraints.py @@ -297,12 +297,20 @@ def test_help_override_with_function(self, dummy_ctx): def test_error_is_overridden_passing_string(self): fake_ctx = make_fake_context(make_options('abcd')) - wrapped = FakeConstraint(satisfied=False) + wrapped = FakeConstraint(satisfied=False, error='__error__') rephrased = Rephraser(wrapped, error='error:\n{param_list}') with pytest.raises(ConstraintViolated) as exc_info: rephrased.check(['a', 'b'], ctx=fake_ctx) assert exc_info.value.message == 'error:\n --a\n --b\n' + def test_error_template_key(self): + fake_ctx = make_fake_context(make_options('abcd')) + wrapped = FakeConstraint(satisfied=False, error='__error__') + rephrased = Rephraser(wrapped, error='{error}\nExtra info here.') + with pytest.raises(ConstraintViolated) as exc_info: + rephrased.check(['a', 'b'], ctx=fake_ctx) + assert str(exc_info.value) == '__error__\nExtra info here.' + def test_error_is_overridden_passing_function(self): params = make_options('abc') fake_ctx = make_fake_context(params) From 49dddaca36242a9bd118a3a888b810ba51e473e4 Mon Sep 17 00:00:00 2001 From: Gianluca Gippetto Date: Wed, 23 Jun 2021 02:22:30 +0200 Subject: [PATCH 4/4] Export HelpRephraser and ErrorRephraser from cloup.constraints --- cloup/constraints/__init__.py | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/cloup/constraints/__init__.py b/cloup/constraints/__init__.py index 0816e3b..c409884 100644 --- a/cloup/constraints/__init__.py +++ b/cloup/constraints/__init__.py @@ -11,6 +11,8 @@ AcceptBetween, And, Constraint, + ErrorRephraser, + HelpRephraser, Operator, Or, Rephraser, @@ -22,11 +24,6 @@ mutually_exclusive, require_all, ) -from ._support import ( - BoundConstraintSpec, - ConstraintMixin, - constraint, - constrained_params, -) +from ._support import (BoundConstraintSpec, ConstraintMixin, constrained_params, constraint) from .conditions import AllSet, AnySet, Equal, IsSet, Not from .exceptions import ConstraintViolated, UnsatisfiableConstraint