diff --git a/crates/red_knot_ide/src/hover.rs b/crates/red_knot_ide/src/hover.rs index 11b1c78c0c1fd..700f8276e119c 100644 --- a/crates/red_knot_ide/src/hover.rs +++ b/crates/red_knot_ide/src/hover.rs @@ -444,7 +444,129 @@ mod tests { } #[test] - fn hover_class_member_declaration() { + fn hover_variable_assignment() { + let test = cursor_test( + r#" + value = 1 + "#, + ); + + assert_snapshot!(test.hover(), @r" + Literal[1] + --------------------------------------------- + ```text + Literal[1] + ``` + --------------------------------------------- + info: lint:hover: Hovered content is + --> main.py:2:13 + | + 2 | value = 1 + | ^^^^^- Cursor offset + | | + | source + | + "); + } + + #[test] + fn hover_augmented_assignment() { + let test = cursor_test( + r#" + value = 1 + value += 2 + "#, + ); + + // We currently show the *previous* value of the variable (1), not the new one (3). + // Showing the new value might be more intuitive for some users, but the actual 'use' + // of the `value` symbol here in read-context is `1`. This comment mainly exists to + // signal that it might be okay to revisit this in the future and reveal 3 instead. + assert_snapshot!(test.hover(), @r" + Literal[1] + --------------------------------------------- + ```text + Literal[1] + ``` + --------------------------------------------- + info: lint:hover: Hovered content is + --> main.py:3:13 + | + 2 | value = 1 + 3 | value += 2 + | ^^^^^- Cursor offset + | | + | source + | + "); + } + + #[test] + fn hover_attribute_assignment() { + let test = cursor_test( + r#" + class C: + attr: int = 1 + + C.attr = 2 + "#, + ); + + assert_snapshot!(test.hover(), @r" + Literal[2] + --------------------------------------------- + ```text + Literal[2] + ``` + --------------------------------------------- + info: lint:hover: Hovered content is + --> main.py:5:13 + | + 3 | attr: int = 1 + 4 | + 5 | C.attr = 2 + | ^^^^^^- Cursor offset + | | + | source + | + "); + } + + #[test] + fn hover_augmented_attribute_assignment() { + let test = cursor_test( + r#" + class C: + attr = 1 + + C.attr += 2 + "#, + ); + + // See the comment in the `hover_augmented_assignment` test above. The same + // reasoning applies here. + assert_snapshot!(test.hover(), @r" + Unknown | Literal[1] + --------------------------------------------- + ```text + Unknown | Literal[1] + ``` + --------------------------------------------- + info: lint:hover: Hovered content is + --> main.py:5:13 + | + 3 | attr = 1 + 4 | + 5 | C.attr += 2 + | ^^^^^^- Cursor offset + | | + | source + | + "); + } + + #[test] + fn hover_annotated_assignment() { let test = cursor_test( r#" class Foo: @@ -452,12 +574,11 @@ mod tests { "#, ); - // TODO: This should be int and not `Never`, https://github.com/astral-sh/ruff/issues/17122 assert_snapshot!(test.hover(), @r" - Never + int --------------------------------------------- ```text - Never + int ``` --------------------------------------------- info: lint:hover: Hovered content is @@ -472,6 +593,64 @@ mod tests { "); } + #[test] + fn hover_annotated_assignment_with_rhs() { + let test = cursor_test( + r#" + class Foo: + a: int = 1 + "#, + ); + + assert_snapshot!(test.hover(), @r" + Literal[1] + --------------------------------------------- + ```text + Literal[1] + ``` + --------------------------------------------- + info: lint:hover: Hovered content is + --> main.py:3:13 + | + 2 | class Foo: + 3 | a: int = 1 + | ^- Cursor offset + | | + | source + | + "); + } + + #[test] + fn hover_annotated_attribute_assignment() { + let test = cursor_test( + r#" + class Foo: + def __init__(self, a: int): + self.a: int = a + "#, + ); + + assert_snapshot!(test.hover(), @r" + int + --------------------------------------------- + ```text + int + ``` + --------------------------------------------- + info: lint:hover: Hovered content is + --> main.py:4:17 + | + 2 | class Foo: + 3 | def __init__(self, a: int): + 4 | self.a: int = a + | ^^^^^^- Cursor offset + | | + | source + | + "); + } + #[test] fn hover_type_narrowing() { let test = cursor_test( diff --git a/crates/red_knot_python_semantic/src/types/infer.rs b/crates/red_knot_python_semantic/src/types/infer.rs index d39b5dc34c419..0d2e934ee681d 100644 --- a/crates/red_knot_python_semantic/src/types/infer.rs +++ b/crates/red_knot_python_semantic/src/types/infer.rs @@ -3143,7 +3143,7 @@ impl<'db> TypeInferenceBuilder<'db> { .. }, ) => { - self.store_expression_type(target, Type::Never); + self.store_expression_type(target, assigned_ty.unwrap_or(Type::unknown())); let object_ty = self.infer_expression(object); @@ -3228,9 +3228,21 @@ impl<'db> TypeInferenceBuilder<'db> { target, simple: _, } = assignment; - self.infer_annotation_expression(annotation, DeferredExpressionState::None); + let annotated = + self.infer_annotation_expression(annotation, DeferredExpressionState::None); self.infer_optional_expression(value.as_deref()); + + // If we have an annotated assignment like `self.attr: int = 1`, we still need to + // do type inference on the `self.attr` target to get types for all sub-expressions. self.infer_expression(target); + + // But here we explicitly overwrite the type for the overall `self.attr` node with + // the annotated type. We do no use `store_expression_type` here, because it checks + // that no type has been stored for the expression before. + let expr_id = target.scoped_expression_id(self.db(), self.scope()); + self.types + .expressions + .insert(expr_id, annotated.inner_type()); } } @@ -3295,6 +3307,11 @@ impl<'db> TypeInferenceBuilder<'db> { } } + // Annotated assignments to non-names are not definitions, so we can only be here + // if the target is a name. In this case, we can simply store types in `target` + // below, instead of calling `infer_expression` (which would return `Never`). + debug_assert!(target.is_name_expr()); + if let Some(value) = value { let inferred_ty = self.infer_expression(value); let inferred_ty = if target @@ -3315,6 +3332,8 @@ impl<'db> TypeInferenceBuilder<'db> { inferred_ty, }, ); + + self.store_expression_type(target, inferred_ty); } else { if self.in_stub() { self.add_declaration_with_binding( @@ -3325,9 +3344,9 @@ impl<'db> TypeInferenceBuilder<'db> { } else { self.add_declaration(target.into(), definition, declared_ty); } - } - self.infer_expression(target); + self.store_expression_type(target, declared_ty.inner_type()); + } } fn infer_augmented_assignment_statement(&mut self, assignment: &ast::StmtAugAssign) { @@ -3416,12 +3435,14 @@ impl<'db> TypeInferenceBuilder<'db> { // Resolve the target type, assuming a load context. let target_type = match &**target { ast::Expr::Name(name) => { - self.store_expression_type(target, Type::Never); - self.infer_name_load(name) + let previous_value = self.infer_name_load(name); + self.store_expression_type(target, previous_value); + previous_value } ast::Expr::Attribute(attr) => { - self.store_expression_type(target, Type::Never); - self.infer_attribute_load(attr) + let previous_value = self.infer_attribute_load(attr); + self.store_expression_type(target, previous_value); + previous_value } _ => self.infer_expression(target), };