fix: avoid variable-shadowing collision in model from_dict - #1485
Open
AndersBennedsgaard wants to merge 1 commit into
Open
fix: avoid variable-shadowing collision in model from_dict#1485AndersBennedsgaard wants to merge 1 commit into
AndersBennedsgaard wants to merge 1 commit into
Conversation
…enerators#1117) `from_dict` used to bind the constructed instance to a local variable named after the class's snake_case module name (e.g. `email = cls(...)`). If any property snake_cased to that same name, the earlier `email = d.pop("email")` assignment got reassigned to a different type, which breaks static type checkers like mypy/pyright even though the generated code works fine at runtime. Instead of picking a "hopefully unique" temporary name, `from_dict` now always returns `cls(...)` directly with no intermediate variable, so the collision is structurally impossible rather than merely unlikely. This requires `additional_properties` to be settable via the constructor, so `init=False` was dropped from that attrs field; as a side effect, `additional_properties` is now also usable with `attrs.evolve()` and the constructor directly. Golden records and integration-tests fixtures regenerated accordingly. Added `ModelWithSelfNamedProperty` to the baseline specs as a regression test: a property whose snake_case name collides with its class's module name, verified via the existing `mypy --strict` check in the e2e suite.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #1117
from_dictused to bind the constructed instance to a local variable named after the class's snake_case module name (e.g.email = cls(...)). If any property snake_cased to that same name, the earlieremail = d.pop("email")assignment got reassigned to a different type, which breaks static type checkers like mypy/pyright even though the generated code works fine at runtime.Instead of picking a "hopefully unique" temporary name,
from_dictnow always returnscls(...)directly with no intermediate variable, so the collision is structurally impossible rather than merely unlikely. This requiresadditional_propertiesto be settable via the constructor, soinit=Falsewas dropped from that attrs field; as a side effect,additional_propertiesis now also usable withattrs.evolve()and the constructor directly.Golden records and integration-tests fixtures regenerated accordingly. Added
ModelWithSelfNamedPropertyto the baseline specs as a regression test: a property whose snake_case name collides with its class's module name, verified via the existingmypy --strictcheck in the e2e suite.pdm run checkrun succesfully.Full disclosure: I used Claude Sonnet to help with the development. Feel free to point our any AI slop, happy to fix it