mirror of
https://github.com/astral-sh/ruff.git
synced 2025-09-29 13:24:57 +00:00
[ty] fix incorrect member narrowing (#19802)
## Summary Reported in: https://github.com/astral-sh/ruff/pull/19795#issuecomment-3161981945 If a root expression is reassigned, narrowing on the member should be invalidated, but there was an oversight in the current implementation. This PR fixes that, and also removes some unnecessary handling. ## Test Plan New tests cases in `narrow/conditionals/nested.md`.
This commit is contained in:
parent
f51a228f04
commit
462adfd0e6
2 changed files with 61 additions and 20 deletions
|
@ -275,7 +275,49 @@ def f(a: A):
|
||||||
a.x = None
|
a.x = None
|
||||||
```
|
```
|
||||||
|
|
||||||
Narrowing is invalidated if a `nonlocal` declaration is made within a lazy scope.
|
The opposite is not true, that is, if a root expression is reassigned, narrowing on the member are
|
||||||
|
no longer valid in the inner lazy scope.
|
||||||
|
|
||||||
|
```py
|
||||||
|
def f(l: list[str | None]):
|
||||||
|
if l[0] is not None:
|
||||||
|
def _():
|
||||||
|
reveal_type(l[0]) # revealed: str | None
|
||||||
|
l = [None]
|
||||||
|
|
||||||
|
def f(l: list[str | None]):
|
||||||
|
l[0] = "a"
|
||||||
|
def _():
|
||||||
|
reveal_type(l[0]) # revealed: str | None
|
||||||
|
l = [None]
|
||||||
|
|
||||||
|
def f(l: list[str | None]):
|
||||||
|
l[0] = "a"
|
||||||
|
def _():
|
||||||
|
l: list[str | None] = [None]
|
||||||
|
def _():
|
||||||
|
# TODO: should be `str | None`
|
||||||
|
reveal_type(l[0]) # revealed: Unknown
|
||||||
|
|
||||||
|
def _():
|
||||||
|
def _():
|
||||||
|
reveal_type(l[0]) # revealed: str | None
|
||||||
|
l: list[str | None] = [None]
|
||||||
|
|
||||||
|
def f(a: A):
|
||||||
|
if a.x is not None:
|
||||||
|
def _():
|
||||||
|
reveal_type(a.x) # revealed: str | None
|
||||||
|
a = A()
|
||||||
|
|
||||||
|
def f(a: A):
|
||||||
|
a.x = "a"
|
||||||
|
def _():
|
||||||
|
reveal_type(a.x) # revealed: str | None
|
||||||
|
a = A()
|
||||||
|
```
|
||||||
|
|
||||||
|
Narrowing is also invalidated if a `nonlocal` declaration is made within a lazy scope.
|
||||||
|
|
||||||
```py
|
```py
|
||||||
def f(non_local: str | None):
|
def f(non_local: str | None):
|
||||||
|
|
|
@ -6758,6 +6758,17 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> {
|
||||||
.parent()
|
.parent()
|
||||||
.is_some_and(|parent| parent == enclosing_scope_file_id);
|
.is_some_and(|parent| parent == enclosing_scope_file_id);
|
||||||
|
|
||||||
|
let has_root_place_been_reassigned = || {
|
||||||
|
let enclosing_place_table = self.index.place_table(enclosing_scope_file_id);
|
||||||
|
enclosing_place_table
|
||||||
|
.parents(place_expr)
|
||||||
|
.any(|enclosing_root_place_id| {
|
||||||
|
enclosing_place_table
|
||||||
|
.place(enclosing_root_place_id)
|
||||||
|
.is_bound()
|
||||||
|
})
|
||||||
|
};
|
||||||
|
|
||||||
// If the reference is in a nested eager scope, we need to look for the place at
|
// If the reference is in a nested eager scope, we need to look for the place at
|
||||||
// the point where the previous enclosing scope was defined, instead of at the end
|
// the point where the previous enclosing scope was defined, instead of at the end
|
||||||
// of the scope. (Note that the semantic index builder takes care of only
|
// of the scope. (Note that the semantic index builder takes care of only
|
||||||
|
@ -6799,28 +6810,16 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> {
|
||||||
// There are no visible bindings / constraint here.
|
// There are no visible bindings / constraint here.
|
||||||
// Don't fall back to non-eager place resolution.
|
// Don't fall back to non-eager place resolution.
|
||||||
EnclosingSnapshotResult::NotFound => {
|
EnclosingSnapshotResult::NotFound => {
|
||||||
let enclosing_place_table =
|
if has_root_place_been_reassigned() {
|
||||||
self.index.place_table(enclosing_scope_file_id);
|
return Place::Unbound.into();
|
||||||
for enclosing_root_place_id in enclosing_place_table.parents(place_expr)
|
|
||||||
{
|
|
||||||
let enclosing_root_place =
|
|
||||||
enclosing_place_table.place(enclosing_root_place_id);
|
|
||||||
if enclosing_root_place.is_bound() {
|
|
||||||
if let Place::Type(_, _) = place(
|
|
||||||
db,
|
|
||||||
enclosing_scope_id,
|
|
||||||
enclosing_root_place,
|
|
||||||
ConsideredDefinitions::AllReachable,
|
|
||||||
)
|
|
||||||
.place
|
|
||||||
{
|
|
||||||
return Place::Unbound.into();
|
|
||||||
}
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
continue;
|
continue;
|
||||||
}
|
}
|
||||||
EnclosingSnapshotResult::NoLongerInEagerContext => {}
|
EnclosingSnapshotResult::NoLongerInEagerContext => {
|
||||||
|
if has_root_place_been_reassigned() {
|
||||||
|
return Place::Unbound.into();
|
||||||
|
}
|
||||||
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
Loading…
Add table
Add a link
Reference in a new issue