Skip to content

Fix: json5/lib.py's load() function declared fp: IO, the bare... - #115

Open
M001N wants to merge 1 commit into
dpranke:mainfrom
M001N:oss-engine/743f358f-e3e616ac
Open

Fix: json5/lib.py's load() function declared fp: IO, the bare...#115
M001N wants to merge 1 commit into
dpranke:mainfrom
M001N:oss-engine/743f358f-e3e616ac

Conversation

@M001N

@M001N M001N commented Aug 17, 2026

Copy link
Copy Markdown

Summary

Changed json5/lib.py: load()'s fp: IO -> fp: Union[IO[str], IO[bytes]] (load() genuinely supports both text- and binary-mode file objects, confirmed by the BytesIO test). dump()'s fp: IO -> fp: IO[str] (dump() only ever writes str, via fp.write(dumps(...))). loads()'s and parse()'s s: str -> s: Union[str, bytes] to make their signatures consistent with their existing documented/tested bytes support, and to keep python -m mypy clean given load()'s now-typed fp.read() result flows into parse().

Problem

dpranke/pyjson5 issue reference: #92

Root Cause

json5/lib.py's load() function declared fp: IO, the bare unparameterized typing.IO generic, which strict type checkers (pyright, and mypy in stricter modes) flag as an incomplete/unknown type. The bare IO (not IO[str]) triggers reportUnknownMemberType-style diagnostics in downstream code calling json5.load(f). dump() had the identical bare IO pattern. Additionally, once fp is properly parameterized, fp.read() becomes a concretely-typed str/bytes value instead of Any, which surfaced a pre-existing latent inconsistency: loads()/parse() are typed as accepting only s: str even though they already handle bytes at runtime (via an isinstance(s, bytes) decode step) — confirmed by tests/lib_test.py's test_encoding, which calls both json5.loads(b'...') and json5.load(io.BytesIO(...)).

Testing

PASS - python -m mypy json5 reports no issues (was previously not exercising this path meaningfully since bare IO/str were effectively Any-typed); full test suite (75 tests across tests/) passes unchanged, confirming zero behavioral impact.

Related Issue

#92

- load()'s fp parameter is now typed Union[IO[str], IO[bytes]] instead
  of the bare, unparameterized IO, since load() supports both text-mode
  and binary-mode file objects (confirmed by tests/lib_test.py's
  test_encoding, which passes an io.BytesIO to json5.load()).
- dump()'s fp parameter is typed IO[str] since it only ever writes str.
- loads()/parse()'s s parameter is now typed Union[str, bytes] to match
  their existing runtime support for byte strings (they already decode
  bytes via an isinstance check), fixing a latent inconsistency that
  the load() fix would otherwise expose as a new mypy error.

Fixes reportUnknownMemberType/partially-unknown-type complaints from
pyright/mypy when calling json5.load(f). Type-annotation-only change;
verified zero behavioral impact via full test suite and `python -m
mypy json5`.
@dpranke

dpranke commented Aug 18, 2026

Copy link
Copy Markdown
Owner

Seems reasonable, thanks for the fix!

Question, though: I notice that the typeshed's hints for the library are actually a bit different. Do you think we should consider something closer to those?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants