Skip to content

[red-knot] More informative hover-types for assignments #17762

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
sharkdp merged 1 commit into from
May 1, 2025
Merged

[red-knot] More informative hover-types for assignments #17762

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
187 changes: 183 additions & 4 deletions crates/red_knot_ide/src/hover.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -444,20 +444,141 @@ mod tests {
}

#[test]
fn hover_class_member_declaration() {
fn hover_variable_assignment() {
let test = cursor_test(
r#"
value<CURSOR> = 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<CURSOR> += 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<CURSOR> = 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<CURSOR> += 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:
a<CURSOR>: int
"#,
);

// 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
Expand All @@ -472,6 +593,64 @@ mod tests {
");
}

#[test]
fn hover_annotated_assignment_with_rhs() {
let test = cursor_test(
r#"
class Foo:
a<CURSOR>: 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<CURSOR>: 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(
Expand Down
37 changes: 29 additions & 8 deletions crates/red_knot_python_semantic/src/types/infer.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down Expand Up @@ -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());
}
}

Expand Down Expand Up @@ -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
Expand All @@ -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(
Expand All @@ -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) {
Expand Down Expand Up @@ -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),
};
Expand Down
Loading