Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 40 additions & 1 deletion crates/ty_python_semantic/resources/mdtest/narrow/match.md
Original file line number Diff line number Diff line change
Expand Up @@ -252,7 +252,7 @@ def _(x: Literal["foo", b"bar"] | int):
pass
case b"bar" if reveal_type(x): # revealed: Literal[b"bar"] | int
pass
case _ if reveal_type(x): # revealed: Literal["foo", b"bar"] | int
case _ if reveal_type(x): # revealed: int | Literal["foo", b"bar"]
pass
```

Expand Down Expand Up @@ -350,6 +350,45 @@ except ValueError:
pass
```

## Narrowing is preserved when a terminal branch prevents a path from flowing through

When one branch of a `match` statement is terminal (e.g. contains `raise`), narrowing from the
non-terminal branches is preserved after the merge point.

```py
class A: ...
class B: ...
class C: ...

def _(x: A | B | C):
match x:
case A():
pass
case B():
pass
case _:
raise ValueError()

reveal_type(x) # revealed: B | (A & ~B)
```

Reassignment in non-terminal branches is also preserved when the default branch is terminal:

```py
def _(number_of_periods: int | None, interval: str):
match interval:
case "monthly":
if number_of_periods is None:
number_of_periods = 1
case "daily":
if number_of_periods is None:
number_of_periods = 30
case _:
raise ValueError("unsupported interval")

reveal_type(number_of_periods) # revealed: int
```

## Narrowing tagged unions of tuples

Narrow unions of tuples based on literal tag elements in `match` statements:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,3 +49,196 @@ def _(x: int | None):

reveal_type(x) # revealed: int
```

## Narrowing is preserved when a terminal branch prevents a path from flowing through

When one branch of an if/elif/else is terminal (e.g. contains `return`), narrowing from the
non-terminal branches is preserved after the merge point.

```py
class A: ...
class B: ...
class C: ...

def _(x: A | B | C):
if isinstance(x, A):
pass
elif isinstance(x, B):
pass
else:
return

# Only the if-branch (A) and elif-branch (B) flow through.
# The else-branch returned, so its narrowing doesn't participate.
reveal_type(x) # revealed: B | (A & ~B)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm a bit confused about why this isn't A | (B & ~A). If we flow through the if we should just get A, if we flow through the elif we have B & ~A -- adding reveal_type(x) inside the branches above confirms. So it's not clear to me why we don't end up with the union of those here.

I guess those are equivalent types (and also equivalent to just A | B?) -- so maybe this is an acceptable (if slightly surprising) transformation that happens due to the TDD implementation?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess those are equivalent types (and also equivalent to just A | B?) -- so maybe this is an acceptable (if slightly surprising) transformation that happens due to the TDD implementation?

Yes, the root cause of this is the reversed ordering of TDD nodes that we introduced in #20098.

This didn't have any observable side effects for reachability constraints (which evaluate to a ternary answer), but it does play a role for narrowing constraints.

Removing that .reverse() changes the result here to A | (B & ~A). But it also breaks ~20 other tests. Some of them are just ordering related, but in some other cases, some union-builder simplifications do not seem to trigger which is a bit concerning. I have not investigated more closely, as we probably still need that optimization.

```

## Narrowing is preserved with multiple terminal branches

```py
class A: ...
class B: ...
class C: ...
class D: ...

def _(x: A | B | C | D):
if isinstance(x, A):
return
elif isinstance(x, B):
pass
elif isinstance(x, C):
pass
else:
return

# Only the elif-B and elif-C branches flow through.
reveal_type(x) # revealed: (C & ~A) | (B & ~A & ~C)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similarly here I'd naively expect (B & ~A) | (C & ~A & ~B) -- but they are equivalent. I'm just curious why it happens.

```

## Multiple sequential if-statements don't leak narrowing

After a complete if/else where both branches flow through (no terminal), narrowing should be
cancelled out at the merge point.

```py
class A: ...
class B: ...
class C: ...

def _(x: A | B | C):
if isinstance(x, A):
pass
else:
pass

# Narrowing cancels out: both paths flow, so type is unchanged.
reveal_type(x) # revealed: A | B | C

if isinstance(x, B):
pass
else:
pass

# Second if-statement's narrowing also cancels out.
reveal_type(x) # revealed: A | B | C
```

## Narrowing after a `NoReturn` call in one branch

When a branch calls a function that returns `NoReturn`/`Never`, we know that branch terminates and
doesn't contribute to the type after the if statement.

```py
import sys

def _(val: int | None):
if val is None:
sys.exit()
reveal_type(val) # revealed: int
```

This also works when the `NoReturn` function is called in the else branch:

```py
import sys

def _(val: int | None):
if val is not None:
pass
else:
sys.exit()
reveal_type(val) # revealed: int
```

And for elif branches:

```py
import sys

def _(val: int | str | None):
if val is None:
sys.exit()
elif isinstance(val, int):
pass
else:
sys.exit()
reveal_type(val) # revealed: int
```

## Narrowing through always-true branches

When a terminal (`return`) is inside an always-true branch, narrowing propagates through because the
else-branch is unreachable and contributes `Never` to the union.

```py
def _(x: int | None):
if True:
if x is None:
return
reveal_type(x) # revealed: int
reveal_type(x) # revealed: int
```

```py
def _(x: int | None):
if 1 + 1 == 2:
if x is None:
return
reveal_type(x) # revealed: int

# TODO: should be `int` (the else-branch of `1 + 1 == 2` is unreachable)
reveal_type(x) # revealed: int | None
```

This also works when the always-true condition is nested inside a narrowing branch:

```py
def _(x: int | None):
if x is None:
if 1 + 1 == 2:
return

# TODO: should be `int` (the inner always-true branch makes the outer
# if-branch terminal)
reveal_type(x) # revealed: int | None
```

## Narrowing from `assert` should not affect reassigned variables

When a variable is reassigned after an `assert`, the narrowing from the assert should not apply to
the new value.

```py
def foo(arg: int) -> int | None:
return None

def bar() -> None:
v = foo(1)
assert v is None

v = foo(2)
# v was reassigned, so the assert narrowing shouldn't apply
reveal_type(v) # revealed: int | None
```

## Narrowing from `NoReturn` should not affect reassigned variables

When a variable is narrowed due to a `NoReturn` call in one branch and then reassigned, the
narrowing should only apply before the reassignment, not after.

```py
import sys

def foo() -> int | None:
return 3

def bar():
v = foo()
if v is None:
sys.exit()
reveal_type(v) # revealed: int

v = foo()
# v was reassigned, so any narrowing shouldn't apply
reveal_type(v) # revealed: int | None
```
Original file line number Diff line number Diff line change
Expand Up @@ -618,9 +618,7 @@ def g(x: int | None):
if x is None:
sys.exit(1)

# TODO: should be just `int`, not `int | None`
# See https://github.com/astral-sh/ty/issues/685
reveal_type(x) # revealed: int | None
reveal_type(x) # revealed: int
```

### Possibly unresolved diagnostics
Expand Down
1 change: 0 additions & 1 deletion crates/ty_python_semantic/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@ pub mod ast_node_ref;
mod db;
mod dunder_all;
pub mod lint;
pub(crate) mod list;
mod node_key;
pub(crate) mod place;
mod program;
Expand Down
Loading