Repository: python-attrs/attrs

Problem statement:
validators.optional should take lists like validator=
We’ve added the support for taking a list of validators to validator=, optional() should be able to take lists too otherwise users have to wrap all validators in the list with optional().


Hints:

Patch:
diff --git a/src/attr/_make.py b/src/attr/_make.py
--- a/src/attr/_make.py
+++ b/src/attr/_make.py
@@ -897,7 +897,7 @@ def __init__(self, default, validator, repr, cmp, hash, init, convert,
         self.default = default
         # If validator is a list/tuple, wrap it using helper validator.
         if validator and isinstance(validator, (list, tuple)):
-            self._validator = _AndValidator(tuple(validator))
+            self._validator = and_(*validator)
         else:
             self._validator = validator
         self.repr = repr
@@ -911,37 +911,18 @@ def validator(self, meth):
         """
         Decorator that adds *meth* to the list of validators.
 
-        Returns meth unchanged.
+        Returns *meth* unchanged.
         """
-        if not isinstance(self._validator, _AndValidator):
-            self._validator = _AndValidator(
-                (self._validator,) if self._validator else ()
-            )
-        self._validator.add(meth)
+        if self._validator is None:
+            self._validator = meth
+        else:
+            self._validator = and_(self._validator, meth)
         return meth
 
 
 _CountingAttr = _add_cmp(_add_repr(_CountingAttr))
 
 
-@attributes(slots=True)
-class _AndValidator(object):
-    """
-    Compose many validators to a single one.
-    """
-    _validators = attr()
-
-    def __call__(self, inst, attr, value):
-        for v in self._validators:
-            v(inst, attr, value)
-
-    def add(self, validator):
-        """
-        Add *validator*.  Shouldn't be called after the class is done.
-        """
-        self._validators += (validator,)
-
-
 @attributes(slots=True)
 class Factory(object):
     """
@@ -981,3 +962,40 @@ def make_class(name, attrs, bases=(object,), **attributes_arguments):
         raise TypeError("attrs argument must be a dict or a list.")
 
     return attributes(**attributes_arguments)(type(name, bases, cls_dict))
+
+
+# These are required by whithin this module so we define them here and merely
+# import into .validators.
+
+
+@attributes(slots=True)
+class _AndValidator(object):
+    """
+    Compose many validators to a single one.
+    """
+    _validators = attr()
+
+    def __call__(self, inst, attr, value):
+        for v in self._validators:
+            v(inst, attr, value)
+
+
+def and_(*validators):
+    """
+    A validator that composes multiple validators into one.
+
+    When called on a value, it runs all wrapped validators.
+
+    :param validators: Arbitrary number of validators.
+    :type validators: callables
+
+    .. versionadded:: 17.1.0
+    """
+    vals = []
+    for validator in validators:
+        vals.extend(
+            validator._validators if isinstance(validator, _AndValidator)
+            else [validator]
+        )
+
+    return _AndValidator(tuple(vals))
diff --git a/src/attr/validators.py b/src/attr/validators.py
--- a/src/attr/validators.py
+++ b/src/attr/validators.py
@@ -4,7 +4,15 @@
 
 from __future__ import absolute_import, division, print_function
 
-from ._make import attr, attributes
+from ._make import attr, attributes, and_, _AndValidator
+
+
+__all__ = [
+    "and_",
+    "instance_of",
+    "optional",
+    "provides",
+]
 
 
 @attributes(repr=False, slots=True)
@@ -93,12 +101,13 @@ class _OptionalValidator(object):
     def __call__(self, inst, attr, value):
         if value is None:
             return
-        return self.validator(inst, attr, value)
+
+        self.validator(inst, attr, value)
 
     def __repr__(self):
         return (
-            "<optional validator for {type} or None>"
-            .format(type=repr(self.validator))
+            "<optional validator for {what} or None>"
+            .format(what=repr(self.validator))
         )
 
 
@@ -108,6 +117,13 @@ def optional(validator):
     which can be set to ``None`` in addition to satisfying the requirements of
     the sub-validator.
 
-    :param validator: A validator that is used for non-``None`` values.
+    :param validator: A validator (or a list of validators) that is used for
+        non-``None`` values.
+    :type validator: callable or :class:`list` of callables.
+
+    .. versionadded:: 15.1.0
+    .. versionchanged:: 17.1.0 *validator* can be a list of validators.
     """
+    if isinstance(validator, list):
+        return _OptionalValidator(_AndValidator(validator))
     return _OptionalValidator(validator)


Test patch:
diff --git a/tests/test_make.py b/tests/test_make.py
--- a/tests/test_make.py
+++ b/tests/test_make.py
@@ -21,6 +21,7 @@
     _AndValidator,
     _CountingAttr,
     _transform_attrs,
+    and_,
     attr,
     attributes,
     fields,
@@ -71,6 +72,8 @@ def v2(_, __):
 
     def test_validator_decorator_single(self):
         """
+        If _CountingAttr.validator is used as a decorator and there is no
+        decorator set, the decorated method is used as the validator.
         """
         a = attr()
 
@@ -78,17 +81,23 @@ def test_validator_decorator_single(self):
         def v():
             pass
 
-        assert _AndValidator((v,)) == a._validator
+        assert v == a._validator
 
-    def test_validator_decorator(self):
+    @pytest.mark.parametrize("wrap", [
+        lambda v: v,
+        lambda v: [v],
+        lambda v: and_(v)
+
+    ])
+    def test_validator_decorator(self, wrap):
         """
-        If _CountingAttr.validator is used as a decorator, the decorated method
-        is added to validators.
+        If _CountingAttr.validator is used as a decorator and there is already
+        a decorator set, the decorators are composed using `and_`.
         """
         def v(_, __):
             pass
 
-        a = attr(validator=[v])
+        a = attr(validator=wrap(v))
 
         @a.validator
         def v2(self, _, __):
diff --git a/tests/test_validators.py b/tests/test_validators.py
--- a/tests/test_validators.py
+++ b/tests/test_validators.py
@@ -7,8 +7,9 @@
 import pytest
 import zope.interface
 
-from attr.validators import instance_of, provides, optional
+from attr.validators import and_, instance_of, provides, optional
 from attr._compat import TYPE
+from attr._make import attributes, attr
 
 from .utils import simple_attr
 
@@ -58,6 +59,53 @@ def test_repr(self):
         ) == repr(v)
 
 
