[ty] Reachability analysis for generic function calls by sharkdp · Pull Request #23419 · astral-sh/ruff (original) (raw)
added the ty
Multi-file analysis & type inference
label
sharkdp marked this pull request as ready for review
sharkdp deleted the david/generic-fn-returns-never branch
knutwannheden pushed a commit to openrewrite/ruff that referenced this pull request
Summary
If a generic function's return type depends on a type variable, and the
argument passed resolves that type variable to Never, the call should
still be treated as terminal:
def identity[T](x: T) -> T:
return x
def f() -> Never:
identity(exit()) # should be detected as terminalThis is a tiny win for correctness, but unfortunately a small drop in performance and slight increase in memory usage, because we need to infer more call expressions upfront. If we think it's not important enough, I'm also okay to change this to a test-only PR that documents this as a known limitation. If we merge it, I might follow up with an idea to simplify the code in a slightly larger refactoring PR.
Memory usage
Insignificant changes on some large internal projects, a 2% decrease in memory usage when running on home-assistant/core.
Ecosystem
No ecosystem changes, as far as I can tell.
Test Plan
New Markdown tests
AlexWaygood added a commit that referenced this pull request
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!
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 xThe 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
KotlinIsland pushed a commit to KotlinIsland/basedpython that referenced this pull request
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
astral-sh/ruff#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!
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 xThe 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
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters
[ Show hidden characters]({{ revealButtonHref }})