From 2fdf92997c9870463113ef36abce345e55f9711b Mon Sep 17 00:00:00 2001
From: Alex Ford <alex.ford@abcellera.com>
Date: Wed, 30 Dec 2020 03:53:43 -0800
Subject: [PATCH] Clarify next-gen auto_attribs inference rules (#742)

* Clarify next-gen auto_attribs inference rules

The next-gen auto_attribs api documentation does not clearly describe
cases where (a) only a subset of attributes are annotated and (b)
`field` definitions are provided for a subset of fields. Update
docstring to reflect current, desirable, behavior.

Add tests to clarify `.define` behavior focused on fully-annotated
classes with partially-defined fields, which is commonly used to add
non-default behavior to a subset of a classes fields. For example:

```python

@attr.define
class NewSchool:
    x: int
    y: list = attr.field()

    @y.validator
    def _validate_y(self, attribute, value):
        if value < 0:
            raise ValueError("y must be positive")

```

The previous docstring *could* be read to imply that:

* The new-school API will not infer auto_attribs if there are any
  unannotated attributes.
* The new-school API will not infer auto_attribs if *any* attr.ib are
  defined, even if those attr.ibs are type annotated.

* Update test to match PR example

* Fix lint error

Co-authored-by: Hynek Schlawack <hs@ox.cx>
---
 src/attr/_next_gen.py  |  4 ++--
 tests/test_next_gen.py | 39 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 41 insertions(+), 2 deletions(-)

diff --git a/src/attr/_next_gen.py b/src/attr/_next_gen.py
index 2b5565c5..2990ad25 100644
--- a/src/attr/_next_gen.py
+++ b/src/attr/_next_gen.py
@@ -42,8 +42,8 @@ def define(
     :param Optional[bool] auto_attribs: If set to `True` or `False`, it behaves
        exactly like `attr.s`. If left `None`, `attr.s` will try to guess:
 
-       1. If all attributes are annotated and no `attr.ib` is found, it assumes
-          *auto_attribs=True*.
+       1. If any attributes are annotated and no unannotated `attr.ib`\ s
+          are found, it assumes *auto_attribs=True*.
        2. Otherwise it assumes *auto_attribs=False* and tries to collect
           `attr.ib`\ s.
 
diff --git a/tests/test_next_gen.py b/tests/test_next_gen.py
index 0ebad8d2..fce01ad4 100644
--- a/tests/test_next_gen.py
+++ b/tests/test_next_gen.py
@@ -112,6 +112,45 @@ class TestNextGen:
 
         assert OldSchool2(1) == OldSchool2(1)
 
+    def test_auto_attribs_detect_fields_and_annotations(self):
+        """
+        define infers auto_attribs=True if fields have type annotations
+        """
+
+        @attr.define
+        class NewSchool:
+            x: int
+            y: list = attr.field()
+
+            @y.validator
+            def _validate_y(self, attribute, value):
+                if value < 0:
+                    raise ValueError("y must be positive")
+
+        assert NewSchool(1, 1) == NewSchool(1, 1)
+        with pytest.raises(ValueError):
+            NewSchool(1, -1)
+        assert list(attr.fields_dict(NewSchool).keys()) == ["x", "y"]
+
+    def test_auto_attribs_partially_annotated(self):
+        """
+        define infers auto_attribs=True if any type annotations are found
+        """
+
+        @attr.define
+        class NewSchool:
+            x: int
+            y: list
+            z = 10
+
+        # fields are defined for any annotated attributes
+        assert NewSchool(1, []) == NewSchool(1, [])
+        assert list(attr.fields_dict(NewSchool).keys()) == ["x", "y"]
+
+        # while the unannotated attributes are left as class vars
+        assert NewSchool.z == 10
+        assert "z" in NewSchool.__dict__
+
     def test_auto_attribs_detect_annotations(self):
         """
         define correctly detects if a class has type annotations.