Merge pull request #3 from python-attrs/master

Sync up to main attrs repository

.pre-commit-config.yaml

@@ -7,10 +7,18 @@ repos:
         # override until resolved: https://github.com/ambv/black/issues/402
         files: \.pyi?$
         types: []
+
+  - repo: https://gitlab.com/pycqa/flake8
+    rev: '3.7.3'
+    hooks:
+    - id: flake8
+      language_version: python3.7
+
   - repo: https://github.com/asottile/seed-isort-config
     rev: v1.5.0
     hooks:
       - id: seed-isort-config
+
   - repo: https://github.com/pre-commit/mirrors-isort
     rev: v4.3.4
     hooks:
@@ -23,5 +31,3 @@ repos:
       - id: trailing-whitespace
       - id: end-of-file-fixer
       - id: debug-statements
-      - id: flake8
-        language_version: python3.7

.travis.yml

@@ -60,7 +60,7 @@ matrix:
 
 
 install:
-  - pip install tox
+  - pip install --upgrade tox
 
 
 script:

changelog.d/482.breaking.rst

@@ -0,0 +1,4 @@
+Fixed a bug where deserialized objects with ``cache_hash=True`` could have incorrect hash code values.
+This change breaks classes with ``cache_hash=True`` when a custom ``__setstate__`` is present.
+An exception will be thrown when applying the ``attrs`` annotation to such a class.
+This limitation is tracked in issue `#494 <https://github.com/python-attrs/attrs/issues/494>`_.

docs/hashing.rst

@@ -37,6 +37,16 @@ Because according to the definition_ from the official Python docs, the returned
    The *correct way* to achieve hashing by id is to set ``@attr.s(cmp=False)``.
    Setting ``@attr.s(hash=False)`` (that implies ``cmp=True``) is almost certainly a *bug*.
 
+   .. warning::
+
+      Be careful when subclassing!
+      Setting ``cmp=False`` on class whose base class has a non-default ``__hash__`` method, will *not* make ``attrs`` remove that ``__hash__`` for you.
+
+      It is part of ``attrs``'s philosophy to only *add* to classes so you have the freedom to customize your classes as you wish.
+      So if you want to *get rid* of methods, you'll have to do it by hand.
+
+      The easiest way to reset ``__hash__`` on a class is adding ``__hash__ = object.__hash__`` in the class body.
+
 #. If two object are not equal, their hash **should** be different.
 
    While this isn't a requirement from a standpoint of correctness, sets and dicts become less effective if there are a lot of identical hashes.

docs/index.rst

@@ -60,6 +60,7 @@ Day-to-Day Usage
 
    license
    backward-compatibility
+   python-2
    contributing
    changelog
 

docs/init.rst

@@ -97,6 +97,7 @@ This is when default values come into play:
    C(a=42, b=[], c=[], d={})
 
 It's important that the decorated method -- or any other method or property! -- doesn't have the same name as the attribute, otherwise it would overwrite the attribute definition.
+You also cannot use type annotations to elide the :func:`attr.ib` call for ``d`` as explained in :doc:`types`.
 
 
 Please note that as with function and method signatures, ``default=[]`` will *not* do what you may think it might do:
@@ -161,7 +162,7 @@ If the value does not pass the validator's standards, it just raises an appropri
       ...
    ValueError: x must be smaller or equal to 42
 
-Again, it's important that the decorated method doesn't have the same name as the attribute.
+Again, it's important that the decorated method doesn't have the same name as the attribute and that you can't elide the call to :func:`attr.ib`.
 
 
 Callables
@@ -350,6 +351,23 @@ If you need to set attributes on a frozen class, you'll have to resort to the :r
 
 Note that you *must not* access the hash code of the object in ``__attrs_post__init__`` if ``cache_hash=True``.
 
+
+Order of Execution
+------------------
+
+If present, the hooks are executed in the following order:
+
+1. For each attribute, in the order it was declared:
+
+   a. default factory
+   b. converter
+
+2. *all* validators
+3. ``__attrs_post_init__``
+
+Notably this means, that you can access all attributes from within your validators, but your converters have to deal with invalid values and have to return a valid value.
+
+
 .. _`Wiki page`: https://github.com/python-attrs/attrs/wiki/Extensions-to-attrs
 .. _`get confused`: https://github.com/python-attrs/attrs/issues/289
 .. _`there is no such thing as a private argument`: https://github.com/hynek/characteristic/issues/6

