Skip to content

Fix remaining ty static type checker warnings and errors. #139

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
roberthdevries wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Fix remaining ty static type checker warnings and errors. #139

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion .github/workflows/python-app.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,9 @@ jobs:
- name: Install the project
run: uv sync --dev
- name: Perform static checks

@dgarske dgarske Aug 6, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 [Medium] New uv run ty check CI gate may never fail the build

The PR's stated purpose is to "allow to run ty check in the pipeline static checks step," but the step as written may be a no-op gate. pyproject.toml contains [tool.ty.rules] with all = "warn", which downgrades every ty rule from its default severity to warning level. ty check exits non-zero for error-level diagnostics; warning-level diagnostics require the --error-on-warning flag to affect the exit code. If that is the case here, the step will print diagnostics to the CI log and still exit 0, so the type regressions this PR just spent effort eliminating could silently reappear without turning CI red.

The multi-command run: | block itself is fine -- GitHub Actions uses bash -e {0} on Linux, so a failing uv run ruff check will still abort before ty check.

Fix: Verify the gate actually blocks: temporarily introduce a deliberate type error on the branch and confirm the job goes red. If it does not, either add --error-on-warning to the CI invocation or promote the rules that should be blocking from "warn" to "error" in [tool.ty.rules].

run: uv run ruff check
run: |
uv run ruff check
uv run ty check
- name: Run tests using the locally built wheel
run: |
uv pip install --reinstall dist/*.whl
Expand Down
4 changes: 2 additions & 2 deletions tests/test_mldsa.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -232,7 +232,7 @@ def test_sign_with_seed_bad_type(mldsa_type, rng, seed: int | str):
message = b"This is a test message for ML-DSA signature"
context = b"Some context for the signature"
with pytest.raises(TypeError):
mldsa_priv.sign_with_seed(message, seed, ctx=context)
mldsa_priv.sign_with_seed(message, seed, ctx=context) # ty: ignore [invalid-argument-type]

def test_make_key_from_seed(mldsa_type):
seed = bytes(MlDsaPrivate.ML_DSA_KEYGEN_SEED_LENGTH)
Expand All @@ -249,4 +249,4 @@ def test_make_key_from_seed_bad_length(mldsa_type, seed_length):
@pytest.mark.parametrize("seed", [0, "seed"])
def test_make_key_from_seed_bad_type(mldsa_type, seed: int | str):
with pytest.raises(TypeError):
MlDsaPrivate.make_key_from_seed(mldsa_type, seed)
MlDsaPrivate.make_key_from_seed(mldsa_type, seed) # ty: ignore [invalid-argument-type]
4 changes: 2 additions & 2 deletions tests/test_mlkem.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -621,7 +621,7 @@ def test_init_pattern_3(mlkem_type):
@pytest.mark.parametrize("rand", [0, "rand"])
def test_make_key_with_random_bad_random_type(mlkem_type, rand: int | str):
with pytest.raises(TypeError):
MlKemPrivate.make_key_with_random(mlkem_type, rand)
MlKemPrivate.make_key_with_random(mlkem_type, rand) # ty: ignore [invalid-argument-type]

@pytest.mark.parametrize("mlkem_type", mlkem_types)
@pytest.mark.parametrize("rand", [0, "rand"])
Expand All @@ -630,7 +630,7 @@ def test_encapsulate_with_random_bad_random_type(mlkem_type, rand: int | str):
assert type(mlkem_pub) is MlKemPublic

with pytest.raises(TypeError):
mlkem_pub.encapsulate_with_random(rand)
mlkem_pub.encapsulate_with_random(rand) # ty: ignore [invalid-argument-type]

@pytest.mark.parametrize("mlkem_type", mlkem_types)
def test_size_properties(mlkem_type):
Expand Down
10 changes: 5 additions & 5 deletions wolfcrypt/ciphers.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1372,7 +1372,7 @@ def decode_key(self, key: BytesOrStr) -> None:
raise WolfCryptError(f"Key decode error ({self.max_signature_size})")

@override
def decode_key_raw(self, qx: BytesOrStr, qy: BytesOrStr, d: BytesOrStr, curve_id: int = ECC_SECP256R1) -> None:
def decode_key_raw(self, qx: BytesOrStr, qy: BytesOrStr, d: BytesOrStr, curve_id: int = ECC_SECP256R1) -> None: # ty: ignore[invalid-method-override]
"""
Decodes an ECC private key from its raw elements: public (Qx,Qy)
and private(d)
Expand All @@ -1394,7 +1394,7 @@ def decode_key_raw(self, qx: BytesOrStr, qy: BytesOrStr, d: BytesOrStr, curve_id
raise WolfCryptApiError("Key decode error", ret)

@override

@dgarske dgarske Aug 6, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 [High] EccPrivate.encode_key() accepts with_curve but silently ignores it

To silence ty's invalid-method-override, the signature of EccPrivate.encode_key was widened from (self) -> bytes to (self, with_curve: bool = True) -> bytes, but the body was not touched. It still calls _lib.wc_EccKeyToDer(self.native_object, key, len(key)). Per the CFFI cdef in scripts/build_ffi.py:1112 (int wc_EccKeyToDer(ecc_key*, byte* output, word32 inLen);), that wolfSSL API has no curve flag at all -- only wc_EccPublicKeyToDer (used by the base EccPublic.encode_key, ciphers.py:1213) takes with_curve. The new parameter is therefore dead: it is accepted, type-checked, documented by its presence in the signature, and then dropped on the floor.

This is a behavioral regression, not just dead code. Before this PR, ecc_priv.encode_key(with_curve=False) raised TypeError: encode_key() takes 1 positional argument but 2 were given -- a loud, immediate failure. After this PR the same call silently returns curve-bearing DER, i.e. exactly the opposite of what the caller asked for. This is the LSP violation ty was warning about, converted from a noisy runtime error into a silent wrong answer.

It is also inconsistent with the rest of this same diff. The four other override mismatches touched here (EccPrivate.decode_key_raw at line 1375, EccPrivate.encode_key_raw at line 1412, Ed25519Private.encode_key at line 1683, Ed448Private.encode_key at line 1892) were all resolved with # ty: ignore[invalid-method-override], deliberately preserving the runtime signature. Only this one method had its signature altered, and it is the one case where doing so…

Fix: Revert the signature to def encode_key(self) -> bytes: and suppress with # ty: ignore[invalid-method-override], matching the four sibling methods fixed the same way in this diff. If the widened signature is intentionally kept for LSP substitutability, the parameter must not be a silent no-op: raise ValueError("with_curve=False is not supported for ECC private keys") when with_curve is falsy, and document the parameter in the docstring.

def encode_key(self) -> bytes:
def encode_key(self, with_curve: bool = True) -> bytes:
"""
Encodes the ECC private key in an ASN sequence.

Expand All @@ -1409,7 +1409,7 @@ def encode_key(self) -> bytes:
return _ffi.buffer(key, ret)[:]

@override
def encode_key_raw(self) -> tuple[bytes, bytes, bytes]:
def encode_key_raw(self) -> tuple[bytes, bytes, bytes]: # ty: ignore[invalid-method-override]
"""
Encodes the ECC private key in its three raw elements

Expand Down Expand Up @@ -1680,7 +1680,7 @@ def decode_key(self, key: BytesOrStr, pub: bytes | None = None) -> None:
raise WolfCryptError(f"Key decode error ({self.max_signature_size})")

@override
def encode_key(self) -> tuple[bytes, bytes]:
def encode_key(self) -> tuple[bytes, bytes]: # ty: ignore[invalid-method-override]
"""
Encodes the ED25519 private key.

Expand Down Expand Up @@ -1889,7 +1889,7 @@ def decode_key(self, key: BytesOrStr, pub: bytes | None = None) -> None:
raise WolfCryptError(f"Key decode error ({self.max_signature_size})")

@override
def encode_key(self) -> tuple[bytes, bytes]:
def encode_key(self) -> tuple[bytes, bytes]: # ty: ignore[invalid-method-override]
"""
Encodes the ED448 private key.

Expand Down
6 changes: 1 addition & 5 deletions wolfcrypt/hashes.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -457,6 +457,7 @@ class _Hmac(_Hash):
digest_size = None
_native_type = "Hmac *"
_native_size = _ffi.sizeof("Hmac")
_type: int

@dgarske dgarske Aug 6, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 [Low] _type should be annotated ClassVar[int], not int

_type: int declares an instance variable, but every concrete subclass assigns it at class level -- HmacSha._type = _TYPE_SHA (line 533), HmacSha256._type = _TYPE_SHA256 (line 545), HmacSha384 (line 557), HmacSha512 (line 569) -- and wolfcrypt/hkdf.py reads it straight off the class object, never an instance: hash_cls._type at hkdf.py:70, hkdf.py:106, and hkdf.py:137, where hash_cls: type[_Hmac]. ClassVar[int] states the actual contract (a class-level constant selecting the wolfCrypt HMAC type id), makes class-object access unambiguous for checkers, and prevents a subclass from accidentally shadowing it per-instance. Worth doing since mypy is also in the dev dependency group and may treat class-object access to a plain instance annotation less permissively than ty.

Fix: Change the annotation to _type: ClassVar[int] and add ClassVar to the typing import at wolfcrypt/hashes.py:26.

_delete = staticmethod(_lib.wc_HmacFree)

@override
Expand Down Expand Up @@ -498,11 +499,6 @@ def new(cls, key: BytesOrStr, string: BytesOrStr | None = None) -> _Hash: # pyl
"""
return cls(key, string)


@property
@abstractmethod
def _type(self) -> int: ...

def _hmac_init(self, hmac: int, key: bytes) -> int:
ret = _lib.wc_HmacInit(self._native_object, _ffi.NULL, -2)

@dgarske dgarske Aug 6, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 [Medium] _Hmac is no longer abstract after removing the _type abstractmethod

The diff deletes the @property @abstractmethod def _type(self) -> int: ... declaration and replaces it with a bare class-body annotation _type: int (line 460). A bare annotation creates no attribute and, critically, is not an abstract member -- so it removes the only thing that was keeping _Hmac abstract.

Walking the ABC bookkeeping: _Hash declares _init, _update, _final, _native_size, _native_type, digest_size, and new as abstract. _Hmac supplies concrete overrides or class attributes for every one of them (digest_size = None, _native_type = "Hmac *", _native_size = _ffi.sizeof("Hmac"), _init, _update, _final, new). With _type gone from __abstractmethods__, that set is now empty and _Hmac is instantiable.

Concretely: _Hmac(b"key") previously raised TypeError: Can't instantiate abstract class _Hmac with abstract method _type before any C resource was touched. It now proceeds into __init__, allocates the native object via _ffi.new(self._native_type), and only then dies at self._hmac_init(self._type, key) (line 479) with AttributeError: '_Hmac' object has no attribute '_type'. The same loss of enforcement applies to any future HMAC variant added to this file: forgetting _type = _TYPE_... used to be a class-instantiation-time error and is now a late AttributeError from inside __init__. _Hmac is a documented extension point (docs/mac.rst:25 has .. autoclass:: _Hmac), so this matters beyond internal use.

Fix: Restore an equivalent guard so a missing _type still fails early and legibly. Either add an __init_subclass__ check as shown, or keep _Hmac explicitly abstract (e.g. leave a trivial abstract member) so _Hmac itself cannot be constructed. At minimum, guard __init__ with an explicit check that produces a clear message instead of a raw AttributeError.

if ret < 0:
Expand Down
2 changes: 1 addition & 1 deletion wolfcrypt/hkdf.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@

if TYPE_CHECKING:
if _lib.HMAC_ENABLED:
from wolfcrypt.hashes import _Hmac
from wolfcrypt.hashes import _Hmac # ty: ignore[possibly-missing-import]


if _lib.HKDF_ENABLED:
Expand Down