diff --git a/crates/ty_python_semantic/resources/mdtest/attributes.md b/crates/ty_python_semantic/resources/mdtest/attributes.md index 993bf64594..22d6663323 100644 --- a/crates/ty_python_semantic/resources/mdtest/attributes.md +++ b/crates/ty_python_semantic/resources/mdtest/attributes.md @@ -338,7 +338,7 @@ class C: for _, self.y in TupleIterable(): pass - # TODO: We should emit a diagnostic here + # error: [not-iterable] for self.z in NonIterable(): pass diff --git a/crates/ty_python_semantic/resources/mdtest/comprehensions/basic.md b/crates/ty_python_semantic/resources/mdtest/comprehensions/basic.md index a32244c09a..6a7c08cfdc 100644 --- a/crates/ty_python_semantic/resources/mdtest/comprehensions/basic.md +++ b/crates/ty_python_semantic/resources/mdtest/comprehensions/basic.md @@ -149,4 +149,7 @@ class Iterable: async def _(): # revealed: @Todo(async iterables/iterators) [reveal_type(x) async for x in Iterable()] + # revealed: @Todo(async iterables/iterators) + # revealed: @Todo(async iterables/iterators) + [(reveal_type(x), reveal_type(y)) async for x in Iterable() async for y in Iterable()] ``` diff --git a/crates/ty_python_semantic/resources/mdtest/unpacking.md b/crates/ty_python_semantic/resources/mdtest/unpacking.md index 1628610800..339b8f008b 100644 --- a/crates/ty_python_semantic/resources/mdtest/unpacking.md +++ b/crates/ty_python_semantic/resources/mdtest/unpacking.md @@ -858,4 +858,6 @@ Unpacking an empty tuple or list shouldn't raise any diagnostics. () = () [] = () () = [] +[] = [] = [] +() = () = () ``` diff --git a/crates/ty_python_semantic/src/semantic_index/builder.rs b/crates/ty_python_semantic/src/semantic_index/builder.rs index 1fc3e3dd1f..1a450ef8eb 100644 --- a/crates/ty_python_semantic/src/semantic_index/builder.rs +++ b/crates/ty_python_semantic/src/semantic_index/builder.rs @@ -32,8 +32,8 @@ use crate::semantic_index::definition::{ }; use crate::semantic_index::expression::{Expression, ExpressionKind}; use crate::semantic_index::place::{ - FileScopeId, NodeWithScopeKey, NodeWithScopeKind, NodeWithScopeRef, PlaceExpr, - PlaceExprWithFlags, PlaceTableBuilder, Scope, ScopeId, ScopeKind, ScopedPlaceId, + FileScopeId, NodeWithScopeKey, NodeWithScopeKind, NodeWithScopeRef, PlaceExprWithFlags, + PlaceTableBuilder, Scope, ScopeId, ScopeKind, ScopedPlaceId, }; use crate::semantic_index::predicate::{ PatternPredicate, PatternPredicateKind, Predicate, PredicateNode, ScopedPredicateId, @@ -1872,11 +1872,9 @@ impl<'ast> Visitor<'ast> for SemanticIndexBuilder<'_, 'ast> { // We will check the target expressions and then delete them. walk_stmt(self, stmt); for target in targets { - if let Ok(target) = PlaceExpr::try_from(target) { - let place_id = self.add_place(PlaceExprWithFlags::new(target)); - self.current_place_table().mark_place_used(place_id); - self.delete_binding(place_id); - } + let place_id = self.add_place(PlaceExprWithFlags::new(target)); + self.current_place_table().mark_place_used(place_id); + self.delete_binding(place_id); } } ast::Stmt::Expr(ast::StmtExpr { @@ -1908,126 +1906,125 @@ impl<'ast> Visitor<'ast> for SemanticIndexBuilder<'_, 'ast> { ast::Expr::Name(ast::ExprName { ctx, .. }) | ast::Expr::Attribute(ast::ExprAttribute { ctx, .. }) | ast::Expr::Subscript(ast::ExprSubscript { ctx, .. }) => { - if let Ok(place_expr) = PlaceExpr::try_from(expr) { - let mut place_expr = PlaceExprWithFlags::new(place_expr); - if self.is_method_of_class().is_some() - && place_expr.is_instance_attribute_candidate() - { - // We specifically mark attribute assignments to the first parameter of a method, - // i.e. typically `self` or `cls`. - let accessed_object_refers_to_first_parameter = self - .current_first_parameter_name - .is_some_and(|fst| place_expr.expr.root_name() == fst); + let mut place_expr = PlaceExprWithFlags::new(expr); + if self.is_method_of_class().is_some() + && place_expr.is_instance_attribute_candidate() + { + // We specifically mark attribute assignments to the first parameter of a method, + // i.e. typically `self` or `cls`. + let accessed_object_refers_to_first_parameter = + self.current_first_parameter_name.is_some_and(|fst| { + place_expr.expr.root_name().is_some_and(|name| name == fst) + }); - if accessed_object_refers_to_first_parameter && place_expr.is_member() { - place_expr.mark_instance_attribute(); + if accessed_object_refers_to_first_parameter && place_expr.is_member() { + place_expr.mark_instance_attribute(); + } + } + + let (is_use, is_definition) = match (ctx, self.current_assignment()) { + (ast::ExprContext::Store, Some(CurrentAssignment::AugAssign(_))) => { + // For augmented assignment, the target expression is also used. + (true, true) + } + (ast::ExprContext::Load, _) => (true, false), + (ast::ExprContext::Store, _) => (false, true), + (ast::ExprContext::Del, _) => (true, true), + (ast::ExprContext::Invalid, _) => (false, false), + }; + let place_id = self.add_place(place_expr); + + if is_use { + self.mark_place_used(place_id); + let use_id = self.current_ast_ids().record_use(expr); + self.current_use_def_map_mut() + .record_use(place_id, use_id, node_key); + } + + if is_definition { + match self.current_assignment() { + Some(CurrentAssignment::Assign { node, unpack }) => { + self.add_definition( + place_id, + AssignmentDefinitionNodeRef { + unpack, + value: &node.value, + target: expr, + }, + ); } - } - - let (is_use, is_definition) = match (ctx, self.current_assignment()) { - (ast::ExprContext::Store, Some(CurrentAssignment::AugAssign(_))) => { - // For augmented assignment, the target expression is also used. - (true, true) + Some(CurrentAssignment::AnnAssign(ann_assign)) => { + self.add_standalone_type_expression(&ann_assign.annotation); + self.add_definition( + place_id, + AnnotatedAssignmentDefinitionNodeRef { + node: ann_assign, + annotation: &ann_assign.annotation, + value: ann_assign.value.as_deref(), + target: expr, + }, + ); } - (ast::ExprContext::Load, _) => (true, false), - (ast::ExprContext::Store, _) => (false, true), - (ast::ExprContext::Del, _) => (true, true), - (ast::ExprContext::Invalid, _) => (false, false), - }; - let place_id = self.add_place(place_expr); - - if is_use { - self.mark_place_used(place_id); - let use_id = self.current_ast_ids().record_use(expr); - self.current_use_def_map_mut() - .record_use(place_id, use_id, node_key); - } - - if is_definition { - match self.current_assignment() { - Some(CurrentAssignment::Assign { node, unpack }) => { - self.add_definition( - place_id, - AssignmentDefinitionNodeRef { - unpack, - value: &node.value, - target: expr, - }, - ); - } - Some(CurrentAssignment::AnnAssign(ann_assign)) => { - self.add_standalone_type_expression(&ann_assign.annotation); - self.add_definition( - place_id, - AnnotatedAssignmentDefinitionNodeRef { - node: ann_assign, - annotation: &ann_assign.annotation, - value: ann_assign.value.as_deref(), - target: expr, - }, - ); - } - Some(CurrentAssignment::AugAssign(aug_assign)) => { - self.add_definition(place_id, aug_assign); - } - Some(CurrentAssignment::For { node, unpack }) => { - self.add_definition( - place_id, - ForStmtDefinitionNodeRef { - unpack, - iterable: &node.iter, - target: expr, - is_async: node.is_async, - }, - ); - } - Some(CurrentAssignment::Named(named)) => { - // TODO(dhruvmanila): If the current scope is a comprehension, then the - // named expression is implicitly nonlocal. This is yet to be - // implemented. - self.add_definition(place_id, named); - } - Some(CurrentAssignment::Comprehension { - unpack, - node, - first, - }) => { - self.add_definition( - place_id, - ComprehensionDefinitionNodeRef { - unpack, - iterable: &node.iter, - target: expr, - first, - is_async: node.is_async, - }, - ); - } - Some(CurrentAssignment::WithItem { - item, - is_async, - unpack, - }) => { - self.add_definition( - place_id, - WithItemDefinitionNodeRef { - unpack, - context_expr: &item.context_expr, - target: expr, - is_async, - }, - ); - } - None => {} + Some(CurrentAssignment::AugAssign(aug_assign)) => { + self.add_definition(place_id, aug_assign); } + Some(CurrentAssignment::For { node, unpack }) => { + self.add_definition( + place_id, + ForStmtDefinitionNodeRef { + unpack, + iterable: &node.iter, + target: expr, + is_async: node.is_async, + }, + ); + } + Some(CurrentAssignment::Named(named)) => { + // TODO(dhruvmanila): If the current scope is a comprehension, then the + // named expression is implicitly nonlocal. This is yet to be + // implemented. + self.add_definition(place_id, named); + } + Some(CurrentAssignment::Comprehension { + unpack, + node, + first, + }) => { + self.add_definition( + place_id, + ComprehensionDefinitionNodeRef { + unpack, + iterable: &node.iter, + target: expr, + first, + is_async: node.is_async, + }, + ); + } + Some(CurrentAssignment::WithItem { + item, + is_async, + unpack, + }) => { + self.add_definition( + place_id, + WithItemDefinitionNodeRef { + unpack, + context_expr: &item.context_expr, + target: expr, + is_async, + }, + ); + } + None => {} } + } - if let Some(unpack_position) = self - .current_assignment_mut() - .and_then(CurrentAssignment::unpack_position_mut) - { - *unpack_position = UnpackPosition::Other; - } + if let Some(unpack_position) = self + .current_assignment_mut() + .and_then(CurrentAssignment::unpack_position_mut) + { + *unpack_position = UnpackPosition::Other; } // Track reachability of attribute expressions to silence `unresolved-attribute` diff --git a/crates/ty_python_semantic/src/semantic_index/place.rs b/crates/ty_python_semantic/src/semantic_index/place.rs index 6b26cd4b7f..4585f612a7 100644 --- a/crates/ty_python_semantic/src/semantic_index/place.rs +++ b/crates/ty_python_semantic/src/semantic_index/place.rs @@ -1,4 +1,3 @@ -use std::convert::Infallible; use std::hash::{Hash, Hasher}; use std::ops::Range; @@ -7,25 +6,80 @@ use hashbrown::hash_table::Entry; use ruff_db::files::File; use ruff_db::parsed::ParsedModuleRef; use ruff_index::{IndexVec, newtype_index}; -use ruff_python_ast as ast; use ruff_python_ast::name::Name; +use ruff_python_ast::{self as ast, ExprRef}; use rustc_hash::FxHasher; use smallvec::{SmallVec, smallvec}; use crate::Db; use crate::ast_node_ref::AstNodeRef; use crate::node_key::NodeKey; +use crate::semantic_index::ast_ids::node_key::ExpressionNodeKey; use crate::semantic_index::reachability_constraints::ScopedReachabilityConstraintId; use crate::semantic_index::{PlaceSet, SemanticIndex, semantic_index}; +/// The root of a place expression. +#[derive(Debug, PartialEq, Eq)] +pub(crate) enum PlaceRoot { + /// A name, e.g. `x` in `x = 1` or `x.y = 1` + Name(Name), + /// An arbitrary expression, e.g. `a()` in `a() = 1` or `a().b = 1` + Expr(ExpressionNodeKey), +} + +impl PlaceRoot { + const fn is_name(&self) -> bool { + matches!(self, PlaceRoot::Name(_)) + } + + pub(crate) const fn as_name(&self) -> Option<&Name> { + match self { + PlaceRoot::Name(name) => Some(name), + PlaceRoot::Expr(_) => None, + } + } + + #[track_caller] + fn expect_name(&self) -> &Name { + match self { + PlaceRoot::Name(name) => name, + PlaceRoot::Expr(_) => panic!("expected PlaceRoot::Name, found PlaceRoot::Expr"), + } + } +} + +impl From<&ast::Expr> for PlaceRoot { + fn from(expr: &ast::Expr) -> Self { + match expr { + ast::Expr::Name(name) => PlaceRoot::Name(name.id.clone()), + _ => PlaceRoot::Expr(ExpressionNodeKey::from(expr)), + } + } +} + +impl std::fmt::Display for PlaceRoot { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + PlaceRoot::Name(name) => write!(f, "{name}"), + // TODO: How to display this? + PlaceRoot::Expr(_) => f.write_str(""), + } + } +} + #[derive(Debug, Clone, PartialEq, Eq, Hash)] pub(crate) enum PlaceExprSubSegment { - /// A member access, e.g. `.y` in `x.y` - Member(ast::name::Name), + /// A name member access, e.g. `.y` in `x.y` + Member(Name), /// An integer-based index access, e.g. `[1]` in `x[1]` IntSubscript(ast::Int), /// A string-based index access, e.g. `["foo"]` in `x["foo"]` StringSubscript(String), + /// An arbitrary subscript, e.g. `[y]` in `x[y]` + // TODO: Specific expressions that are valid in this position can be extracted as separate enum + // variants in the future to provide precise information like `NameSubscript` for the above + // example. + AnySubscript(ExpressionNodeKey), } impl PlaceExprSubSegment { @@ -40,137 +94,33 @@ impl PlaceExprSubSegment { /// An expression that can be the target of a `Definition`. #[derive(Eq, PartialEq, Debug)] pub struct PlaceExpr { - root_name: Name, + /// The root of the place expression, which can be a [`Name`] or any arbitrary expression. + root: PlaceRoot, + + /// The sub-segments of the place expression. + /// + /// This is relevant only for attribute and subscript expressions, e.g. `x.y` or `x[0].y`. This + /// will be empty for simple names like `x` or `y`. sub_segments: SmallVec<[PlaceExprSubSegment; 1]>, } -impl std::fmt::Display for PlaceExpr { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - write!(f, "{}", self.root_name)?; - for segment in &self.sub_segments { - match segment { - PlaceExprSubSegment::Member(name) => write!(f, ".{name}")?, - PlaceExprSubSegment::IntSubscript(int) => write!(f, "[{int}]")?, - PlaceExprSubSegment::StringSubscript(string) => write!(f, "[\"{string}\"]")?, - } - } - Ok(()) - } -} - -impl TryFrom<&ast::name::Name> for PlaceExpr { - type Error = Infallible; - - fn try_from(name: &ast::name::Name) -> Result { - Ok(PlaceExpr::name(name.clone())) - } -} - -impl TryFrom for PlaceExpr { - type Error = Infallible; - - fn try_from(name: ast::name::Name) -> Result { - Ok(PlaceExpr::name(name)) - } -} - -impl TryFrom<&ast::ExprAttribute> for PlaceExpr { - type Error = (); - - fn try_from(attr: &ast::ExprAttribute) -> Result { - let mut place = PlaceExpr::try_from(&*attr.value)?; - place - .sub_segments - .push(PlaceExprSubSegment::Member(attr.attr.id.clone())); - Ok(place) - } -} - -impl TryFrom for PlaceExpr { - type Error = (); - - fn try_from(attr: ast::ExprAttribute) -> Result { - let mut place = PlaceExpr::try_from(&*attr.value)?; - place - .sub_segments - .push(PlaceExprSubSegment::Member(attr.attr.id)); - Ok(place) - } -} - -impl TryFrom<&ast::ExprSubscript> for PlaceExpr { - type Error = (); - - fn try_from(subscript: &ast::ExprSubscript) -> Result { - let mut place = PlaceExpr::try_from(&*subscript.value)?; - match &*subscript.slice { - ast::Expr::NumberLiteral(ast::ExprNumberLiteral { - value: ast::Number::Int(index), - .. - }) => { - place - .sub_segments - .push(PlaceExprSubSegment::IntSubscript(index.clone())); - } - ast::Expr::StringLiteral(string) => { - place - .sub_segments - .push(PlaceExprSubSegment::StringSubscript( - string.value.to_string(), - )); - } - _ => { - return Err(()); - } - } - Ok(place) - } -} - -impl TryFrom for PlaceExpr { - type Error = (); - - fn try_from(subscript: ast::ExprSubscript) -> Result { - PlaceExpr::try_from(&subscript) - } -} - -impl TryFrom<&ast::Expr> for PlaceExpr { - type Error = (); - - fn try_from(expr: &ast::Expr) -> Result { - match expr { - ast::Expr::Name(name) => Ok(PlaceExpr::name(name.id.clone())), - ast::Expr::Attribute(attr) => PlaceExpr::try_from(attr), - ast::Expr::Subscript(subscript) => PlaceExpr::try_from(subscript), - _ => Err(()), - } - } -} - -impl TryFrom> for PlaceExpr { - type Error = (); - - fn try_from(expr: ast::ExprRef) -> Result { - match expr { - ast::ExprRef::Name(name) => Ok(PlaceExpr::name(name.id.clone())), - ast::ExprRef::Attribute(attr) => PlaceExpr::try_from(attr), - ast::ExprRef::Subscript(subscript) => PlaceExpr::try_from(subscript), - _ => Err(()), - } - } -} - impl PlaceExpr { pub(crate) fn name(name: Name) -> Self { Self { - root_name: name, + root: PlaceRoot::Name(name), sub_segments: smallvec![], } } - pub(crate) fn root_name(&self) -> &Name { - &self.root_name + fn from_expr(expr: ExpressionNodeKey) -> Self { + Self { + root: PlaceRoot::Expr(expr), + sub_segments: smallvec![], + } + } + + pub(super) fn root_name(&self) -> Option<&Name> { + self.root.as_name() } pub(crate) fn sub_segments(&self) -> &[PlaceExprSubSegment] { @@ -179,26 +129,26 @@ impl PlaceExpr { pub(crate) fn as_name(&self) -> Option<&Name> { if self.is_name() { - Some(&self.root_name) + Some(self.root.expect_name()) } else { None } } - /// Assumes that the place expression is a name. + /// Assumes that the place expression is a name, panics if it is not. #[track_caller] pub(crate) fn expect_name(&self) -> &Name { debug_assert_eq!(self.sub_segments.len(), 0); - &self.root_name + self.root.expect_name() } /// Is the place just a name? pub fn is_name(&self) -> bool { - self.sub_segments.is_empty() + self.root.is_name() && self.sub_segments.is_empty() } pub fn is_name_and(&self, f: impl FnOnce(&str) -> bool) -> bool { - self.is_name() && f(&self.root_name) + self.as_name().is_some_and(|name| f(name.as_str())) } /// Does the place expression have the form `.member`? @@ -208,6 +158,19 @@ impl PlaceExpr { .is_some_and(|last| last.as_member().is_some()) } + /// Returns `true` if this is a valid place expression that can be used in a definition. + pub(crate) fn is_valid(&self) -> bool { + self.root.is_name() + && self.sub_segments.iter().all(|segment| { + matches!( + segment, + PlaceExprSubSegment::Member(_) + | PlaceExprSubSegment::IntSubscript(_) + | PlaceExprSubSegment::StringSubscript(_) + ) + }) + } + fn root_exprs(&self) -> RootExprs<'_> { RootExprs { expr_ref: self.into(), @@ -216,6 +179,77 @@ impl PlaceExpr { } } +impl From<&ast::Expr> for PlaceExpr { + fn from(expr: &ast::Expr) -> Self { + match expr { + ast::Expr::Name(name) => PlaceExpr::name(name.id.clone()), + ast::Expr::Attribute(attr) => PlaceExpr::from(attr), + ast::Expr::Subscript(subscript) => PlaceExpr::from(subscript), + _ => PlaceExpr::from_expr(ExpressionNodeKey::from(expr)), + } + } +} + +impl From> for PlaceExpr { + fn from(expr: ExprRef<'_>) -> Self { + match expr { + ExprRef::Name(name) => PlaceExpr::name(name.id.clone()), + ExprRef::Attribute(attr) => PlaceExpr::from(attr), + ExprRef::Subscript(subscript) => PlaceExpr::from(subscript), + _ => PlaceExpr::from_expr(ExpressionNodeKey::from(expr)), + } + } +} + +impl From<&ast::name::Name> for PlaceExpr { + fn from(name: &ast::name::Name) -> Self { + PlaceExpr::name(name.clone()) + } +} + +impl From<&ast::ExprAttribute> for PlaceExpr { + fn from(value: &ast::ExprAttribute) -> Self { + let mut place = PlaceExpr::from(&*value.value); + place + .sub_segments + .push(PlaceExprSubSegment::Member(value.attr.id.clone())); + place + } +} + +impl From<&ast::ExprSubscript> for PlaceExpr { + fn from(subscript: &ast::ExprSubscript) -> Self { + let mut place = PlaceExpr::from(&*subscript.value); + place.sub_segments.push(match &*subscript.slice { + ast::Expr::NumberLiteral(ast::ExprNumberLiteral { + value: ast::Number::Int(index), + .. + }) => PlaceExprSubSegment::IntSubscript(index.clone()), + ast::Expr::StringLiteral(string) => { + PlaceExprSubSegment::StringSubscript(string.value.to_string()) + } + expr => PlaceExprSubSegment::AnySubscript(ExpressionNodeKey::from(expr)), + }); + place + } +} + +impl std::fmt::Display for PlaceExpr { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!(f, "{}", self.root)?; + for segment in &self.sub_segments { + match segment { + PlaceExprSubSegment::Member(name) => write!(f, ".{name}")?, + PlaceExprSubSegment::IntSubscript(int) => write!(f, "[{int}]")?, + PlaceExprSubSegment::StringSubscript(string) => write!(f, "[\"{string}\"]")?, + // TODO: How to display this? Raw text or source generator? + PlaceExprSubSegment::AnySubscript(_) => write!(f, "[]")?, + } + } + Ok(()) + } +} + /// A [`PlaceExpr`] with flags, e.g. whether it is used, bound, an instance attribute, etc. #[derive(Eq, PartialEq, Debug)] pub struct PlaceExprWithFlags { @@ -230,9 +264,9 @@ impl std::fmt::Display for PlaceExprWithFlags { } impl PlaceExprWithFlags { - pub(crate) fn new(expr: PlaceExpr) -> Self { + pub(crate) fn new(expr: &ast::Expr) -> Self { PlaceExprWithFlags { - expr, + expr: PlaceExpr::from(expr), flags: PlaceFlags::empty(), } } @@ -263,8 +297,8 @@ impl PlaceExprWithFlags { /// parameter of the method (i.e. `self`). To answer those questions, /// use [`Self::as_instance_attribute`]. pub(super) fn as_instance_attribute_candidate(&self) -> Option<&Name> { - if self.expr.sub_segments.len() == 1 { - self.expr.sub_segments[0].as_member() + if let [sub_segment] = self.expr.sub_segments.as_slice() { + sub_segment.as_member() } else { None } @@ -340,28 +374,53 @@ impl PlaceExprWithFlags { } #[derive(Clone, Copy, Eq, PartialEq, Debug, Hash)] -pub struct PlaceExprRef<'a> { - pub(crate) root_name: &'a Name, +pub(crate) enum PlaceRootRef<'a> { + Name(&'a Name), + Expr(ExpressionNodeKey), +} + +impl<'a> From<&'a PlaceRoot> for PlaceRootRef<'a> { + fn from(root: &'a PlaceRoot) -> Self { + match root { + PlaceRoot::Name(name) => PlaceRootRef::Name(name), + PlaceRoot::Expr(expr) => PlaceRootRef::Expr(*expr), + } + } +} + +impl PartialEq for PlaceRootRef<'_> { + fn eq(&self, other: &PlaceRoot) -> bool { + match (self, other) { + (PlaceRootRef::Name(name), PlaceRoot::Name(other_name)) => name == &other_name, + (PlaceRootRef::Expr(expr), PlaceRoot::Expr(other_expr)) => expr == other_expr, + _ => false, + } + } +} + +#[derive(Clone, Copy, Eq, PartialEq, Debug, Hash)] +pub(crate) struct PlaceExprRef<'a> { + pub(crate) root: PlaceRootRef<'a>, /// Sub-segments is empty for a simple target (e.g. `foo`). pub(crate) sub_segments: &'a [PlaceExprSubSegment], } impl PartialEq for PlaceExprRef<'_> { fn eq(&self, other: &PlaceExpr) -> bool { - self.root_name == &other.root_name && self.sub_segments == &other.sub_segments[..] + self.root == other.root && self.sub_segments == &other.sub_segments[..] } } impl PartialEq> for PlaceExpr { fn eq(&self, other: &PlaceExprRef<'_>) -> bool { - &self.root_name == other.root_name && &self.sub_segments[..] == other.sub_segments + other.root == self.root && &self.sub_segments[..] == other.sub_segments } } impl<'e> From<&'e PlaceExpr> for PlaceExprRef<'e> { fn from(expr: &'e PlaceExpr) -> Self { PlaceExprRef { - root_name: &expr.root_name, + root: PlaceRootRef::from(&expr.root), sub_segments: &expr.sub_segments, } } @@ -381,7 +440,7 @@ impl<'e> Iterator for RootExprs<'e> { } self.len -= 1; Some(PlaceExprRef { - root_name: self.expr_ref.root_name, + root: self.expr_ref.root, sub_segments: &self.expr_ref.sub_segments[..self.len], }) } @@ -713,7 +772,10 @@ impl PlaceTable { // Special case for simple names (e.g. "foo"). Only hash the name so // that a lookup by name can find it (see `place_by_name`). if place_expr.sub_segments.is_empty() { - place_expr.root_name.as_str().hash(&mut hasher); + match place_expr.root { + PlaceRootRef::Name(name) => name.as_str().hash(&mut hasher), + PlaceRootRef::Expr(expr) => expr.hash(&mut hasher), + } } else { place_expr.hash(&mut hasher); } diff --git a/crates/ty_python_semantic/src/semantic_model.rs b/crates/ty_python_semantic/src/semantic_model.rs index d6795e6d92..0f0264fa0b 100644 --- a/crates/ty_python_semantic/src/semantic_model.rs +++ b/crates/ty_python_semantic/src/semantic_model.rs @@ -122,7 +122,10 @@ impl HasType for ast::ExprRef<'_> { let scope = file_scope.to_scope_id(model.db, model.file); let expression_id = self.scoped_expression_id(model.db, scope); - infer_scope_types(model.db, scope).expression_type(expression_id) + tracing::info!("ExprRef: {:?}", self); + let ty = infer_scope_types(model.db, scope).expression_type(expression_id); + tracing::info!("Inferred type: {}", ty.display(model.db)); + ty } } diff --git a/crates/ty_python_semantic/src/types/infer.rs b/crates/ty_python_semantic/src/types/infer.rs index 4cfa98c2a9..e533a13564 100644 --- a/crates/ty_python_semantic/src/types/infer.rs +++ b/crates/ty_python_semantic/src/types/infer.rs @@ -358,8 +358,8 @@ fn unpack_cycle_recover<'db>( salsa::CycleRecoveryAction::Iterate } -fn unpack_cycle_initial<'db>(_db: &'db dyn Db, _unpack: Unpack<'db>) -> UnpackResult<'db> { - UnpackResult::cycle_fallback(Type::Never) +fn unpack_cycle_initial<'db>(db: &'db dyn Db, unpack: Unpack<'db>) -> UnpackResult<'db> { + UnpackResult::cycle_fallback(unpack.value_scope(db), Type::Never) } /// Returns the type of the nearest enclosing class for the given scope. @@ -513,11 +513,25 @@ impl<'db> TypeInference<'db> { ) } + pub(super) fn extend(&mut self, inference: &TypeInference<'db>) { + debug_assert_eq!(self.scope, inference.scope); + + self.bindings.extend(inference.bindings.iter()); + self.declarations.extend(inference.declarations.iter()); + self.expressions.extend(inference.expressions.iter()); + self.deferred.extend(inference.deferred.iter()); + self.cycle_fallback_type = self.cycle_fallback_type.or(inference.cycle_fallback_type); + } + + pub(super) fn set_diagnostics(&mut self, diagnostics: TypeCheckDiagnostics) { + self.diagnostics = diagnostics; + } + pub(crate) fn diagnostics(&self) -> &TypeCheckDiagnostics { &self.diagnostics } - fn shrink_to_fit(&mut self) { + pub(super) fn shrink_to_fit(&mut self) { self.expressions.shrink_to_fit(); self.bindings.shrink_to_fit(); self.declarations.shrink_to_fit(); @@ -667,19 +681,8 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> { } fn extend(&mut self, inference: &TypeInference<'db>) { - debug_assert_eq!(self.types.scope, inference.scope); - - self.types.bindings.extend(inference.bindings.iter()); - self.types - .declarations - .extend(inference.declarations.iter()); - self.types.expressions.extend(inference.expressions.iter()); - self.types.deferred.extend(inference.deferred.iter()); + self.types.extend(inference); self.context.extend(inference.diagnostics()); - self.types.cycle_fallback_type = self - .types - .cycle_fallback_type - .or(inference.cycle_fallback_type); } fn file(&self) -> File { @@ -1637,11 +1640,35 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> { } ty.inner_type() }); - if !bound_ty.is_assignable_to(db, declared_ty) { + + if let Some( + attr_expr @ ast::ExprAttribute { + value: object, + ctx: ExprContext::Store, + attr, + .. + }, + ) = node.as_expr_attribute() + { + if !self.validate_attribute_assignment( + attr_expr, + self.expression_type(object), + attr.id(), + bound_ty, + true, + ) { + // If the attribute assignment is invalid, we still want to bind the type + // to the attribute, so we don't report it again. + if !declared_ty.is_unknown() { + bound_ty = declared_ty; + } + } + } else if !bound_ty.is_assignable_to(db, declared_ty) { report_invalid_assignment(&self.context, node, declared_ty, bound_ty); // allow declarations to override inference in case of invalid assignment bound_ty = declared_ty; } + // In the following cases, the bound type may not be the same as the RHS value type. if let AnyNodeRef::ExprAttribute(ast::ExprAttribute { value, attr, .. }) = node { let value_ty = self @@ -2724,14 +2751,7 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> { for item in items { let target = item.optional_vars.as_deref(); if let Some(target) = target { - self.infer_target(target, &item.context_expr, |builder, context_expr| { - // TODO: `infer_with_statement_definition` reports a diagnostic if `ctx_manager_ty` isn't a context manager - // but only if the target is a name. We should report a diagnostic here if the target isn't a name: - // `with not_context_manager as a.x: ... - builder - .infer_standalone_expression(context_expr) - .enter(builder.db()) - }); + self.infer_target(target, &item.context_expr, false); } else { // Call into the context expression inference to validate that it evaluates // to a valid context manager. @@ -2753,23 +2773,25 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> { let target = with_item.target(self.module()); let target_ty = if with_item.is_async() { - let _context_expr_ty = self.infer_standalone_expression(context_expr); + let _context_expr_type = self.infer_standalone_expression(context_expr); todo_type!("async `with` statement") } else { match with_item.target_kind() { TargetKind::Sequence(unpack_position, unpack) => { let unpacked = infer_unpack_types(self.db(), unpack); - let target_ast_id = target.scoped_expression_id(self.db(), self.scope()); + // Only copy the inference results if this is the first assignment to avoid any + // duplication e.g., diagnostics. if unpack_position == UnpackPosition::First { - self.context.extend(unpacked.diagnostics()); + self.extend(unpacked.types()); } + let target_ast_id = target.scoped_expression_id(self.db(), self.scope()); unpacked.expression_type(target_ast_id) } TargetKind::Single => { - let context_expr_ty = self.infer_standalone_expression(context_expr); + let context_expr_type = self.infer_standalone_expression(context_expr); self.infer_context_expression( context_expr, - context_expr_ty, + context_expr_type, with_item.is_async(), ) } @@ -3186,30 +3208,48 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> { } = assignment; for target in targets { - self.infer_target(target, value, |builder, value_expr| { - builder.infer_standalone_expression(value_expr) - }); + self.infer_target(target, value, false); } } /// Infer the (definition) types involved in a `target` expression. /// /// This is used for assignment statements, for statements, etc. with a single or multiple - /// targets (unpacking). If `target` is an attribute expression, we check that the assignment - /// is valid. For 'target's that are definitions, this check happens elsewhere. + /// targets (unpacking). /// - /// The `infer_value_expr` function is used to infer the type of the `value` expression which - /// are not `Name` expressions. The returned type is the one that is eventually assigned to the - /// `target`. - fn infer_target(&mut self, target: &ast::Expr, value: &ast::Expr, infer_value_expr: F) - where - F: Fn(&mut TypeInferenceBuilder<'db, '_>, &ast::Expr) -> Type<'db>, - { - let assigned_ty = match target { - ast::Expr::Name(_) => None, - _ => Some(infer_value_expr(self, value)), - }; - self.infer_target_impl(target, assigned_ty); + /// # Panics + /// + /// Panics if the `value` expression is not a standalone expression. + fn infer_target( + &mut self, + target: &ast::Expr, + value: &ast::Expr, + is_first_comprehension: bool, + ) { + // TODO: Starred target + // TODO: Subscript target + match target { + ast::Expr::Name(name) => self.infer_definition(name), + ast::Expr::Attribute(attribute) => self.infer_definition(attribute), + ast::Expr::List(ast::ExprList { elts, .. }) + | ast::Expr::Tuple(ast::ExprTuple { elts, .. }) => { + for element in elts { + self.infer_target(element, value, is_first_comprehension); + } + // If the target list / tuple is empty, we still need to infer the value expression + // to store its type. + if elts.is_empty() { + self.infer_standalone_expression(value); + } + } + _ => { + // TODO: Remove this once we handle all possible assignment targets + if !is_first_comprehension { + self.infer_standalone_expression(value); + } + self.infer_expression(target); + } + } } /// Make sure that the attribute assignment `obj.attribute = value` is valid. @@ -3695,71 +3735,31 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> { } } - fn infer_target_impl(&mut self, target: &ast::Expr, assigned_ty: Option>) { - match target { - ast::Expr::Name(name) => self.infer_definition(name), - ast::Expr::List(ast::ExprList { elts, .. }) - | ast::Expr::Tuple(ast::ExprTuple { elts, .. }) => { - let mut assigned_tys = match assigned_ty { - Some(Type::Tuple(tuple)) => Either::Left(tuple.tuple(self.db()).all_elements()), - Some(_) | None => Either::Right(std::iter::empty()), - }; - - for element in elts { - self.infer_target_impl(element, assigned_tys.next()); - } - } - ast::Expr::Attribute( - attr_expr @ ast::ExprAttribute { - value: object, - ctx: ExprContext::Store, - attr, - .. - }, - ) => { - self.store_expression_type(target, assigned_ty.unwrap_or(Type::unknown())); - - let object_ty = self.infer_expression(object); - - if let Some(assigned_ty) = assigned_ty { - self.validate_attribute_assignment( - attr_expr, - object_ty, - attr.id(), - assigned_ty, - true, - ); - } - } - _ => { - // TODO: Remove this once we handle all possible assignment targets. - self.infer_expression(target); - } - } - } - fn infer_assignment_definition( &mut self, assignment: &AssignmentDefinitionKind<'db>, definition: Definition<'db>, ) { - let value = assignment.value(self.module()); let target = assignment.target(self.module()); - let mut target_ty = match assignment.target_kind() { + let target_ty = match assignment.target_kind() { TargetKind::Sequence(unpack_position, unpack) => { let unpacked = infer_unpack_types(self.db(), unpack); - // Only copy the diagnostics if this is the first assignment to avoid duplicating the - // unpack assignments. + // Only copy the inference results if this is the first assignment to avoid any + // duplication e.g., diagnostics. if unpack_position == UnpackPosition::First { - self.context.extend(unpacked.diagnostics()); + self.extend(unpacked.types()); } - let target_ast_id = target.scoped_expression_id(self.db(), self.scope()); unpacked.expression_type(target_ast_id) } TargetKind::Single => { - let value_ty = self.infer_standalone_expression(value); + let value = assignment.value(self.module()); + + // The type of the value expression always needs to be inferred even though there + // are special cases that doesn't require this because of the invariant that every + // expression should have a type inferred for it. + let value_type = self.infer_standalone_expression(value); // `TYPE_CHECKING` is a special variable that should only be assigned `False` // at runtime, but is always considered `True` in type checking. @@ -3772,20 +3772,18 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> { report_invalid_type_checking_constant(&self.context, target.into()); } Type::BooleanLiteral(true) + } else if let Some(special_form) = target.as_name_expr().and_then(|name| { + SpecialFormType::try_from_file_and_name(self.db(), self.file(), &name.id) + }) { + Type::SpecialForm(special_form) } else if self.in_stub() && value.is_ellipsis_literal_expr() { Type::unknown() } else { - value_ty + value_type } } }; - if let Some(special_form) = target.as_name_expr().and_then(|name| { - SpecialFormType::try_from_file_and_name(self.db(), self.file(), &name.id) - }) { - target_ty = Type::SpecialForm(special_form); - } - self.store_expression_type(target, target_ty); self.add_binding(target.into(), definition, target_ty); } @@ -3882,7 +3880,7 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> { // If the target of an assignment is not one of the place expressions we support, // then they are not definitions, so we can only be here if the target is in a form supported as a place expression. // In this case, we can simply store types in `target` below, instead of calling `infer_expression` (which would return `Never`). - debug_assert!(PlaceExpr::try_from(target).is_ok()); + // debug_assert!(PlaceExpr::try_from(target).is_ok()); if let Some(value) = value { let inferred_ty = self.infer_expression(value); @@ -4044,15 +4042,7 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> { is_async: _, } = for_statement; - self.infer_target(target, iter, |builder, iter_expr| { - // TODO: `infer_for_statement_definition` reports a diagnostic if `iter_ty` isn't iterable - // but only if the target is a name. We should report a diagnostic here if the target isn't a name: - // `for a.x in not_iterable: ... - builder - .infer_standalone_expression(iter_expr) - .iterate(builder.db()) - }); - + self.infer_target(target, iter, false); self.infer_body(body); self.infer_body(orelse); } @@ -4072,8 +4062,10 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> { match for_stmt.target_kind() { TargetKind::Sequence(unpack_position, unpack) => { let unpacked = infer_unpack_types(self.db(), unpack); + // Only copy the inference results if this is the first assignment to avoid any + // duplication e.g., diagnostics. if unpack_position == UnpackPosition::First { - self.context.extend(unpacked.diagnostics()); + self.extend(unpacked.types()); } let target_ast_id = target.scoped_expression_id(self.db(), self.scope()); unpacked.expression_type(target_ast_id) @@ -5054,21 +5046,7 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> { is_async: _, } = comprehension; - self.infer_target(target, iter, |builder, iter_expr| { - // TODO: `infer_comprehension_definition` reports a diagnostic if `iter_ty` isn't iterable - // but only if the target is a name. We should report a diagnostic here if the target isn't a name: - // `[... for a.x in not_iterable] - if is_first { - infer_same_file_expression_type( - builder.db(), - builder.index.expression(iter_expr), - builder.module(), - ) - } else { - builder.infer_standalone_expression(iter_expr) - } - .iterate(builder.db()) - }); + self.infer_target(target, iter, is_first); for expr in ifs { self.infer_expression(expr); } @@ -5079,8 +5057,8 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> { comprehension: &ComprehensionDefinitionKind<'db>, definition: Definition<'db>, ) { - let iterable = comprehension.iterable(self.module()); let target = comprehension.target(self.module()); + let iterable = comprehension.iterable(self.module()); let mut infer_iterable_type = || { let expression = self.index.expression(iterable); @@ -5092,7 +5070,7 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> { // (2) We must *not* call `self.extend()` on the result of the type inference, // because `ScopedExpressionId`s are only meaningful within their own scope, so // we'd add types for random wrong expressions in the current scope - if comprehension.is_first() && target.is_name_expr() { + if comprehension.is_first() { let lookup_scope = self .index .parent_scope_id(self.scope().file_scope_id(self.db())) @@ -5119,7 +5097,11 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> { TargetKind::Sequence(unpack_position, unpack) => { let unpacked = infer_unpack_types(self.db(), unpack); if unpack_position == UnpackPosition::First { - self.context.extend(unpacked.diagnostics()); + if comprehension.is_first() { + self.context.extend(unpacked.types().diagnostics()); + } else { + self.extend(unpacked.types()); + } } let target_ast_id = target.scoped_expression_id(self.db(), unpack.target_scope(self.db())); @@ -5135,10 +5117,7 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> { } }; - self.types.expressions.insert( - target.scoped_expression_id(self.db(), self.scope()), - target_type, - ); + self.store_expression_type(target, target_type); self.add_binding(target.into(), definition, target_type); } @@ -5954,10 +5933,13 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> { } None } - Some(expr) => match PlaceExpr::try_from(expr) { - Ok(place_expr) => place_table(db, scope).place_id_by_expr(&place_expr), - Err(()) => None, - }, + Some(expr) => { + let place_expr = PlaceExpr::from(expr); + if !place_expr.is_valid() { + return None; + } + place_table(db, scope).place_id_by_expr(&place_expr) + } }; match return_ty { @@ -6116,7 +6098,7 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> { id: symbol_name, ctx: _, } = name_node; - let Ok(expr) = PlaceExpr::try_from(symbol_name); + let expr = PlaceExpr::from(symbol_name); let db = self.db(); let (resolved, constraint_keys) = @@ -6490,11 +6472,11 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> { ) -> Type<'db> { let target = target.into(); - if let Ok(place_expr) = PlaceExpr::try_from(target) { - self.narrow_place_with_applicable_constraints(&place_expr, target_ty, constraint_keys) - } else { - target_ty - } + self.narrow_place_with_applicable_constraints( + &PlaceExpr::from(target), + target_ty, + constraint_keys, + ) } /// Infer the type of a [`ast::ExprAttribute`] expression, assuming a load context. @@ -6512,13 +6494,13 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> { let mut constraint_keys = vec![]; let mut assigned_type = None; - if let Ok(place_expr) = PlaceExpr::try_from(attribute) { - let (resolved, keys) = - self.infer_place_load(&place_expr, ast::ExprRef::Attribute(attribute)); - constraint_keys.extend(keys); - if let Place::Type(ty, Boundness::Bound) = resolved.place { - assigned_type = Some(ty); - } + let (resolved, keys) = self.infer_place_load( + &PlaceExpr::from(attribute), + ast::ExprRef::Attribute(attribute), + ); + constraint_keys.extend(keys); + if let Place::Type(ty, Boundness::Bound) = resolved.place { + assigned_type = Some(ty); } let resolved_type = value_type @@ -8043,17 +8025,17 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> { // If `value` is a valid reference, we attempt type narrowing by assignment. if !value_ty.is_unknown() { - if let Ok(expr) = PlaceExpr::try_from(subscript) { - let (place, keys) = - self.infer_place_load(&expr, ast::ExprRef::Subscript(subscript)); - constraint_keys.extend(keys); - if let Place::Type(ty, Boundness::Bound) = place.place { - // Even if we can obtain the subscript type based on the assignments, we still perform default type inference - // (to store the expression type and to report errors). - let slice_ty = self.infer_expression(slice); - self.infer_subscript_expression_types(value, value_ty, slice_ty); - return ty; - } + let (place, keys) = self.infer_place_load( + &PlaceExpr::from(subscript), + ast::ExprRef::Subscript(subscript), + ); + constraint_keys.extend(keys); + if let Place::Type(ty, Boundness::Bound) = place.place { + // Even if we can obtain the subscript type based on the assignments, we still perform default type inference + // (to store the expression type and to report errors). + let slice_ty = self.infer_expression(slice); + self.infer_subscript_expression_types(value, value_ty, slice_ty); + return ty; } } @@ -8560,7 +8542,7 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> { pub(super) fn finish(mut self) -> TypeInference<'db> { self.infer_region(); - self.types.diagnostics = self.context.finish(); + self.types.set_diagnostics(self.context.finish()); self.types.shrink_to_fit(); self.types } diff --git a/crates/ty_python_semantic/src/types/narrow.rs b/crates/ty_python_semantic/src/types/narrow.rs index 09517ef03e..d055957ffb 100644 --- a/crates/ty_python_semantic/src/types/narrow.rs +++ b/crates/ty_python_semantic/src/types/narrow.rs @@ -248,12 +248,14 @@ fn negate_if<'db>(constraints: &mut NarrowingConstraints<'db>, db: &'db dyn Db, } fn place_expr(expr: &ast::Expr) -> Option { - match expr { - ast::Expr::Name(name) => Some(PlaceExpr::name(name.id.clone())), - ast::Expr::Attribute(attr) => PlaceExpr::try_from(attr).ok(), - ast::Expr::Subscript(subscript) => PlaceExpr::try_from(subscript).ok(), - ast::Expr::Named(named) => PlaceExpr::try_from(named.target.as_ref()).ok(), - _ => None, + let place_expr = match expr { + ast::Expr::Named(named) => PlaceExpr::from(named.target.as_ref()), + _ => PlaceExpr::from(expr), + }; + if place_expr.is_valid() { + Some(place_expr) + } else { + None } } diff --git a/crates/ty_python_semantic/src/types/unpacker.rs b/crates/ty_python_semantic/src/types/unpacker.rs index f963fb0255..0efc4545ca 100644 --- a/crates/ty_python_semantic/src/types/unpacker.rs +++ b/crates/ty_python_semantic/src/types/unpacker.rs @@ -10,18 +10,34 @@ use crate::Db; use crate::semantic_index::ast_ids::{HasScopedExpressionId, ScopedExpressionId}; use crate::semantic_index::place::ScopeId; use crate::types::tuple::{FixedLengthTupleSpec, TupleSpec, TupleType}; -use crate::types::{Type, TypeCheckDiagnostics, infer_expression_types, todo_type}; +use crate::types::{Type, infer_expression_types, todo_type}; use crate::unpack::{UnpackKind, UnpackValue}; use super::context::InferContext; use super::diagnostic::INVALID_ASSIGNMENT; +use super::infer::TypeInference; use super::{KnownClass, UnionType}; /// Unpacks the value expression type to their respective targets. pub(crate) struct Unpacker<'db, 'ast> { context: InferContext<'db, 'ast>, + + /// The type inference result for the unpacked value expression. + /// + /// This does not include the target types, which are stored in `targets` field. + value_types: TypeInference<'db>, + + /// The scope in which the target expressions belongs to. target_scope: ScopeId<'db>, + + /// The scope in which the value expression belongs to. + /// + /// For more information, see [`value_scope`]. + /// + /// [`value_scope`]: crate::unpack::Unpack::value_scope value_scope: ScopeId<'db>, + + /// The inferred types of the target expressions involved in the unpacking. targets: FxHashMap>, } @@ -34,6 +50,7 @@ impl<'db, 'ast> Unpacker<'db, 'ast> { ) -> Self { Self { context: InferContext::new(db, target_scope, module), + value_types: TypeInference::empty(value_scope), targets: FxHashMap::default(), target_scope, value_scope, @@ -55,11 +72,20 @@ impl<'db, 'ast> Unpacker<'db, 'ast> { "Unpacking target must be a list or tuple expression" ); - let value_type = infer_expression_types(self.db(), value.expression()).expression_type( - value.scoped_expression_id(self.db(), self.value_scope, self.module()), - ); + let value_types_result = infer_expression_types(self.db(), value.expression()); - let value_type = match value.kind() { + // Make sure the inference result and diagnostics are stored in the unpacker so that it's + // propagated outside the unpacker via the `UnpackResult`. + self.value_types.extend(value_types_result); + self.context.extend(value_types_result.diagnostics()); + + let mut value_type = value_types_result.expression_type(value.scoped_expression_id( + self.db(), + self.value_scope, + self.module(), + )); + + value_type = match value.kind() { UnpackKind::Assign => { if self.context.in_stub() && value @@ -335,18 +361,27 @@ impl<'db, 'ast> Unpacker<'db, 'ast> { pub(crate) fn finish(mut self) -> UnpackResult<'db> { self.targets.shrink_to_fit(); + + self.value_types.set_diagnostics(self.context.finish()); + self.value_types.shrink_to_fit(); + UnpackResult { - diagnostics: self.context.finish(), + types: self.value_types, targets: self.targets, cycle_fallback_type: None, } } } -#[derive(Debug, Default, PartialEq, Eq, salsa::Update)] +#[derive(Debug, PartialEq, Eq, salsa::Update)] pub(crate) struct UnpackResult<'db> { targets: FxHashMap>, - diagnostics: TypeCheckDiagnostics, + + /// The type inference result for the unpacked value expression. + /// + /// This does not include the target types, which are stored in `targets` field. This does + /// include all the diagnostics that were collected during the unpacking process. + types: TypeInference<'db>, /// The fallback type for missing expressions. /// @@ -377,15 +412,14 @@ impl<'db> UnpackResult<'db> { .or(self.cycle_fallback_type) } - /// Returns the diagnostics in this unpacking assignment. - pub(crate) fn diagnostics(&self) -> &TypeCheckDiagnostics { - &self.diagnostics + pub(super) fn types(&self) -> &TypeInference<'db> { + &self.types } - pub(crate) fn cycle_fallback(cycle_fallback_type: Type<'db>) -> Self { + pub(crate) fn cycle_fallback(scope_id: ScopeId<'db>, cycle_fallback_type: Type<'db>) -> Self { Self { targets: FxHashMap::default(), - diagnostics: TypeCheckDiagnostics::default(), + types: TypeInference::empty(scope_id), cycle_fallback_type: Some(cycle_fallback_type), } }