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

crates/ruff_linter/src/checkers/ast/mod.rs

@@ -684,7 +684,8 @@ impl SemanticSyntaxContext for Checker<'_> {
             | SemanticSyntaxErrorKind::LoadBeforeNonlocalDeclaration { .. }
             | SemanticSyntaxErrorKind::NonlocalAndGlobal(_)
             | SemanticSyntaxErrorKind::AnnotatedGlobal(_)
-            | SemanticSyntaxErrorKind::AnnotatedNonlocal(_) => {
+            | SemanticSyntaxErrorKind::AnnotatedNonlocal(_)
+            | SemanticSyntaxErrorKind::NoBindingForNonlocal(_) => {
                 self.semantic_errors.borrow_mut().push(error);
             }
         }

crates/ruff_python_parser/src/semantic_errors.rs

@@ -989,6 +989,9 @@ impl Display for SemanticSyntaxError {
             SemanticSyntaxErrorKind::AnnotatedNonlocal(name) => {
                 write!(f, "annotated name `{name}` can't be nonlocal")
             }
+            SemanticSyntaxErrorKind::NoBindingForNonlocal(name) => {
+                write!(f, "no binding for nonlocal `{name}` found")
+            }
         }
     }
 }
@@ -1346,6 +1349,20 @@ pub enum SemanticSyntaxErrorKind {
 
     /// Represents a type annotation on a variable that's been declared nonlocal
     AnnotatedNonlocal(String),
+
+    /// Represents a `nonlocal` statement that doesn't match any enclosing definition.
+    ///
+    /// ## Examples
+    ///
+    /// ```python
+    /// def f():
+    ///     nonlocal x  # error
+    ///
+    /// y = 1
+    /// def f():
+    ///     nonlocal y  # error (the global `y` isn't considered)
+    /// ```
+    NoBindingForNonlocal(String),
 }
 
 #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, get_size2::GetSize)]

crates/ty_python_semantic/resources/mdtest/public_types.md

@@ -312,8 +312,7 @@ def outer() -> None:
     set_x()
 
     def inner() -> None:
-        # TODO: this should ideally be `None | Literal[1]`. Mypy and pyright support this.
-        reveal_type(x)  # revealed: None
+        reveal_type(x)  # revealed: Literal[1] | None
     inner()
 ```
 

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]
+    reveal_type(x)  # revealed: Unknown | Literal["56", 42]
     x = "56"
     reveal_type(x)  # revealed: Literal["56"]
 ```

crates/ty_python_semantic/resources/mdtest/scopes/moduletype_attrs.md

@@ -78,7 +78,7 @@ reveal_type(module.__spec__)  # revealed: Unknown | ModuleSpec | None
 
 def nested_scope():
     global __loader__
-    reveal_type(__loader__)  # revealed: LoaderProtocol | None
+    reveal_type(__loader__)  # revealed: Unknown | LoaderProtocol | None
     __loader__ = 56  # error: [invalid-assignment] "Object of type `Literal[56]` is not assignable to `LoaderProtocol | None`"
 ```
 

crates/ty_python_semantic/resources/mdtest/scopes/nonlocal.md

@@ -104,16 +104,20 @@ def a():
 
             def d():
                 nonlocal x
-                reveal_type(x)  # revealed: Literal[3, 2]
+                # 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[3, 4, 2]
                 x = 4
                 reveal_type(x)  # revealed: Literal[4]
 
                 def e():
-                    reveal_type(x)  # revealed: Literal[4, 3, 2]
+                    reveal_type(x)  # revealed: Literal[3, 4, 2]
 ```
 
-However, currently the union of types that we build is incomplete. We walk parent scopes, but not
-sibling scopes, child scopes, second-cousin-once-removed scopes, etc:
+In addition to parent scopes, we also consider sibling scopes, child scopes,
+second-cousin-once-removed scopes, etc. However, we only fall back to bindings from other scopes
+when a place is unbound or possibly unbound in the current scope. In other words, nested bindings
+are only included in the "public type" of a variable:
 
 ```py
 def a():
@@ -121,13 +125,15 @@ def a():
     def b():
         nonlocal x
         x = 2
+        reveal_type(x)  # revealed: Literal[2]
 
     def c():
         def d():
             nonlocal x
             x = 3
-        # TODO: This should include 2 and 3.
-        reveal_type(x)  # revealed: Literal[1]
+        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]
 ```
 
 ## Local variable bindings "look ahead" to any assignment in the current scope
