From 3040bdabbc627fc6a4d151a9cb2e6f9d4177b6f2 Mon Sep 17 00:00:00 2001 From: Hynek Schlawack Date: Thu, 26 Oct 2017 12:55:34 +0200 Subject: [PATCH] Refactor class creation (#272) Instead of adding and possibly later deleting attributes, the class creation is delegated into a cleaner building pattern. --- CHANGELOG.rst | 2 +- README.rst | 6 +- changelog.d/269.change.rst | 1 + changelog.d/270.change.rst | 1 + changelog.d/272.change.rst | 1 + conftest.py | 5 +- docs/examples.rst | 6 +- docs/how-does-it-work.rst | 6 +- src/attr/_make.py | 423 +++++++++++++++++++++++++----------- tests/test_annotations.py | 6 +- tests/test_dark_magic.py | 92 ++++++++ tests/test_init_subclass.py | 44 ++++ tests/test_make.py | 126 ++++++----- tests/test_slots.py | 35 ++- tests/utils.py | 9 +- 15 files changed, 568 insertions(+), 195 deletions(-) create mode 100644 changelog.d/269.change.rst create mode 100644 changelog.d/270.change.rst create mode 100644 changelog.d/272.change.rst create mode 100644 tests/test_init_subclass.py diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 9bbe20402..18ecd9113 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -230,7 +230,7 @@ Changes: ^^^^^^^^ - ``__slots__`` have arrived! - Classes now can automatically be `slots `_-style (and save your precious memory) just by passing ``slots=True``. + Classes now can automatically be `slots `_-style (and save your precious memory) just by passing ``slots=True``. `#35 `_ - Allow the case of initializing attributes that are set to ``init=False``. This allows for clean initializer parameter lists while being able to initialize attributes to default values. diff --git a/README.rst b/README.rst index f41bd08d1..8a7348347 100644 --- a/README.rst +++ b/README.rst @@ -97,6 +97,9 @@ Testimonials -- **Kenneth Reitz**, author of `requests `_, Python Overlord at Heroku, `on paper no less `_ +.. -end- + +.. -project-information- Getting Help ============ @@ -105,9 +108,6 @@ Please use the ``python-attrs`` tag on `StackOverflow `_ ``__slots__``. +Normal Python classes can avoid using a separate dictionary for each instance of a class by `defining `_ ``__slots__``. For ``attrs`` classes it's enough to set ``slots=True``: .. doctest:: @@ -505,9 +505,11 @@ Slot classes are a little different than ordinary, dictionary-backed classes: ... AttributeError: 'Coordinates' object has no attribute 'z' -- Since non-slot classes cannot be turned into slot classes after they have been created, ``attr.s(.., slots=True)`` will *replace* the class it is applied to with a copy. +- Since non-slot classes cannot be turned into slot classes after they have been created, ``attr.s(slots=True)`` will *replace* the class it is applied to with a copy. In almost all cases this isn't a problem, but we mention it for the sake of completeness. + * One notable problem is that certain metaclass features like ``__init_subclass__`` do not work with slot classes. + - Using :mod:`pickle` with slot classes requires pickle protocol 2 or greater. Python 2 uses protocol 0 by default so the protocol needs to be specified. Python 3 uses protocol 3 by default. diff --git a/docs/how-does-it-work.rst b/docs/how-does-it-work.rst index d80988c2b..c20becd1f 100644 --- a/docs/how-does-it-work.rst +++ b/docs/how-does-it-work.rst @@ -17,14 +17,16 @@ In order to ensure that sub-classing works as you'd expect it to work, ``attrs`` Please note that ``attrs`` does *not* call ``super()`` *ever*. It will write dunder methods to work on *all* of those attributes which also has performance benefits due to fewer function calls. -Once ``attrs`` knows what attributes it has to work on, it writes the requested dunder methods and attaches them to your class. +Once ``attrs`` knows what attributes it has to work on, it writes the requested dunder methods and -- depending on whether you wish to have ``__slots__`` -- creates a new class for you (``slots=True``) or attaches them to the original class (``slots=False``). +While creating new classes is more elegant, we've run into several edge cases surrounding metaclasses that make it impossible to go this route unconditionally. + To be very clear: if you define a class with a single attribute without a default value, the generated ``__init__`` will look *exactly* how you'd expect: .. doctest:: >>> import attr, inspect >>> @attr.s - ... class C: + ... class C(object): ... x = attr.ib() >>> print(inspect.getsource(C.__init__)) def __init__(self, x): diff --git a/src/attr/_make.py b/src/attr/_make.py index 89edd312b..eaac0843a 100644 --- a/src/attr/_make.py +++ b/src/attr/_make.py @@ -181,22 +181,27 @@ class MyClassAttributes(tuple): return globs[attr_class_name] +# Tuple class for extracted attributes from a class definition. +# `super_attrs` is a subset of `attrs`. +_Attributes = _make_attr_tuple_class("_Attributes", [ + "attrs", # all attributes to build dunder methods for + "super_attrs", # attributes that have been inherited from super classes +]) + + def _transform_attrs(cls, these): """ - Transform all `_CountingAttr`s on a class into `Attribute`s and save the - list in `__attrs_attrs__` while potentially deleting them from *cls*. + Transform all `_CountingAttr`s on a class into `Attribute`s. If *these* is passed, use that and don't look for them on the class. - Return a list of tuples of (attribute name, attribute). + Return an `_Attributes`. """ if these is None: ca_list = [(name, attr) for name, attr in cls.__dict__.items() if isinstance(attr, _CountingAttr)] - for name, _ in ca_list: - delattr(cls, name) else: ca_list = [(name, ca) for name, ca @@ -206,35 +211,53 @@ def _transform_attrs(cls, these): ann = getattr(cls, "__annotations__", {}) non_super_attrs = [ - Attribute.from_counting_attr(name=attr_name, ca=ca, - type=ann.get(attr_name)) + Attribute.from_counting_attr( + name=attr_name, + ca=ca, + type=ann.get(attr_name), + ) for attr_name, ca in ca_list ] - super_cls = [] - non_super_names = set(a.name for a in non_super_attrs) - for c in reversed(cls.__mro__[1:-1]): - sub_attrs = getattr(c, "__attrs_attrs__", None) + # Walk *down* the MRO for attributes. While doing so, we collect the names + # of attributes we've seen in `take_attr_names` and ignore their + # redefinitions deeper in the hierarchy. + super_attrs = [] + taken_attr_names = set(a.name for a in non_super_attrs) + for super_cls in cls.__mro__[1:-1]: + sub_attrs = getattr(super_cls, "__attrs_attrs__", None) if sub_attrs is not None: - super_cls.extend( - a for a in sub_attrs - if a not in super_cls and a.name not in non_super_names - ) + # We iterate over sub_attrs backwards so we can reverse the whole + # list in the end and get all attributes in the order they have + # been defined. + for a in reversed(sub_attrs): + if a.name not in taken_attr_names: + super_attrs.append(a) + taken_attr_names.add(a.name) - attr_names = [a.name for a in super_cls + non_super_attrs] + # Now reverse the list, such that the attributes are sorted by *descending* + # age. IOW: the oldest attribute definition is at the head of the list. + super_attrs.reverse() + + attr_names = [a.name for a in super_attrs + non_super_attrs] AttrsClass = _make_attr_tuple_class(cls.__name__, attr_names) - cls.__attrs_attrs__ = AttrsClass(super_cls + [ - Attribute.from_counting_attr(name=attr_name, ca=ca, - type=ann.get(attr_name)) - for attr_name, ca - in ca_list - ]) + attrs = AttrsClass( + super_attrs + [ + Attribute.from_counting_attr( + name=attr_name, + ca=ca, + type=ann.get(attr_name) + ) + for attr_name, ca + in ca_list + ] + ) had_default = False - for a in cls.__attrs_attrs__: + for a in attrs: if had_default is True and a.default is NOTHING and a.init is True: raise ValueError( "No mandatory attributes allowed after an attribute with a " @@ -246,7 +269,7 @@ def _transform_attrs(cls, these): a.init is not False: had_default = True - return ca_list + return _Attributes((attrs, super_attrs)) def _frozen_setattrs(self, name, value): @@ -263,6 +286,177 @@ def _frozen_delattrs(self, name): raise FrozenInstanceError() +class _ClassBuilder(object): + """ + Iteratively build *one* class. + """ + __slots__ = ( + "_cls", "_cls_dict", "_attrs", "_super_names", "_attr_names", "_slots", + "_frozen", "_has_post_init", + ) + + def __init__(self, cls, these, slots, frozen): + attrs, super_attrs = _transform_attrs(cls, these) + + self._cls = cls + self._cls_dict = dict(cls.__dict__) if slots else {} + self._attrs = attrs + self._super_names = set(a.name for a in super_attrs) + self._attr_names = tuple(a.name for a in attrs) + self._slots = slots + self._frozen = frozen or _has_frozen_superclass(cls) + self._has_post_init = bool(getattr(cls, "__attrs_post_init__", False)) + + self._cls_dict["__attrs_attrs__"] = self._attrs + + if frozen: + self._cls_dict["__setattr__"] = _frozen_setattrs + self._cls_dict["__delattr__"] = _frozen_delattrs + + def __repr__(self): + return "<_ClassBuilder(cls={cls})>".format(cls=self._cls.__name__) + + def build_class(self): + """ + Finalize class based on the accumulated configuration. + + Builder cannot be used anymore after calling this method. + """ + if self._slots is True: + return self._create_slots_class() + else: + return self._patch_original_class() + + def _patch_original_class(self): + """ + Apply accumulated methods and return the class. + """ + cls = self._cls + super_names = self._super_names + + # Clean class of attribute definitions (`attr.ib()`s). + for name in self._attr_names: + if name not in super_names and \ + getattr(cls, name, None) is not None: + delattr(cls, name) + + # Attach our dunder methods. + for name, value in self._cls_dict.items(): + setattr(cls, name, value) + + return cls + + def _create_slots_class(self): + """ + Build and return a new class with a `__slots__` attribute. + """ + super_names = self._super_names + cd = { + k: v + for k, v in iteritems(self._cls_dict) + if k not in tuple(self._attr_names) + ("__dict__",) + } + + # We only add the names of attributes that aren't inherited. + # Settings __slots__ to inherited attributes wastes memory. + cd["__slots__"] = tuple( + name + for name in self._attr_names + if name not in super_names + ) + + qualname = getattr(self._cls, "__qualname__", None) + if qualname is not None: + cd["__qualname__"] = qualname + + attr_names = tuple(self._attr_names) + + def slots_getstate(self): + """ + Automatically created by attrs. + """ + return tuple(getattr(self, name) for name in attr_names) + + def slots_setstate(self, state): + """ + Automatically created by attrs. + """ + __bound_setattr = _obj_setattr.__get__(self, Attribute) + for name, value in zip(attr_names, state): + __bound_setattr(name, value) + + # slots and frozen require __getstate__/__setstate__ to work + cd["__getstate__"] = slots_getstate + cd["__setstate__"] = slots_setstate + + # Create new class based on old class and our methods. + cls = type(self._cls)( + self._cls.__name__, + self._cls.__bases__, + cd, + ) + + # The following is a fix for + # https://github.com/python-attrs/attrs/issues/102. On Python 3, + # if a method mentions `__class__` or uses the no-arg super(), the + # compiler will bake a reference to the class in the method itself + # as `method.__closure__`. Since we replace the class with a + # clone, we rewrite these references so it keeps working. + for item in cls.__dict__.values(): + if isinstance(item, (classmethod, staticmethod)): + # Class- and staticmethods hide their functions inside. + # These might need to be rewritten as well. + closure_cells = getattr(item.__func__, "__closure__", None) + else: + closure_cells = getattr(item, "__closure__", None) + + if not closure_cells: # Catch None or the empty list. + continue + for cell in closure_cells: + if cell.cell_contents is self._cls: + set_closure_cell(cell, cls) + + return cls + + def add_repr(self, ns): + self._cls_dict["__repr__"] = _make_repr(self._attrs, ns=ns) + return self + + def add_str(self): + repr_ = self._cls_dict.get("__repr__") + if repr_ is None: + raise ValueError( + "__str__ can only be generated if a __repr__ exists." + ) + + self._cls_dict["__str__"] = repr_ + return self + + def make_unhashable(self): + self._cls_dict["__hash__"] = None + return self + + def add_hash(self): + self._cls_dict["__hash__"] = _make_hash(self._attrs) + return self + + def add_init(self): + self._cls_dict["__init__"] = _make_init( + self._attrs, + self._has_post_init, + self._frozen, + ) + return self + + def add_cmp(self): + cd = self._cls_dict + + cd["__eq__"], cd["__ne__"], cd["__lt__"], cd["__le__"], cd["__gt__"], \ + cd["__ge__"] = _make_cmp(self._attrs) + + return self + + def attrs(maybe_cls=None, these=None, repr_ns=None, repr=True, cmp=True, hash=None, init=True, slots=False, frozen=False, str=False): @@ -339,7 +533,7 @@ def attrs(maybe_cls=None, these=None, repr_ns=None, circumvent that limitation by using ``object.__setattr__(self, "attribute_name", value)``. - .. _slots: https://docs.python.org/3.5/reference/datamodel.html#slots + .. _slots: https://docs.python.org/3/reference/datamodel.html#slots .. versionadded:: 16.0.0 *slots* .. versionadded:: 16.1.0 *frozen* @@ -352,70 +546,31 @@ def wrap(cls): if getattr(cls, "__class__", None) is None: raise TypeError("attrs only works with new-style classes.") - if repr is False and str is True: - raise ValueError( - "__str__ can only be generated if a __repr__ exists." - ) - - ca_list = _transform_attrs(cls, these) + builder = _ClassBuilder(cls, these, slots, frozen) - # Can't just re-use frozen name because Python's scoping. :( - # Can't compare function objects because Python 2 is terrible. :( - effectively_frozen = _has_frozen_superclass(cls) or frozen if repr is True: - cls = _add_repr(cls, ns=repr_ns, str=str) + builder.add_repr(repr_ns) + if str is True: + builder.add_str() if cmp is True: - cls = _add_cmp(cls) + builder.add_cmp() if hash is not True and hash is not False and hash is not None: + # Can't use `hash in` because 1 == True for example. raise TypeError( "Invalid value for hash. Must be True, False, or None." ) elif hash is False or (hash is None and cmp is False): pass elif hash is True or (hash is None and cmp is True and frozen is True): - cls = _add_hash(cls) + builder.add_hash() else: - cls.__hash__ = None + builder.make_unhashable() if init is True: - cls = _add_init(cls, effectively_frozen) - if effectively_frozen is True: - cls.__setattr__ = _frozen_setattrs - cls.__delattr__ = _frozen_delattrs - if slots is True: - # slots and frozen require __getstate__/__setstate__ to work - cls = _add_pickle(cls) - if slots is True: - cls_dict = dict(cls.__dict__) - attr_names = tuple(t[0] for t in ca_list) - cls_dict["__slots__"] = attr_names - for ca_name in attr_names: - # It might not actually be in there, e.g. if using 'these'. - cls_dict.pop(ca_name, None) - cls_dict.pop("__dict__", None) - old_cls = cls - - qualname = getattr(cls, "__qualname__", None) - cls = type(cls)(cls.__name__, cls.__bases__, cls_dict) - if qualname is not None: - cls.__qualname__ = qualname - - # The following is a fix for - # https://github.com/python-attrs/attrs/issues/102. On Python 3, - # if a method mentions `__class__` or uses the no-arg super(), the - # compiler will bake a reference to the class in the method itself - # as `method.__closure__`. Since we replace the class with a - # clone, we rewrite these references so it keeps working. - for item in cls.__dict__.values(): - closure_cells = getattr(item, "__closure__", None) - if not closure_cells: # Catch None or the empty list. - continue - for cell in closure_cells: - if cell.cell_contents is old_cls: - set_closure_cell(cell, cls) + builder.add_init() - return cls + return builder.build_class() # maybe_cls's type depends on the usage of the decorator. It's a class # if it's used as `@attrs` but ``None`` if used as `@attrs()`. @@ -460,14 +615,12 @@ def _attrs_to_tuple(obj, attrs): return tuple(getattr(obj, a.name) for a in attrs) -def _add_hash(cls, attrs=None): - """ - Add a hash method to *cls*. - """ - if attrs is None: - attrs = [a - for a in cls.__attrs_attrs__ - if a.hash is True or (a.hash is None and a.cmp is True)] +def _make_hash(attrs): + attrs = tuple( + a + for a in attrs + if a.hash is True or (a.hash is None and a.cmp is True) + ) def hash_(self): """ @@ -475,16 +628,19 @@ def hash_(self): """ return hash(_attrs_to_tuple(self, attrs)) - cls.__hash__ = hash_ - return cls + return hash_ -def _add_cmp(cls, attrs=None): +def _add_hash(cls, attrs): """ - Add comparison methods to *cls*. + Add a hash method to *cls*. """ - if attrs is None: - attrs = [a for a in cls.__attrs_attrs__ if a.cmp] + cls.__hash__ = _make_hash(attrs) + return cls + + +def _make_cmp(attrs): + attrs = [a for a in attrs if a.cmp] def attrs_to_tuple(obj): """ @@ -547,22 +703,31 @@ def ge(self, other): else: return NotImplemented - cls.__eq__ = eq - cls.__ne__ = ne - cls.__lt__ = lt - cls.__le__ = le - cls.__gt__ = gt - cls.__ge__ = ge + return eq, ne, lt, le, gt, ge + + +def _add_cmp(cls, attrs=None): + """ + Add comparison methods to *cls*. + """ + if attrs is None: + attrs = cls.__attrs_attrs__ + + cls.__eq__, cls.__ne__, cls.__lt__, cls.__le__, cls.__gt__, cls.__ge__ = \ + _make_cmp(attrs) return cls -def _add_repr(cls, ns=None, attrs=None, str=False): +def _make_repr(attrs, ns): """ - Add a repr method to *cls*. If *str* is True, also add __str__. + Make a repr method for *attr_names* adding *ns* to the full name. """ - if attrs is None: - attrs = [a for a in cls.__attrs_attrs__ if a.repr] + attr_names = tuple( + a.name + for a in attrs + if a.repr + ) def repr_(self): """ @@ -580,21 +745,32 @@ def repr_(self): return "{0}({1})".format( class_name, - ", ".join(a.name + "=" + repr(getattr(self, a.name)) - for a in attrs) + ", ".join( + name + "=" + repr(getattr(self, name)) + for name in attr_names + ) ) - cls.__repr__ = repr_ - if str is True: - cls.__str__ = repr_ - return cls + return repr_ -def _add_init(cls, frozen): +def _add_repr(cls, ns=None, attrs=None): """ - Add a __init__ method to *cls*. If *frozen* is True, make it immutable. + Add a repr method to *cls*. """ - attrs = [a for a in cls.__attrs_attrs__ - if a.init or a.default is not NOTHING] + if attrs is None: + attrs = cls.__attrs_attrs__ + + repr_ = _make_repr(attrs, ns) + cls.__repr__ = repr_ + return cls + + +def _make_init(attrs, post_init, frozen): + attrs = [ + a + for a in attrs + if a.init or a.default is not NOTHING + ] # We cache the generated init methods for the same kinds of attributes. sha1 = hashlib.sha1() @@ -606,7 +782,7 @@ def _add_init(cls, frozen): script, globs = _attrs_to_script( attrs, frozen, - getattr(cls, "__attrs_post_init__", False), + post_init, ) locs = {} bytecode = compile(script, unique_filename, "exec") @@ -630,30 +806,19 @@ def _add_init(cls, frozen): script.splitlines(True), unique_filename ) - cls.__init__ = init - return cls + + return init -def _add_pickle(cls): +def _add_init(cls, frozen): """ - Add pickle helpers, needed for frozen and slotted classes + Add a __init__ method to *cls*. If *frozen* is True, make it immutable. """ - def _slots_getstate__(obj): - """ - Play nice with pickle. - """ - return tuple(getattr(obj, a.name) for a in fields(obj.__class__)) - - def _slots_setstate__(obj, state): - """ - Play nice with pickle. - """ - __bound_setattr = _obj_setattr.__get__(obj, Attribute) - for a, value in zip(fields(obj.__class__), state): - __bound_setattr(a.name, value) - - cls.__getstate__ = _slots_getstate__ - cls.__setstate__ = _slots_setstate__ + cls.__init__ = _make_init( + cls.__attrs_attrs__, + getattr(cls, "__attrs_post_init__", False), + frozen, + ) return cls diff --git a/tests/test_annotations.py b/tests/test_annotations.py index ee5d4f5f1..e91a2ffcf 100644 --- a/tests/test_annotations.py +++ b/tests/test_annotations.py @@ -1,13 +1,15 @@ """ Tests for PEP-526 type annotations. + +Python 3.6+ only. """ +import typing + import pytest import attr -import typing - class TestAnnotations: """ diff --git a/tests/test_dark_magic.py b/tests/test_dark_magic.py index e8ef89375..f0f19bef4 100644 --- a/tests/test_dark_magic.py +++ b/tests/test_dark_magic.py @@ -1,3 +1,7 @@ +""" +End-to-end tests. +""" + from __future__ import absolute_import, division, print_function import pickle @@ -283,3 +287,91 @@ class SubOverwrite(Super): x = attr.ib(default=attr.Factory(list)) assert SubOverwrite([]) == SubOverwrite() + + def test_dict_patch_class(self): + """ + dict-classes are never replaced. + """ + class C(object): + x = attr.ib() + + C_new = attr.s(C) + + assert C_new is C + + def test_hash_by_id(self): + """ + With dict classes, hashing by ID is active for hash=False even on + Python 3. This is incorrect behavior but we have to retain it for + backward compatibility. + """ + @attr.s(hash=False) + class HashByIDBackwardCompat(object): + x = attr.ib() + + assert ( + hash(HashByIDBackwardCompat(1)) != hash(HashByIDBackwardCompat(1)) + ) + + @attr.s(hash=False, cmp=False) + class HashByID(object): + x = attr.ib() + + assert hash(HashByID(1)) != hash(HashByID(1)) + + @attr.s(hash=True) + class HashByValues(object): + x = attr.ib() + + assert hash(HashByValues(1)) == hash(HashByValues(1)) + + def test_handles_different_defaults(self): + """ + Unhashable defaults + subclassing values work. + """ + @attr.s + class Unhashable(object): + pass + + @attr.s + class C(object): + x = attr.ib(default=Unhashable()) + + @attr.s + class D(C): + pass + + @pytest.mark.parametrize("slots", [True, False]) + def test_hash_false_cmp_false(self, slots): + """ + hash=False and cmp=False make a class hashable by ID. + """ + @attr.s(hash=False, cmp=False, slots=slots) + class C(object): + pass + + assert hash(C()) != hash(C()) + + def test_overwrite_super(self): + """ + Super classes can overwrite each other and the attributes are added + in the order they are defined. + """ + @attr.s + class C(object): + c = attr.ib(default=100) + x = attr.ib(default=1) + b = attr.ib(default=23) + + @attr.s + class D(C): + a = attr.ib(default=42) + x = attr.ib(default=2) + d = attr.ib(default=3.14) + + @attr.s + class E(D): + y = attr.ib(default=3) + z = attr.ib(default=4) + + assert "E(c=100, b=23, a=42, x=2, d=3.14, y=3, z=4)" == repr(E()) diff --git a/tests/test_init_subclass.py b/tests/test_init_subclass.py new file mode 100644 index 000000000..b66130ede --- /dev/null +++ b/tests/test_init_subclass.py @@ -0,0 +1,44 @@ +""" +Tests for `__init_subclass__` related tests. + +Python 3.6+ only. +""" + +import pytest + +import attr + + +@pytest.mark.parametrize("slots", [True, False]) +def test_init_subclass_vanilla(slots): + """ + `super().__init_subclass__` can be used if the subclass is not an attrs + class both with dict and slots classes. + """ + @attr.s(slots=slots) + class Base: + def __init_subclass__(cls, param, **kw): + super().__init_subclass__(**kw) + cls.param = param + + class Vanilla(Base, param="foo"): + pass + + assert "foo" == Vanilla().param + + +def test_init_subclass_attrs(): + """ + `__init_subclass__` works with attrs classes as long as slots=False. + """ + @attr.s(slots=False) + class Base: + def __init_subclass__(cls, param, **kw): + super().__init_subclass__(**kw) + cls.param = param + + @attr.s + class Attrs(Base, param="foo"): + pass + + assert "foo" == Attrs().param diff --git a/tests/test_make.py b/tests/test_make.py index 1c613e38d..5eb6f13a4 100644 --- a/tests/test_make.py +++ b/tests/test_make.py @@ -21,6 +21,8 @@ Attribute, Factory, _AndValidator, + _Attributes, + _ClassBuilder, _CountingAttr, _transform_attrs, and_, @@ -136,13 +138,23 @@ class TestTransformAttrs(object): """ Tests for `_transform_attrs`. """ + def test_no_modifications(self): + """ + Doesn't attach __attrs_attrs__ to the class anymore. + """ + C = make_tc() + _transform_attrs(C, None) + + assert None is getattr(C, "__attrs_attrs__", None) + def test_normal(self): """ Transforms every `_CountingAttr` and leaves others (a) be. """ C = make_tc() - _transform_attrs(C, None) - assert ["z", "y", "x"] == [a.name for a in C.__attrs_attrs__] + attrs, _, = _transform_attrs(C, None) + + assert ["z", "y", "x"] == [a.name for a in attrs] def test_empty(self): """ @@ -152,23 +164,18 @@ def test_empty(self): class C(object): pass - _transform_attrs(C, None) - - assert () == C.__attrs_attrs__ + assert _Attributes(((), [])) == _transform_attrs(C, None) - @pytest.mark.parametrize("attribute", [ - "z", - "y", - "x", - ]) - def test_transforms_to_attribute(self, attribute): + def test_transforms_to_attribute(self): """ All `_CountingAttr`s are transformed into `Attribute`s. """ C = make_tc() - _transform_attrs(C, None) + attrs, super_attrs = _transform_attrs(C, None) - assert isinstance(getattr(fields(C), attribute), Attribute) + assert [] == super_attrs + assert 3 == len(attrs) + assert all(isinstance(a, Attribute) for a in attrs) def test_conflicting_defaults(self): """ @@ -191,50 +198,20 @@ class C(object): def test_these(self): """ - If these is passed, use it and ignore body. + If these is passed, use it and ignore body and super classes. """ - class C(object): - y = attr.ib() - - _transform_attrs(C, {"x": attr.ib()}) - assert ( - simple_attr("x"), - ) == C.__attrs_attrs__ - assert isinstance(C.y, _CountingAttr) - - def test_recurse(self): - """ - Collect attributes from all sub-classes. - """ - class A(object): - a = None - - class B(A): - b = attr.ib() - - _transform_attrs(B, None) - - class C(B): - c = attr.ib() - - _transform_attrs(C, None) - - class D(C): - d = attr.ib() - - _transform_attrs(D, None) + class Base(object): + z = attr.ib() - class E(D): - e = attr.ib() + class C(Base): + y = attr.ib() - _transform_attrs(E, None) + attrs, super_attrs = _transform_attrs(C, {"x": attr.ib()}) + assert [] == super_attrs assert ( - simple_attr("b"), - simple_attr("c"), - simple_attr("d"), - simple_attr("e"), - ) == E.__attrs_attrs__ + simple_attr("x"), + ) == attrs class TestAttributes(object): @@ -250,6 +227,7 @@ def test_catches_old_style(self): @attr.s class C: pass + assert ("attrs only works with new-style classes.",) == e.value.args def test_sets_attrs(self): @@ -259,6 +237,7 @@ def test_sets_attrs(self): @attr.s class C(object): x = attr.ib() + assert "x" == C.__attrs_attrs__[0].name assert all(isinstance(a, Attribute) for a in C.__attrs_attrs__) @@ -269,6 +248,7 @@ def test_empty(self): @attr.s class C3(object): pass + assert "C3()" == repr(C3()) assert C3() == C3() @@ -798,3 +778,45 @@ def test_empty_metadata_singleton(self, list_of_attrs): C = make_class("C", dict(zip(gen_attr_names(), list_of_attrs))) for a in fields(C)[1:]: assert a.metadata is fields(C)[0].metadata + + +class TestClassBuilder(object): + """ + Tests for `_ClassBuilder`. + """ + def test_repr_str(self): + """ + Trying to add a `__str__` without having a `__repr__` raises a + ValueError. + """ + with pytest.raises(ValueError) as ei: + make_class("C", {}, repr=False, str=True) + + assert ( + "__str__ can only be generated if a __repr__ exists.", + ) == ei.value.args + + def test_repr(self): + """ + repr of builder itself makes sense. + """ + class C(object): + pass + + b = _ClassBuilder(C, None, True, True) + + assert "<_ClassBuilder(cls=C)>" == repr(b) + + def test_returns_self(self): + """ + All methods return the builder for chaining. + """ + class C(object): + x = attr.ib() + + b = _ClassBuilder(C, None, True, True) + + cls = b.add_cmp().add_hash().add_init().add_repr("ns").add_str() \ + .build_class() + + assert "ns.C(x=1)" == repr(cls(1)) diff --git a/tests/test_slots.py b/tests/test_slots.py index b930b2ef1..676bda51e 100644 --- a/tests/test_slots.py +++ b/tests/test_slots.py @@ -8,7 +8,7 @@ try: from pympler.asizeof import asizeof has_pympler = True -except: # Won't be an import error. +except BaseException: # Won't be an import error. has_pympler = False import attr @@ -129,10 +129,13 @@ class C2Slots(C1): z = attr.ib() c2 = C2Slots(x=1, y=2, z="test") + assert 1 == c2.x assert 2 == c2.y assert "test" == c2.z + c2.t = "test" # This will work, using the base class. + assert "test" == c2.t assert 1 == c2.method() @@ -142,8 +145,11 @@ class C2Slots(C1): assert set(["z"]) == set(C2Slots.__slots__) c3 = C2Slots(x=1, y=3, z="test") + assert c3 > c2 + c2_ = C2Slots(x=1, y=2, z="test") + assert c2 == c2_ assert "C2Slots(x=1, y=2, z='test')" == repr(c2) @@ -365,3 +371,30 @@ def my_subclass(self): assert non_slot_instance.my_subclass() is C2 assert slot_instance.my_subclass() is C2Slots + + +@pytest.mark.skipif(PY2, reason="closure cell rewriting is PY3-only.") +@pytest.mark.parametrize("slots", [True, False]) +def test_closure_cell_rewriting_cls_static(slots): + """ + Slot classes support proper closure cell rewriting for class- and static + methods. + """ + # Python can reuse closure cells, so we create new classes just for + # this test. + + @attr.s(slots=slots) + class C: + @classmethod + def clsmethod(cls): + return __class__ # noqa: F821 + + assert C.clsmethod() is C + + @attr.s(slots=slots) + class D: + @staticmethod + def statmethod(): + return __class__ # noqa: F821 + + assert D.statmethod() is D diff --git a/tests/utils.py b/tests/utils.py index 86f457477..36b624981 100644 --- a/tests/utils.py +++ b/tests/utils.py @@ -218,8 +218,13 @@ class HypClass: def post_init(self): pass cls_dict["__attrs_post_init__"] = post_init - return make_class("HypClass", cls_dict, - slots=slots_flag, frozen=frozen_flag) + + return make_class( + "HypClass", + cls_dict, + slots=slots_flag, + frozen=frozen_flag, + ) # st.recursive works by taking a base strategy (in this case, simple_classes)