WIP straight up Claude implementation just to look at the ecosystem report
diff --git a/crates/ty_python_semantic/resources/mdtest/type_qualifiers/final.md b/crates/ty_python_semantic/resources/mdtest/type_qualifiers/final.md index 967c28f..36ba336 100644 --- a/crates/ty_python_semantic/resources/mdtest/type_qualifiers/final.md +++ b/crates/ty_python_semantic/resources/mdtest/type_qualifiers/final.md
@@ -666,7 +666,7 @@ k = "" for i in range(10): - # TODO: This should be an error; it's a reassignment + # error: [invalid-assignment] "Reassignment of `Final` symbol `k` is not allowed" k += " " k: Final[str] = ""
diff --git a/crates/ty_python_semantic/src/semantic_index.rs b/crates/ty_python_semantic/src/semantic_index.rs index e49d318..c6c5577 100644 --- a/crates/ty_python_semantic/src/semantic_index.rs +++ b/crates/ty_python_semantic/src/semantic_index.rs
@@ -47,7 +47,7 @@ pub(crate) use self::use_def::{ ApplicableConstraints, BindingWithConstraints, BindingWithConstraintsIterator, - DeclarationWithConstraint, DeclarationsIterator, LiveBinding, + DeclarationWithConstraint, DeclarationsIterator, LiveBinding, LiveDeclaration, }; /// Returns the semantic index for `file`. @@ -135,12 +135,14 @@ #[derive(Debug, Clone, Default, PartialEq, Eq, Update, get_size2::GetSize)] pub(crate) struct LoopHeader { bindings: FxHashMap<ScopedPlaceId, SmallVec<[LiveBinding; 1]>>, + declarations: FxHashMap<ScopedPlaceId, SmallVec<[LiveDeclaration; 1]>>, } impl LoopHeader { pub(crate) fn new() -> Self { Self { bindings: FxHashMap::default(), + declarations: FxHashMap::default(), } } @@ -148,6 +150,13 @@ self.bindings.entry(place).or_default().push(binding); } + pub(crate) fn add_declaration(&mut self, place: ScopedPlaceId, declaration: LiveDeclaration) { + self.declarations + .entry(place) + .or_default() + .push(declaration); + } + pub(crate) fn bindings_for_place( &self, place: ScopedPlaceId, @@ -158,6 +167,17 @@ .into_iter() .flatten() } + + pub(crate) fn declarations_for_place( + &self, + place: ScopedPlaceId, + ) -> impl Iterator<Item = LiveDeclaration> + '_ { + self.declarations + .get(&place) + .map(|v: &SmallVec<[LiveDeclaration; 1]>| v.iter().copied()) + .into_iter() + .flatten() + } } /// A Salsa token for retrieving a `LoopHeader`. See `get_loop_header` below.
diff --git a/crates/ty_python_semantic/src/semantic_index/builder.rs b/crates/ty_python_semantic/src/semantic_index/builder.rs index 76cb3c3..d69c514 100644 --- a/crates/ty_python_semantic/src/semantic_index/builder.rs +++ b/crates/ty_python_semantic/src/semantic_index/builder.rs
@@ -748,17 +748,27 @@ if category.is_binding() && !is_loop_header { self.mark_place_bound(place); } - if category.is_declaration() { + if category.is_declaration() && !is_loop_header { self.mark_place_declared(place); } let use_def = self.current_use_def_map_mut(); match category { DefinitionCategory::DeclarationAndBinding => { - use_def.record_declaration_and_binding(place, definition); - self.delete_associated_bindings(place); + if is_loop_header { + use_def.record_loop_header_declaration_and_binding(place, definition); + } else { + use_def.record_declaration_and_binding(place, definition); + self.delete_associated_bindings(place); + } } - DefinitionCategory::Declaration => use_def.record_declaration(place, definition), + DefinitionCategory::Declaration => { + if is_loop_header { + use_def.record_loop_header_declaration(place, definition); + } else { + use_def.record_declaration(place, definition); + } + } DefinitionCategory::Binding => { // Loop-header bindings don't shadow prior bindings. let previous_definitions = if is_loop_header { @@ -839,49 +849,80 @@ } } - /// Create loop header definitions for all places that are bound within a loop. Return the - /// `LoopToken` referenced by those definitions, and the set of bound place IDs. + /// Create loop header definitions for all places that are bound and/or declared within a loop. + /// Return the `LoopToken` referenced by those definitions, the set of bound place IDs, and + /// the set of declared place IDs. fn synthesize_loop_header_definitions( &mut self, loop_stmt: LoopStmtRef<'ast>, bound_places: Vec<PlaceExpr>, - ) -> (LoopToken<'db>, FxHashSet<ScopedPlaceId>) { + declared_places: Vec<PlaceExpr>, + ) -> ( + LoopToken<'db>, + FxHashSet<ScopedPlaceId>, + FxHashSet<ScopedPlaceId>, + ) { let loop_token = LoopToken::new(self.db); let mut bound_place_ids: FxHashSet<ScopedPlaceId> = FxHashSet::default(); - for place_expr in bound_places { - let place_id = self.add_place(place_expr); - if bound_place_ids.insert(place_id) { + let mut declared_place_ids: FxHashSet<ScopedPlaceId> = FxHashSet::default(); + // First pass: resolve all place expressions to IDs to know the category of each. + let bound_ids: Vec<_> = bound_places + .into_iter() + .map(|p| self.add_place(p)) + .collect(); + let declared_ids: Vec<_> = declared_places + .into_iter() + .map(|p| self.add_place(p)) + .collect(); + for &id in &bound_ids { + bound_place_ids.insert(id); + } + for &id in &declared_ids { + declared_place_ids.insert(id); + } + // Second pass: create a loop header definition for each unique place. + let mut seen: FxHashSet<ScopedPlaceId> = FxHashSet::default(); + for place_id in bound_ids.iter().chain(&declared_ids).copied() { + if seen.insert(place_id) { let loop_header_ref = LoopHeaderDefinitionNodeRef { loop_stmt, place: place_id, loop_token, + is_binding: bound_place_ids.contains(&place_id), + is_declaration: declared_place_ids.contains(&place_id), }; - // Note that `DefinitionKind::LoopHeader` doesn't shadow prior bindings. self.push_additional_definition(place_id, loop_header_ref); } } - (loop_token, bound_place_ids) + (loop_token, bound_place_ids, declared_place_ids) } - /// Build a `LoopHeader` that tracks all the variables bound in a loop, which will be visible - /// to uses in the same loop via "loop header definitions". We call this after merging control - /// flow from all the loop-back edges, most importantly at the end of the loop body, and also - /// at any `continue` statements. + /// Build a `LoopHeader` that tracks all the variables bound and/or declared in a loop, which + /// will be visible to uses in the same loop via "loop header definitions". We call this after + /// merging control flow from all the loop-back edges, most importantly at the end of the loop + /// body, and also at any `continue` statements. fn populate_loop_header( &mut self, - loop_header_places: &FxHashSet<ScopedPlaceId>, + bound_place_ids: &FxHashSet<ScopedPlaceId>, + declared_place_ids: &FxHashSet<ScopedPlaceId>, loop_token: LoopToken<'db>, ) { let mut loop_header = LoopHeader::new(); let use_def = self.current_use_def_map_mut(); // Collect bindings. - for place_id in loop_header_places { + for place_id in bound_place_ids { for live_binding in use_def.loop_back_bindings(*place_id) { loop_header.add_binding(*place_id, live_binding); } } + // Collect declarations. + for place_id in declared_place_ids { + for live_declaration in use_def.loop_back_declarations(*place_id) { + loop_header.add_declaration(*place_id, live_declaration); + } + } // Mark the reachability and narrowing constraints as used. - for place_id in loop_header_places { + for place_id in bound_place_ids { for live_binding in loop_header.bindings_for_place(*place_id) { use_def .reachability_constraints @@ -891,6 +932,13 @@ .mark_used(live_binding.narrowing_constraint); } } + for place_id in declared_place_ids { + for live_declaration in loop_header.declarations_for_place(*place_id) { + use_def + .reachability_constraints + .mark_used(live_declaration.reachability_constraint); + } + } // The `LoopHeader` needs to be visible to uses within the loop body that we've already // walked, but all our Salsa state is generally immutable. `specify` is how we work around // that. See this section of the Salsa docs: @@ -2172,17 +2220,18 @@ node_index: _, }, ) => { - // Pre-walk the loop to collect all the bound places, then create a loop header - // definition for each bound place. See `struct LoopHeader` for more on this. Loop + // Pre-walk the loop to collect all the bound and declared places, then create a + // loop header definition for each. See `struct LoopHeader` for more on this. Loop // header definitions stash a token to look up the `LoopHeader` later, so that we // can populate the header lazily. - let bound_places = loop_bindings_visitor::collect_while_loop_bindings(while_stmt); + let loop_defs = loop_bindings_visitor::collect_while_loop_definitions(while_stmt); let mut maybe_loop_header_info = None; - // Avoid allocating a `LoopToken` if there are no bound places in this loop. - if !bound_places.is_empty() { + // Avoid allocating a `LoopToken` if there are no definitions in this loop. + if !loop_defs.bound_places.is_empty() || !loop_defs.declared_places.is_empty() { maybe_loop_header_info = Some(self.synthesize_loop_header_definitions( LoopStmtRef::While(while_stmt), - bound_places, + loop_defs.bound_places, + loop_defs.declared_places, )); } @@ -2208,10 +2257,12 @@ self.flow_merge(continue_state); } - // Collect all the loop-back bindings (including the `continue` states we just - // merged) and populate the `LoopHeader`. - if let Some((loop_token, bound_place_ids)) = maybe_loop_header_info { - self.populate_loop_header(&bound_place_ids, loop_token); + // Collect all the loop-back bindings and declarations (including the `continue` + // states we just merged) and populate the `LoopHeader`. + if let Some((loop_token, bound_place_ids, declared_place_ids)) = + maybe_loop_header_info + { + self.populate_loop_header(&bound_place_ids, &declared_place_ids, loop_token); } // We execute the `else` branch once the condition evaluates to false. This could @@ -2289,17 +2340,18 @@ let pre_loop = self.flow_snapshot(); - // Pre-walk the loop to collect all the bound places, then create a loop header - // definition for each bound place. See `struct LoopHeader` for more on this. Loop + // Pre-walk the loop to collect all the bound and declared places, then create a + // loop header definition for each. See `struct LoopHeader` for more on this. Loop // header definitions stash a token to look up the `LoopHeader` later, so that we // can populate the header lazily. - let bound_places = loop_bindings_visitor::collect_for_loop_bindings(for_stmt); + let loop_defs = loop_bindings_visitor::collect_for_loop_definitions(for_stmt); let mut maybe_loop_header_info = None; - // Avoid allocating a `LoopToken` if there are no bound places in this loop. - if !bound_places.is_empty() { + // Avoid allocating a `LoopToken` if there are no definitions in this loop. + if !loop_defs.bound_places.is_empty() || !loop_defs.declared_places.is_empty() { maybe_loop_header_info = Some(self.synthesize_loop_header_definitions( LoopStmtRef::For(for_stmt), - bound_places, + loop_defs.bound_places, + loop_defs.declared_places, )); } @@ -2316,10 +2368,12 @@ self.flow_merge(continue_state); } - // Collect all the loop-back bindings (including the `continue` states we just - // merged) and populate the `LoopHeader`. - if let Some((loop_token, bound_place_ids)) = maybe_loop_header_info { - self.populate_loop_header(&bound_place_ids, loop_token); + // Collect all the loop-back bindings and declarations (including the `continue` + // states we just merged) and populate the `LoopHeader`. + if let Some((loop_token, bound_place_ids, declared_place_ids)) = + maybe_loop_header_info + { + self.populate_loop_header(&bound_place_ids, &declared_place_ids, loop_token); } // We may execute the `else` clause without ever executing the body, so merge in
diff --git a/crates/ty_python_semantic/src/semantic_index/builder/loop_bindings_visitor.rs b/crates/ty_python_semantic/src/semantic_index/builder/loop_bindings_visitor.rs index 7491857..929c9e9 100644 --- a/crates/ty_python_semantic/src/semantic_index/builder/loop_bindings_visitor.rs +++ b/crates/ty_python_semantic/src/semantic_index/builder/loop_bindings_visitor.rs
@@ -4,33 +4,46 @@ use crate::semantic_index::place::PlaceExpr; use crate::semantic_index::symbol::Symbol; -/// Do a pre-walk of a `while` loop to collect all the places that are bound, prior to visiting the -/// loop with `SemanticIndexBuilder`. This walk includes bindings in nested loops, but not in -/// nested scopes. (I.e. we don't descend into function bodies or class definitions.) We need this -/// pre-walk so that we can synthesize "loop header definitions" that are visible to the loop body -/// (and condition). See `LoopHeader`. +/// Collected definitions from a loop body: bound places and declared places. +pub(crate) struct LoopDefinitions { + pub(crate) bound_places: Vec<PlaceExpr>, + pub(crate) declared_places: Vec<PlaceExpr>, +} + +/// Do a pre-walk of a `while` loop to collect all the places that are bound and/or declared, prior +/// to visiting the loop with `SemanticIndexBuilder`. This walk includes definitions in nested +/// loops, but not in nested scopes. (I.e. we don't descend into function bodies or class +/// definitions.) We need this pre-walk so that we can synthesize "loop header definitions" that are +/// visible to the loop body (and condition). See `LoopHeader`. /// TODO: Handle `nonlocal` bindings from nested scopes somehow. -pub(crate) fn collect_while_loop_bindings(while_stmt: &ast::StmtWhile) -> Vec<PlaceExpr> { +pub(crate) fn collect_while_loop_definitions(while_stmt: &ast::StmtWhile) -> LoopDefinitions { let mut collector = LoopBindingsVisitor::default(); collector.visit_expr(&while_stmt.test); collector.visit_body(&while_stmt.body); - collector.bound_places + LoopDefinitions { + bound_places: collector.bound_places, + declared_places: collector.declared_places, + } } -/// Like `collect_while_loop_bindings` above, but for `for` loops. -pub(crate) fn collect_for_loop_bindings(for_stmt: &ast::StmtFor) -> Vec<PlaceExpr> { +/// Like `collect_while_loop_definitions` above, but for `for` loops. +pub(crate) fn collect_for_loop_definitions(for_stmt: &ast::StmtFor) -> LoopDefinitions { let mut collector = LoopBindingsVisitor::default(); collector.add_place_from_target(&for_stmt.target); collector.visit_body(&for_stmt.body); - collector.bound_places + LoopDefinitions { + bound_places: collector.bound_places, + declared_places: collector.declared_places, + } } -/// The visitor that powers `collect_while_loop_bindings` and `collect_for_loop_bindings`. +/// The visitor that powers `collect_while_loop_definitions` and `collect_for_loop_definitions`. /// /// This visitor doesn't walk nested function/class definitions since those are different scopes. #[derive(Debug, Default)] pub(crate) struct LoopBindingsVisitor { bound_places: Vec<PlaceExpr>, + declared_places: Vec<PlaceExpr>, } impl LoopBindingsVisitor { @@ -77,7 +90,12 @@ self.visit_expr(&node.value); } ast::Stmt::AnnAssign(node) => { + // An annotated assignment is always a declaration. + if let ast::Expr::Name(name) = &*node.target { + self.declared_places.push(PlaceExpr::from_expr_name(name)); + } if let Some(value) = &node.value { + // With a value, it's also a binding. self.add_place_from_target(&node.target); self.visit_expr(value); } @@ -118,27 +136,39 @@ ast::Stmt::Import(node) => { for alias in &node.names { let name = alias.asname.as_ref().unwrap_or(&alias.name); + // Imports are DeclarationAndBinding. self.bound_places .push(PlaceExpr::Symbol(Symbol::new(name.id.clone()))); + self.declared_places + .push(PlaceExpr::Symbol(Symbol::new(name.id.clone()))); } } ast::Stmt::ImportFrom(node) => { for alias in &node.names { if &*alias.name != "*" { let name = alias.asname.as_ref().unwrap_or(&alias.name); + // ImportFrom are DeclarationAndBinding. self.bound_places .push(PlaceExpr::Symbol(Symbol::new(name.id.clone()))); + self.declared_places + .push(PlaceExpr::Symbol(Symbol::new(name.id.clone()))); } } } ast::Stmt::FunctionDef(node) => { + // Function definitions are DeclarationAndBinding. self.bound_places .push(PlaceExpr::Symbol(Symbol::new(node.name.id.clone()))); + self.declared_places + .push(PlaceExpr::Symbol(Symbol::new(node.name.id.clone()))); // Don't descend into function bodies - they're different scopes. } ast::Stmt::ClassDef(node) => { + // Class definitions are DeclarationAndBinding. self.bound_places .push(PlaceExpr::Symbol(Symbol::new(node.name.id.clone()))); + self.declared_places + .push(PlaceExpr::Symbol(Symbol::new(node.name.id.clone()))); // Don't descend into class bodies - they're different scopes. } ast::Stmt::Match(node) => { @@ -200,7 +230,7 @@ use ruff_python_parser::parse_module; use ruff_python_trivia::textwrap::dedent; - // Test collecting `while` loop bindings. + // Test collecting `while` loop definitions. fn collect_while_loop_place_names(code: &str) -> Vec<String> { let parsed = parse_module(code).expect("valid Python code"); @@ -208,7 +238,24 @@ let ast::Stmt::While(while_stmt) = stmt else { panic!("Expected a while statement"); }; - collect_while_loop_bindings(while_stmt) + collect_while_loop_definitions(while_stmt) + .bound_places + .into_iter() + .map(|place| match place { + PlaceExpr::Symbol(sym) => sym.name().to_string(), + PlaceExpr::Member(member) => member.to_string(), + }) + .collect() + } + + fn collect_while_loop_declared_names(code: &str) -> Vec<String> { + let parsed = parse_module(code).expect("valid Python code"); + let stmt = &parsed.suite()[0]; + let ast::Stmt::While(while_stmt) = stmt else { + panic!("Expected a while statement"); + }; + collect_while_loop_definitions(while_stmt) + .declared_places .into_iter() .map(|place| match place { PlaceExpr::Symbol(sym) => sym.name().to_string(), @@ -264,7 +311,27 @@ assert_eq!(bindings, vec!["x", "y"]); } - // Test collecting `for` loop bindings. + #[test] + fn test_collect_while_loop_declarations() { + let declarations = collect_while_loop_declared_names(&dedent( + " + while True: + x: int = 1 + y: str + import mod_a + from pkg import name_b + def func(): ... + class MyClass: ... + z = 42 + ", + )); + assert_eq!( + declarations, + vec!["x", "y", "mod_a", "name_b", "func", "MyClass"] + ); + } + + // Test collecting `for` loop definitions. fn collect_for_loop_place_names(code: &str) -> Vec<String> { let parsed = parse_module(code).expect("valid Python code"); @@ -272,7 +339,8 @@ let ast::Stmt::For(for_stmt) = stmt else { panic!("Expected a for statement"); }; - collect_for_loop_bindings(for_stmt) + collect_for_loop_definitions(for_stmt) + .bound_places .into_iter() .map(|place| match place { PlaceExpr::Symbol(sym) => sym.name().to_string(), @@ -466,11 +534,34 @@ expected_bindings.insert(0, "for_loop_var"); } - let bindings = match loop_kind { - LoopKind::While => collect_while_loop_place_names(&code_snippet), - LoopKind::For => collect_for_loop_place_names(&code_snippet), + let definitions = match loop_kind { + LoopKind::While => { + let parsed = parse_module(&code_snippet).expect("valid Python code"); + let stmt = &parsed.suite()[0]; + let ast::Stmt::While(while_stmt) = stmt else { + panic!("Expected a while statement"); + }; + collect_while_loop_definitions(while_stmt) + } + LoopKind::For => { + let parsed = parse_module(&code_snippet).expect("valid Python code"); + let stmt = &parsed.suite()[0]; + let ast::Stmt::For(for_stmt) = stmt else { + panic!("Expected a for statement"); + }; + collect_for_loop_definitions(for_stmt) + } }; + let bindings: Vec<String> = definitions + .bound_places + .into_iter() + .map(|place| match place { + PlaceExpr::Symbol(sym) => sym.name().to_string(), + PlaceExpr::Member(member) => member.to_string(), + }) + .collect(); + assert_eq!(bindings, expected_bindings); } }
diff --git a/crates/ty_python_semantic/src/semantic_index/definition.rs b/crates/ty_python_semantic/src/semantic_index/definition.rs index 99bacd9..87a1057 100644 --- a/crates/ty_python_semantic/src/semantic_index/definition.rs +++ b/crates/ty_python_semantic/src/semantic_index/definition.rs
@@ -492,6 +492,8 @@ pub(crate) loop_stmt: LoopStmtRef<'ast>, pub(crate) place: ScopedPlaceId, pub(crate) loop_token: LoopToken<'db>, + pub(crate) is_binding: bool, + pub(crate) is_declaration: bool, } #[derive(Copy, Clone, Debug)] @@ -673,6 +675,8 @@ loop_stmt, place, loop_token, + is_binding, + is_declaration, }) => DefinitionKind::LoopHeader(LoopHeaderDefinitionKind { loop_token, loop_stmt: match loop_stmt { @@ -680,6 +684,8 @@ LoopStmtRef::For(stmt) => LoopStmtKind::For(AstNodeRef::new(parsed, stmt)), }, place, + is_binding, + is_declaration, }), } } @@ -1025,8 +1031,16 @@ | DefinitionKind::WithItem(_) | DefinitionKind::MatchPattern(_) | DefinitionKind::ImportFromSubmodule(_) - | DefinitionKind::ExceptHandler(_) - | DefinitionKind::LoopHeader(_) => DefinitionCategory::Binding, + | DefinitionKind::ExceptHandler(_) => DefinitionCategory::Binding, + DefinitionKind::LoopHeader(loop_header) => { + match (loop_header.is_binding, loop_header.is_declaration) { + (true, true) => DefinitionCategory::DeclarationAndBinding, + (true, false) => DefinitionCategory::Binding, + (false, true) => DefinitionCategory::Declaration, + // Every loop header should be at least one of binding or declaration. + (false, false) => DefinitionCategory::Binding, + } + } } } @@ -1367,6 +1381,8 @@ loop_token: LoopToken<'db>, loop_stmt: LoopStmtKind, place: ScopedPlaceId, + is_binding: bool, + is_declaration: bool, } #[derive(Clone, Debug, get_size2::GetSize)] @@ -1384,6 +1400,14 @@ self.place } + pub(crate) fn is_binding(&self) -> bool { + self.is_binding + } + + pub(crate) fn is_declaration(&self) -> bool { + self.is_declaration + } + pub(crate) fn range(&self, module: &ParsedModuleRef) -> TextRange { match &self.loop_stmt { LoopStmtKind::While(stmt) => stmt.node(module).range(),
diff --git a/crates/ty_python_semantic/src/semantic_index/use_def.rs b/crates/ty_python_semantic/src/semantic_index/use_def.rs index c0597e8..a901e7b 100644 --- a/crates/ty_python_semantic/src/semantic_index/use_def.rs +++ b/crates/ty_python_semantic/src/semantic_index/use_def.rs
@@ -259,8 +259,8 @@ use crate::semantic_index::scope::{FileScopeId, ScopeKind, ScopeLaziness}; use crate::semantic_index::symbol::ScopedSymbolId; use crate::semantic_index::use_def::place_state::{ - Bindings, Declarations, EnclosingSnapshot, LiveBindingsIterator, LiveDeclaration, - LiveDeclarationsIterator, PlaceState, + Bindings, Declarations, EnclosingSnapshot, LiveBindingsIterator, LiveDeclarationsIterator, + PlaceState, }; use crate::semantic_index::{EnclosingSnapshotResult, SemanticIndex}; use crate::types::{PossiblyNarrowedPlaces, Truthiness, Type}; @@ -268,7 +268,7 @@ mod place_state; pub(super) use place_state::PreviousDefinitions; -pub(crate) use place_state::{LiveBinding, ScopedDefinitionId}; +pub(crate) use place_state::{LiveBinding, LiveDeclaration, ScopedDefinitionId}; /// Applicable definitions and constraints for every use of a name. #[derive(Debug, PartialEq, Eq, salsa::Update, get_size2::GetSize)] @@ -1210,7 +1210,7 @@ self.bindings_by_definition .insert(declaration, place_state.bindings().clone()); - place_state.record_declaration(def_id, self.reachability); + place_state.record_declaration(def_id, self.reachability, PreviousDefinitions::AreShadowed); let definitions = match place { ScopedPlaceId::Symbol(symbol) => &mut self.reachable_symbol_definitions[symbol], @@ -1238,7 +1238,7 @@ ScopedPlaceId::Symbol(symbol) => &mut self.symbol_states[symbol], ScopedPlaceId::Member(member) => &mut self.member_states[member], }; - place_state.record_declaration(def_id, self.reachability); + place_state.record_declaration(def_id, self.reachability, PreviousDefinitions::AreShadowed); place_state.record_binding( def_id, self.reachability, @@ -1377,6 +1377,93 @@ bindings.iter().copied() } + /// Get a snapshot of the current declarations for a place. Like `loop_back_bindings` but for + /// declarations in the loop body that are visible via loop-back. + pub(super) fn loop_back_declarations( + &self, + place: ScopedPlaceId, + ) -> impl Iterator<Item = LiveDeclaration> + '_ { + let declarations = match place { + ScopedPlaceId::Symbol(symbol) => self.symbol_states[symbol].declarations(), + ScopedPlaceId::Member(member) => self.member_states[member].declarations(), + }; + + declarations.iter().copied() + } + + /// Record a loop header declaration. Doesn't shadow prior declarations. + pub(super) fn record_loop_header_declaration( + &mut self, + place: ScopedPlaceId, + declaration: Definition<'db>, + ) { + let def_id = self + .all_definitions + .push(DefinitionState::Defined(declaration)); + + let place_state = match place { + ScopedPlaceId::Symbol(symbol) => &mut self.symbol_states[symbol], + ScopedPlaceId::Member(member) => &mut self.member_states[member], + }; + + self.bindings_by_definition + .insert(declaration, place_state.bindings().clone()); + place_state.record_declaration(def_id, self.reachability, PreviousDefinitions::AreKept); + + let definitions = match place { + ScopedPlaceId::Symbol(symbol) => &mut self.reachable_symbol_definitions[symbol], + ScopedPlaceId::Member(member) => &mut self.reachable_member_definitions[member], + }; + + definitions.declarations.record_declaration( + def_id, + self.reachability, + PreviousDefinitions::AreKept, + ); + } + + /// Record a loop header that is both a declaration and binding. + /// Doesn't shadow prior definitions. + pub(super) fn record_loop_header_declaration_and_binding( + &mut self, + place: ScopedPlaceId, + definition: Definition<'db>, + ) { + let def_id = self + .all_definitions + .push(DefinitionState::Defined(definition)); + let place_state = match place { + ScopedPlaceId::Symbol(symbol) => &mut self.symbol_states[symbol], + ScopedPlaceId::Member(member) => &mut self.member_states[member], + }; + place_state.record_declaration(def_id, self.reachability, PreviousDefinitions::AreKept); + place_state.record_binding( + def_id, + self.reachability, + self.is_class_scope, + place.is_symbol(), + PreviousDefinitions::AreKept, + ); + + let reachable_definitions = match place { + ScopedPlaceId::Symbol(symbol) => &mut self.reachable_symbol_definitions[symbol], + ScopedPlaceId::Member(member) => &mut self.reachable_member_definitions[member], + }; + + reachable_definitions.declarations.record_declaration( + def_id, + self.reachability, + PreviousDefinitions::AreKept, + ); + reachable_definitions.bindings.record_binding( + def_id, + self.reachability, + self.is_class_scope, + place.is_symbol(), + PreviousDefinitions::AreKept, + ); + } + /// Restore the current builder places state to the given snapshot. pub(super) fn restore(&mut self, snapshot: FlowSnapshot) { // We never remove places from `place_states` (it's an IndexVec, and the place
diff --git a/crates/ty_python_semantic/src/semantic_index/use_def/place_state.rs b/crates/ty_python_semantic/src/semantic_index/use_def/place_state.rs index 71833f3..55660be 100644 --- a/crates/ty_python_semantic/src/semantic_index/use_def/place_state.rs +++ b/crates/ty_python_semantic/src/semantic_index/use_def/place_state.rs
@@ -78,10 +78,10 @@ } /// One of the live declarations for a single place at some point in control flow. -#[derive(Clone, Debug, PartialEq, Eq, get_size2::GetSize)] -pub(super) struct LiveDeclaration { - pub(super) declaration: ScopedDefinitionId, - pub(super) reachability_constraint: ScopedReachabilityConstraintId, +#[derive(Clone, Copy, Debug, PartialEq, Eq, get_size2::GetSize)] +pub(crate) struct LiveDeclaration { + pub(crate) declaration: ScopedDefinitionId, + pub(crate) reachability_constraint: ScopedReachabilityConstraintId, } pub(super) type LiveDeclarationsIterator<'a> = std::slice::Iter<'a, LiveDeclaration>; @@ -419,11 +419,12 @@ &mut self, declaration_id: ScopedDefinitionId, reachability_constraint: ScopedReachabilityConstraintId, + previous_definitions: PreviousDefinitions, ) { self.declarations.record_declaration( declaration_id, reachability_constraint, - PreviousDefinitions::AreShadowed, + previous_definitions, ); } @@ -651,6 +652,7 @@ sym.record_declaration( ScopedDefinitionId::from_u32(1), ScopedReachabilityConstraintId::ALWAYS_TRUE, + PreviousDefinitions::AreShadowed, ); assert_declarations(&sym, &["1"]); @@ -662,10 +664,12 @@ sym.record_declaration( ScopedDefinitionId::from_u32(1), ScopedReachabilityConstraintId::ALWAYS_TRUE, + PreviousDefinitions::AreShadowed, ); sym.record_declaration( ScopedDefinitionId::from_u32(2), ScopedReachabilityConstraintId::ALWAYS_TRUE, + PreviousDefinitions::AreShadowed, ); assert_declarations(&sym, &["2"]); @@ -678,12 +682,14 @@ sym.record_declaration( ScopedDefinitionId::from_u32(1), ScopedReachabilityConstraintId::ALWAYS_TRUE, + PreviousDefinitions::AreShadowed, ); let mut sym2 = PlaceState::undefined(ScopedReachabilityConstraintId::ALWAYS_TRUE); sym2.record_declaration( ScopedDefinitionId::from_u32(2), ScopedReachabilityConstraintId::ALWAYS_TRUE, + PreviousDefinitions::AreShadowed, ); sym.merge(sym2, &mut reachability_constraints); @@ -698,6 +704,7 @@ sym.record_declaration( ScopedDefinitionId::from_u32(1), ScopedReachabilityConstraintId::ALWAYS_TRUE, + PreviousDefinitions::AreShadowed, ); let sym2 = PlaceState::undefined(ScopedReachabilityConstraintId::ALWAYS_TRUE);
diff --git a/crates/ty_python_semantic/src/types/infer/builder.rs b/crates/ty_python_semantic/src/types/infer/builder.rs index 36ff0f0..df4fdcd 100644 --- a/crates/ty_python_semantic/src/types/infer/builder.rs +++ b/crates/ty_python_semantic/src/types/infer/builder.rs
@@ -136,8 +136,8 @@ TypeAliasType, TypeAndQualifiers, TypeContext, TypeQualifiers, TypeVarBoundOrConstraints, TypeVarBoundOrConstraintsEvaluation, TypeVarConstraints, TypeVarDefaultEvaluation, TypeVarIdentity, TypeVarInstance, TypeVarKind, TypeVarVariance, TypedDictType, UnionBuilder, - UnionType, UnionTypeInstance, any_over_type, binding_type, definition_expression_type, - infer_complete_scope_types, infer_scope_types, todo_type, + UnionType, UnionTypeInstance, any_over_type, binding_type, declaration_type, + definition_expression_type, infer_complete_scope_types, infer_scope_types, todo_type, }; use crate::types::{CallableTypes, overrides}; use crate::types::{ClassBase, add_inferred_python_version_hint_to_diagnostic}; @@ -5027,37 +5027,71 @@ .index .use_def_map(self.scope().file_scope_id(self.db())); - let mut union = UnionBuilder::new(db).recursively_defined(RecursivelyDefined::Yes); + if loop_header_kind.is_binding() { + let mut union = UnionBuilder::new(db).recursively_defined(RecursivelyDefined::Yes); - for live_binding in loop_header.bindings_for_place(place) { - // Skip unreachable bindings. - if !use_def.is_reachable(db, live_binding.reachability_constraint) { - continue; + for live_binding in loop_header.bindings_for_place(place) { + // Skip unreachable bindings. + if !use_def.is_reachable(db, live_binding.reachability_constraint) { + continue; + } + + // Boundness analysis is handled by looking at these bindings again in + // `place_from_bindings_impl`. Here we're only concerned with the type. + let def_state = use_def.definition(live_binding.binding); + let def = match def_state { + DefinitionState::Defined(def) => def, + DefinitionState::Deleted | DefinitionState::Undefined => continue, + }; + + // This loop header is visible to itself. Filter it out to avoid a pointless cycle. + if def == definition { + continue; + } + + let binding_ty = binding_type(db, def); + let narrowed_ty = use_def + .narrowing_evaluator(live_binding.narrowing_constraint) + .narrow(db, binding_ty, place); + + union.add_in_place(narrowed_ty); } - // Boundness analysis is handled by looking at these bindings again in - // `place_from_bindings_impl`. Here we're only concerned with the type. - let def_state = use_def.definition(live_binding.binding); - let def = match def_state { - DefinitionState::Defined(def) => def, - DefinitionState::Deleted | DefinitionState::Undefined => continue, - }; - - // This loop header is visible to itself. Filter it out to avoid a pointless cycle. - if def == definition { - continue; - } - - let binding_ty = binding_type(db, def); - let narrowed_ty = use_def - .narrowing_evaluator(live_binding.narrowing_constraint) - .narrow(db, binding_ty, place); - - union.add_in_place(narrowed_ty); + self.bindings + .insert(definition, union.build(), self.multi_inference_state); } - self.bindings - .insert(definition, union.build(), self.multi_inference_state); + if loop_header_kind.is_declaration() { + let mut union = UnionBuilder::new(db); + let mut qualifiers = TypeQualifiers::empty(); + + for live_declaration in loop_header.declarations_for_place(place) { + // Skip unreachable declarations. + if !use_def.is_reachable(db, live_declaration.reachability_constraint) { + continue; + } + + let def_state = use_def.definition(live_declaration.declaration); + let def = match def_state { + DefinitionState::Defined(def) => def, + DefinitionState::Deleted | DefinitionState::Undefined => continue, + }; + + // This loop header is visible to itself. Filter it out to avoid a pointless cycle. + if def == definition { + continue; + } + + let ty = declaration_type(db, def); + union.add_in_place(ty.inner_type()); + qualifiers |= ty.qualifiers(); + } + + let declared_ty = + TypeAndQualifiers::new(union.build(), TypeOrigin::Declared, qualifiers); + self.declarations + .insert(definition, declared_ty, self.multi_inference_state); + } } fn infer_match_statement(&mut self, match_statement: &ast::StmtMatch) {