From eb2773f0bbb94d1031e092a0efbae7f7a3760c2b Mon Sep 17 00:00:00 2001 From: Latiff Danieyal Date: Fri, 24 Jul 2026 15:41:54 +0800 Subject: [PATCH] chore: pyright strict mode Step 3 of the strict-mode plan. Flips typeCheckingMode to strict with ten whole-rule exceptions, each disabling a category that is pure noise for this codebase rather than a suppression of real findings: - reportUnknown{Variable,Argument,Member,Parameter,Lambda}Type: lxml-stubs and Pydantic internals surface many values as Unknown (~90 hits), nothing actionable behind them. - reportPrivateUsage: the UBL models share a deliberate cross-module internal API (_leaf, _money, _UblModel, _Base). By design. - reportUnnecessaryIsInstance: Pydantic mode="before" validators genuinely receive Any at runtime, so their isinstance checks are load-bearing. - reportMissingParameterType: annotating every test helper/lambda is noise; src is covered by mypy --strict anyway. With those off, strict surfaced 8 real findings, fixed individually rather than blanket-suppressed: - services/models.py: removed _as_items, genuinely dead -- defined, never called anywhere. A real find strict earned. - ubl/_base.py: added __all__ = ["_UblModel", "_leaf", "_money"]. These are used across sibling modules; the reportUnusedFunction/Class flags were a private-name heuristic not seeing cross-module imports. __all__ marks them exported and is more honest than a suppression. - scripts/extract_codes.py: TABLES typed as dict[str, _TableSpec] (a TypedDict). Previously spec["out"] inferred as str | list[tuple], which the `OUT / spec["out"]` path join rejected. The TypedDict gives each key its own type. - tests/unit/test_auth.py: annotated a local dict[str, object]; an inferred dict[str, str | int] is not assignable there because dict value types are invariant. - ubl/invoice.py, ubl/reference.py: the two `is not None` serializer guards deferred from #20. Statically dead, but they protect model_construct()-built models (which bypass validation) from raising during serialization. Suppressed per-line with reportUnnecessaryComparison and the rationale in a comment. Verified load-bearing: removing both ignores brings the 2 errors straight back. pyright now runs strict in CI (the config drives it) and reports 0. All gates green: ruff, ruff format, mypy, pyright, 405 tests, lock in sync. Co-Authored-By: Claude Opus 4.8 (1M context) --- pyrightconfig.json | 30 ++++++++++++++++++++++++++++-- scripts/extract_codes.py | 12 +++++++++++- src/myinvois/services/models.py | 10 ---------- src/myinvois/ubl/_base.py | 6 ++++++ src/myinvois/ubl/invoice.py | 6 +++++- src/myinvois/ubl/reference.py | 4 +++- tests/unit/test_auth.py | 5 ++++- 7 files changed, 57 insertions(+), 16 deletions(-) diff --git a/pyrightconfig.json b/pyrightconfig.json index b826e8b..7240e21 100644 --- a/pyrightconfig.json +++ b/pyrightconfig.json @@ -10,6 +10,32 @@ // repeated here rather than silently dropped. "exclude": ["**/node_modules", "**/.*", "**/__pycache__", ".venv", "dist"], "pythonVersion": "3.11", - "typeCheckingMode": "standard", - "reportMissingTypeStubs": false + + // Strict, minus the rules that are pure noise for this codebase's idioms. + // Each exception is a whole category we would otherwise suppress inline + // dozens of times, which hides real findings rather than surfacing them. + "typeCheckingMode": "strict", + + "reportMissingTypeStubs": false, + + // lxml-stubs and Pydantic's internals expose many values as Unknown. These + // fire ~90 times across the models and builders with nothing actionable + // behind them; turning them off keeps strict's genuine checks visible. + "reportUnknownVariableType": false, + "reportUnknownArgumentType": false, + "reportUnknownMemberType": false, + "reportUnknownParameterType": false, + "reportUnknownLambdaType": false, + + // The UBL models share a deliberate cross-module internal API (`_leaf`, + // `_money`, `_UblModel`, `_Base`). That is by design, not a leak. + "reportPrivateUsage": false, + + // Pydantic `mode="before"` validators genuinely receive `Any` at runtime, + // so the isinstance/type checks they perform are necessary, not redundant. + "reportUnnecessaryIsInstance": false, + + // Requiring an explicit annotation on every test helper and lambda is noise + // for a test suite; src is covered by mypy --strict regardless. + "reportMissingParameterType": false } diff --git a/scripts/extract_codes.py b/scripts/extract_codes.py index c64934d..1677e4d 100644 --- a/scripts/extract_codes.py +++ b/scripts/extract_codes.py @@ -10,13 +10,23 @@ import json import re from pathlib import Path +from typing import TypedDict + + +class _TableSpec(TypedDict): + out: str + columns: list[tuple[str, str]] + PHPSDK = Path("/tmp/phpsdk/src/Ubl/Constant") OUT = Path(__file__).resolve().parents[1] / "src" / "myinvois" / "codes" / "_data" OUT.mkdir(parents=True, exist_ok=True) # Map: PHP class => {output_filename, columns[(php_const, json_key)]} -TABLES = { +#: Per-table extraction spec. A TypedDict so ``spec["out"]`` types as ``str`` +#: and ``spec["columns"]`` as the column list, rather than unifying to a union +#: that neither ``Path / spec["out"]`` nor the column loop can use. +TABLES: dict[str, _TableSpec] = { "StateCodes": { "out": "states.json", "columns": [("CODE", "code"), ("STATE", "name")], diff --git a/src/myinvois/services/models.py b/src/myinvois/services/models.py index c99a65d..58ec97c 100644 --- a/src/myinvois/services/models.py +++ b/src/myinvois/services/models.py @@ -75,16 +75,6 @@ class DocumentTypeList(_Base): result: list[DocumentType] -# A small helper used by services to wrap raw dicts as models where the LHDN -# response shape is a plain list (no `result` wrapper). -def _as_items(raw: Any, *, model: type[BaseModel], key: str = "result") -> list[BaseModel]: - if isinstance(raw, dict) and key in raw and isinstance(raw[key], list): - return [model.model_validate(item) for item in raw[key]] - if isinstance(raw, list): - return [model.model_validate(item) for item in raw] - return [] - - # --------------------------------------------------------------------------- # Phase 5 — Submit documents + Get submission response models # Spec: https://sdk.myinvois.hasil.gov.my/einvoicingapi/02-submit-documents/ diff --git a/src/myinvois/ubl/_base.py b/src/myinvois/ubl/_base.py index 0fcc799..102479e 100644 --- a/src/myinvois/ubl/_base.py +++ b/src/myinvois/ubl/_base.py @@ -18,6 +18,12 @@ from pydantic import BaseModel, ConfigDict +# These are the module's internal API, imported by the sibling model modules +# (address.py, party.py, tax.py, …) rather than used within this file. Naming +# them in __all__ marks them as exported so a "used only via import" checker +# does not read them as dead. +__all__ = ["_UblModel", "_leaf", "_money"] + class _UblModel(BaseModel): """Base for every UBL structural model. diff --git a/src/myinvois/ubl/invoice.py b/src/myinvois/ubl/invoice.py index e49ae98..fde4e1f 100644 --- a/src/myinvois/ubl/invoice.py +++ b/src/myinvois/ubl/invoice.py @@ -235,7 +235,11 @@ def _dl_list(seq: list[Any]) -> list[dict[str, Any]]: "IssueTime": _leaf(self.issue_date_time.strftime("%H:%M:%SZ")), } - if self.invoice_type_code is not None: + # Statically always-true (the field is required), but kept as a guard + # for models built via `model_construct()`, which bypasses validation + # and can leave a required field as None. Serializing such a model + # should skip the key, not raise. + if self.invoice_type_code is not None: # pyright: ignore[reportUnnecessaryComparison] tc_val = ( self.invoice_type_code.value if isinstance(self.invoice_type_code, DocumentTypeCode) diff --git a/src/myinvois/ubl/reference.py b/src/myinvois/ubl/reference.py index c006136..9956f44 100644 --- a/src/myinvois/ubl/reference.py +++ b/src/myinvois/ubl/reference.py @@ -148,7 +148,9 @@ def _must_have_id(self) -> OrderReference: @model_serializer def _ser(self) -> dict[str, Any]: out: dict[str, Any] = {} - if self.id is not None: + # Statically always-true (id is required), but kept as a guard for + # models built via `model_construct()`, which bypasses validation. + if self.id is not None: # pyright: ignore[reportUnnecessaryComparison] out["ID"] = _leaf(self.id) if self.sales_order_id is not None: out["SalesOrderID"] = _leaf(self.sales_order_id) diff --git a/tests/unit/test_auth.py b/tests/unit/test_auth.py index 584cebc..944438e 100644 --- a/tests/unit/test_auth.py +++ b/tests/unit/test_auth.py @@ -56,7 +56,10 @@ def test_oauth2_token_from_response() -> None: def test_oauth2_token_defaults_scheme_to_bearer() -> None: - resp = {"access_token": "X", "expires_in": 60} # no token_type + # Annotated `dict[str, object]`: an inferred `dict[str, str | int]` is not + # assignable to `from_response`'s `dict[str, object]` parameter, because + # dict value types are invariant. + resp: dict[str, object] = {"access_token": "X", "expires_in": 60} # no token_type assert OAuth2Token.from_response(resp, fetched_at=0.0).token_type == "Bearer"