Match filter attributes by identity instead of equality

changelog.d/864.change.md

@@ -0,0 +1 @@
+`attrs.filters.include` and `attrs.filters.exclude` now match `attrs.Attribute` instances by identity instead of equality, so filtering by an attribute of one class no longer also matches an identically-configured attribute of a different class.

src/attr/filters.py

@@ -9,12 +9,18 @@
 
 def _split_what(what):
     """
-    Returns a tuple of `frozenset`s of classes and attributes.
+    Returns a `frozenset` of classes, a `frozenset` of names, and a `tuple` of
+    attributes.
+
+    Attributes are kept as a tuple and matched by identity rather than put in a
+    `frozenset`: `Attribute` equality ignores the owning class, so two
+    identically-configured attributes on different classes compare equal and
+    set membership would match the wrong one (see #864).
     """
     return (
         frozenset(cls for cls in what if isinstance(cls, type)),
         frozenset(cls for cls in what if isinstance(cls, str)),
-        frozenset(cls for cls in what if isinstance(cls, Attribute)),
+        tuple(cls for cls in what if isinstance(cls, Attribute)),
     )
 
 
@@ -39,7 +45,7 @@ def include_(attribute, value):
         return (
             value.__class__ in cls
             or attribute.name in names
-            or attribute in attrs
+            or any(attribute is a for a in attrs)
         )
 
     return include_
@@ -66,7 +72,7 @@ def exclude_(attribute, value):
         return not (
             value.__class__ in cls
             or attribute.name in names
-            or attribute in attrs
+            or any(attribute is a for a in attrs)
         )
 
     return exclude_

tests/test_filters.py

@@ -30,7 +30,7 @@ def test_splits(self):
         assert (
             frozenset((int, str)),
             frozenset(("abcd", "123")),
-            frozenset((fields(C).a,)),
+            (fields(C).a,),
         ) == _split_what((str, "123", fields(C).a, int, "abcd"))
 
 
@@ -79,6 +79,24 @@ def test_drop_class(self, incl, value):
         i = include(*incl)
         assert i(fields(C).a, value) is False
 
+    def test_matches_attribute_by_identity(self):
+        """
+        An identically-configured attribute on a different class is not
+        included: attributes are matched by identity, not equality (#864).
+        """
+
+        @attr.s
+        class D:
+            a = attr.ib()
+            b = attr.ib()
+
+        assert fields(C).a == fields(D).a
+        assert fields(C).a is not fields(D).a
+
+        i = include(fields(C).a)
+        assert i(fields(C).a, 42) is True
+        assert i(fields(D).a, 42) is False
+
 
 class TestExclude:
     """
@@ -124,3 +142,23 @@ def test_drop_class(self, excl, value):
         """
         e = exclude(*excl)
         assert e(fields(C).a, value) is False
+
+    def test_matches_attribute_by_identity(self):
+        """
+        An identically-configured attribute on a different class is not
+        excluded: attributes are matched by identity, not equality (#864).
+        """
+
+        @attr.s
+        class D:
+            a = attr.ib()
+            b = attr.ib()
+
+        assert fields(C).a == fields(D).a
+        assert fields(C).a is not fields(D).a
+
+        e = exclude(fields(C).a)
+        # C.a is excluded ...
+        assert e(fields(C).a, 42) is False
+        # ... but the equal-but-distinct D.a is not.
+        assert e(fields(D).a, 42) is True