Skip to content

Commit

Permalink
[red-knot] Fix diagnostic range for non-iterable unpacking assignments (
Browse files Browse the repository at this point in the history
#15994)

## Summary

I noticed that the diagnostic range in specific unpacking assignments is
wrong. For this example

```py
a, b = 1
```

we previously got (see first commit):

```
error: lint:not-iterable
 --> /src/mdtest_snippet.py:1:1
  |
1 | a, b = 1
  | ^^^^ Object of type `Literal[1]` is not iterable
  |
```

and with this change, we get:

```
error: lint:not-iterable
 --> /src/mdtest_snippet.py:1:8
  |
1 | a, b = 1
  |        ^ Object of type `Literal[1]` is not iterable
  |
```

## Test Plan

New snapshot tests.
  • Loading branch information
sharkdp authored Feb 6, 2025
1 parent 5588c75 commit e345307
Show file tree
Hide file tree
Showing 5 changed files with 115 additions and 5 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
# Unpacking

<!-- snapshot-diagnostics -->

## Right hand side not iterable

```py
a, b = 1 # error: [not-iterable]
```

## Too many values to unpack

```py
a, b = (1, 2, 3) # error: [invalid-assignment]
```

## Too few values to unpack

```py
a, b = (1,) # error: [invalid-assignment]
```
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
---
source: crates/red_knot_test/src/lib.rs
expression: snapshot
---
---
mdtest name: unpacking.md - Unpacking - Right hand side not iterable
mdtest path: crates/red_knot_python_semantic/resources/mdtest/diagnostics/unpacking.md
---

# Python source files

## mdtest_snippet.py

```
1 | a, b = 1 # error: [not-iterable]
```

# Diagnostics

```
error: lint:not-iterable
--> /src/mdtest_snippet.py:1:8
|
1 | a, b = 1 # error: [not-iterable]
| ^ Object of type `Literal[1]` is not iterable
|
```
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
---
source: crates/red_knot_test/src/lib.rs
expression: snapshot
---
---
mdtest name: unpacking.md - Unpacking - Too few values to unpack
mdtest path: crates/red_knot_python_semantic/resources/mdtest/diagnostics/unpacking.md
---

# Python source files

## mdtest_snippet.py

```
1 | a, b = (1,) # error: [invalid-assignment]
```

# Diagnostics

```
error: lint:invalid-assignment
--> /src/mdtest_snippet.py:1:1
|
1 | a, b = (1,) # error: [invalid-assignment]
| ^^^^ Not enough values to unpack (expected 2, got 1)
|
```
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
---
source: crates/red_knot_test/src/lib.rs
expression: snapshot
---
---
mdtest name: unpacking.md - Unpacking - Too many values to unpack
mdtest path: crates/red_knot_python_semantic/resources/mdtest/diagnostics/unpacking.md
---

# Python source files

## mdtest_snippet.py

```
1 | a, b = (1, 2, 3) # error: [invalid-assignment]
```

# Diagnostics

```
error: lint:invalid-assignment
--> /src/mdtest_snippet.py:1:1
|
1 | a, b = (1, 2, 3) # error: [invalid-assignment]
| ^^^^ Too many values to unpack (expected 2, got 3)
|
```
15 changes: 10 additions & 5 deletions crates/red_knot_python_semantic/src/types/unpacker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -62,10 +62,15 @@ impl<'db> Unpacker<'db> {
.unwrap_with_diagnostic(&self.context, value.as_any_node_ref(self.db()));
}

self.unpack_inner(target, value_ty);
self.unpack_inner(target, value.as_any_node_ref(self.db()), value_ty);
}

fn unpack_inner(&mut self, target: &ast::Expr, value_ty: Type<'db>) {
fn unpack_inner(
&mut self,
target: &ast::Expr,
value_expr: AnyNodeRef<'db>,
value_ty: Type<'db>,
) {
match target {
ast::Expr::Name(target_name) => {
self.targets.insert(
Expand All @@ -74,7 +79,7 @@ impl<'db> Unpacker<'db> {
);
}
ast::Expr::Starred(ast::ExprStarred { value, .. }) => {
self.unpack_inner(value, value_ty);
self.unpack_inner(value, value_expr, value_ty);
}
ast::Expr::List(ast::ExprList { elts, .. })
| ast::Expr::Tuple(ast::ExprTuple { elts, .. }) => {
Expand Down Expand Up @@ -153,7 +158,7 @@ impl<'db> Unpacker<'db> {
Type::LiteralString
} else {
ty.iterate(self.db())
.unwrap_with_diagnostic(&self.context, AnyNodeRef::from(target))
.unwrap_with_diagnostic(&self.context, value_expr)
};
for target_type in &mut target_types {
target_type.push(ty);
Expand All @@ -167,7 +172,7 @@ impl<'db> Unpacker<'db> {
[] => Type::unknown(),
types => UnionType::from_elements(self.db(), types),
};
self.unpack_inner(element, element_ty);
self.unpack_inner(element, value_expr, element_ty);
}
}
_ => {}
Expand Down

0 comments on commit e345307

Please sign in to comment.