fix structure for union of attr classes with default values

HISTORY.rst

@@ -2,6 +2,11 @@
 History
 =======
 
+1.1.2 (unrelased)
+-----------------
+* Disambigualation do not pick non required field.
+  (`#108 <https://github.com/Tinche/cattrs/pull/108>`_)
+
 1.1.1 (2020-10-30)
 ------------------
 * Add metadata for supported Python versions.

src/cattr/disambiguators.py

@@ -10,7 +10,7 @@
     Type,
 )
 
-from attr import fields
+from attr import fields, NOTHING
 
 from cattr._compat import get_origin
 
@@ -42,7 +42,14 @@ def create_uniq_field_dis_func(*classes: Type) -> Callable:
             if not uniq:
                 m = "{} has no usable unique attributes.".format(cl)
                 raise ValueError(m)
-            uniq_attrs_dict[next(iter(uniq))] = cl
+            # We need a unique attribute with no default.
+            cl_fields = fields(get_origin(cl) or cl)
+            for attr_name in uniq:
+                if getattr(cl_fields, attr_name).default is NOTHING:
+                    break
+            else:
+                raise ValueError(f"{cl} has no usable non-default attributes.")
+            uniq_attrs_dict[attr_name] = cl
         else:
             fallback = cl
 

tests/test_disambigutors.py

@@ -1,8 +1,11 @@
 """Tests for auto-disambiguators."""
+from typing import Any
+
 import attr
 import pytest
 
-from hypothesis import assume, given
+from attr import NOTHING
+from hypothesis import HealthCheck, assume, given, settings
 
 from cattr.disambiguators import create_uniq_field_dis_func
 
@@ -40,6 +43,18 @@ class D(object):
         # No unique fields on either class.
         create_uniq_field_dis_func(C, D)
 
+    @attr.s
+    class E:
+        pass
+
+    @attr.s
+    class F:
+        b = attr.ib(default=Any)
+
+    with pytest.raises(ValueError):
+        # no usable non-default attributes
+        create_uniq_field_dis_func(E, F)
+
 
 @given(simple_classes(defaults=False))
 def test_fallback(cl_and_vals):
@@ -63,9 +78,12 @@ class A(object):
         fn({"xyz": 1}) is A  # Uses the fallback.
 
 
+@settings(
+    suppress_health_check=[HealthCheck.filter_too_much, HealthCheck.too_slow]
+)
 @given(simple_classes(), simple_classes())
 def test_disambiguation(cl_and_vals_a, cl_and_vals_b):
-    """Disambiguation should work when there are unique fields."""
+    """Disambiguation should work when there are unique required fields."""
     cl_a, vals_a = cl_and_vals_a
     cl_b, vals_b = cl_and_vals_b
 
@@ -76,6 +94,10 @@ def test_disambiguation(cl_and_vals_a, cl_and_vals_b):
     assume(len(req_b))
 
     assume((req_a - req_b) or (req_b - req_a))
+    for attr_name in req_a - req_b:
+        assume(getattr(attr.fields(cl_a), attr_name).default is NOTHING)
+    for attr_name in req_b - req_a:
+        assume(getattr(attr.fields(cl_b), attr_name).default is NOTHING)
 
     fn = create_uniq_field_dis_func(cl_a, cl_b)
 

tests/test_structure.py

@@ -29,6 +29,7 @@
 )
 from pytest import raises
 
+import attr
 from cattr import Converter
 from cattr._compat import is_bare, is_union_type
 from cattr.converters import NoneType
@@ -351,3 +352,21 @@ class Bar(Foo):
     converter.register_structure_hook(Bar, lambda obj, cls: cls("bar"))
     assert converter.structure(None, Foo).value == "foo"
     assert converter.structure(None, Bar).value == "bar"
+
+
+def test_structure_union_edge_case():
+    converter = Converter()
+
+    @attr.s(auto_attribs=True)
+    class A:
+        a1: Any
+        a2: Optional[Any] = None
+
+    @attr.s(auto_attribs=True)
+    class B:
+        b1: Any
+        b2: Optional[Any] = None
+
+    assert converter.structure(
+        [{"a1": "foo"}, {"b1": "bar"}], List[Union[A, B]]
+    ) == [A("foo"), B("bar")]