Repository: marshmallow-code/marshmallow

Issue:
Don't check for False return values from validators
Currently, validators are allowed to return `False` which results in `ValidationError` with a generic message.

https://github.com/marshmallow-code/marshmallow/blob/9ac3241b78b444033f901d51b431ac862d1e21cc/src/marshmallow/fields.py#L249-L251

I'm not convinced this behavior is that useful, and we could improve validation/deserialization performance by removing it. It also is a simpler API, avoiding confusion like in https://github.com/marshmallow-code/marshmallow/issues/1458

Slating this for 4.0 since it's a breaking change.

Discussion:
sloria: yeah, in practice it's much better to use a function so that you can have helpful error messages. as you mentined, the lambdas in the tests are only there out of convenience.

Patch:
diff --git a/CHANGELOG.rst b/CHANGELOG.rst
index d8747862..ee4a6c48 100644
--- a/CHANGELOG.rst
+++ b/CHANGELOG.rst
@@ -4,6 +4,8 @@ Changelog
 4.0.0 (unreleased)
 ******************
 
+See :ref:`upgrading_4_0` for a guide on updating your code.
+
 - *Backwards-incompatible*: Remove implicit field creation, i.e. using the ``fields`` or ``additional`` class Meta options with undeclared fields (:issue:`1356`).
 - *Backwards-incompatible*: Use `datetime.date.fromisoformat`, `datetime.time.fromisoformat`, and `datetime.datetime.fromisoformat` from the standard library to deserialize dates, times and datetimes (:pr:`2078`).
 
@@ -12,8 +14,9 @@ As a consequence of this change:
   - YYYY-MM-DD is now accepted as a datetime and deserialized as naive 00:00 AM.
   - `from_iso_date`, `from_iso_time` and `from_iso_datetime` are removed from `marshmallow.utils`
 
+- *Backwards-incompatible*: Custom validators must raise a `ValidationError <marshmallow.exceptions.ValidationError>` for invalid values.
+  Returning `False` is no longer supported (:issue:`1775`).
 - *Backwards-incompatible*: Rename ``schema`` parameter to ``parent`` in `marshmallow.fields.Field._bind_to_schema` (:issue:`1360`).
-
 - *Backwards-incompatible*: `marshmallow.fields.TimeDelta` no longer truncates float values when
   deserializing (:pr:`2654`). This allows microseconds to be preserved, e.g.
 
