feat: add mypy plugin for type-checking structs.replace kwargs#992
feat: add mypy plugin for type-checking structs.replace kwargs#992lukasK9999 wants to merge 1 commit intojcrist:mainfrom
Conversation
Add `msgspec.mypy` plugin that validates keyword arguments passed to
`msgspec.structs.replace()`. Without the plugin, mypy accepts any kwargs
due to the `**changes: Any` signature in the stubs.
The plugin hooks into `get_function_signature_hook` and generates a
call-site-specific signature with the struct's fields as optional keyword
arguments, following the same approach used by mypy's built-in attrs
`evolve()` plugin.
Supports: plain structs, frozen structs, generics, unions of structs,
TypeVar bounds, and Any passthrough.
Usage:
# mypy.ini or pyproject.toml
[mypy]
plugins = [msgspec.mypy]
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
Mypy support for replace is something I've been missing when using msgspec. We use frozen structs heavily, and without this we usually default to attrs, which has this built in via its mypy plugin. The implementation closely follows mypy's existing attrs evolve() plugin — the core algorithm is essentially the same (get the init signature, extract field names/types, build a typed CallableType), with the only real difference being struct detection via MRO lookup instead of the attrs_attrs marker. It's AI-generated, but I've tested it against all the cases I could think of: plain/frozen structs, generics, unions, |
|
Given that the attrs and dataclass mypy plugin are part of mypy, would it make sense to have this there too ? If you closely copied attrs implementation, there is maybe even room to share common parts ? |
|
attrs is somewhat exceptional historically. It predates dataclasses, and dataclasses were directly inspired by attrs, so mypy’s built-in support for both evolved together around very similar semantics. Since dataclasses are stdlib, core support in mypy is necessary; attrs then remained as an established special case. For newer third-party libraries, the more common pattern is to ship plugin support in the library itself (e.g. Pydantic), which is why keeping msgspec support in msgspec currently seems closer to current practice. |
Siyet
left a comment
There was a problem hiding this comment.
Thanks for the PR. I'll lay out the historical context first, then walk through code-level concerns.
Context. #19 was opened by @jcrist in 2022 asking for a mypy plugin and was closed as completed in 2023 once dataclass_transform (PEP 681) landed. msgspec already uses it on Struct (src/msgspec/__init__.pyi:88), so constructor calls (Point("oops", "bad")) are already validated without a custom plugin.
What dataclass_transform does not cover is msgspec.structs.replace, because PEP 681 only describes class constructors, not arbitrary helper functions. The current stub (src/msgspec/structs.pyi:7) accepts **changes: Any, so mypy can't catch typos or wrong types in replace() calls today. This PR closes exactly that gap.
Re. @UnknownPlatypus's question about upstreaming into mypy or sharing with attrs: the author's reply is correct. mypy only ships plugins for stdlib (dataclasses) and attrs as a historical exception. Newer libraries (e.g. Pydantic) ship plugins in-tree, which is what this PR does. Sharing code with attrs is not practical because the attrs evolve plugin lives inside mypy core (mypy/plugins/attrs.py), not as a reusable library. Functionally this PR is a port of evolve_function_sig_callback adapted for msgspec, which is fine.
Code-level review:
-
Documentation missing. Per the contributing guidelines, features need docs in the same PR. Add a section near
structs.replace(or a dedicated "Type checking" page) showing how to enable the plugin and what it catches. Without this, the feature is invisible to users. -
format_type_bareimport points to the wrong place. The PR has:try: from mypy.typeops import format_type_bare except ImportError: from mypy.checker import format_type_bare
On current mypy (1.20) the primary import raises
ImportErrorbecauseformat_type_baredoes not live inmypy.typeops. The fallback frommypy.checkersucceeds, so the code works, but that's accidental. The canonical location ismypy.messages, which is also where the attrs evolve plugin imports it from:from mypy.messages import format_type_bare. Replace the try/except with a singlefrom mypy.messages import format_type_bare. Simpler, and matches mypy core's own usage. -
Minimum mypy version is not declared. Pin it, either via a new optional extra:
[project.optional-dependencies] mypy = ["mypy>=X.Y"]
or document it in the plugin docstring. CI should also test against that minimum so internal API drift gets caught here, not at users.
-
Test runtime cost. 18 tests, each spinning up a fresh
mypy_api.runwith a tempdir and--no-incremental, will add notable seconds to CI. The existingtests/typing/test_mypy.pybatches all type checks into a single mypy invocation againstbasic_typing_examples.pyusing# revealed typecomments. Consider following the same pattern, or at least batch all "should pass" cases into one file and all "should fail" cases into another. Two mypy invocations beats 18. -
Test gap:
Optional[Struct]. Add a case forreplace(maybe_a, x=1)where the input isA | None.relevant_items()will filter outNone, so it should behave the same as the bare-Acase, but it's worth pinning down in a test, otherwise a future regression inrelevant_items()would slip through. -
Long messages in
_fail_not_struct. The ternaries inside parenthesized concatenations of f-strings are hard to scan. Split them into a clean if/else with named string templates. Easier to read and survives future tweaks. -
Scope of the plugin in the docstring. The current docstring says "Provides type-checking support for
msgspec.structs.replace", which can be read as "among other things". State explicitly that the plugin only hooksmsgspec.structs.replaceand is not intended to grow to cover other functions. Sets the right expectations and prevents follow-up "can you also handle X?" issues.
Positive notes:
- Approach is correct:
get_function_signature_hookis the minimal-impact way to do this. - Union handling via
meet_typescorrectly enforces "only fields common to all variants", which matches what's safe at type-check time. Thetest_replace_union_incompatible_fieldtest pins this. - TypeVar bound handling via recursive descent into
upper_boundis idiomatic. expand_type_by_instanceis the right primitive for substituting generic params.- Plugin structure (
_replace_sig_callback,_get_expanded_struct_types,_meet_fields,_fail_not_struct) mirrors the attrs evolve plugin almost 1-to-1, which is good for maintainability and review. - Test coverage is conceptually broad (plain, frozen, generic, union, typevar, any, function-return).
Next steps. I don't rule out that we may end up rejecting this PR. But I'd suggest first cleaning it up against the points above (at minimum docs, the import fix, min mypy version, scope in docstring), and only then pinging the project maintainer and one of the msgspec contributors who works on typing in CPython for the final call on whether to accept. Not tagging them yet to avoid distracting them prematurely.
|
Since structs support |
Summary
msgspec.mypyplugin that validates keyword arguments passed tomsgspec.structs.replace()**changes: Anysignature in the stubs — including invalid field names and wrong typesevolve()plugin: hooks intoget_function_signature_hookand generates a call-site-specific signature with the struct's fields as optional keyword argumentsUsage
What it catches
Supported cases
Struct[T])Test plan
tests/typing/test_mypy_plugin.py— all passingtests/typing/test_mypy.pystill passes (no regression)🤖 Generated with Claude Code