+def always_pass(_, __, ___):
+    """
+    Toy validator that always passses.
+    """
+
+
+def always_fail(_, __, ___):
+    """
+    Toy validator that always fails.
+    """
+    0/0
+
+
+class TestAnd(object):
+    def test_success(self):
+        """
+        Succeeds if all wrapped validators succeed.
+        """
+        v = and_(instance_of(int), always_pass)
+
+        v(None, simple_attr("test"), 42)
+
+    def test_fail(self):
+        """
+        Fails if any wrapped validator fails.
+        """
+        v = and_(instance_of(int), always_fail)
+
+        with pytest.raises(ZeroDivisionError):
+            v(None, simple_attr("test"), 42)
+
+    def test_sugar(self):
+        """
+        `and_(v1, v2, v3)` and `[v1, v2, v3]` are equivalent.
+        """
+        @attributes
+        class C(object):
+            a1 = attr("a1", validator=and_(
+                instance_of(int),
+            ))
+            a2 = attr("a2", validator=[
+                instance_of(int),
+            ])
+
+        assert C.__attrs_attrs__[0].validator == C.__attrs_attrs__[1].validator
+
+
 class IFoo(zope.interface.Interface):
     """
     An interface.
@@ -111,29 +159,33 @@ def test_repr(self):
         ) == repr(v)
 
 
+@pytest.mark.parametrize("validator", [
+    instance_of(int),
+    [always_pass, instance_of(int)],
+])
 class TestOptional(object):
     """
     Tests for `optional`.
     """
-    def test_success_with_type(self):
+    def test_success(self, validator):
         """
-        Nothing happens if types match.
+        Nothing happens if validator succeeds.
         """
-        v = optional(instance_of(int))
+        v = optional(validator)
         v(None, simple_attr("test"), 42)
 
-    def test_success_with_none(self):
+    def test_success_with_none(self, validator):
         """
         Nothing happens if None.
         """
-        v = optional(instance_of(int))
+        v = optional(validator)
         v(None, simple_attr("test"), None)
 
-    def test_fail(self):
+    def test_fail(self, validator):
         """
         Raises `TypeError` on wrong types.
         """
-        v = optional(instance_of(int))
+        v = optional(validator)
         a = simple_attr("test")
         with pytest.raises(TypeError) as e:
             v(None, a, "42")
@@ -144,13 +196,21 @@ def test_fail(self):
 
         ) == e.value.args
 
-    def test_repr(self):
+    def test_repr(self, validator):
         """
         Returned validator has a useful `__repr__`.
         """
-        v = optional(instance_of(int))
-        assert (
-            ("<optional validator for <instance_of validator for type "
-             "<{type} 'int'>> or None>")
-            .format(type=TYPE)
-        ) == repr(v)
+        v = optional(validator)
+
+        if isinstance(validator, list):
+            assert (
+                ("<optional validator for _AndValidator(_validators=[{func}, "
+                 "<instance_of validator for type <{type} 'int'>>]) or None>")
+                .format(func=repr(always_pass), type=TYPE)
+            ) == repr(v)
+        else:
+            assert (
+                ("<optional validator for <instance_of validator for type "
+                 "<{type} 'int'>> or None>")
+                .format(type=TYPE)
+            ) == repr(v)