diff --git a/changelog.d/1147.change.md b/changelog.d/1147.change.md new file mode 100644 index 000000000..17c62638f --- /dev/null +++ b/changelog.d/1147.change.md @@ -0,0 +1 @@ +Checking mandatory vs non-mandatory attribute order is now performed after the field transformer, since the field transformer may change attributes and/or their order. diff --git a/src/attr/_make.py b/src/attr/_make.py index a7a4075af..b1550eb8a 100644 --- a/src/attr/_make.py +++ b/src/attr/_make.py @@ -449,6 +449,10 @@ def _transform_attrs( attrs = base_attrs + own_attrs + if field_transformer is not None: + attrs = field_transformer(cls, attrs) + + # Check attr order after executing the field_transformer. # Mandatory vs non-mandatory attr order only matters when they are part of # the __init__ signature and when they aren't kw_only (which are moved to # the end and can be mandatory or non-mandatory in any order, as they will @@ -462,9 +466,6 @@ def _transform_attrs( if had_default is False and a.default is not NOTHING: had_default = True - if field_transformer is not None: - attrs = field_transformer(cls, attrs) - # Resolve default field alias after executing field_transformer. # This allows field_transformer to differentiate between explicit vs # default aliases and supply their own defaults. diff --git a/tests/test_hooks.py b/tests/test_hooks.py index 9c37a98cd..6b3420639 100644 --- a/tests/test_hooks.py +++ b/tests/test_hooks.py @@ -4,6 +4,8 @@ from datetime import datetime +import pytest + import attr @@ -30,7 +32,7 @@ class C: y = attr.ib(type=int) z: float = attr.ib() - assert results == [("x", None), ("y", int), ("z", float)] + assert [("x", None), ("y", int), ("z", float)] == results def test_hook_applied_auto_attrib(self): """ @@ -49,7 +51,7 @@ class C: x: int y: str = attr.ib() - assert results == [("x", int), ("y", str)] + assert [("x", int), ("y", str)] == results def test_hook_applied_modify_attrib(self): """ @@ -66,7 +68,8 @@ class C: y: float c = C(x="3", y="3.14") - assert c == C(x=3, y=3.14) + + assert C(x=3, y=3.14) == c def test_hook_remove_field(self): """ @@ -82,7 +85,7 @@ class C: x: int y: float - assert attr.asdict(C(2.7)) == {"y": 2.7} + assert {"y": 2.7} == attr.asdict(C(2.7)) def test_hook_add_field(self): """ @@ -98,7 +101,7 @@ def hook(cls, attribs): class C: x: int - assert attr.asdict(C(1, 2)) == {"x": 1, "new": 2} + assert {"x": 1, "new": 2} == attr.asdict(C(1, 2)) def test_hook_override_alias(self): """ @@ -118,13 +121,73 @@ class NameCase: 1, 2, 3 ) + def test_hook_reorder_fields(self): + """ + It is possible to reorder fields via the hook. + """ + + def hook(cls, attribs): + return sorted(attribs, key=lambda x: x.metadata["field_order"]) + + @attr.s(field_transformer=hook) + class C: + x: int = attr.ib(metadata={"field_order": 1}) + y: int = attr.ib(metadata={"field_order": 0}) + + assert {"x": 0, "y": 1} == attr.asdict(C(1, 0)) + + def test_hook_reorder_fields_before_order_check(self): + """ + It is possible to reorder fields via the hook before order-based errors are raised. + + Regression test for #1147. + """ + + def hook(cls, attribs): + return sorted(attribs, key=lambda x: x.metadata["field_order"]) + + @attr.s(field_transformer=hook) + class C: + x: int = attr.ib(metadata={"field_order": 1}, default=0) + y: int = attr.ib(metadata={"field_order": 0}) + + assert {"x": 0, "y": 1} == attr.asdict(C(1)) + + def test_hook_conflicting_defaults_after_reorder(self): + """ + Raises `ValueError` if attributes with defaults are followed by + mandatory attributes after the hook reorders fields. + + Regression test for #1147. + """ + + def hook(cls, attribs): + return sorted(attribs, key=lambda x: x.metadata["field_order"]) + + with pytest.raises(ValueError) as e: + + @attr.s(field_transformer=hook) + class C: + x: int = attr.ib(metadata={"field_order": 1}) + y: int = attr.ib(metadata={"field_order": 0}, default=0) + + assert ( + "No mandatory attributes allowed after an attribute with a " + "default value or factory. Attribute in question: Attribute" + "(name='x', default=NOTHING, validator=None, repr=True, " + "eq=True, eq_key=None, order=True, order_key=None, " + "hash=None, init=True, " + "metadata=mappingproxy({'field_order': 1}), type='int', converter=None, " + "kw_only=False, inherited=False, on_setattr=None, alias=None)", + ) == e.value.args + def test_hook_with_inheritance(self): """ The hook receives all fields from base classes. """ def hook(cls, attribs): - assert [a.name for a in attribs] == ["x", "y"] + assert ["x", "y"] == [a.name for a in attribs] # Remove Base' "x" return attribs[1:] @@ -136,7 +199,7 @@ class Base: class Sub(Base): y: int - assert attr.asdict(Sub(2)) == {"y": 2} + assert {"y": 2} == attr.asdict(Sub(2)) def test_attrs_attrclass(self): """ @@ -151,7 +214,7 @@ class C: x: int fields_type = type(attr.fields(C)) - assert fields_type.__name__ == "CAttributes" + assert "CAttributes" == fields_type.__name__ assert issubclass(fields_type, tuple) @@ -187,12 +250,12 @@ class Parent: ) result = attr.asdict(inst, value_serializer=hook) - assert result == { + assert { "a": {"x": 1, "y": ["2020-07-01T00:00:00"]}, "b": [{"x": 2, "y": ["2020-07-02T00:00:00"]}], "c": {"spam": {"x": 3, "y": ["2020-07-03T00:00:00"]}}, "d": {"eggs": "2020-07-04T00:00:00"}, - } + } == result def test_asdict_calls(self): """ @@ -217,7 +280,7 @@ class Parent: inst = Parent(a=Child(1), b=[Child(2)], c={"spam": Child(3)}) attr.asdict(inst, value_serializer=hook) - assert calls == [ + assert [ (inst, "a", inst.a), (inst.a, "x", inst.a.x), (inst, "b", inst.b), @@ -225,4 +288,4 @@ class Parent: (inst, "c", inst.c), (None, None, "spam"), (inst.c["spam"], "x", inst.c["spam"].x), - ] + ] == calls