fix report unsafe operator type on subclasses #1361
Conversation
| def __radd__(self, other) -> Self: | ||
| return self | ||
| assert_type(A() + B(), A) # E: `A.__add__` is deprecated | ||
| assert_type(A() + B(), A | B) # E: `A.__add__` is deprecated |
There was a problem hiding this comment.
does this A | B imply we choose A | B in this example?
is it worth adding a test for that, and should it be just A?
There was a problem hiding this comment.
The current behavior for these reflected operators is:
- Try the "forward" operator, if that works, use the return type from that
- If the forward operator fails, try the reflected operator, using it's return type if it succeeds
- If both the forward and reflected operators fail, error and use the return type from the forward operator
is this level of strictness practical? might be worth a discussion
There was a problem hiding this comment.
thanks for the contribution! we tested this change against instagram and it introduced a lot of unrelated type errors. not entirely sure we want to merge it just yet. it might be worth discussing it more at an office hours or on discord if you're still interested in this issue
| ]; | ||
| self.try_binop_calls(&calls_to_try, range, errors, &context) | ||
| let class_type_of = |ty: &Type| -> Option<ClassType> { | ||
| match ty { |
There was a problem hiding this comment.
mapping over type like this is almost never what we want: it's hard to maintain and prone to errors
| def __radd__(self, other) -> Self: | ||
| return self | ||
| assert_type(A() + B(), A) # E: `A.__add__` is deprecated | ||
| assert_type(A() + B(), A | B) # E: `A.__add__` is deprecated |
There was a problem hiding this comment.
The current behavior for these reflected operators is:
- Try the "forward" operator, if that works, use the return type from that
- If the forward operator fails, try the reflected operator, using it's return type if it succeeds
- If both the forward and reflected operators fail, error and use the return type from the forward operator
is this level of strictness practical? might be worth a discussion
yangdanny97
left a comment
yangdanny97
left a comment
There was a problem hiding this comment.
Review automatically exported from Phabricator review in Meta.
4 participants
fix #1176