diff --git a/docs/quickstart.rst b/docs/quickstart.rst
index c6bc2426..17318f90 100644
--- a/docs/quickstart.rst
+++ b/docs/quickstart.rst
@@ -246,7 +246,7 @@ There are a number of built-in validators in the :ref:`marshmallow.validate <api
 You may implement your own validators.
 A validator is a callable that accepts a single argument, the value to validate.
 If validation fails, the callable should raise a :exc:`ValidationError <marshmallow.exceptions.ValidationError>`
-with a useful error message or return ``False`` (for a generic error message).
+with an error message.
 
 .. code-block:: python
 
diff --git a/docs/upgrading.rst b/docs/upgrading.rst
index 27465d13..f2eba38d 100644
--- a/docs/upgrading.rst
+++ b/docs/upgrading.rst
@@ -3,6 +3,61 @@ Upgrading to Newer Releases
 
 This section documents migration paths to new releases.
 
+.. _upgrading_4_0:
+
+Upgrading to 4.0
+++++++++++++++++
+
+Validators must raise a ValidationError
+***************************************
+
+In marshmallow 4.0, validators must raise a :exc:`ValidationError <marshmallow.exceptions.ValidationError>` when the value is invalid.
+Returning `False` from a validator is no longer supported.
+
+.. code-block:: python
+
+    from marshmallow import Schema, fields
+
+
+    # 3.x
+    class UserSchema(Schema):
+        password = fields.String(validate=lambda x: x == "password")
+
+
+    # 4.x
+    def validate_password(val):
+        if val != "password":
+            raise ValidationError("Invalid password.")
+
+
+    class UserSchema(Schema):
+        password = fields.String(validate=validate_password)
+
+
+If you want to use anonymous functions, you can use this helper function.
+
+.. code-block:: python
+
+    import typing
+
+    from marshmallow import Schema, fields
+
+
+    def predicate(
+        func: typing.Callable[[typing.Any], typing.Any],
+    ) -> typing.Callable[[typing.Any], None]:
+        def validate(value: typing.Any) -> None:
+            if not func(value):
+                raise ValidationError("Invalid value.")
+
+        return validate
+
+
+    # Usage
+    class UserSchema(Schema):
+        password = fields.String(validate=predicate(lambda x: x == "password"))
+
+
 Upgrading to 3.3
 ++++++++++++++++
 
diff --git a/src/marshmallow/fields.py b/src/marshmallow/fields.py
index c14c89cc..9fdd12e1 100644
--- a/src/marshmallow/fields.py
+++ b/src/marshmallow/fields.py
@@ -250,7 +250,7 @@ class Field(FieldABC):
 
     @property
     def _validate_all(self):
-        return And(*self.validators, error=self.error_messages["validator_failed"])
+        return And(*self.validators)
 
     def make_error(self, key: str, **kwargs) -> ValidationError:
         """Helper method to make a `ValidationError` with an error message
diff --git a/src/marshmallow/validate.py b/src/marshmallow/validate.py
index e4536d88..2ad18832 100644
--- a/src/marshmallow/validate.py
+++ b/src/marshmallow/validate.py
@@ -58,14 +58,10 @@ class And(Validator):
         # ValidationError: ['Must be greater than or equal to 0.', 'Not an even value.']
 
     :param validators: Validators to combine.
-    :param error: Error message to use when a validator returns ``False``.
     """
 
-    default_error_message = "Invalid value."
-
-    def __init__(self, *validators: types.Validator, error: str | None = None):
+    def __init__(self, *validators: types.Validator):
         self.validators = tuple(validators)
-        self.error = error or self.default_error_message  # type: str
 
     def _repr_args(self) -> str:
         return f"validators={self.validators!r}"
@@ -75,9 +71,7 @@ class And(Validator):
         kwargs = {}
         for validator in self.validators:
             try:
-                r = validator(value)
-                if not isinstance(validator, Validator) and r is False:
-                    raise ValidationError(self.error)
+                validator(value)
             except ValidationError as err:
                 kwargs.update(err.kwargs)
                 if isinstance(err.messages, dict):


Test patch:
diff --git a/tests/base.py b/tests/base.py
index 65c9b8e4..e3de8689 100644
--- a/tests/base.py
+++ b/tests/base.py
@@ -2,6 +2,7 @@
 
 import datetime as dt
 import functools
+import typing
 import uuid
 from enum import Enum, IntEnum
 from zoneinfo import ZoneInfo
@@ -63,19 +64,32 @@ ALL_FIELDS = [
 ##### Custom asserts #####
 
 
-def assert_date_equal(d1, d2):
+def assert_date_equal(d1: dt.date, d2: dt.date) -> None:
     assert d1.year == d2.year
     assert d1.month == d2.month
     assert d1.day == d2.day
 
 
-def assert_time_equal(t1, t2):
+def assert_time_equal(t1: dt.time, t2: dt.time) -> None:
     assert t1.hour == t2.hour
     assert t1.minute == t2.minute
     assert t1.second == t2.second
     assert t1.microsecond == t2.microsecond
 
 
+##### Validation #####
+
+
+def predicate(
+    func: typing.Callable[[typing.Any], typing.Any],
+) -> typing.Callable[[typing.Any], None]:
+    def validate(value: typing.Any) -> None:
+        if not func(value):
+            raise ValidationError("Invalid value.")
+
+    return validate
+
+
 ##### Models #####
 
 
diff --git a/tests/test_decorators.py b/tests/test_decorators.py
index aa18ea97..854f87fc 100644
--- a/tests/test_decorators.py
+++ b/tests/test_decorators.py
@@ -11,9 +11,11 @@ from marshmallow import (
     post_load,
     pre_dump,
     pre_load,
+    validate,
     validates,
     validates_schema,
 )
+from tests.base import predicate
 
 
 @pytest.mark.parametrize("partial_val", (True, False))
@@ -333,8 +335,8 @@ class TestValidatesDecorator:
 
     def test_precedence(self):
         class Schema2(ValidatesSchema):
-            foo = fields.Int(validate=lambda n: n != 42)
-            bar = fields.Int(validate=lambda n: n == 1)
+            foo = fields.Int(validate=predicate(lambda n: n != 42))
+            bar = fields.Int(validate=validate.Equal(1))
 
             @validates("bar")
             def validate_bar(self, value):
@@ -351,7 +353,7 @@ class TestValidatesDecorator:
         errors = schema.validate({"bar": 3})
         assert "bar" in errors
         assert len(errors["bar"]) == 1
-        assert "Invalid value." in errors["bar"][0]
+        assert "Must be equal to 1." in errors["bar"][0]
 
         errors = schema.validate({"bar": 1})
         assert "bar" in errors
@@ -631,7 +633,7 @@ class TestValidatesSchemaDecorator:
 
     def test_skip_on_field_errors(self):
         class MySchema(Schema):
-            foo = fields.Int(required=True, validate=lambda n: n == 3)
+            foo = fields.Int(required=True, validate=validate.Equal(3))
             bar = fields.Int(required=True)
 
             @validates_schema(skip_on_field_errors=True)
diff --git a/tests/test_deserialization.py b/tests/test_deserialization.py
index 5c1e6d0d..e71e8999 100644
--- a/tests/test_deserialization.py
+++ b/tests/test_deserialization.py
@@ -9,7 +9,6 @@ import pytest
 
 from marshmallow import EXCLUDE, INCLUDE, RAISE, Schema, fields, validate
 from marshmallow.exceptions import ValidationError
-from marshmallow.validate import Equal
 from tests.base import (
     ALL_FIELDS,
     DateEnum,
@@ -18,6 +17,7 @@ from tests.base import (
     assert_date_equal,
     assert_time_equal,
     central,
+    predicate,
 )
 
 
@@ -1404,25 +1404,13 @@ class TestFieldDeserialization:
         assert sch.load({"foo": 24})["foo"] == 42
 
     def test_field_deserialization_with_user_validator_function(self):
-        field = fields.String(validate=lambda s: s.lower() == "valid")
+        field = fields.String(validate=predicate(lambda s: s.lower() == "valid"))
         assert field.deserialize("Valid") == "Valid"
         with pytest.raises(ValidationError) as excinfo:
             field.deserialize("invalid")
         assert excinfo.value.args[0][0] == "Invalid value."
         assert type(excinfo.value) is ValidationError
 
-    def test_field_deserialization_with_user_validator_class_that_returns_bool(self):
-        class MyValidator:
-            def __call__(self, val):
-                if val == "valid":
-                    return True
-                return False
-
-        field = fields.Field(validate=MyValidator())
-        assert field.deserialize("valid") == "valid"
-        with pytest.raises(ValidationError, match="Invalid value."):
-            field.deserialize("invalid")
-
     def test_field_deserialization_with_user_validator_that_raises_error_with_list(
         self,
     ):
@@ -1435,17 +1423,11 @@ class TestFieldDeserialization:
         errors = MySchema().validate({"foo": 42})
         assert errors["foo"] == ["err1", "err2"]
 
-    def test_validator_must_return_false_to_raise_error(self):
-        # validator returns None, so anything validates
-        field = fields.String(validate=lambda s: None)
-        assert field.deserialize("Valid") == "Valid"
-        # validator returns False, so nothing validates
-        field2 = fields.String(validate=lambda s: False)
-        with pytest.raises(ValidationError):
-            field2.deserialize("invalid")
-
     def test_field_deserialization_with_validator_with_nonascii_input(self):
-        field = fields.String(validate=lambda s: False)
+        def validate(val):
+            raise ValidationError("oops")
+
+        field = fields.String(validate=validate)
         with pytest.raises(ValidationError) as excinfo:
             field.deserialize("привет")
         assert type(excinfo.value) is ValidationError
@@ -1454,22 +1436,22 @@ class TestFieldDeserialization:
         validators_gen = (
             func
             for func in (
-                lambda s: s.lower() == "valid",
-                lambda s: s.lower()[::-1] == "dilav",
+                predicate(lambda s: s.lower() == "valid"),
+                predicate(lambda s: s.lower()[::-1] == "dilav"),
             )
         )
 
         m_colletion_type = [
             fields.String(
                 validate=[
-                    lambda s: s.lower() == "valid",
-                    lambda s: s.lower()[::-1] == "dilav",
+                    predicate(lambda s: s.lower() == "valid"),
+                    predicate(lambda s: s.lower()[::-1] == "dilav"),
                 ]
             ),
             fields.String(
                 validate=(
-                    lambda s: s.lower() == "valid",
-                    lambda s: s.lower()[::-1] == "dilav",
+                    predicate(lambda s: s.lower() == "valid"),
+                    predicate(lambda s: s.lower()[::-1] == "dilav"),
                 )
             ),
             fields.String(validate=validators_gen),
@@ -1480,14 +1462,6 @@ class TestFieldDeserialization:
             with pytest.raises(ValidationError, match="Invalid value."):
                 field.deserialize("invalid")
 
-    def test_field_deserialization_with_custom_error_message(self):
-        field = fields.String(
-            validate=lambda s: s.lower() == "valid",
-            error_messages={"validator_failed": "Bad value."},
-        )
-        with pytest.raises(ValidationError, match="Bad value."):
-            field.deserialize("invalid")
-
 
 # No custom deserialization behavior, so a dict is returned
 class SimpleUserSchema(Schema):
@@ -1498,13 +1472,13 @@ class SimpleUserSchema(Schema):
 class Validator(Schema):
     email = fields.Email()
     colors = fields.Str(validate=validate.OneOf(["red", "blue"]))
-    age = fields.Integer(validate=lambda n: n > 0)
+    age = fields.Integer(validate=validate.Range(min=0, min_inclusive=False))
 
 
 class Validators(Schema):
     email = fields.Email()
     colors = fields.Str(validate=validate.OneOf(["red", "blue"]))
-    age = fields.Integer(validate=[lambda n: n > 0, lambda n: n < 100])
+    age = fields.Integer(validate=[validate.Range(1, 99)])
 
 
 class TestSchemaDeserialization:
@@ -1813,16 +1787,14 @@ class TestSchemaDeserialization:
         assert "Something went wrong" in errors["foo"]
 
     def test_multiple_errors_can_be_stored_for_a_field(self):
-        def validate_with_bool(n):
-            return False
+        def validate1(n):
+            raise ValidationError("error one")
 
-        def validate_with_error(n):
-            raise ValidationError("foo is not valid")
+        def validate2(n):
+            raise ValidationError("error two")
 
         class MySchema(Schema):
-            foo = fields.Field(
-                required=True, validate=[validate_with_bool, validate_with_error]
-            )
+            foo = fields.Field(required=True, validate=[validate1, validate2])
 
         with pytest.raises(ValidationError) as excinfo:
             MySchema().load({"foo": "bar"})
@@ -1832,11 +1804,11 @@ class TestSchemaDeserialization:
         assert len(errors["foo"]) == 2
 
     def test_multiple_errors_can_be_stored_for_an_email_field(self):
-        def validate_with_bool(val):
-            return False
+        def validate(val):
+            raise ValidationError("Invalid value.")
 
         class MySchema(Schema):
-            email = fields.Email(validate=[validate_with_bool])
+            email = fields.Email(validate=[validate])
 
         with pytest.raises(ValidationError) as excinfo:
             MySchema().load({"email": "foo"})
@@ -1845,11 +1817,11 @@ class TestSchemaDeserialization:
         assert "Not a valid email address." in errors["email"][0]
 
     def test_multiple_errors_can_be_stored_for_a_url_field(self):
-        def validate_with_bool(val):
-            return False
+        def validator(val):
+            raise ValidationError("Not a valid URL.")
 
         class MySchema(Schema):
-            url = fields.Url(validate=[validate_with_bool])
+            url = fields.Url(validate=[validator])
 
         with pytest.raises(ValidationError) as excinfo:
             MySchema().load({"url": "foo"})
@@ -2059,18 +2031,26 @@ class TestSchemaDeserialization:
         assert data == {"foo": "hi", "bar": "okay", "alpha.beta": "woah!"}
 
 
-validators_gen = (func for func in [lambda x: x <= 24, lambda x: 18 <= x])
+validators_gen = (
+    func for func in [predicate(lambda x: x <= 24), predicate(lambda x: 18 <= x)]
+)
 
-validators_gen_float = (func for func in [lambda f: f <= 4.1, lambda f: f >= 1.0])
+validators_gen_float = (
+    func for func in [predicate(lambda f: f <= 4.1), predicate(lambda f: f >= 1.0)]
+)
 
 validators_gen_str = (
-    func for func in [lambda n: len(n) == 3, lambda n: n[1].lower() == "o"]
+    func
+    for func in [
+        predicate(lambda n: len(n) == 3),
+        predicate(lambda n: n[1].lower() == "o"),
+    ]
 )
 
 
 class TestValidation:
     def test_integer_with_validator(self):
-        field = fields.Integer(validate=lambda x: 18 <= x <= 24)
+        field = fields.Integer(validate=validate.Range(18, 24))
         out = field.deserialize("20")
         assert out == 20
         with pytest.raises(ValidationError):
@@ -2079,8 +2059,12 @@ class TestValidation:
     @pytest.mark.parametrize(
         "field",
         [
-            fields.Integer(validate=[lambda x: x <= 24, lambda x: 18 <= x]),
-            fields.Integer(validate=(lambda x: x <= 24, lambda x: 18 <= x)),
+            fields.Integer(
+                validate=[predicate(lambda x: x <= 24), predicate(lambda x: 18 <= x)]
+            ),
+            fields.Integer(
+                validate=(predicate(lambda x: x <= 24), predicate(lambda x: 18 <= x))
+            ),
             fields.Integer(validate=validators_gen),
         ],
     )
@@ -2093,8 +2077,12 @@ class TestValidation:
     @pytest.mark.parametrize(
         "field",
         [
-            fields.Float(validate=[lambda f: f <= 4.1, lambda f: f >= 1.0]),
-            fields.Float(validate=(lambda f: f <= 4.1, lambda f: f >= 1.0)),
+            fields.Float(
+                validate=[predicate(lambda f: f <= 4.1), predicate(lambda f: f >= 1.0)]
+            ),
+            fields.Float(
+                validate=(predicate(lambda f: f <= 4.1), predicate(lambda f: f >= 1.0))
+            ),
             fields.Float(validate=validators_gen_float),
         ],
     )
@@ -2104,14 +2092,14 @@ class TestValidation:
             field.deserialize(4.2)
 
     def test_string_validator(self):
-        field = fields.String(validate=lambda n: len(n) == 3)
+        field = fields.String(validate=validate.Length(equal=3))
         assert field.deserialize("Joe") == "Joe"
         with pytest.raises(ValidationError):
             field.deserialize("joseph")
 
     def test_function_validator(self):
         field = fields.Function(
-            lambda d: d.name.upper(), validate=lambda n: len(n) == 3
+            lambda d: d.name.upper(), validate=validate.Length(equal=3)
         )
         assert field.deserialize("joe")
         with pytest.raises(ValidationError):
@@ -2122,11 +2110,17 @@ class TestValidation:
         [
             fields.Function(
                 lambda d: d.name.upper(),
-                validate=[lambda n: len(n) == 3, lambda n: n[1].lower() == "o"],
+                validate=[
+                    validate.Length(equal=3),
+                    predicate(lambda n: n[1].lower() == "o"),
+                ],
             ),
             fields.Function(
                 lambda d: d.name.upper(),
-                validate=(lambda n: len(n) == 3, lambda n: n[1].lower() == "o"),
+                validate=(
+                    predicate(lambda n: len(n) == 3),
+                    predicate(lambda n: n[1].lower() == "o"),
+                ),
             ),
             fields.Function(lambda d: d.name.upper(), validate=validators_gen_str),
         ],
@@ -2139,21 +2133,21 @@ class TestValidation:
     def test_method_validator(self):
         class MethodSerializer(Schema):
             name = fields.Method(
-                "get_name", deserialize="get_name", validate=lambda n: len(n) == 3
+                "get_name", deserialize="get_name", validate=validate.Length(equal=3)
             )
 
             def get_name(self, val):
                 return val.upper()
 
         assert MethodSerializer().load({"name": "joe"})
-        with pytest.raises(ValidationError, match="Invalid value."):
+        with pytest.raises(ValidationError, match="Length must be 3."):
             MethodSerializer().load({"name": "joseph"})
 
     # Regression test for https://github.com/marshmallow-code/marshmallow/issues/269
     def test_nested_data_is_stored_when_validation_fails(self):
         class SchemaA(Schema):
             x = fields.Integer()
-            y = fields.Integer(validate=lambda n: n > 0)
+            y = fields.Integer(validate=validate.Range(min=1))
             z = fields.Integer()
 
         class SchemaB(Schema):
@@ -2174,19 +2168,6 @@ class TestValidation:
         assert "y" in errors["n"]
         assert data == {"w": 90, "n": {"x": 90, "z": 180}}
 
-    def test_false_value_validation(self):
-        class Sch(Schema):
-            lamb = fields.Raw(validate=lambda x: x is False)
-            equal = fields.Raw(validate=Equal(False))
-
-        errors = Sch().validate({"lamb": False, "equal": False})
-        assert not errors
-        errors = Sch().validate({"lamb": True, "equal": True})
-        assert "lamb" in errors
-        assert errors["lamb"] == ["Invalid value."]
-        assert "equal" in errors
-        assert errors["equal"] == ["Must be equal to False."]
-
     def test_nested_partial_load(self):
         class SchemaA(Schema):
             x = fields.Integer(required=True)
diff --git a/tests/test_schema.py b/tests/test_schema.py
index 2d1c267b..801663b6 100644
--- a/tests/test_schema.py
+++ b/tests/test_schema.py
@@ -14,6 +14,7 @@ from marshmallow import (
     class_registry,
     fields,
     utils,
+    validate,
     validates,
     validates_schema,
 )
@@ -72,9 +73,12 @@ def test_load_resets_errors():
 
 
 def test_load_validation_error_stores_input_data_and_valid_data():
+    def validator(val):
+        raise ValidationError("oops")
+
     class MySchema(Schema):
         always_valid = fields.DateTime()
-        always_invalid = fields.Field(validate=[lambda v: False])
+        always_invalid = fields.Field(validate=[validator])
 
     schema = MySchema()
     input_data = {
@@ -117,8 +121,11 @@ def test_errored_fields_do_not_appear_in_output():
         def _serialize(self, val, attr, obj):
             raise ValidationError("oops")
 
+    def validator(val):
+        raise ValidationError("oops")
+
     class MySchema(Schema):
-        foo = MyField(validate=lambda x: False)
+        foo = MyField(validate=validator)
 
     sch = MySchema()
     with pytest.raises(ValidationError) as excinfo:
@@ -164,8 +171,8 @@ def test_boolean_can_dump_unhashable(value):
 
 def test_multiple_errors_can_be_stored_for_a_given_index():
     class MySchema(Schema):
-        foo = fields.Str(validate=lambda x: len(x) > 3)
-        bar = fields.Int(validate=lambda x: x > 3)
+        foo = fields.Str(validate=validate.Length(min=4))
+        bar = fields.Int(validate=validate.Range(min=4))
 
     sch = MySchema()
     valid = {"foo": "loll", "bar": 42}
@@ -2135,11 +2142,14 @@ def test_deserialization_with_required_field():
 
 
 def test_deserialization_with_required_field_and_custom_validator():
+    def validator(val):
+        if val.lower() not in {"red", "blue"}:
+            raise ValidationError("Color must be red or blue")
+
     class ValidatingSchema(Schema):
         color = fields.String(
             required=True,
-            validate=lambda x: x.lower() == "red" or x.lower() == "blue",
-            error_messages={"validator_failed": "Color must be red or blue"},
+            validate=validator,
         )
 
     with pytest.raises(ValidationError) as excinfo: