recurse into decorator-wrapped methods when rewriting closure cells for slotted classes

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`.

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

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 <https://github.com/python-attrs/attrs/issues/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:
+            # 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):
@@ -2570,6 +2613,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,

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()
 

tests/test_converters.py

@@ -364,3 +364,61 @@ 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"")

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.

tests/test_slots.py

@@ -497,6 +497,59 @@ 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():