Start adding typing information.

[roberthdevries]

This is very preliminary and only provides type information for random.py and utils.py

[JeremiahM37]

Skoll Code Review

Scan type: review
Overall recommendation: REQUEST_CHANGES
Findings: 6 total — 4 posted, 2 skipped
4 finding(s) posted as inline comments (see file-level comments below)

Posted findings

• [High] __builtins__.bytes is broken as a type annotation in non-__main__ modules — wolfcrypt/random.py:57,70
• [Medium] Unused from _cffi_backend import FFI import in random.py — wolfcrypt/random.py:25
• [Medium] __init__.pyi stub declares __all__ = ["lib"] but does not declare lib itself — wolfcrypt/_ffi/__init__.pyi:5
• [Low] Docstring in t2b() is now inaccurate after behavioral change — wolfcrypt/utils.py:30-31

Skipped findings

• [Medium] Random._delete in __del__ is less shutdown-safe than self._delete
• [Info] Missing newline at end of file in __init__.pyi

Review generated by Skoll

[JeremiahM37]

🔴 [High] __builtins__.bytes is broken as a type annotation in non-__main__ modules

The return type annotation -> __builtins__.bytes is incorrect. In Python, __builtins__ is a dict (not a module) in every module except __main__. Since random.py is always imported as a module, __builtins__ is a dict, and __builtins__.bytes would raise AttributeError (dicts don't have a .bytes attribute). While from __future__ import annotations prevents runtime evaluation (PEP 563), this annotation will: (1) cause AttributeError if anyone calls typing.get_type_hints(Random.byte), (2) fail validation with type checkers like mypy/pyright, and (3) confuse IDE autocompletion. The built-in bytes type is already in scope — no qualification is needed.

def byte(self) -> __builtins__.bytes:
    ...
    def bytes(self, length) -> __builtins__.bytes:

Recommendation: Replace __builtins__.bytes with plain bytes in both return type annotations. The builtin bytes is always available without qualification.

[JeremiahM37]

🟠 [Medium] Unused from _cffi_backend import FFI import in random.py

The import from _cffi_backend import FFI was added but FFI is never referenced anywhere in random.py. The annotations use __builtins__.bytes (which should be plain bytes), not FFI. This import will cause a linter warning (e.g., F401 unused import) and adds a hard dependency on _cffi_backend at import time that didn't exist before. Note that _cffi_backend is a C extension module; importing it directly is unusual — the codebase already imports through wolfcrypt._ffi.

from _cffi_backend import FFI

Recommendation: Remove the from _cffi_backend import FFI import since it is not used.

[JeremiahM37]

🟠 [Medium] __init__.pyi stub declares __all__ = ["lib"] but does not declare lib itself

The stub file declares __all__ = ["lib"] but never defines or imports lib. Additionally, ffi is declared as a module attribute (line 3) but is excluded from __all__, even though random.py imports it with from wolfcrypt._ffi import ffi. While __all__ doesn't restrict direct imports, it signals the module's public API to type checkers and documentation tools. The stub should be consistent with actual module exports.

import _cffi_backend

ffi: _cffi_backend.FFI

__all__ = ["lib"]

Recommendation: Either add ffi to __all__ and declare lib as a submodule reference, or remove __all__ from the stub if it's not meaningful.

[JeremiahM37]

🔵 [Low] Docstring in t2b() is now inaccurate after behavioral change

The docstring says Passes through bytes, bytearray, and memoryview unchanged. but the implementation now wraps all three with bytes(), which creates a new bytes object. While bytes(some_bytes) returns an equivalent bytes object, bytes(some_bytearray) and bytes(some_memoryview) perform a copy and type conversion. The docstring should reflect the new behavior.

"""
    Converts text to binary.

    Passes through bytes, bytearray, and memoryview unchanged.
    Encodes str to UTF-8 bytes.
    """

Recommendation: Update the docstring to say the inputs are converted to bytes, not passed through unchanged.