@@ -365,10 +371,10 @@ def f():
     x = 1
     def g():
         nonlocal x
-        reveal_type(x)  # revealed: Literal[1]
+        reveal_type(x)  # revealed: int
         x += 1
-        reveal_type(x)  # revealed: Literal[2]
-    # TODO: should be `Unknown | Literal[1]`
+        reveal_type(x)  # revealed: int
+    # TODO: should be `int`
     reveal_type(x)  # revealed: Literal[1]
 ```
 

crates/ty_python_semantic/src/place.rs

@@ -666,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
@@ -725,11 +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(declared_ty, 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(
-                    UnionType::from_elements(db, [inferred_ty, declared_ty]),
+                    nested_bindings().add(inferred_ty).add(declared_ty).build(),
                     if boundness_analysis == BoundnessAnalysis::AssumeBound {
                         Boundness::Bound
                     } else {
@@ -740,7 +781,8 @@ fn place_by_id<'db>(
 
             PlaceAndQualifiers { place, qualifiers }
         }
-        // Place is undeclared, return the union of `Unknown` with the inferred type
+        // Place is undeclared, return the inferred type, and union it with `Unknown` if the place
+        // is public.
         Ok(PlaceAndQualifiers {
             place: Place::Unbound,
             qualifiers: _,
@@ -749,6 +791,19 @@ fn place_by_id<'db>(
             let boundness_analysis = bindings.boundness_analysis;
             let mut inferred = place_from_bindings_impl(db, bindings, requires_explicit_reexport);
 
+            // If there are nested bindings, union whatever we inferred from those into what we've
+            // inferred here.
+            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);
+                    }
+                }
+            }
+
             if boundness_analysis == BoundnessAnalysis::AssumeBound {
                 if let Place::Type(ty, Boundness::PossiblyUnbound) = inferred {
                     inferred = Place::Type(ty, Boundness::Bound);

crates/ty_python_semantic/src/semantic_index/builder.rs

@@ -68,8 +68,40 @@ impl Loop {
 
 struct ScopeInfo {
     file_scope_id: FileScopeId,
+
     /// Current loop state; None if we are not currently visiting a loop
     current_loop: Option<Loop>,
+
+    /// `nonlocal` variables from scopes nested inside of this one that haven't yet been resolved
+    /// to a definition. They might end up resolving in this scope, or in an enclosing scope.
+    ///
+    /// When we pop scopes, we merge any unresolved nonlocals into the parent scope's collection.
+    /// The reason we need to track them for each scope separately, instead of using one map for
+    /// the whole builder, is because of sibling scope arrangements like this:
+    /// ```py
+    /// def f():
+    ///     def g():
+    ///         # When we pop `g`, this `x` goes in `f`'s set of unresolved nonlocals.
+    ///         nonlocal x
+    ///     def h():
+    ///         # When we pop `h`, this binding of `x` will *not* resolve the nonlocal from `g`,
+    ///         # because it's not in `h`'s set of unresolved nonlocals.
+    ///         x = 1
+    ///     # When we pop `f`, this binding of `x` will resolve the nonlocal from `g`.
+    ///     x = 1
+    /// ```
+    ///
+    /// Currently we only track explicit nonlocals, because ordinary "free" variables referring to
+    /// enclosing scopes can't be bound and can't trigger semantic syntax errors. Callers who need
+    /// to resolve free variables (e.g. `infer_place_load`) need to walk parent scopes until they
+    /// find one where `Symbol::is_local` or `Symbol::is_global` is true. If we wanted to
+    /// pre-record more of that information, we could expand this.
+    unresolved_nonlocals: FxHashMap<ast::name::Name, Vec<UnresolvedNonlocal>>,
+}
+
+struct UnresolvedNonlocal {
+    scope_id: FileScopeId,
+    range: TextRange,
 }
 
 pub(super) struct SemanticIndexBuilder<'db, 'ast> {
@@ -275,6 +307,7 @@ impl<'db, 'ast> SemanticIndexBuilder<'db, 'ast> {
         self.scope_stack.push(ScopeInfo {
             file_scope_id,
             current_loop: None,
+            unresolved_nonlocals: FxHashMap::default(),
         });
     }
 
@@ -446,6 +479,7 @@ impl<'db, 'ast> SemanticIndexBuilder<'db, 'ast> {
 
         let ScopeInfo {
             file_scope_id: popped_scope_id,
+            unresolved_nonlocals: mut popped_unresolved_nonlocals,
             ..
         } = self
             .scope_stack
@@ -458,13 +492,113 @@ impl<'db, 'ast> SemanticIndexBuilder<'db, 'ast> {
 
         let popped_scope = &mut self.scopes[popped_scope_id];
         popped_scope.extend_descendants(children_end);
+        let is_eager = popped_scope.is_eager();
+        let kind = popped_scope.kind();
 
-        if popped_scope.is_eager() {
+        if is_eager {
             self.record_eager_snapshots(popped_scope_id);
         } else {
             self.record_lazy_snapshots(popped_scope_id);
         }
 
+        // If we've popped a scope that nonlocals from nested (previously popped) scopes can refer
+        // to (i.e. not a class body), try to resolve them.
+        if kind.is_function_like() || popped_scope_id.is_global() {
+            popped_unresolved_nonlocals.retain(|name, nonlocals_with_this_name| {
+                let popped_place_table = &self.place_tables[popped_scope_id];
+                if let Some(symbol_id) = popped_place_table.symbol_id(name.as_str()) {
+                    let symbol = popped_place_table.symbol(symbol_id);
+                    let symbol_is_resolved = symbol.is_local() || symbol.is_global();
+                    let resolution_is_global = symbol.is_global() || popped_scope_id.is_global();
+                    if symbol_is_resolved {
+                        for &mut UnresolvedNonlocal {
+                            scope_id: nested_scope_id,
+                            range,
+                        } in nonlocals_with_this_name
+                        {
+                            if resolution_is_global {
+                                // It's a syntax error for a nonlocal variable to resolve to the
+                                // global scope or to a `global` statement in an enclosing scope.
+                                self.report_semantic_error(SemanticSyntaxError {
+                                    kind: SemanticSyntaxErrorKind::NoBindingForNonlocal(
+                                        name.clone().into(),
+                                    ),
+                                    range,
+                                    python_version: self.python_version,
+                                });
+                                continue;
+                            }
+                            let nested_place_table = &self.place_tables[nested_scope_id];
+                            let nested_symbol_id = nested_place_table.symbol_id(name).unwrap();
+                            let nested_symbol = nested_place_table.symbol(nested_symbol_id);
+                            if nested_symbol.is_bound() {
+                                self.place_tables[popped_scope_id]
+                                    .add_nested_scope_with_binding(symbol_id, nested_scope_id);
+                            }
+                        }
+                        // This name was resolved. Remove it and its references.
+                        return false;
+                    }
+                }
+                // This name was not resolved. Retain it. We'll add it to the parent scope's
+                // collection below.
+                true
+            });
+        }
+
+        if popped_scope_id.is_global() {
+            // If we've popped the global/module scope, still-unresolved `nonlocal` variables are
+            // another syntax error.
+            debug_assert!(self.scope_stack.is_empty());
+            for (name, nonlocals) in &popped_unresolved_nonlocals {
+                for nonlocal in nonlocals {
+                    self.report_semantic_error(SemanticSyntaxError {
+                        kind: SemanticSyntaxErrorKind::NoBindingForNonlocal(name.clone().into()),
+                        range: nonlocal.range,
+                        python_version: self.python_version,
+                    });
+                }
+            }
+        } else {
+            // Otherwise, add any still-unresolved nonlocals from nested scopes to the parent
+            // scope's collection.
+            let parent_unresolved_nonlocals = &mut self
+                .scope_stack
+                .last_mut() // current_scope_info_mut() would be a borrock error here
+                .expect("this is not the global/module scope")
+                .unresolved_nonlocals;
+            for (name, variables) in popped_unresolved_nonlocals {
+                parent_unresolved_nonlocals
+                    .entry(name)
+                    .or_default()
+                    .extend(variables);
+            }
+
+            // Also, update the global/module symbol table with any bound `global` variables in
+            // this scope. We do this here, rather than when we visit the `global` statement,
+            // because at that point we don't know whether the variable is bound.
+            let mut bound_global_symbols = Vec::new();
+            for symbol in self.place_tables[popped_scope_id].symbols() {
+                // Record bindings of global variables in a temporary Vec as a borrowck workaround.
+                // We could get clever here, but `global` variables are relatively rare, and
+                // allocating a small Vec in those cases isn't expensive.
+                if symbol.is_global() && symbol.is_bound() {
+                    bound_global_symbols.push(symbol.name().clone());
+                }
+            }
+            // Update the global scope with those bindings, now that `self.place_tables` is no
+            // longer borrowed.
+            for symbol_name in bound_global_symbols {
+                // Add this symbol to the global scope, if it isn't there already.
+                let global_symbol_id = self.add_symbol_to_scope(symbol_name, FileScopeId::global());
+                // Update the global place table with this reference. Doing this here rather than
+                // when we first encounter the `global` statement lets us see whether the symbol is
+                // bound.
+                self.place_tables[FileScopeId::global()]
+                    .add_nested_scope_with_binding(global_symbol_id, popped_scope_id);
+            }
+        }
+
         popped_scope_id
     }
 
@@ -512,22 +646,36 @@ impl<'db, 'ast> SemanticIndexBuilder<'db, 'ast> {
 
     /// Add a symbol to the place table and the use-def map.
     /// Return the [`ScopedPlaceId`] that uniquely identifies the symbol in both.
-    fn add_symbol(&mut self, name: Name) -> ScopedSymbolId {
-        let (symbol_id, added) = self.current_place_table_mut().add_symbol(Symbol::new(name));
+    fn add_symbol_to_scope(&mut self, name: Name, scope_id: FileScopeId) -> ScopedSymbolId {
+        let (symbol_id, added) = self.place_tables[scope_id].add_symbol(Symbol::new(name));
         if added {
-            self.current_use_def_map_mut().add_place(symbol_id.into());
+            self.use_def_maps[scope_id].add_place(symbol_id.into());
         }
         symbol_id
     }
 
+    fn add_symbol(&mut self, name: Name) -> ScopedSymbolId {
+        self.add_symbol_to_scope(name, self.current_scope())
+    }
+
+    /// Add a place to the place table and the use-def map.
+    /// Return the [`ScopedPlaceId`] that uniquely identifies the place in both.
+    fn add_place_to_scope(
+        &mut self,
+        place_expr: PlaceExpr,
+        scope_id: FileScopeId,
+    ) -> ScopedPlaceId {
+        let (place_id, added) = self.place_tables[scope_id].add_place(place_expr);
+        if added {
+            self.use_def_maps[scope_id].add_place(place_id);
+        }
+        place_id
+    }
+
     /// Add a place to the place table and the use-def map.
     /// Return the [`ScopedPlaceId`] that uniquely identifies the place in both.
     fn add_place(&mut self, place_expr: PlaceExpr) -> ScopedPlaceId {
-        let (place_id, added) = self.current_place_table_mut().add_place(place_expr);
-        if added {
-            self.current_use_def_map_mut().add_place(place_id);
-        }
-        place_id
+        self.add_place_to_scope(place_expr, self.current_scope())
     }
 
     #[track_caller]
@@ -2104,10 +2252,20 @@ impl<'ast> Visitor<'ast> for SemanticIndexBuilder<'_, 'ast> {
                             range: name.range,
                             python_version: self.python_version,
                         });
+                        // Never mark a symbol both global and nonlocal, even in this error case.
+                        continue;
+                    }
+                    // Assuming none of the rules above are violated, repeated `global`
+                    // declarations are allowed and ignored.
+                    if symbol.is_global() {
+                        continue;
                     }
                     self.current_place_table_mut()
                         .symbol_mut(symbol_id)
                         .mark_global();
+                    // We'll add this symbol to the global scope in `pop_scope`, after we resolve
+                    // nonlocals. That lets us record whether it's bound in this scope, which we
+                    // don't know yet.
                 }
                 walk_stmt(self, stmt);
             }
@@ -2137,19 +2295,38 @@ impl<'ast> Visitor<'ast> for SemanticIndexBuilder<'_, 'ast> {
                             range: name.range,
                             python_version: self.python_version,
                         });
