Fix conditional_types remainder for structural types #20450
Conversation
| proposed_type = make_simplified_union(proposed_items) | ||
|
|
||
| if isinstance(proper_type, AnyType): | ||
| return proposed_type, current_type |
There was a problem hiding this comment.
By the way, would it be technically more correct to return AnyType(TypeOfAny.from_another_any, source_any=proper_type) for the else branch, rather than proper_type itself?
There was a problem hiding this comment.
Not sure, I've lost track of what all TypeOfAny is used for. My instinct is that it is correct as is, e.g. if you had a ternary expression that you're narrowing, feels like you might want to preserve the original Any
| return default, UninhabitedType() | ||
| elif not is_overlapping_types(current_type, proposed_type, ignore_promotions=True): | ||
| return default, remainder | ||
| if not is_overlapping_types(current_type, proposed_type, ignore_promotions=True): |
There was a problem hiding this comment.
Converted the elif to if because the if not any(type_range.is_upper_bound for type_range in proposed_type_ranges): is not guaranteed to return anymore.
| ) | ||
| ): | ||
| # Expression is always of one of the types in proposed_type_ranges | ||
| return default, UninhabitedType() |
There was a problem hiding this comment.
This return was problematic in the structural case when current_type=Any | Proto, restricting Proto away should leave us with Any and not Never.
|
According to mypy_primer, this change doesn't affect type check results on a corpus of open source code. ✅ |
hauntsaninja
left a comment
hauntsaninja
left a comment
There was a problem hiding this comment.
This is great, thank you for the quick fix and for the original PR :-)
hauntsaninja
changed the title
2 participants
Fixes #20445
Fixes a regression in
conditional_typewhen given a union ofAny | Protocol