-
-
Notifications
You must be signed in to change notification settings - Fork 3k
Report incompatible assignments for descriptors with overloaded __set__ methods. #9893
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
Conversation
…__ methods (fixes python#7205). An assertion expected the second result of method `ExpressionChecker.check_call` to be a `CallableType`, but it is `AnyType` in the situation causing the crash. Hence, this commit replaces this assertion by handling both types differently. This involves modifying the second invocation of method `ExpressionChecker.check_call` a little to make sure it reports the invalid assignment in both cases. The new test case `testSettingDescriptorWithOverloadedDunderSet1` deals with overloading the value argument only and `testSettingDescriptorWithOverloadedDunderSet2` deals with overloading both the instance and the value argument.
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.
This fixes a pretty bad crash. Looks good -- thanks for the PR!
Just left a few minor comments, otherwise this is ready to merge.
test-data/unit/check-classes.test
Outdated
a = A() | ||
a.f = '' | ||
a.f = 1 | ||
a.f = 1.5 |
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.
Can you add a # E
comment here as a hint to the reader about where the error happens, like this:
a.f = 1.5 # E
(This is only a comment and has no semantic significance.)
test-data/unit/check-classes.test
Outdated
b = B() | ||
a.f = '' | ||
b.f = 1 | ||
a.f = 1 |
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.
Add # E
comments to the lines that produce errors, similar to above.
mypy/checker.py
Outdated
# And now we type check the call second time, to show errors related | ||
# to wrong arguments count, etc. | ||
# And now we in fact type check the call, to show errors related to wrong arguments | ||
# count, etc., replacing the type context von non-overloaded setters only. |
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.
Typo: 'von'
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.
Glad I can help. Thanks for your remarks. I have adopted them as suggested.
Glad I can help. Thanks for your remarks. I have adopted them as suggested. |
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!
Fixes #7205
Hello everyone,
this is my first contribution to mypy, I hope to meet your style well enough.
I added two test cases which cause mypy to crash without the provided changes. At least on my machine, the changes do not affect any other tests.
For short explanations, please see the commit message and the (modifications of the) inline comments.