+                        // Never mark a symbol both global and nonlocal, even in this error case.
+                        continue;
+                    }
+                    // Check whether this is the module scope, where `nonlocal` isn't allowed.
+                    let scope_id = self.current_scope();
+                    if scope_id.is_global() {
+                        // The SemanticSyntaxChecker will report an error for this.
+                        continue;
+                    }
+                    // Assuming none of the rules above are violated, repeated `nonlocal`
+                    // declarations are allowed and ignored.
+                    if symbol.is_nonlocal() {
+                        continue;
                     }
-                    // The variable is required to exist in an enclosing scope, but that definition
-                    // might come later. For example, this is example legal, but we can't check
-                    // that here, because we haven't gotten to `x = 1`:
-                    // ```py
-                    // def f():
-                    //     def g():
-                    //         nonlocal x
-                    //     x = 1
-                    // ```
                     self.current_place_table_mut()
                         .symbol_mut(symbol_id)
                         .mark_nonlocal();
+                    // Add this symbol to the parent scope's set of unresolved nonlocals. (It would
+                    // also work to add it to this scope's set, which will get folded into the
+                    // parent's in `pop_scope`. But since it can't possibly resolve here, we might
+                    // as well try to spare an allocation.) We checked above that we aren't in the
+                    // module scope, so there's definitely a parent scope.
+                    let parent_scope_index = self.scope_stack.len() - 2;
+                    let parent_scope_info = &mut self.scope_stack[parent_scope_index];
+                    parent_scope_info
+                        .unresolved_nonlocals
+                        .entry(name.id.clone())
+                        .or_default()
+                        .push(UnresolvedNonlocal {
+                            scope_id,
+                            range: name.range,
+                        });
                 }
                 walk_stmt(self, stmt);
             }