docs/python-2.rst

@@ -0,0 +1,25 @@
+Python 2 Statement
+==================
+
+While ``attrs`` has always been a Python 3-first package, we the maintainers are aware that Python 2 will not magically disappear in 2020.
+We are also aware that ``attrs`` is an important building block in many people's systems and livelihoods.
+
+As such, we do **not** have any immediate plans to drop Python 2 support in ``attrs``.
+We intend to support is as long as it will be technically feasible for us.
+
+Feasibility in this case means:
+
+1. Possibility to run the tests on our development computers,
+2. and **free** CI options.
+
+This can mean that we will have to run our tests on PyPy, whose maintainters have unequivocally declared that they do not intend to stop the development and maintenance of their Python 2-compatible line at all.
+And this can mean that at some point, a sponsor will have to step up and pay for bespoke CI setups.
+
+**However**: there is no promise of new features coming to ``attrs`` running under Python 2.
+It is up to our discretion alone, to decide whether the introduced complexity or awkwardness are worth it, or whether we choose to make a feature available on modern platforms only.
+
+
+Summary
+-------
+
+We will do our best to support existing users, but nobody is entitled to the latest and greatest features on a platform that is officially end of life.

docs/types.rst

@@ -23,9 +23,19 @@ That means that on modern Python versions, the declaration part of the example f
    >>> attr.fields(SomeClass).a_number.type
    <class 'int'>
 
-You can still use :func:`attr.ib` for advanced features, but you don't have to.
+You will still need :func:`attr.ib` for advanced features, but not for the common cases.
 
-Please note that these types are *only metadata* that can be queried from the class and they aren't used for anything out of the box!
+One of those features are the decorator-based features like defaults.
+It's important to remember that ``attrs`` doesn't do any magic behind your back.
+All the decorators are implemented using an object that is returned by the call to :func:`attr.ib`.
+
+Attributes that only carry a class annotation do not have that object so trying to call a method on it will inevitably fail.
+
+*****
+
+Please note that types -- however added -- are *only metadata* that can be queried from the class and they aren't used for anything out of the box!
+
+In practice, their biggest usefulness shows in combination with mypy.
 
 
 mypy

pyproject.toml

@@ -1,5 +1,6 @@
 [build-system]
-requires = ["setuptools", "wheel"]
+requires = ["setuptools>=40.6.0", "wheel"]
+build-backend = "setuptools.build_meta"
 
 
 [tool.black]

src/attr/_make.py

@@ -529,6 +529,26 @@ def _patch_original_class(self):
         for name, value in self._cls_dict.items():
             setattr(cls, name, value)
 
+        # Attach __setstate__. This is necessary to clear the hash code
+        # cache on deserialization. See issue
+        # https://github.com/python-attrs/attrs/issues/482 .
+        # Note that this code only handles setstate for dict classes.
+        # For slotted classes, see similar code in _create_slots_class .
+        if self._cache_hash:
+            existing_set_state_method = getattr(cls, "__setstate__", None)
+            if existing_set_state_method:
+                raise NotImplementedError(
+                    "Currently you cannot use hash caching if "
+                    "you specify your own __setstate__ method."
+                    "See https://github.com/python-attrs/attrs/issues/494 ."
+                )
+
+            def cache_hash_set_state(chss_self, _):
+                # clear hash code cache
+                setattr(chss_self, _hash_cache_field, None)
+
+            setattr(cls, "__setstate__", cache_hash_set_state)
+
         return cls
 
     def _create_slots_class(self):
