-
-
Notifications
You must be signed in to change notification settings - Fork 3k
Fix crash when a dataclass with a no init InitVar is inherited #7390
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.
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
Fix crash when a dataclass with a no init InitVar is inherited #7390
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for fixing the crash! Looks good (just a few nits) but it would be nice to have a test case. Test cases for dataclasses live in test-data/unit/check-dataclasses.test
.
mypy/plugins/dataclasses.py
Outdated
if info[attr.name].type is None: | ||
try: | ||
node = info[attr.name] | ||
except KeyError: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that it would be slightly cleaner if you'd use a if info.get(attr.name) is None
check instead of try/except here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed.
mypy/plugins/dataclasses.py
Outdated
del info.names[attr.name] | ||
try: | ||
del info.names[attr.name] | ||
except KeyError: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Similar to above, I'd slightly prefer an if statement instead of try/except here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed.
Thanks for the review! I updated the code and added a test. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the updates! Looks good now.
Fixes #7320.