diff --git a/crates/ty_python_semantic/resources/mdtest/public_types.md b/crates/ty_python_semantic/resources/mdtest/public_types.md index 7d5a55b01a..72016a054c 100644 --- a/crates/ty_python_semantic/resources/mdtest/public_types.md +++ b/crates/ty_python_semantic/resources/mdtest/public_types.md @@ -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() ``` diff --git a/crates/ty_python_semantic/resources/mdtest/scopes/global.md b/crates/ty_python_semantic/resources/mdtest/scopes/global.md index 87f2c8dafa..26cf45cf44 100644 --- a/crates/ty_python_semantic/resources/mdtest/scopes/global.md +++ b/crates/ty_python_semantic/resources/mdtest/scopes/global.md @@ -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"] ``` diff --git a/crates/ty_python_semantic/resources/mdtest/scopes/nonlocal.md b/crates/ty_python_semantic/resources/mdtest/scopes/nonlocal.md index fefda9f3bf..15e471ebdd 100644 --- a/crates/ty_python_semantic/resources/mdtest/scopes/nonlocal.md +++ b/crates/ty_python_semantic/resources/mdtest/scopes/nonlocal.md @@ -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] ``` diff --git a/crates/ty_python_semantic/src/place.rs b/crates/ty_python_semantic/src/place.rs index 58d68a4569..c5ece6975a 100644 --- a/crates/ty_python_semantic/src/place.rs +++ b/crates/ty_python_semantic/src/place.rs @@ -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); } }