From 9cf1e261980b4a8bae0b61e7ee3586ad70be87b3 Mon Sep 17 00:00:00 2001 From: Zo Bot Date: Sat, 27 Jun 2026 03:52:48 +0000 Subject: [PATCH 1/4] decode bytes/bytearray inputs in to_bool os.environb yields bytes on POSIX and the documented use case for to_bool is reading values out of environment variables. The truthy and falsy lookup tuples only contain str and int, so a bytes('true') input raised 'Cannot convert value to bool: b\'true\'' even though its ASCII-decoded value is the canonical truthy literal. Decode bytes and bytearray as ASCII before the lowercase + lookup step, matching the str path. Non-ASCII bytes raise ValueError ('Cannot convert value to bool: ...') the same way an unknown string does, since the decoded text would still not match any known literal. Unknown byte values raise the same ValueError. --- src/attr/converters.py | 15 +++++++++++ tests/test_converters.py | 54 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 69 insertions(+) diff --git a/src/attr/converters.py b/src/attr/converters.py index 0a79deef0..543aff0ff 100644 --- a/src/attr/converters.py +++ b/src/attr/converters.py @@ -135,6 +135,7 @@ def to_bool(val): - ``"on"`` - ``"1"`` - ``1`` + - bytes/bytearray whose ASCII-decoded value matches any of the strings above Values mapping to `False`: @@ -144,12 +145,26 @@ def to_bool(val): - ``"off"`` - ``"0"`` - ``0`` + - bytes/bytearray whose ASCII-decoded value matches any of the strings above Raises: ValueError: For any other value. + .. versionchanged:: 26.2 + bytes/bytearray inputs are decoded as ASCII before lookup, so values + read from environment variables as bytes (e.g. on Windows or from + ``os.environb``) work without a separate ``.decode("ascii")`` step at + the call site. + .. versionadded:: 21.3.0 """ + if isinstance(val, (bytes, bytearray)): + try: + val = val.decode("ascii") + except UnicodeDecodeError as e: + msg = f"Cannot convert value to bool: {val!r}" + raise ValueError(msg) from e + if isinstance(val, str): val = val.lower() diff --git a/tests/test_converters.py b/tests/test_converters.py index 5726ae210..7fa631df4 100644 --- a/tests/test_converters.py +++ b/tests/test_converters.py @@ -364,3 +364,57 @@ def test_falsy(self): assert not to_bool("f") assert not to_bool("no") assert not to_bool("off") + + @pytest.mark.parametrize("value", [b"true", b"t", b"yes", b"y", b"on", b"1"]) + def test_truthy_bytes(self, value): + """ + Bytes values that decode to a truthy keyword match the str truthy path. + """ + assert to_bool(value) is True + + @pytest.mark.parametrize( + "value", [b"false", b"f", b"no", b"n", b"off", b"0"] + ) + def test_falsy_bytes(self, value): + """ + Bytes values that decode to a falsy keyword match the str falsy path. + """ + assert to_bool(value) is False + + @pytest.mark.parametrize("value", [b"TRUE", b"Yes", b"OFF", b"1"]) + def test_bytes_are_lowercased(self, value): + """ + Bytes inputs follow the same case-insensitive lookup as str inputs. + """ + assert to_bool(value) is bool(value.decode("ascii").lower() in + {"true", "t", "yes", "y", "on", "1"}) + + def test_bytearray_truthy(self): + """ + bytearray is a bytes subclass and should be decoded the same way. + """ + assert to_bool(bytearray(b"true")) is True + assert to_bool(bytearray(b"off")) is False + + def test_bytes_non_ascii_raises(self): + """ + Bytes that are not valid ASCII raise ValueError with the same shape as + other unconvertible values. + """ + with pytest.raises(ValueError, match="Cannot convert value to bool"): + to_bool(b"\xfftrue") + + def test_bytes_unknown_keyword_raises(self): + """ + Bytes that decode to a value not in either keyword set still raise. + """ + with pytest.raises(ValueError, match="Cannot convert value to bool"): + to_bool(b"maybe") + + def test_bytes_empty_raises(self): + """ + An empty bytes value is not a valid keyword, so it raises the same + error as an empty string would. + """ + with pytest.raises(ValueError, match="Cannot convert value to bool"): + to_bool(b"") From fb2d7d0c68e03898f5a52dddd593a4f50fe916a1 Mon Sep 17 00:00:00 2001 From: Zo Bot Date: Mon, 6 Jul 2026 13:55:26 +0000 Subject: [PATCH 2/4] make_class: accept Attribute instances in 'these' (#1424) When a class is rebuilt from attrs.fields() via attrs.make_class, the 'these' mapping contains Attribute instances rather than _CountingAttr objects. from_counting_attr was only reading the private _default, _validator, and _converter attributes, which Attribute does not expose, so the rebuild crashed with "'Attribute' object has no attribute '_default'". Detect an Attribute argument and read the public default/validator/ converter attributes directly. Add a regression test and a changelog entry. --- changelog.d/1424.change.md | 1 + src/attr/_make.py | 30 ++++++++++++++++++++++++++ tests/test_make.py | 44 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 75 insertions(+) create mode 100644 changelog.d/1424.change.md diff --git a/changelog.d/1424.change.md b/changelog.d/1424.change.md new file mode 100644 index 000000000..2f781292c --- /dev/null +++ b/changelog.d/1424.change.md @@ -0,0 +1 @@ +`attrs.make_class()` now works when its `attrs` argument is a dict of existing `attrs.Attribute` instances (e.g. from `attrs.fields(cls)`), not just fresh `attr.ib()` / `attrs.field()` call \ No newline at end of file diff --git a/src/attr/_make.py b/src/attr/_make.py index 793bfd89d..a7f392af5 100644 --- a/src/attr/_make.py +++ b/src/attr/_make.py @@ -2570,6 +2570,36 @@ def from_counting_attr( elif ca.type is not None: msg = f"Type annotation and type argument cannot both be present for '{name}'." raise ValueError(msg) + + # `from_counting_attr` is also called with already-built `Attribute` + # instances (e.g. when a class is reconstructed from + # `attrs.fields(cls)` via `attrs.make_class`, see #1424). In that case + # `ca` is an `Attribute` and exposes the public `default`/`validator`/ + # `converter` attributes rather than the `_default`/`_validator`/ + # `_converter` internals of `_CountingAttr`. + if isinstance(ca, Attribute): + return cls( + name, + ca.default, + ca.validator, + ca.repr, + None, + ca.hash, + ca.init, + False, + ca.metadata, + type, + ca.converter, + kw_only if ca.kw_only is None else ca.kw_only, + ca.eq, + ca.eq_key, + ca.order, + ca.order_key, + ca.on_setattr, + ca.alias, + ca.alias_is_default, + ) + return cls( name, ca._default, diff --git a/tests/test_make.py b/tests/test_make.py index f83f85234..f932a634d 100644 --- a/tests/test_make.py +++ b/tests/test_make.py @@ -1535,6 +1535,50 @@ def test_make_class_ordered(self): assert "C(a=1, b=2)" == repr(C()) + def test_make_class_from_attrs_fields(self): + """ + make_class can rebuild a class from attrs.fields() output, not only + fresh _CountingAttr / attr.ib() values. + + https://github.com/python-attrs/attrs/issues/1424 + """ + + @attr.s + class Source: + a = attr.ib(type=int) + b = attr.ib( + default="x", validator=attr.validators.instance_of(str) + ) + c = attr.ib( + default=0, + converter=attr.converters.optional(int), + ) + + # Convert the source's _CountingAttr-derived fields into proper + # Attribute instances via make_class, then re-run make_class on those + # Attribute instances. The second pass previously failed with + # "'Attribute' object has no attribute '_default'". + rebuilt = attr.make_class( + "Rebuilt", + {f.name: f for f in attr.fields(Source)}, + ) + + # Order is preserved end-to-end. + assert ["a", "b", "c"] == [a.name for a in attr.fields(rebuilt)] + + # `a` has no default and the rest do, so call with positional a and + # rely on defaults to fill b/c. `7` for c is converted via int. + ri = rebuilt(5) + assert 5 == ri.a + assert "x" == ri.b + assert 0 == ri.c + + ri = rebuilt(5, "hello") + assert "hello" == ri.b + + ri = rebuilt(5, "hello", "7") + assert 7 == ri.c + def test_generic_dynamic_class(self): """ make_class can create generic dynamic classes. From 0d20013b61e6d960ec8f613fee3e76339caa8693 Mon Sep 17 00:00:00 2001 From: Zo Bot Date: Tue, 7 Jul 2026 03:49:04 +0000 Subject: [PATCH 3/4] recurse into decorator-wrapped methods when rewriting closure cells for slotted classes --- changelog.d/1038.bugfix.md | 2 + src/attr/_make.py | 87 ++++++++++++++++++++++++++++---------- tests/test_slots.py | 50 ++++++++++++++++++++++ 3 files changed, 117 insertions(+), 22 deletions(-) create mode 100644 changelog.d/1038.bugfix.md diff --git a/changelog.d/1038.bugfix.md b/changelog.d/1038.bugfix.md new file mode 100644 index 000000000..f0c80b042 --- /dev/null +++ b/changelog.d/1038.bugfix.md @@ -0,0 +1,2 @@ +Slotted classes now correctly handle `super()` and `__class__` references inside methods that are wrapped by a decorator. +The closure-cell rewrite that keeps `super()` and `__class__` working after the slot-based class is rebuilt now recurses into decorator-wrapped methods instead of stopping at the outer wrapper, so the inner method's baked class reference points at the rebuilt class. Without the fix, calling a decorated method on a slotted attrs class that used `super()` raised `TypeError: super(type, obj): obj must be an instance or subtype of type`. diff --git a/src/attr/_make.py b/src/attr/_make.py index a7f392af5..bd69f8863 100644 --- a/src/attr/_make.py +++ b/src/attr/_make.py @@ -556,6 +556,63 @@ def _make_cached_property_getattr(cached_properties, original_getattr, cls): )["__getattr__"] +def _rewrite_closure_cells(item, old_cls, new_cls): + """ + Rewrite ``__class__`` / ``super()`` closure cells in *item* to point at + *new_cls* instead of *old_cls*. + + This is invoked on the class dict of the cloned slotted attrs class so + that a method which references ``__class__`` or calls the no-arg + ``super()`` keeps working — the compiler bakes a reference to the + surrounding class into the method's ``__closure__``, and we just + replaced that class with a clone. + + A single pass through the class dict is not enough: a method may be + wrapped by a decorator whose closure cell holds the *original* method, + and the closure cell that actually points at the class lives on that + inner method (see `GH-1038 `_). + We recurse into cell contents so decorator-wrapped methods get their + inner ``__class__`` cell rewritten too. + + Classmethod, staticmethod, and property descriptors are unwrapped + before their ``__closure__`` is inspected. The recursion descends + into any cell whose contents are themselves a function (the typical + decorator-wraps-function case) but does not look inside arbitrary + callables — we only want to rewrite closure cells the compiler put + there as part of a ``super()`` / ``__class__`` reference, not user + data that happens to live in a cell. + """ + if isinstance(item, (classmethod, staticmethod)): + # Class- and staticmethods hide their functions inside. + # These might need to be rewritten as well. + target = item.__func__ + elif isinstance(item, property): + # Workaround for property `super()` shortcut (PY3-only). + # There is no universal way for other descriptors. + target = item.fget + else: + target = item + + closure_cells = getattr(target, "__closure__", None) + if not closure_cells: # Catch None or the empty list. + return + + for cell in closure_cells: + try: + contents = cell.cell_contents + except ValueError: # noqa: PERF203 + # ValueError: Cell is empty + continue + + if contents is old_cls: + cell.cell_contents = new_cls + elif isinstance(contents, types.FunctionType): + # Decorator wrapping: the wrapper's cell points at the + # underlying function, whose own closure may carry the + # ``__class__`` / ``super()`` reference we need to rewrite. + _rewrite_closure_cells(contents, old_cls, new_cls) + + def _frozen_setattrs(self, name, value): """ Attached to frozen classes as __setattr__. @@ -970,31 +1027,17 @@ def _create_slots_class(self): # compiler will bake a reference to the class in the method itself # as `method.__closure__`. Since we replace the class with a # clone, we rewrite these references so it keeps working. + # + # The rewrite is recursive because a method may be wrapped by a + # decorator (decorator returns a wrapper function whose closure + # cell contains the original method; see GH-1038). Plain iteration + # over the class dict stops at the wrapper, so the inner method's + # baked `__class__` is left pointing at the old class and + # ``super()`` raises ``TypeError`` at call time. for item in itertools.chain( cls.__dict__.values(), additional_closure_functions_to_update ): - if isinstance(item, (classmethod, staticmethod)): - # Class- and staticmethods hide their functions inside. - # These might need to be rewritten as well. - closure_cells = getattr(item.__func__, "__closure__", None) - elif isinstance(item, property): - # Workaround for property `super()` shortcut (PY3-only). - # There is no universal way for other descriptors. - closure_cells = getattr(item.fget, "__closure__", None) - else: - closure_cells = getattr(item, "__closure__", None) - - if not closure_cells: # Catch None or the empty list. - continue - for cell in closure_cells: - try: - match = cell.cell_contents is self._cls - except ValueError: # noqa: PERF203 - # ValueError: Cell is empty - pass - else: - if match: - cell.cell_contents = cls + _rewrite_closure_cells(item, self._cls, cls) return cls def add_repr(self, ns): diff --git a/tests/test_slots.py b/tests/test_slots.py index a74c32b03..ff564286e 100644 --- a/tests/test_slots.py +++ b/tests/test_slots.py @@ -497,6 +497,56 @@ def statmethod(): assert D.statmethod() is D + def test_decorated_method_with_super(self, slots): + """ + Closure cell rewriting recurses through wrapped methods (GH-1038). + + When a method is wrapped by a decorator, the wrapper function's + closure cell holds the original (unwrapped) method, which in turn + has its own closure cell baking a reference to ``__class__``. The + previous closure walker only inspected the outer wrapper, so the + inner method's ``__class__`` / no-arg ``super()`` reference kept + pointing at the pre-slot-rebuild class and raised ``TypeError`` at + call time on slotted attrs classes. + + The parametrized ``slots`` fixture covers both the slots-on and + slots-off paths; the bug is slots-specific. + """ + def trace(method): + def wrapper(self, *args, **kwargs): + return method(self, *args, **kwargs) + return wrapper + + @attr.s(slots=slots) + class Parent: + def f(self): + return "Parent.f" + + @attr.s(slots=slots) + class Child(Parent): + @trace + def f(self): + return super().f() + "+Child.f" + + # Non-slotted classes were always correct; the regression was only + # on slots=True. Assert both paths work. + assert Child().f() == "Parent.f+Child.f" + + # And the static case: an unwrapped subclass that mentions + # ``__class__`` inside a method that lives behind a decorator. + def returns_class(method): + def wrapper(self, *args, **kwargs): + return method(self, *args, **kwargs) + return wrapper + + @attr.s(slots=slots) + class WithClass: + @returns_class + def who(self): + return __class__ + + assert WithClass().who() is WithClass + @pytest.mark.skipif(PYPY, reason="__slots__ only block weakref on CPython") def test_not_weakrefable(): From 902ebb73bd1e4bbf55e129b4ed1d6862ccc5ab6d Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Tue, 7 Jul 2026 03:49:32 +0000 Subject: [PATCH 4/4] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- changelog.d/1424.change.md | 2 +- src/attr/_make.py | 2 +- tests/test_converters.py | 10 +++++++--- tests/test_slots.py | 3 +++ 4 files changed, 12 insertions(+), 5 deletions(-) diff --git a/changelog.d/1424.change.md b/changelog.d/1424.change.md index 2f781292c..87b5dc2f2 100644 --- a/changelog.d/1424.change.md +++ b/changelog.d/1424.change.md @@ -1 +1 @@ -`attrs.make_class()` now works when its `attrs` argument is a dict of existing `attrs.Attribute` instances (e.g. from `attrs.fields(cls)`), not just fresh `attr.ib()` / `attrs.field()` call \ No newline at end of file +`attrs.make_class()` now works when its `attrs` argument is a dict of existing `attrs.Attribute` instances (e.g. from `attrs.fields(cls)`), not just fresh `attr.ib()` / `attrs.field()` call diff --git a/src/attr/_make.py b/src/attr/_make.py index bd69f8863..06ba96a47 100644 --- a/src/attr/_make.py +++ b/src/attr/_make.py @@ -600,7 +600,7 @@ def _rewrite_closure_cells(item, old_cls, new_cls): for cell in closure_cells: try: contents = cell.cell_contents - except ValueError: # noqa: PERF203 + except ValueError: # ValueError: Cell is empty continue diff --git a/tests/test_converters.py b/tests/test_converters.py index 7fa631df4..5f2d0b092 100644 --- a/tests/test_converters.py +++ b/tests/test_converters.py @@ -365,7 +365,9 @@ def test_falsy(self): assert not to_bool("no") assert not to_bool("off") - @pytest.mark.parametrize("value", [b"true", b"t", b"yes", b"y", b"on", b"1"]) + @pytest.mark.parametrize( + "value", [b"true", b"t", b"yes", b"y", b"on", b"1"] + ) def test_truthy_bytes(self, value): """ Bytes values that decode to a truthy keyword match the str truthy path. @@ -386,8 +388,10 @@ def test_bytes_are_lowercased(self, value): """ Bytes inputs follow the same case-insensitive lookup as str inputs. """ - assert to_bool(value) is bool(value.decode("ascii").lower() in - {"true", "t", "yes", "y", "on", "1"}) + assert to_bool(value) is bool( + value.decode("ascii").lower() + in {"true", "t", "yes", "y", "on", "1"} + ) def test_bytearray_truthy(self): """ diff --git a/tests/test_slots.py b/tests/test_slots.py index ff564286e..9fd561831 100644 --- a/tests/test_slots.py +++ b/tests/test_slots.py @@ -512,9 +512,11 @@ def test_decorated_method_with_super(self, slots): The parametrized ``slots`` fixture covers both the slots-on and slots-off paths; the bug is slots-specific. """ + def trace(method): def wrapper(self, *args, **kwargs): return method(self, *args, **kwargs) + return wrapper @attr.s(slots=slots) @@ -537,6 +539,7 @@ def f(self): def returns_class(method): def wrapper(self, *args, **kwargs): return method(self, *args, **kwargs) + return wrapper @attr.s(slots=slots)