diff --git a/changelog.d/1479.change b/changelog.d/1479.change new file mode 100644 index 000000000..62628680e --- /dev/null +++ b/changelog.d/1479.change @@ -0,0 +1 @@ +Field aliases are now resolved *before* calling `field_transformer`, so transformers receive fully populated `Attribute` objects with usable `alias` values instead of `None`. diff --git a/docs/extending.md b/docs/extending.md index c3a475ae4..cc724bd81 100644 --- a/docs/extending.md +++ b/docs/extending.md @@ -248,13 +248,13 @@ Data(a=3, b='spam', c=datetime.datetime(2020, 5, 4, 13, 37)) ``` Or, perhaps you would prefer to generate dataclass-compatible `__init__` signatures via a default field *alias*. -Note, *field_transformer* operates on {class}`attrs.Attribute` instances before the default private-attribute handling is applied so explicit user-provided aliases can be detected. +Note, *field_transformer* receives {class}`attrs.Attribute` instances with default aliases already resolved (e.g., leading-underscore stripping has been applied), so you can compare against the default to detect explicit user-provided aliases. ```{doctest} >>> def dataclass_names(cls, fields): ... return [ ... field.evolve(alias=field.name) -... if not field.alias +... if field.alias == field.name.lstrip("_") ... else field ... for field in fields ... ] diff --git a/src/attr/_make.py b/src/attr/_make.py index 32e42976e..38f30e7e9 100644 --- a/src/attr/_make.py +++ b/src/attr/_make.py @@ -463,6 +463,15 @@ def _transform_attrs( attrs = base_attrs + own_attrs + # Resolve default field alias before executing field_transformer, + # so that the transformer receives fully populated Attribute objects + # with usable alias values instead of None. + # See: https://github.com/python-attrs/attrs/issues/1479 + for a in attrs: + if not a.alias: + # Evolve is very slow, so we hold our nose and do it dirty. + _OBJ_SETATTR.__get__(a)("alias", _default_init_alias_for(a.name)) + if field_transformer is not None: attrs = tuple(field_transformer(cls, attrs)) @@ -480,12 +489,10 @@ def _transform_attrs( if had_default is False and a.default is not NOTHING: had_default = True - # Resolve default field alias after executing field_transformer. - # This allows field_transformer to differentiate between explicit vs - # default aliases and supply their own defaults. + # Resolve default field alias for any new attributes that the + # field_transformer may have added without setting an alias. for a in attrs: if not a.alias: - # Evolve is very slow, so we hold our nose and do it dirty. _OBJ_SETATTR.__get__(a)("alias", _default_init_alias_for(a.name)) # Create AttrsClass *after* applying the field_transformer since it may @@ -2585,6 +2592,18 @@ def evolve(self, **changes): new._setattrs(changes.items()) + # If the name changed but alias was not explicitly provided in + # changes, update the alias if it was auto-generated (i.e., it + # matches the default alias for the *old* name). + if ( + "name" in changes + and "alias" not in changes + and self.alias == _default_init_alias_for(self.name) + ): + _OBJ_SETATTR.__get__(new)( + "alias", _default_init_alias_for(new.name) + ) + return new # Don't use _add_pickle since fields(Attribute) doesn't work diff --git a/tests/test_hooks.py b/tests/test_hooks.py index 930c750af..d2eb87146 100644 --- a/tests/test_hooks.py +++ b/tests/test_hooks.py @@ -178,7 +178,7 @@ class C: "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)", + "kw_only=False, inherited=False, on_setattr=None, alias='x')", ) == e.value.args def test_hook_with_inheritance(self): @@ -233,6 +233,104 @@ class Base: assert ["x"] == [a.name for a in attr.fields(Base)] + def test_hook_alias_available(self): + """ + The field_transformer receives attributes with default aliases + already resolved, not None. + + Regression test for #1479. + """ + seen_aliases = [] + + def hook(cls, attribs): + seen_aliases[:] = [(a.name, a.alias) for a in attribs] + return attribs + + @attr.s(auto_attribs=True, field_transformer=hook) + class C: + _private: int + _explicit: int = attr.ib(alias="_explicit") + public: int + + assert [ + ("_private", "private"), + ("_explicit", "_explicit"), + ("public", "public"), + ] == seen_aliases + + def test_hook_evolve_name_updates_auto_alias(self): + """ + When a field_transformer evolves a field's name, the alias is + automatically updated if it was auto-generated. + + Regression test for #1479. + """ + + def hook(cls, attribs): + return [a.evolve(name="renamed") for a in attribs] + + @attr.s(auto_attribs=True, field_transformer=hook) + class C: + _original: int + + assert "renamed" == attr.fields(C).renamed.alias + + def test_hook_evolve_name_keeps_explicit_alias(self): + """ + When a field_transformer evolves a field's name but the field had + an explicit alias, the alias is preserved. + + Regression test for #1479. + """ + + def hook(cls, attribs): + return [a.evolve(name="renamed") for a in attribs] + + @attr.s(auto_attribs=True, field_transformer=hook) + class C: + original: int = attr.ib(alias="my_alias") + + assert "my_alias" == attr.fields(C).renamed.alias + + def test_hook_new_field_without_alias(self): + """ + When a field_transformer adds a brand-new field without setting an + alias, the post-transformer alias resolution fills it in. + + Regression test for #1479. + """ + + def hook(cls, attribs): + return [ + *attribs, + attr.Attribute( + name="_extra", + default=0, + validator=None, + repr=True, + cmp=None, + hash=None, + init=True, + metadata={}, + type=int, + converter=None, + kw_only=False, + eq=True, + eq_key=None, + order=True, + order_key=None, + on_setattr=None, + alias=None, + inherited=False, + ), + ] + + @attr.s(auto_attribs=True, field_transformer=hook) + class C: + x: int + + assert "extra" == attr.fields(C)._extra.alias + class TestAsDictHook: def test_asdict(self): diff --git a/tests/test_make.py b/tests/test_make.py index 6f4ab57a8..69a9dfaf6 100644 --- a/tests/test_make.py +++ b/tests/test_make.py @@ -243,7 +243,7 @@ class C: "eq=True, eq_key=None, order=True, order_key=None, " "hash=None, init=True, " "metadata=mappingproxy({}), type=None, converter=None, " - "kw_only=False, inherited=False, on_setattr=None, alias=None)", + "kw_only=False, inherited=False, on_setattr=None, alias='y')", ) == e.value.args def test_kw_only(self):