From c0d42e964c86e59da46e82ccaabcdea005beb77a Mon Sep 17 00:00:00 2001 From: Pranav Mishra Date: Tue, 7 Jul 2026 13:56:06 -0700 Subject: [PATCH] Match filter attributes by identity instead of equality attrs.filters.include/exclude stored the passed Attribute instances in a frozenset and tested membership with `attribute in attrs`. Attribute equality ignores the owning class, so two identically-configured attributes on different classes compare equal - filtering by one class's attribute also matched the other class's. Keep the attributes in a tuple and match them by identity instead. Field-name strings still match by name across classes, which covers the cross-class use case. Fixes #864 Co-Authored-By: Claude Opus 4.8 --- changelog.d/864.change.md | 1 + src/attr/filters.py | 14 ++++++++++---- tests/test_filters.py | 40 ++++++++++++++++++++++++++++++++++++++- 3 files changed, 50 insertions(+), 5 deletions(-) create mode 100644 changelog.d/864.change.md diff --git a/changelog.d/864.change.md b/changelog.d/864.change.md new file mode 100644 index 000000000..73b2e3620 --- /dev/null +++ b/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. diff --git a/src/attr/filters.py b/src/attr/filters.py index 689b1705a..3ac4f6e6c 100644 --- a/src/attr/filters.py +++ b/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_ diff --git a/tests/test_filters.py b/tests/test_filters.py index 08314fa88..f1ca7e237 100644 --- a/tests/test_filters.py +++ b/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