Repository: python-attrs/attrs
Pull request #590 (2019-10-21 15:59:12)

Fix issue 589
only the first item in the following list actually applies :)

# Pull Request Check List

- [x ] Added **tests** for changed code.
- [x ] New features have been added to our [Hypothesis testing strategy](https://github.com/python-attrs/attrs/blob/master/tests/strategies.py).
- [x ] Changes or additions to public APIs are reflected in our type stubs (files ending in ``.pyi``).
    - [x ] ...and used in the stub test file `tests/typing_example.py`.
- [x ] Updated **documentation** for changed code.
    - [x ] New functions/classes have to be added to `docs/api.rst` by hand.
    - [x ] Changes to the signature of `@attr.s()` have to be added by hand too.
    - [x ] Changed/added classes/methods/functions have appropriate `versionadded`, `versionchanged`, or `deprecated` [directives](http://www.sphinx-doc.org/en/stable/markup/para.html#directive-versionadded).
- [x ] Documentation in `.rst` files is written using [semantic newlines](https://rhodesmill.org/brandon/2012/one-sentence-per-line/).
- [x ] Changes (and possible deprecations) have news fragments in [`changelog.d`](https://github.com/python-attrs/attrs/blob/master/changelog.d).

If you have *any* questions to *any* of the points above, just **submit and ask**!  This checklist is here to *help* you, not to deter you from contributing!


smarie: hum it seems that some very old mods leaked in the rst file. It should not be part of the PR. I'm trying to remove it right now
smarie: rather closing and re-creating a proper one: the tree seems quite full of dirt here, sorry.
smarie: The diff seems ok now. 

Sorry for the bad tree (commits from 2017 mentioned above), if that is a problem I can delete my attrs fork and start from a fresh one. 
hynek: - Could you please add a changelog entry? You've checked it in the checklist but I can't find it. ;)
- Please run Black (or just `tox -e lint`), it's failing the CI.

Thanks!
smarie: Oh sorry - sure, I'll do.

Actually I'll close this PR and open another one with cleaner history.

hynek: Don't worry about the history, we squash on merge anyways.
smarie: The tox lint config does not seem to take into account proxy environment variables. I am located behind a corporate proxy so it fails with ` fatal: unable to access 'https://github.com/psf/black/': Failed to connect to github.com port 443: Timed out`

I'll try later or when back home. Alternately if you have the opportunity to see the issue in `test_slots.py`, do not hesitate to propose a review comment

Thanks again hynek


hynek: Feel free to add an `passenv` to tox.ini
smarie: Should be fine now. Do not hesitate to change the test description if that does not suit you - I was rather verbose :)
hynek: Line 559 is too long 🙈
smarie: That's what happens when I try to complete a PR while doing other things in parallel :) sorry hynek. This is now ok.

Patch:
diff --git a/changelog.d/590.change.rst b/changelog.d/590.change.rst
new file mode 100644
index 0000000..1bee757
--- /dev/null
+++ b/changelog.d/590.change.rst
@@ -0,0 +1,1 @@
+Fixed a ``ValueError: Cell is empty`` bug that could happen in some rare edge cases.
diff --git a/src/attr/_make.py b/src/attr/_make.py
index f32ae95..c28c625 100644
--- a/src/attr/_make.py
+++ b/src/attr/_make.py
@@ -641,8 +641,13 @@ class _ClassBuilder(object):
             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)
+                try:
+                    match = cell.cell_contents is self._cls
+                except ValueError:  # ValueError: Cell is empty
+                    pass
+                else:
+                    if match:
+                        set_closure_cell(cell, cls)
 
         return cls
 


Test patch:
diff --git a/tests/test_slots.py b/tests/test_slots.py
index 1fda4c1..1d83297 100644
--- a/tests/test_slots.py
+++ b/tests/test_slots.py
@@ -543,3 +543,28 @@ def tests_weakref_does_not_add_with_weakref_attribute():
     w = weakref.ref(c)
 
     assert c is w()
+
+
+def test_slots_empty_cell():
+    """
+    Tests that no `ValueError: Cell is empty` exception is raised when
+    closure cells are present with no contents in a `slots=True` class.
+    (issue https://github.com/python-attrs/attrs/issues/589)
+
+    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 `attrs` replaces the class with a clone,
+    `_ClassBuilder._create_slots_class(self)` will rewrite these references so
+    it keeps working. This method was not properly covering the edge case where
+    the closure cell was empty, we fixed it and this is the non-regression
+    test.
+    """
+
+    @attr.s(slots=True)
+    class C(object):
+        field = attr.ib()
+
+        def f(self, a):
+            super(C, self).__init__()
+
+    C(field=1)