Fix narrowing with chained comparison#21150
Conversation
This comment has been minimized.
This comment has been minimized.
| def f1(a: str | None, b: str | None) -> None: | ||
| if None is not a == b: | ||
| reveal_type(a) # N: Revealed type is "builtins.str" | ||
| reveal_type(b) # N: Revealed type is "builtins.str | None" |
There was a problem hiding this comment.
This should also be str. I will fix this in a subsequent PR (this one is not a regression, it will need net new code)
| # TODO: support regular and len() narrowing in the same chain. | ||
| if any(m != ({}, {}) for m in partial_type_maps): | ||
| return reduce_conditional_maps(partial_type_maps) | ||
| if any(len(m[0]) or len(m[1]) for m in partial_type_maps): |
There was a problem hiding this comment.
TBH I don't understand how is this different from what it was before. Is this just some drive-by performance micro-optimization?
There was a problem hiding this comment.
Yeah, just a micro optimisation
| if any(m != ({}, {}) for m in partial_type_maps): | ||
| return reduce_conditional_maps(partial_type_maps) | ||
| if any(len(m[0]) or len(m[1]) for m in partial_type_maps): | ||
| return reduce_conditional_maps(partial_type_maps, use_meet=True) |
There was a problem hiding this comment.
I am in favour of this, but just as a warning, this may have some subtle side-effect w.r.t. Any, so you may want to add some tests involving Any as well.
There was a problem hiding this comment.
Added some tests, but couldn't find one with a behaviour change. If there is one, I think it should result in fewer Any's, and we don't see anything on primer, so that's good
|
According to mypy_primer, this change doesn't affect type check results on a corpus of open source code. ✅ |
2 participants
Fixes #21149 (the regression part of it, still working on the bad behaviour present in previous mypy versions)