[ty] Improve isinstance() reachability analysis by AlexWaygood · Pull Request #24077 · astral-sh/ruff (original) (raw)
Summary
Our reachability analysis for isinstance() constraints currently doesn't match the sophistication of our type narrowing in this situation. This was obscured in our tests by the fact that assert_never(), reveal_type and assert_type calls can in fact create their own reachability constraints!
assert_type, reveal_type and assert_never are all generic functions that return an object of the same type as the object passed into the function. Following #23419, that means that if you pass an object inferred as having the type Never into one of these functions, all code following that call will be inferred as being unreachable. In other words, this test was passing entirely by accident, because of additional reachability constraints introduced by the reveal-type and assert_never calls!
| def h[T: int | str](x: T) -> T: |
|---|
| if isinstance(x, int): |
| return x |
| elif isinstance(x, str): |
| return x |
| else: |
| reveal_type(x) # revealed: Never |
| assert_never(x) |
Remove the else branch from that test, and you get a false-positive invalid-return-type diagnostic complaining that the function can implicitly return None -- which is clearly not the case:
def h[T: int | str](x: T) -> T: if isinstance(x, int): return x elif isinstance(x, str): return x
The fix is to adapt our isinstance() special casing so that it has the same sophistication as our type narrowing machinery here, where we understand that T & ~U & ~S must resolve to Never if T is a type variable bound to the union S | U. In a similar way, an else branch (explicit or implicit) can never be taken in either of the h functions above.
Test Plan
Added mdtests that fail on main