refactor: review feedback

Author: boblat

src/_algopy_testing/primitives/fixed_bytes.py

@@ -6,7 +6,7 @@
 import typing
 
 if typing.TYPE_CHECKING:
-    from collections.abc import Iterator
+    from collections.abc import Callable, Iterator
 
 from itertools import zip_longest
 
@@ -24,18 +24,19 @@
 _TBytesLength_Arg = typing.TypeVar("_TBytesLength_Arg", bound=int)
 
 
-def _create_class_from_type(cls: type, length_t: type) -> type:
+def _get_or_create_class_from_type(cls: type, length_t: type) -> type:
     _length = get_int_literal_from_type_generic(length_t)
-    return _create_class(cls, _length, length_t)
+    return _get_or_create_class(cls, _length, length_t)
 
 
-def _create_class_from_int(cls: type, length: int) -> type:
+def _get_or_create_class_from_int(cls: type, length: int) -> type:
     length_t = get_type_generic_from_int_literal(length)
-    return _create_class(cls, length, length_t)
+    return _get_or_create_class(cls, length, length_t)
 
 
-def _create_class(cls: type, length: int, length_t: type) -> type:
-    cache = cls.__concrete__ if hasattr(cls, "__concrete__") else {}
+def _get_or_create_class(cls: type, length: int, length_t: type) -> type:
+    """Get or create a type that is parametrized with element_t and length."""
+    cache = getattr(cls, "__concrete__", {})
     if c := cache.get(length_t, None):
         assert isinstance(c, type)
         return c
@@ -55,12 +56,11 @@ def _create_class(cls: type, length: int, length_t: type) -> type:
 class _FixedBytesMeta(type):
     __concrete__: typing.ClassVar[dict[type, type]] = {}
 
-    # get or create a type that is parametrized with element_t and length
     def __getitem__(cls, length_t: type) -> type:
         if length_t == typing.Any:
             return cls
 
