Repository: marshmallow-code/marshmallow
Pull request #2772 (2025-01-13 06:09:49)

Check for fields declared as instances at class declaration rather than at instantiation
in the spirit of erroring earlier. also obviates the `is_instance_or_subclass` util


Patch:
diff --git a/CHANGELOG.rst b/CHANGELOG.rst
index d78bdb63..8b8561df 100644
--- a/CHANGELOG.rst
+++ b/CHANGELOG.rst
@@ -62,9 +62,6 @@ As a consequence of this change:
 - Improve performance and minimize float precision loss of `marshmallow.fields.TimeDelta` serialization (:pr:`2654`).
 - *Backwards-incompatible*: Remove ``serialization_type`` parameter from
   `marshmallow.fields.TimeDelta` (:pr:`2654`).
-- Rename ``json_data`` parameter of `marshmallow.Schema.loads` to ``s``
-  for compatibility with most render module implementations (`json`, ``simplejson``, etc.) (:pr:`2764`).
-  Also make it a positional-only argument.
 
 Thanks :user:`ddelange` for the PR.
 
@@ -98,6 +95,11 @@ Thanks :user:`ddelange` for the PR.
 - Methods decorated with `marshmallow.pre_load`, `marshmallow.post_load`, `marshmallow.validates_schema`,
   receive ``unknown`` as a keyword argument (:pr:`1632`).
   Thanks :user:`jforand` for the PR.
+- Rename ``json_data`` parameter of `marshmallow.Schema.loads` to ``s``
+  for compatibility with most render module implementations (`json`, ``simplejson``, etc.) (:pr:`2764`).
+  Also make it a positional-only argument.
+- Incorrectly declaring a field using a field class rather than instance
+  errors at class declaration time (previously happended when the schema was instantiated) (:pr:`2772`).
 
 Deprecations/Removals:
 
@@ -133,8 +135,8 @@ Documentation:
 
 Deprecations:
 
-- The ``ordered`` `class Meta <marshmallow.Schema.Meta>` option is deprecated (:issue:`2146`, :pr:`2762`). 
-  Field order is already preserved by default. Set `marshmallow.Schema.dict_class` to `collections.OrderedDict` 
+- The ``ordered`` `class Meta <marshmallow.Schema.Meta>` option is deprecated (:issue:`2146`, :pr:`2762`).
+  Field order is already preserved by default. Set `marshmallow.Schema.dict_class` to `collections.OrderedDict`
   to maintain the previous behavior.
 
 3.25.1 (2025-01-11)
diff --git a/src/marshmallow/schema.py b/src/marshmallow/schema.py
index 0298274e..f1ed1201 100644
--- a/src/marshmallow/schema.py
+++ b/src/marshmallow/schema.py
@@ -33,7 +33,6 @@ from marshmallow.utils import (
     RAISE,
     get_value,
     is_collection,
-    is_instance_or_subclass,
     missing,
     set_value,
     validate_unknown_parameter_value,
@@ -45,11 +44,17 @@ def _get_fields(attrs) -> list[tuple[str, Field]]:
 
     :param attrs: Mapping of class attributes
     """
-    return [
-        (field_name, field_value)
-        for field_name, field_value in attrs.items()
-        if is_instance_or_subclass(field_value, ma_fields.Field)
-    ]
+    ret = []
+    for field_name, field_value in attrs.items():
+        if isinstance(field_value, type) and issubclass(field_value, ma_fields.Field):
+            raise TypeError(
+                f'Field for "{field_name}" must be declared as a '
+                "Field instance, not a class. "
+                f'Did you mean "fields.{field_value.__name__}()"?'
+            )
+        elif isinstance(field_value, ma_fields.Field):
+            ret.append((field_name, field_value))
+    return ret
 
 
 # This function allows Schemas to inherit from non-Schema classes and ensures
@@ -1050,20 +1055,7 @@ class Schema(metaclass=SchemaMeta):
             field_obj.load_only = True
         if field_name in self.dump_only:
             field_obj.dump_only = True
-        try:
-            field_obj._bind_to_schema(field_name, self)
-        except TypeError as error:
-            # Field declared as a class, not an instance. Ignore type checking because
-            # we handle unsupported arg types, i.e. this is dead code from
-            # the type checker's perspective.
-            if isinstance(field_obj, type) and issubclass(field_obj, ma_fields.Field):
-                msg = (
-                    f'Field for "{field_name}" must be declared as a '
-                    "Field instance, not a class. "
-                    f'Did you mean "fields.{field_obj.__name__}()"?'  # type: ignore
-                )
-                raise TypeError(msg) from error
-            raise error
+        field_obj._bind_to_schema(field_name, self)
         self.on_bind_field(field_name, field_obj)
 
     def _invoke_dump_processors(
diff --git a/src/marshmallow/utils.py b/src/marshmallow/utils.py
index 24ebd4f4..34fa517a 100644
--- a/src/marshmallow/utils.py
+++ b/src/marshmallow/utils.py
@@ -48,14 +48,6 @@ def is_collection(obj) -> bool:
     return is_iterable_but_not_string(obj) and not isinstance(obj, Mapping)
 
 
-def is_instance_or_subclass(val, class_) -> bool:
-    """Return True if ``val`` is either a subclass or instance of ``class_``."""
-    try:
-        return issubclass(val, class_)
-    except TypeError:
-        return isinstance(val, class_)
-
-
 # https://stackoverflow.com/a/27596917
 def is_aware(datetime: dt.datetime) -> bool:
     return (


Test patch:
diff --git a/tests/test_schema.py b/tests/test_schema.py
index df176e46..8b896a47 100644
--- a/tests/test_schema.py
+++ b/tests/test_schema.py
@@ -537,13 +537,12 @@ def test_function_field(serialized_user, user):
 
 
 def test_fields_must_be_declared_as_instances(user):
-    class BadUserSchema(Schema):
-        name = fields.String
-
     with pytest.raises(
         TypeError, match='Field for "name" must be declared as a Field instance'
     ):
-        BadUserSchema().dump(user)
+
+        class BadUserSchema(Schema):
+            name = fields.String
 
 
 # regression test