@@ -2177,6 +2354,9 @@ impl<'ast> Visitor<'ast> for SemanticIndexBuilder<'_, 'ast> {
                             // foo()
                             // ```
                             symbol.mark_bound();
+                            // TODO: `mark_used` might be redundant here, since `walk_stmt` visits
+                            // the deleted expression, and `visit_expr` considers `del` to be a
+                            // use.
                             symbol.mark_used();
                         }
 

crates/ty_python_semantic/src/semantic_index/member.rs

@@ -39,11 +39,6 @@ impl Member {
         self.flags.contains(MemberFlags::IS_BOUND)
     }
 
-    /// Is the place declared in its containing scope?
-    pub(crate) fn is_declared(&self) -> bool {
-        self.flags.contains(MemberFlags::IS_DECLARED)
-    }
-
     pub(super) fn mark_bound(&mut self) {
         self.insert_flags(MemberFlags::IS_BOUND);
     }

crates/ty_python_semantic/src/semantic_index/place.rs

@@ -80,13 +80,6 @@ impl<'a> PlaceExprRef<'a> {
         matches!(self, PlaceExprRef::Symbol(_))
     }
 
-    pub(crate) fn is_declared(self) -> bool {
-        match self {
-            Self::Symbol(symbol) => symbol.is_declared(),
-            Self::Member(member) => member.is_declared(),
-        }
-    }
-
     pub(crate) const fn is_bound(self) -> bool {
         match self {
             PlaceExprRef::Symbol(symbol) => symbol.is_bound(),
@@ -233,6 +226,14 @@ impl PlaceTable {
     ) -> Option<ScopedMemberId> {
         self.members.place_id_by_instance_attribute_name(name)
     }
+
+    pub(crate) fn nested_scopes_with_bindings(&self, symbol_id: ScopedSymbolId) -> &[FileScopeId] {
+        if let Some(scopes) = self.symbols.nested_scopes_with_bindings.get(&symbol_id) {
+            scopes
+        } else {
+            &[]
+        }
+    }
 }
 
 #[derive(Default)]
@@ -375,6 +376,15 @@ impl PlaceTableBuilder {
         }
     }
 
