Repository: python-attrs/attrs (python)

Issue:
Attrs deletes any non-None class attributes
Currently, `attrs` will delete any non-`None` class attributes when `_ClassBuilder._delete_attribs` is `True` (i.e. when attributes are specified via class attributes, not directly via `these` field).
According to comment on [_make.py:502](https://github.com/python-attrs/attrs/blob/master/src/attr/_make.py#L502), this is intended to remove `attr.ib` instances from the class.
But it does not play well with attributes declared using PEP 484 type annotations and `auto_attribs=True` argument to `attr.s`, because any attribute with non-`None` default value specified will be pruned from the class, while attributes with `None` value are retained.
This leads to unexpected behaviour in many cases, in particular when generating documentation using `Sphinx` `autosummary` extension which relies on class introspection.
I think we should remove only `attr.ib` instances, not any not-None fields. At least in `auto_attribs` mode. Or at least fix the comment.

Attrs version checked: 19.1.0
Code to reproduce:
```
import attr
import typing

@attr.s(auto_attribs=True)
class Foo:
    bar: int = 42
    baz: typing.Any = None

print([f for f in dir(Foo) if not f.startswith('_')])  # ['baz']
print(Foo.baz)  # 42
print(Foo.bar)  # AttributeError
```

Pull request description:
Ensure that bare attributes with default None are removed too
Fixes #523 

Classic None-as-sentinel-where-None-is-a-valid-value-blunder.

Patch:
diff --git a/changelog.d/523.change.rst b/changelog.d/523.change.rst
new file mode 100644
index 0000000..86bf8cd
--- /dev/null
+++ b/changelog.d/523.change.rst
@@ -0,0 +1,1 @@
+When collecting attributes using ``@attr.s(auto_attribs=True)``, attributes with a default of ``None`` are now deleted too.
diff --git a/changelog.d/556.change.rst b/changelog.d/556.change.rst
new file mode 100644
index 0000000..86bf8cd
--- /dev/null
+++ b/changelog.d/556.change.rst
@@ -0,0 +1,1 @@
+When collecting attributes using ``@attr.s(auto_attribs=True)``, attributes with a default of ``None`` are now deleted too.
diff --git a/src/attr/_make.py b/src/attr/_make.py
index 2e96542..283aec6 100644
--- a/src/attr/_make.py
+++ b/src/attr/_make.py
@@ -42,6 +42,9 @@ _hash_cache_field = "_attrs_cached_hash"
 
 _empty_metadata_singleton = metadata_proxy({})
 
+# Unique object for unequivocal getattr() defaults.
+_sentinel = object()
+
 
 class _Nothing(object):
     """
@@ -504,7 +507,7 @@ class _ClassBuilder(object):
             for name in self._attr_names:
                 if (
                     name not in base_names
-                    and getattr(cls, name, None) is not None
+                    and getattr(cls, name, _sentinel) != _sentinel
                 ):
                     try:
                         delattr(cls, name)


Test patch:
diff --git a/tests/test_annotations.py b/tests/test_annotations.py
index cbf5bd0..1f23f1d 100644
--- a/tests/test_annotations.py
+++ b/tests/test_annotations.py
@@ -265,3 +265,20 @@ class TestAnnotations:
             x: int
 
         assert 1 == C(1).x
+
+    def test_removes_none_too(self):
+        """
+        Regression test for #523: make sure defaults that are set to None are
+        removed too.
+        """
+
+        @attr.s(auto_attribs=True)
+        class C:
+            x: int = 42
+            y: typing.Any = None
+
+        with pytest.raises(AttributeError):
+            C.x
+
+        with pytest.raises(AttributeError):
+            C.y