@@ -581,13 +601,22 @@ def slots_getstate(self):
             """
             return tuple(getattr(self, name) for name in state_attr_names)
 
+        hash_caching_enabled = self._cache_hash
+
         def slots_setstate(self, state):
             """
             Automatically created by attrs.
             """
             __bound_setattr = _obj_setattr.__get__(self, Attribute)
             for name, value in zip(state_attr_names, state):
                 __bound_setattr(name, value)
+            # Clearing the hash code cache on deserialization is needed
+            # because hash codes can change from run to run. See issue
+            # https://github.com/python-attrs/attrs/issues/482 .
+            # Note that this code only handles setstate for slotted classes.
+            # For dict classes, see similar code in _patch_original_class .
+            if hash_caching_enabled:
+                __bound_setattr(_hash_cache_field, None)
 
         # slots and frozen require __getstate__/__setstate__ to work
         cd["__getstate__"] = slots_getstate

tests/test_dunders.py

@@ -5,6 +5,7 @@
 from __future__ import absolute_import, division, print_function
 
 import copy
+import pickle
 
 import pytest
 
@@ -453,6 +454,86 @@ def __hash__(self):
         assert 2 == uncached_instance.hash_counter.times_hash_called
         assert 1 == cached_instance.hash_counter.times_hash_called
 
+    def test_cache_hash_serialization(self):
+        """
+        Tests that the hash cache is cleared on deserialization to fix
+        https://github.com/python-attrs/attrs/issues/482 .
+        """
+
+        # First, check that our fix didn't break serialization without
+        # hash caching.
+        # We don't care about the result of this; we just want to make sure we
+        # can do it without exceptions.
+        hash(pickle.loads(pickle.dumps(HashCacheSerializationTestUncached)))
+
+        def assert_hash_code_not_cached_across_serialization(original):
+            # Now check our fix for #482 for when hash caching is enabled.
+            original_hash = hash(original)
+            round_tripped = pickle.loads(pickle.dumps(original))
+            # What we want to guard against is having a stale hash code
+            # when a field's hash code differs in a new interpreter after
+            # deserialization.  This is tricky to test because we are,
+            # of course, still running in the same interpreter.  So
+            # after deserialization we reach in and change the value of
+            # a field to simulate the field changing its hash code. We then
+            # check that the object's hash code changes, indicating that we
+            # don't have a stale hash code.
+            # This could fail in two ways: (1) pickle.loads could get the hash
+            # code of the deserialized value (triggering it to cache) before
+            # we alter the field value.  This doesn't happen in our tested
+            # Python versions.  (2) "foo" and "something different" could
+            # have a hash collision on this interpreter run.   But this is
+            # extremely improbable and would just result in one buggy test run.
+            round_tripped.foo_string = "something different"
+            assert original_hash != hash(round_tripped)
+
+        # Slots and non-slots classes implement __setstate__ differently,
+        # so we need to test both cases.
+        assert_hash_code_not_cached_across_serialization(
+            HashCacheSerializationTestCached()
+        )
+        assert_hash_code_not_cached_across_serialization(
+            HashCacheSerializationTestCachedSlots()
+        )
+
+    def test_caching_and_custom_setstate(self):
+        """
+        The combination of a custom __setstate__ and cache_hash=True is caught
+        with a helpful message.
+
+        This is needed because we handle clearing the cache after
+        deserialization with a custom __setstate__. It is possible to make both
+        work, but it requires some thought about how to go about it, so it has
+        not yet been implemented.
+        """
+        with pytest.raises(
+            NotImplementedError,
+            match="Currently you cannot use hash caching if you "
+            "specify your own __setstate__ method.",
+        ):
+
+            @attr.attrs(hash=True, cache_hash=True)
+            class NoCacheHashAndCustomSetState(object):
+                def __setstate__(self, state):
+                    pass
+
+
+# these are for use in TestAddHash.test_cache_hash_serialization
+# they need to be out here so they can be un-pickled
+@attr.attrs(hash=True, cache_hash=False)
+class HashCacheSerializationTestUncached(object):
+    foo_string = attr.ib(default="foo")
+
+
+@attr.attrs(hash=True, cache_hash=True)
+class HashCacheSerializationTestCached(object):
+    foo_string = attr.ib(default="foo")
+
+
+@attr.attrs(slots=True, hash=True, cache_hash=True)
+class HashCacheSerializationTestCachedSlots(object):
+    foo_string = attr.ib(default="foo")
+
 
 class TestAddInit(object):
     """

tox.ini

@@ -1,5 +1,6 @@
 [tox]
 envlist = typing,lint,py27,py34,py35,py36,py37,pypy,pypy3,manifest,docs,doctest,pypi-description,changelog,coverage-report
+isolated_build = True
 
 
 [testenv]