-
-
Notifications
You must be signed in to change notification settings - Fork 2.8k
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
Add error code for mutable covariant override #16399
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.
We should accept:
class X:
x: float
class Y(X):
x = 5
When I made this same patch I did this by only doing the check if lvalue_type
This comment has been minimized.
This comment has been minimized.
Also #3208 (comment) is similar hole, might be worth opening separate issue for it |
Why should we allow this?
You may be tempted to change these rules, but believe me they are very old, and may break some unexpected things (especially in the daemon). Or did you mean we should allow
Yes, this is a separate story, and there are problems not just for variables, but for methods as well. |
According to mypy_primer, this change doesn't affect type check results on a corpus of open source code. ✅ |
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.
Ah, I didn't realise x = 5
and self.x = 5
were different. Thanks for adding the test!
Then yes, this looks good to me! Note that if we ever want to turn this code on by default, we should consider the change... IIRC fixed several mypy_primer hits when I tried out my diff
Fixes #3208
Interestingly, we already prohibit this when the override is a mutable property (goes through
FuncDef
-related code), and in multiple inheritance. The logic there is not very principled, but I just left a TODO instead of extending the scope of this PR.