-        return _create_class_from_type(cls, length_t)
+        return _get_or_create_class_from_type(cls, length_t)
 
 
 class FixedBytes(
@@ -103,8 +103,6 @@ def __bool__(self) -> bool:
     def __len__(self) -> int:
         return self._length
 
-    # mypy suggests due to Liskov below should be other: object
-    # need to consider ramifications here, ignoring it for now
     def __eq__(self, other: FixedBytes[_TBytesLength_Arg] | Bytes | bytes) -> bool:  # type: ignore[override]
         """FixedBytes can be compared using the `==` operator with another FixedBytes,
         Bytes or bytes."""
@@ -134,17 +132,18 @@ def __radd__(self, other: FixedBytes[_TBytesLength_Arg] | Bytes | bytes) -> Byte
         result = as_bytes(other) + self.value
         return _checked_result(result, "+")
 
+    def __iadd__(self, _other: Bytes | typing.Self | bytes) -> typing.Self:  # type: ignore[misc]
+        raise TypeError("FixedBytes does not support in-place addition")
+
     @property
     def length(self) -> UInt64:
         """Returns the specified length of the FixedBytes."""
         return UInt64(self._length)
 
-    def __getitem__(
-        self, index: UInt64 | int | slice
-    ) -> Bytes:  # maps to substring/substring3 if slice, extract/extract3 otherwise?
+    def __getitem__(self, index: UInt64 | int | slice) -> Bytes:
         """Returns a Bytes containing a single byte if indexed with UInt64 or int
         otherwise the substring of bytes described by the slice."""
-        value = self.value[None : self.length]
+        value = self.value[: self.length]
         if isinstance(index, slice):
             return Bytes(value[index])
         else:
@@ -174,15 +173,15 @@ def __and__(self, other: FixedBytes[_TBytesLength_Arg] | bytes | Bytes) -> typin
 
         Returns FixedBytes if other has the same length, otherwise returns Bytes.
         """
-        return self._operate_bitwise(other, "and_")
+        return self._operate_bitwise(other, operator.and_)
 
     def __rand__(self, other: bytes) -> Bytes:
         return self & other
 
     def __iand__(self, other: Bytes | typing.Self | bytes) -> typing.Self:  # type: ignore[misc]
         other_bytes = as_bytes(other)
         other_fixed_bytes = self.__class__(other_bytes)
-        result = self._operate_bitwise(other_fixed_bytes, "and_")
+        result = self._operate_bitwise(other_fixed_bytes, operator.and_)
         assert isinstance(result, self.__class__)
         return result
 
@@ -191,15 +190,15 @@ def __or__(self, other: typing.Self) -> typing.Self: ...  # type: ignore[overloa
     @typing.overload
     def __or__(self, other: FixedBytes[_TBytesLength_Arg] | bytes | Bytes) -> Bytes: ...
     def __or__(self, other: FixedBytes[_TBytesLength_Arg] | bytes | Bytes) -> typing.Self | Bytes:
-        return self._operate_bitwise(other, "or_")
+        return self._operate_bitwise(other, operator.or_)
 
     def __ror__(self, other: bytes) -> Bytes:
         return self | other
 
     def __ior__(self, other: Bytes | typing.Self | bytes) -> typing.Self:  # type: ignore[misc]
         other_bytes = as_bytes(other)
         other_fixed_bytes = self.__class__(other_bytes)
-        result = self._operate_bitwise(other_fixed_bytes, "or_")
+        result = self._operate_bitwise(other_fixed_bytes, operator.or_)
         assert isinstance(result, self.__class__)
         return result
 
@@ -208,15 +207,15 @@ def __xor__(self, other: typing.Self) -> typing.Self: ...  # type: ignore[overlo
     @typing.overload
     def __xor__(self, other: FixedBytes[_TBytesLength_Arg] | bytes | Bytes) -> Bytes: ...
     def __xor__(self, other: FixedBytes[_TBytesLength_Arg] | bytes | Bytes) -> typing.Self | Bytes:
-        return self._operate_bitwise(other, "xor")
+        return self._operate_bitwise(other, operator.xor)
 
     def __rxor__(self, other: bytes) -> Bytes:
         return self ^ other
 
     def __ixor__(self, other: Bytes | typing.Self | bytes) -> typing.Self:  # type: ignore[misc]
         other_bytes = as_bytes(other)
         other_fixed_bytes = self.__class__(other_bytes)
-        result = self._operate_bitwise(other_fixed_bytes, "xor")
+        result = self._operate_bitwise(other_fixed_bytes, operator.xor)
         assert isinstance(result, self.__class__)
         return result
 
@@ -231,9 +230,8 @@ def __invert__(self) -> typing.Self:
     def _operate_bitwise(
         self,
         other: FixedBytes[_TBytesLength_Arg] | bytes | Bytes,
-        operator_name: str,
+        op: Callable[[int, int], int],
     ) -> typing.Self | Bytes:
-        op = getattr(operator, operator_name)
         maybe_bytes = as_bytes(other)
         # pad the shorter of self.value and other bytes with leading zero
         # by reversing them as zip_longest fills at the end
@@ -259,32 +257,46 @@ def from_base32(cls, value: str) -> typing.Self:
         """Creates FixedBytes from a base32 encoded string e.g.
         `FixedBytes.from_base32("74======")`"""
         bytes_value = base64.b32decode(value)
-        c = _create_class_from_int(cls, len(bytes_value)) if not hasattr(cls, "_length") else cls
-        return c(bytes_value)  # type: ignore[no-any-return]
+        c = cls._ensure_class_with_length(bytes_value)
+        return c(bytes_value)
 
     @classmethod
     def from_base64(cls, value: str) -> typing.Self:
         """Creates FixedBytes from a base64 encoded string e.g.
         `FixedBytes.from_base64("RkY=")`"""
         bytes_value = base64.b64decode(value)
-        c = _create_class_from_int(cls, len(bytes_value)) if not hasattr(cls, "_length") else cls
-        return c(bytes_value)  # type: ignore[no-any-return]
+        c = cls._ensure_class_with_length(bytes_value)
+        return c(bytes_value)
 
     @classmethod
     def from_hex(cls, value: str) -> typing.Self:
-        """Creates FixedBytes from a hex/octal encoded string e.g. `FixedBytes.from_hex("FF")`"""
+        """Creates FixedBytes from a hex/octal encoded string e.g.
+        `FixedBytes.from_hex("FF")`"""
         bytes_value = base64.b16decode(value)
-        c = _create_class_from_int(cls, len(bytes_value)) if not hasattr(cls, "_length") else cls
-        return c(bytes_value)  # type: ignore[no-any-return]
+        c = cls._ensure_class_with_length(bytes_value)
+        return c(bytes_value)
 
     @classmethod
     def from_bytes(cls, value: Bytes | bytes) -> typing.Self:
         """Construct an instance from the underlying bytes (no validation)"""
         bytes_value = as_bytes(value)
-        c = _create_class_from_int(cls, len(bytes_value)) if not hasattr(cls, "_length") else cls
+        c = cls._ensure_class_with_length(bytes_value)
         result = c()
         result.value = bytes_value
-        return result  # type: ignore[no-any-return]
+        return result
+
+    @classmethod
+    def _ensure_class_with_length(cls, bytes_value: bytes) -> type[typing.Self]:
+        """Returns the appropriate class for the given bytes value.
+
+        If cls has a fixed _length, returns cls. Otherwise, returns or creates a
+        specialized class with the length set to match bytes_value.
+        """
+        return (
+            _get_or_create_class_from_int(cls, len(bytes_value))
+            if not hasattr(cls, "_length")
+            else cls
+        )
 
     @property
     def bytes(self) -> Bytes:

tests/models/test_box.py

@@ -20,6 +20,7 @@
 )
 from _algopy_testing.primitives.biguint import BigUInt
 from _algopy_testing.primitives.bytes import Bytes
+from _algopy_testing.primitives.fixed_bytes import FixedBytes
 from _algopy_testing.primitives.string import String
 from _algopy_testing.primitives.uint64 import UInt64
 from _algopy_testing.state.box import Box
@@ -387,7 +388,7 @@ class Swapped2(Struct):
     b: Array[UInt64]
 
 
-def test_arrays_and_struct_in_boxes(context: AlgopyTestContext) -> None:  # noqa: ARG001
+def test_arrays_and_struct_in_boxes(context: AlgopyTestContext) -> None:  # noqa: ARG001, PLR0915
     # Array
     arr1 = Array([UInt64(1), UInt64(2), UInt64(3)])
     arr2 = Array([UInt64(4), UInt64(5), UInt64(6)])
@@ -460,6 +461,19 @@ def test_arrays_and_struct_in_boxes(context: AlgopyTestContext) -> None:  # noqa
     box5.value.a[1] = UInt64(20)
     assert list(box5.value.a) == [UInt64(1), UInt64(20), UInt64(3)]
 
+    # FixedBytes in FixedArray
+    box6 = Box(
+        FixedArray[FixedBytes[typing.Literal[1024]], typing.Literal[10]], key=b"test_array_6"
+    )
+    box6.value = FixedArray(
+        [FixedBytes[typing.Literal[1024]].from_hex(f"0{x}" * 1024) for x in range(10)]
+    )
+    assert box6.length == 10 * 1024
+    assert box6.value[0].bytes == b"\x00" * 1024
+    assert box6.value[9].bytes == b"\x09" * 1024
+    box6.value[1] = FixedBytes[typing.Literal[1024]].from_hex("11" * 1024)
+    assert box6.value[1].bytes == b"\x11" * 1024
+
 
 def test_box() -> None:
     with algopy_testing_context():

tests/primitives/test_fixed_bytes.py

@@ -54,7 +54,8 @@ def test_fixed_bytes_init_wrong_length(value: bytes | Bytes, message: str) -> No
 )
 def test_fixed_bytes_bool(
     value: FixedBytes,  # type: ignore[type-arg]
-    expected: bool,  # noqa: FBT001
+    *,
+    expected: bool,
 ) -> None:
     assert bool(value) == expected
 
@@ -596,6 +597,21 @@ def test_fixed_bytes_contains_edge_cases() -> None:
     assert b"testing" not in fb  # longer than haystack
 
 
+def test_in_place_addition() -> None:
+    """Test that in-place addition is not supported."""
+    a = FixedBytes[typing.Literal[4]](b"test")
+    b = FixedBytes[typing.Literal[4]](b"data")
+
+    with pytest.raises(TypeError, match="FixedBytes does not support in-place addition"):
+        a += b
+
+    with pytest.raises(TypeError, match="FixedBytes does not support in-place addition"):
+        a += b"hello"
+
+    with pytest.raises(TypeError, match="FixedBytes does not support in-place addition"):
+        a += Bytes(b"hello")
+
+
 def test_augmented_assignment() -> None:
     """Test that augmented assignment operators are not supported."""
     a = FixedBytes[typing.Literal[2]](b"\x0f\x0f")