+    pub(super) fn add_nested_scope_with_binding(
+        &mut self,
+        this_scope_symbol_id: ScopedSymbolId,
+        nested_scope: FileScopeId,
+    ) {
+        self.symbols
+            .add_nested_scope_with_binding(this_scope_symbol_id, nested_scope);
+    }
+
     pub(crate) fn finish(self) -> PlaceTable {
         PlaceTable {
             symbols: self.symbols.build(),

crates/ty_python_semantic/src/semantic_index/symbol.rs

@@ -1,8 +1,9 @@
+use crate::semantic_index::scope::FileScopeId;
 use bitflags::bitflags;
 use hashbrown::hash_table::Entry;
 use ruff_index::{IndexVec, newtype_index};
 use ruff_python_ast::name::Name;
-use rustc_hash::FxHasher;
+use rustc_hash::{FxHashMap, FxHasher};
 use std::hash::{Hash as _, Hasher as _};
 use std::ops::{Deref, DerefMut};
 
@@ -156,6 +157,12 @@ pub(super) struct SymbolTable {
     ///
     /// Uses a hash table to avoid storing the name twice.
     map: hashbrown::HashTable<ScopedSymbolId>,
+
+    // Variables defined in this scope, and not marked `global` or `nonlocal` here, which are also
+    // bound in nested scopes (by being marked `global` or `nonlocal` there). These (keys) are
+    // similar to what CPython calls "cell" variables, except that this scope may also be the
+    // global scope.
+    pub(super) nested_scopes_with_bindings: FxHashMap<ScopedSymbolId, Vec<FileScopeId>>,
 }
 
 impl SymbolTable {
@@ -245,12 +252,33 @@ impl SymbolTableBuilder {
         }
     }
 
+    pub(super) fn add_nested_scope_with_binding(
+        &mut self,
+        this_scope_symbol_id: ScopedSymbolId,
+        nested_scope: FileScopeId,
+    ) {
+        let bindings = self
+            .table
+            .nested_scopes_with_bindings
+            .entry(this_scope_symbol_id)
+            .or_default();
+        debug_assert!(
+            !bindings.contains(&nested_scope),
+            "the same scoped symbol shouldn't get added more than once",
+        );
+        bindings.push(nested_scope);
+    }
+
     pub(super) fn build(self) -> SymbolTable {
         let mut table = self.table;
         table.symbols.shrink_to_fit();
         table
             .map
             .shrink_to_fit(|id| SymbolTable::hash_name(&table.symbols[*id].name));
+        table.nested_scopes_with_bindings.shrink_to_fit();
+        for scopes_vec in table.nested_scopes_with_bindings.values_mut() {
+            scopes_vec.shrink_to_fit();
+        }
         table
     }
 }

crates/ty_python_semantic/src/types/builder.rs

@@ -202,7 +202,7 @@ enum ReduceResult<'db> {
 
 // TODO increase this once we extend `UnionElement` throughout all union/intersection
 // representations, so that we can make large unions of literals fast in all operations.
-const MAX_UNION_LITERALS: usize = 200;
+const MAX_UNION_LITERALS: usize = 100;
 
 pub(crate) struct UnionBuilder<'db> {
     elements: Vec<UnionElement<'db>>,

crates/ty_python_semantic/src/types/infer.rs

@@ -37,7 +37,6 @@
 //! be considered a bug.)
 
 use itertools::{Either, Itertools};
-use ruff_db::diagnostic::{Annotation, DiagnosticId, Severity};
 use ruff_db::files::File;
 use ruff_db::parsed::{ParsedModuleRef, parsed_module};
 use ruff_python_ast::visitor::{Visitor, walk_expr};
@@ -85,7 +84,7 @@ use crate::semantic_index::place::{PlaceExpr, PlaceExprRef};
 use crate::semantic_index::scope::{
     FileScopeId, NodeWithScopeKind, NodeWithScopeRef, ScopeId, ScopeKind,
 };
-use crate::semantic_index::symbol::ScopedSymbolId;
+use crate::semantic_index::symbol::{ScopedSymbolId, Symbol};
 use crate::semantic_index::{
     ApplicableConstraints, EnclosingSnapshotResult, SemanticIndex, place_table, semantic_index,
 };
@@ -2548,9 +2547,9 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> {
             ast::Stmt::Raise(raise) => self.infer_raise_statement(raise),
             ast::Stmt::Return(ret) => self.infer_return_statement(ret),
             ast::Stmt::Delete(delete) => self.infer_delete_statement(delete),
-            ast::Stmt::Nonlocal(nonlocal) => self.infer_nonlocal_statement(nonlocal),
             ast::Stmt::Global(global) => self.infer_global_statement(global),
-            ast::Stmt::Break(_)
+            ast::Stmt::Nonlocal(_)
+            | ast::Stmt::Break(_)
             | ast::Stmt::Continue(_)
             | ast::Stmt::Pass(_)
             | ast::Stmt::IpyEscapeCommand(_) => {
@@ -5355,75 +5354,6 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> {
         }
     }
 
-    fn infer_nonlocal_statement(&mut self, nonlocal: &ast::StmtNonlocal) {
-        let ast::StmtNonlocal {
-            node_index: _,
-            range,
-            names,
-        } = nonlocal;
-        let db = self.db();
-        let scope = self.scope();
-        let file_scope_id = scope.file_scope_id(db);
-        let current_file = self.file();
-        'names: for name in names {
-            // Walk up parent scopes looking for a possible enclosing scope that may have a
-            // definition of this name visible to us. Note that we skip the scope containing the
-            // use that we are resolving, since we already looked for the place there up above.
-            for (enclosing_scope_file_id, _) in self.index.ancestor_scopes(file_scope_id).skip(1) {
-                // Class scopes are not visible to nested scopes, and `nonlocal` cannot refer to
-                // globals, so check only function-like scopes.
-                let enclosing_scope_id = enclosing_scope_file_id.to_scope_id(db, current_file);
-                if !enclosing_scope_id.is_function_like(db) {
-                    continue;
-                }
-                let enclosing_place_table = self.index.place_table(enclosing_scope_file_id);
-                let Some(enclosing_symbol_id) = enclosing_place_table.symbol_id(name) else {
-                    // This scope doesn't define this name. Keep going.
-                    continue;
-                };
-                let enclosing_symbol = enclosing_place_table.symbol(enclosing_symbol_id);
-                // We've found a definition for this name in an enclosing function-like scope.
-                // Either this definition is the valid place this name refers to, or else we'll
-                // emit a syntax error. Either way, we won't walk any more enclosing scopes. Note
-                // that there are differences here compared to `infer_place_load`: A regular load
-                // (e.g. `print(x)`) is allowed to refer to a global variable (e.g. `x = 1` in the
-                // global scope), and similarly it's allowed to refer to a local variable in an
-                // enclosing function that's declared `global` (e.g. `global x`). However, the
-                // `nonlocal` keyword can't refer to global variables (that's a `SyntaxError`), and
-                // it also can't refer to local variables in enclosing functions that are declared
-                // `global` (also a `SyntaxError`).
-                if enclosing_symbol.is_global() {
-                    // A "chain" of `nonlocal` statements is "broken" by a `global` statement. Stop
-                    // looping and report that this `nonlocal` statement is invalid.
-                    break;
-                }
-                if !enclosing_symbol.is_bound()
-                    && !enclosing_symbol.is_declared()
-                    && !enclosing_symbol.is_nonlocal()
-                {
-                    debug_assert!(enclosing_symbol.is_used());
-                    // The name is only referenced here, not defined. Keep going.
-                    continue;
-                }
-                // We found a definition. We've checked that the name isn't `global` in this scope,
-                // but it's ok if it's `nonlocal`. If a "chain" of `nonlocal` statements fails to
-                // lead to a valid binding, the outermost one will be an error; we don't need to
-                // walk the whole chain for each one.
-                continue 'names;
-            }
-            // There's no matching binding in an enclosing scope. This `nonlocal` statement is
-            // invalid.
-            if let Some(builder) = self
-                .context
-                .report_diagnostic(DiagnosticId::InvalidSyntax, Severity::Error)
-            {
-                builder
-                    .into_diagnostic(format_args!("no binding for nonlocal `{name}` found"))
-                    .annotate(Annotation::primary(self.context.span(*range)));
-            }
-        }
-    }
-
     fn module_type_from_name(&self, module_name: &ModuleName) -> Option<Type<'db>> {
         resolve_module(self.db(), module_name)
             .map(|module| Type::module_literal(self.db(), self.file(), module))
@@ -6724,8 +6654,6 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> {
             // definition of this name visible to us (would be `LOAD_DEREF` at runtime.)
             // Note that we skip the scope containing the use that we are resolving, since we
             // already looked for the place there up above.
-            let mut nonlocal_union_builder = UnionBuilder::new(db);
-            let mut found_some_definition = false;
             for (enclosing_scope_file_id, _) in self.index.ancestor_scopes(file_scope_id).skip(1) {
                 // Class scopes are not visible to nested scopes, and we need to handle global
                 // scope differently (because an unbound name there falls back to builtins), so
@@ -6821,22 +6749,17 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> {
                 // see a `global` declaration, stop walking scopes and proceed to the global
                 // handling below. (If we're walking from a prior/inner scope where this variable
                 // is `nonlocal`, then this is a semantic syntax error, but we don't enforce that
-                // here. See `infer_nonlocal_statement`.)
-                if enclosing_place
-                    .as_symbol()
-                    .is_some_and(super::super::semantic_index::symbol::Symbol::is_global)
-                {
+                // here. See `SemanticSyntaxBuilder::pop_scope`.)
+                if enclosing_place.as_symbol().is_some_and(Symbol::is_global) {
                     break;
                 }
 
-                // If the name is declared or bound in this scope, figure out its type. This might
-                // resolve the name and end the walk. But if the name is declared `nonlocal` in
-                // this scope, we'll keep walking enclosing scopes and union this type with the
-                // other types we find. (It's a semantic syntax error to declare a type for a
-                // `nonlocal` variable, but we don't enforce that here. See the
-                // `ast::Stmt::AnnAssign` handling in `SemanticIndexBuilder::visit_stmt`.)
-                if enclosing_place.is_bound() || enclosing_place.is_declared() {
-                    let local_place_and_qualifiers = place(
+                // If we've reached the scope where the name is local (bound or declared, and not
+                // marked `global` or `nonlocal`), end the walk and infer its "public" type. This
+                // considers bindings from nested scopes, not only those we just walked but also
+                // sibling/cousin scopes.
+                if enclosing_place.as_symbol().is_some_and(Symbol::is_local) {
+                    return place(
                         db,
                         enclosing_scope_id,
                         place_expr,
@@ -6849,28 +6772,6 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> {
                             &constraint_keys,
                         )
                     });
-                    // We could have Place::Unbound here, despite the checks above, for example if
-                    // this scope contains a `del` statement but no binding or declaration.
-                    if let Place::Type(type_, boundness) = local_place_and_qualifiers.place {
-                        nonlocal_union_builder.add_in_place(type_);
-                        // `ConsideredDefinitions::AllReachable` never returns PossiblyUnbound
-                        debug_assert_eq!(boundness, Boundness::Bound);
-                        found_some_definition = true;
-                    }
-
-                    if !enclosing_place
-                        .as_symbol()
-                        .is_some_and(super::super::semantic_index::symbol::Symbol::is_nonlocal)
-                    {
-                        // We've reached a function-like scope that marks this name bound or
-                        // declared but doesn't mark it `nonlocal`. The name is therefore resolved,
-                        // and we won't consider any scopes outside of this one.
-                        return if found_some_definition {
-                            Place::Type(nonlocal_union_builder.build(), Boundness::Bound).into()
-                        } else {
-                            Place::Unbound.into()
-                        };
-                    }
                 }
             }