Python
/
ruff
mirror of https://github.com/astral-sh/ruff
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

don't do nested bindings inference if we have a declared type

This commit is contained in:
Jack O'Connor 2025-08-08 14:14:09 -07:00
parent b82bbcd51c
commit abe6a36c80
4 changed files with 54 additions and 46 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
crates/ty_python_semantic/resources/mdtest/public_types.md
View File

@ -312,7 +312,7 @@ def outer() -> None:
set_x()
def inner() -> None:
reveal_type(x) # revealed: None | Literal[1]
reveal_type(x) # revealed: Literal[1] | None
inner()
```

2
crates/ty_python_semantic/resources/mdtest/scopes/global.md
View File

@ -201,7 +201,7 @@ x = 42
def f():
global x
reveal_type(x) # revealed: Unknown | Literal[42, "56"]
reveal_type(x) # revealed: Unknown | Literal["56", 42]
x = "56"
reveal_type(x) # revealed: Literal["56"]
```

6
crates/ty_python_semantic/resources/mdtest/scopes/nonlocal.md
View File

@ -106,12 +106,12 @@ def a():
nonlocal x
# It's counterintuitive that 4 gets included here, since we haven't reached the
# binding in this scope, but this function might get called more than once.
reveal_type(x) # revealed: Literal[2, 3, 4]
reveal_type(x) # revealed: Literal[3, 4, 2]
x = 4
reveal_type(x) # revealed: Literal[4]
def e():
reveal_type(x) # revealed: Literal[2, 3, 4]
reveal_type(x) # revealed: Literal[3, 4, 2]
```
In addition to parent scopes, we also consider sibling scopes, child scopes,
@ -131,7 +131,7 @@ def a():
def d():
nonlocal x
x = 3
reveal_type(x) # revealed: Literal[1, 2, 3]
reveal_type(x) # revealed: Literal[2, 3, 1]
# `x` is local here, so we don't look at nested scopes.
reveal_type(x) # revealed: Literal[1]
```

90
crates/ty_python_semantic/src/place.rs
View File

@ -651,32 +651,6 @@ fn place_by_id<'db>(
) -> PlaceAndQualifiers<'db> {
let use_def = use_def_map(db, scope);
// If there are any nested bindings (via `global` or `nonlocal` variables) for this symbol,
// infer them and union the results. Nested bindings aren't allowed to have declarations or
// qualifiers, and we can just union their inferred types.
let mut nested_bindings_union = UnionBuilder::new(db);
if let Some(symbol_id) = place_id.as_symbol() {
let current_place_table = place_table(db, scope);
let symbol = current_place_table.symbol(symbol_id);
for nested_file_scope_id in place_table(db, scope).nested_scopes_with_bindings(symbol_id) {
let nested_scope_id = nested_file_scope_id.to_scope_id(db, scope.file(db));
let nested_place_table = place_table(db, nested_scope_id);
let nested_symbol_id = nested_place_table
.symbol_id(symbol.name())
.expect("nested_scopes_with_bindings says this reference exists");
let nested_place = place_by_id(
db,
nested_scope_id,
ScopedPlaceId::Symbol(nested_symbol_id),
RequiresExplicitReExport::No,
ConsideredDefinitions::AllReachable,
);
if let Place::Type(nested_type, _) = nested_place.place {
nested_bindings_union.add_in_place(nested_type);
}
}
}
// If the place is declared, the public type is based on declarations; otherwise, it's based
// on inference from bindings.
@ -692,6 +666,47 @@ fn place_by_id<'db>(
ConsideredDefinitions::AllReachable => use_def.all_reachable_bindings(place_id),
};
// If there are any nested bindings (via `global` or `nonlocal` variables) for this symbol,
// infer them and union the results. Note that this is potentially recursive, and we can
// trigger fixed-point iteration here in cases like this:
// ```
// def f():
// x = 1
// def g():
// nonlocal x
// x += 1
// ```
let nested_bindings = || {
// For performance reasons, avoid creating a union unless we have more one binding.
let mut union = UnionBuilder::new(db);
if let Some(symbol_id) = place_id.as_symbol() {
let current_place_table = place_table(db, scope);
let symbol = current_place_table.symbol(symbol_id);
for &nested_file_scope_id in
place_table(db, scope).nested_scopes_with_bindings(symbol_id)
{
let nested_scope_id = nested_file_scope_id.to_scope_id(db, scope.file(db));
let nested_place_table = place_table(db, nested_scope_id);
let nested_symbol_id = nested_place_table
.symbol_id(symbol.name())
.expect("nested_scopes_with_bindings says this reference exists");
let place = place_by_id(
db,
nested_scope_id,
ScopedPlaceId::Symbol(nested_symbol_id),
RequiresExplicitReExport::No,
ConsideredDefinitions::AllReachable,
);
// Nested bindings aren't allowed to have declarations or qualifiers, so we can
// just extract their inferred types.
if let Place::Type(nested_type, _) = place.place {
union.add_in_place(nested_type);
}
}
}
union
};
// If a symbol is undeclared, but qualified with `typing.Final`, we use the right-hand side
// inferred type, without unioning with `Unknown`, because it can not be modified.
if let Some(qualifiers) = declared
@ -751,17 +766,11 @@ fn place_by_id<'db>(
// TODO: We probably don't want to report `Bound` here. This requires a bit of
// design work though as we might want a different behavior for stubs and for
// normal modules.
Place::Type(
nested_bindings_union.add(declared_ty).build(),
Boundness::Bound,
)
Place::Type(nested_bindings().add(declared_ty).build(), Boundness::Bound)
}
// Place is possibly undeclared and (possibly) bound
Place::Type(inferred_ty, boundness) => Place::Type(
nested_bindings_union
.add(inferred_ty)
.add(declared_ty)
.build(),
nested_bindings().add(inferred_ty).add(declared_ty).build(),
if boundness_analysis == BoundnessAnalysis::AssumeBound {
Boundness::Bound
} else {
@ -784,13 +793,12 @@ fn place_by_id<'db>(
// If there are nested bindings, union whatever we inferred from those into what we've
// inferred here.
if let Some(nested_bindings_type) = nested_bindings_union.try_build() {
match &mut inferred {
Place::Type(inferred_type, _) => {
*inferred_type =
UnionType::from_elements(db, [*inferred_type, nested_bindings_type]);
}
Place::Unbound => {
match &mut inferred {
Place::Type(inferred_type, _) => {
*inferred_type = nested_bindings().add(*inferred_type).build();
}
Place::Unbound => {
if let Some(nested_bindings_type) = nested_bindings().try_build() {
inferred = Place::Type(nested_bindings_type, Boundness::PossiblyUnbound);
}
}