diff --git a/crates/red_knot_python_semantic/resources/mdtest/attributes.md b/crates/red_knot_python_semantic/resources/mdtest/attributes.md index 2e352ed264..5a704decdf 100644 --- a/crates/red_knot_python_semantic/resources/mdtest/attributes.md +++ b/crates/red_knot_python_semantic/resources/mdtest/attributes.md @@ -102,7 +102,7 @@ reveal_type(C.pure_instance_variable) # revealed: str # and pyright allow this. C.pure_instance_variable = "overwritten on class" -# TODO: this should be an error (incompatible types in assignment) +# error: [invalid-assignment] "Object of type `Literal[1]` is not assignable to `str`" c_instance.pure_instance_variable = 1 ``` @@ -191,7 +191,7 @@ c_instance.pure_class_variable1 = "value set on instance" C.pure_class_variable1 = "overwritten on class" -# TODO: should raise an error (incompatible types in assignment) +# error: [invalid-assignment] "Object of type `Literal[1]` is not assignable to `str`" C.pure_class_variable1 = 1 class Subclass(C): @@ -436,6 +436,40 @@ class Foo: ... reveal_type(Foo.__class__) # revealed: Literal[type] ``` +## Module attributes + +```py path=mod.py +global_symbol: str = "a" +``` + +```py +import mod + +reveal_type(mod.global_symbol) # revealed: str +mod.global_symbol = "b" + +# error: [invalid-assignment] "Object of type `Literal[1]` is not assignable to `str`" +mod.global_symbol = 1 + +# error: [invalid-assignment] "Object of type `Literal[1]` is not assignable to `str`" +(_, mod.global_symbol) = (..., 1) + +# TODO: this should be an error, but we do not understand list unpackings yet. +[_, mod.global_symbol] = [1, 2] + +class IntIterator: + def __next__(self) -> int: + return 42 + +class IntIterable: + def __iter__(self) -> IntIterator: + return IntIterator() + +# error: [invalid-assignment] "Object of type `int` is not assignable to `str`" +for mod.global_symbol in IntIterable(): + pass +``` + ## Literal types ### Function-literal attributes diff --git a/crates/red_knot_python_semantic/src/types.rs b/crates/red_knot_python_semantic/src/types.rs index 593bde2daf..ab3da8fab4 100644 --- a/crates/red_knot_python_semantic/src/types.rs +++ b/crates/red_knot_python_semantic/src/types.rs @@ -3313,6 +3313,14 @@ impl<'db> IterationOutcome<'db> { } } } + + fn unwrap_without_diagnostic(self) -> Type<'db> { + match self { + Self::Iterable { element_ty } => element_ty, + Self::NotIterable { .. } => Type::unknown(), + Self::PossiblyUnboundDunderIter { element_ty, .. } => element_ty, + } + } } #[derive(Debug, Copy, Clone, PartialEq, Eq)] diff --git a/crates/red_knot_python_semantic/src/types/infer.rs b/crates/red_knot_python_semantic/src/types/infer.rs index bcff5f92af..2ffae813b5 100644 --- a/crates/red_knot_python_semantic/src/types/infer.rs +++ b/crates/red_knot_python_semantic/src/types/infer.rs @@ -1972,33 +1972,75 @@ impl<'db> TypeInferenceBuilder<'db> { } = assignment; for target in targets { - self.infer_target(target, value); + self.infer_target(target, value, |_, ty| ty); } } - /// Infer the definition types involved in a `target` expression. + /// 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). + /// 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. + /// + /// The `to_assigned_ty` function is used to convert the inferred type of the `value` expression + /// to the type that is eventually assigned to the `target`. /// /// # Panics /// /// If the `value` is not a standalone expression. - fn infer_target(&mut self, target: &ast::Expr, value: &ast::Expr) { + fn infer_target(&mut self, target: &ast::Expr, value: &ast::Expr, to_assigned_ty: F) + where + F: Fn(&'db dyn Db, Type<'db>) -> Type<'db>, + { + let assigned_ty = match target { + ast::Expr::Name(_) => None, + _ => { + let value_ty = self.infer_standalone_expression(value); + + Some(to_assigned_ty(self.db(), value_ty)) + } + }; + self.infer_target_impl(target, assigned_ty); + } + + 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.elements(self.db()).into_iter().copied()) + } + Some(_) | None => Either::Right(std::iter::empty()), + }; + for element in elts { - self.infer_target(element, value); + self.infer_target_impl(element, assigned_tys.next()); } - if elts.is_empty() { - self.infer_standalone_expression(value); + } + ast::Expr::Attribute( + lhs_expr @ ast::ExprAttribute { + ctx: ExprContext::Store, + .. + }, + ) => { + let attribute_expr_ty = self.infer_attribute_expression(lhs_expr); + self.store_expression_type(target, attribute_expr_ty); + + if let Some(assigned_ty) = assigned_ty { + if !assigned_ty.is_assignable_to(self.db(), attribute_expr_ty) { + report_invalid_assignment( + &self.context, + target.into(), + attribute_expr_ty, + assigned_ty, + ); + } } } _ => { // TODO: Remove this once we handle all possible assignment targets. - self.infer_standalone_expression(value); self.infer_expression(target); } } @@ -2267,7 +2309,10 @@ impl<'db> TypeInferenceBuilder<'db> { is_async: _, } = for_statement; - self.infer_target(target, iter); + self.infer_target(target, iter, |db, iter_ty| { + iter_ty.iterate(db).unwrap_without_diagnostic() + }); + self.infer_body(body); self.infer_body(orelse); } @@ -3418,7 +3463,7 @@ impl<'db> TypeInferenceBuilder<'db> { ExprContext::Store => { let value_ty = self.infer_expression(value); - if let Type::Instance(instance) = value_ty { + let symbol = if let Type::Instance(instance) = value_ty { let instance_member = instance.class.instance_member(self.db(), attr); if instance_member.is_class_var() { self.context.report_lint( @@ -3430,9 +3475,16 @@ impl<'db> TypeInferenceBuilder<'db> { ), ); } - } - Type::Never + instance_member.0 + } else { + value_ty.member(self.db(), attr) + }; + + // TODO: The unbound-case might also yield a diagnostic, but we can not activate + // this yet until we understand implicit instance attributes (those that are not + // defined in the class body), as there would be too many false positives. + symbol.ignore_possibly_unbound().unwrap_or(Type::unknown()) } ExprContext::Del => { self.infer_expression(value);