Keep Item fields in definition order (#7694)

scrapy/item.py

@@ -25,6 +25,25 @@ class Field(dict[str, Any]):
     """Container of field metadata"""
 
 
+def _ordered_field_names(cls: type) -> list[str]:
+    """Return the names of the :class:`Field` attributes of *cls* in definition
+    order.
+
+    Fields declared in base classes come first, ordered from the topmost base
+    to the most derived class. Within each class, fields keep their definition
+    order. A field redefined in a subclass keeps the position of its first
+    definition.
+    """
+    names: list[str] = []
+    seen: set[str] = set()
+    for base in reversed(cls.__mro__):
+        for name, value in vars(base).items():
+            if isinstance(value, Field) and name not in seen:
+                seen.add(name)
+                names.append(name)
+    return names
+
+
 class ItemMeta(ABCMeta):
     """Metaclass_ of :class:`Item` that handles field definitions.
 
@@ -39,13 +58,9 @@ class ItemMeta(ABCMeta):
         _class = super().__new__(mcs, "x_" + class_name, new_bases, attrs)
 
         fields = getattr(_class, "fields", {})
-        new_attrs = {}
-        for n in dir(_class):
-            v = getattr(_class, n)
-            if isinstance(v, Field):
-                fields[n] = v
-            elif n in attrs:
-                new_attrs[n] = attrs[n]
+        for n in _ordered_field_names(_class):
+            fields[n] = getattr(_class, n)
+        new_attrs = {n: v for n, v in attrs.items() if not isinstance(v, Field)}
 
         new_attrs["fields"] = fields
         new_attrs["_class"] = _class
@@ -80,6 +95,14 @@ class Item(MutableMapping[str, Any], object_ref, metaclass=ItemMeta):
     #: those populated. The keys are the field names and the values are the
     #: :class:`Field` objects used in the :ref:`Item declaration
     #: <topics-items-declaring>`.
+    #:
+    #: Fields are kept in definition order: fields declared in base classes
+    #: come first, followed by fields declared in subclasses, and a field
+    #: redefined in a subclass keeps the position of its first definition.
+    #:
+    #: .. versionchanged:: VERSION
+    #:    Fields are now returned in definition order rather than alphabetical
+    #:    order.
     fields: dict[str, Field]
 
     def __init__(self, *args: Any, **kwargs: Any):

tests/test_feedexport.py

@@ -746,7 +746,7 @@ class TestFeedExport(TestFeedExportBase):
         ]
 
         formats = {
-            "csv": b"baz,egg,foo\r\n,spam1,bar1\r\n",
+            "csv": b"foo,egg,baz\r\nbar1,spam1,\r\n",
             "json": b'[\n{"hello": "world2", "foo": "bar2"}\n]',
             "jsonlines": (
                 b'{"foo": "bar1", "egg": "spam1"}\n{"hello": "world2", "foo": "bar2"}\n'

tests/test_item.py

@@ -142,6 +142,30 @@ class TestItem:
         self.assertSortedEqual(list(item.keys()), ["new"])
         self.assertSortedEqual(list(item.values()), ["New"])
 
+    def test_fields_order(self):
+        class TestItem(Item):
+            name = Field()
+            keys = Field()
+            values = Field()
+
+        assert list(TestItem.fields) == ["name", "keys", "values"]
+
+    def test_fields_order_inheritance(self):
+        class ParentItem(Item):
+            name = Field()
+            keys = Field()
+            values = Field()
+
+        class TestItem(ParentItem):
+            extra = Field()
+            keys = Field(serializer=str)
+
+        # Inherited fields come first, in their definition order, followed by
+        # the fields newly defined in the subclass. A redefined field keeps the
+        # position of its first definition while taking the new metadata.
+        assert list(TestItem.fields) == ["name", "keys", "values", "extra"]
+        assert TestItem.fields["keys"] == {"serializer": str}
+
     def test_metaclass_inheritance(self):
         class ParentItem(Item):
             name = Field()