Skip to content

Commit e345307

Browse files
sharkdpsharkdp
authored
[red-knot] Fix diagnostic range for non-iterable unpacking assignments (#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.
1 parent 5588c75 commit e345307

5 files changed

Lines changed: 115 additions & 5 deletions

File tree

‎crates/red_knot_python_semantic/resources/mdtest/diagnostics/unpacking.md‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
# Unpacking
2+
3+
<!-- snapshot-diagnostics -->
4+
5+
## Right hand side not iterable
6+
7+
```py
8+
a, b = 1 # error: [not-iterable]
9+
```
10+
11+
## Too many values to unpack
12+
13+
```py
14+
a, b = (1, 2, 3) # error: [invalid-assignment]
15+
```
16+
17+
## Too few values to unpack
18+
19+
```py
20+
a, b = (1,) # error: [invalid-assignment]
21+
```

‎crates/red_knot_python_semantic/resources/mdtest/snapshots/unpacking.md_-_Unpacking_-_Right_hand_side_not_iterable.snap‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
---
2+
source: crates/red_knot_test/src/lib.rs
3+
expression: snapshot
4+
---
5+
---
6+
mdtest name: unpacking.md - Unpacking - Right hand side not iterable
7+
mdtest path: crates/red_knot_python_semantic/resources/mdtest/diagnostics/unpacking.md
8+
---
9+
10+
# Python source files
11+
12+
## mdtest_snippet.py
13+
14+
```
15+
1 | a, b = 1 # error: [not-iterable]
16+
```
17+
18+
# Diagnostics
19+
20+
```
21+
error: lint:not-iterable
22+
--> /src/mdtest_snippet.py:1:8
23+
|
24+
1 | a, b = 1 # error: [not-iterable]
25+
| ^ Object of type `Literal[1]` is not iterable
26+
|
27+
28+
```

‎crates/red_knot_python_semantic/resources/mdtest/snapshots/unpacking.md_-_Unpacking_-_Too_few_values_to_unpack.snap‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
---
2+
source: crates/red_knot_test/src/lib.rs
3+
expression: snapshot
4+
---
5+
---
6+
mdtest name: unpacking.md - Unpacking - Too few values to unpack
7+
mdtest path: crates/red_knot_python_semantic/resources/mdtest/diagnostics/unpacking.md
8+
---
9+
10+
# Python source files
11+
12+
## mdtest_snippet.py
13+
14+
```
15+
1 | a, b = (1,) # error: [invalid-assignment]
16+
```
17+
18+
# Diagnostics
19+
20+
```
21+
error: lint:invalid-assignment
22+
--> /src/mdtest_snippet.py:1:1
23+
|
24+
1 | a, b = (1,) # error: [invalid-assignment]
25+
| ^^^^ Not enough values to unpack (expected 2, got 1)
26+
|
27+
28+
```

‎crates/red_knot_python_semantic/resources/mdtest/snapshots/unpacking.md_-_Unpacking_-_Too_many_values_to_unpack.snap‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
---
2+
source: crates/red_knot_test/src/lib.rs
3+
expression: snapshot
4+
---
5+
---
6+
mdtest name: unpacking.md - Unpacking - Too many values to unpack
7+
mdtest path: crates/red_knot_python_semantic/resources/mdtest/diagnostics/unpacking.md
8+
---
9+
10+
# Python source files
11+
12+
## mdtest_snippet.py
13+
14+
```
15+
1 | a, b = (1, 2, 3) # error: [invalid-assignment]
16+
```
17+
18+
# Diagnostics
19+
20+
```
21+
error: lint:invalid-assignment
22+
--> /src/mdtest_snippet.py:1:1
23+
|
24+
1 | a, b = (1, 2, 3) # error: [invalid-assignment]
25+
| ^^^^ Too many values to unpack (expected 2, got 3)
26+
|
27+
28+
```

‎crates/red_knot_python_semantic/src/types/unpacker.rs‎

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -62,10 +62,15 @@ impl<'db> Unpacker<'db> {
6262
.unwrap_with_diagnostic(&self.context, value.as_any_node_ref(self.db()));
6363
}
6464

65-
self.unpack_inner(target, value_ty);
65+
self.unpack_inner(target, value.as_any_node_ref(self.db()), value_ty);
6666
}
6767

68-
fn unpack_inner(&mut self, target: &ast::Expr, value_ty: Type<'db>) {
68+
fn unpack_inner(
69+
&mut self,
70+
target: &ast::Expr,
71+
value_expr: AnyNodeRef<'db>,
72+
value_ty: Type<'db>,
73+
) {
6974
match target {
7075
ast::Expr::Name(target_name) => {
7176
self.targets.insert(
@@ -74,7 +79,7 @@ impl<'db> Unpacker<'db> {
7479
);
7580
}
7681
ast::Expr::Starred(ast::ExprStarred { value, .. }) => {
77-
self.unpack_inner(value, value_ty);
82+
self.unpack_inner(value, value_expr, value_ty);
7883
}
7984
ast::Expr::List(ast::ExprList { elts, .. })
8085
| ast::Expr::Tuple(ast::ExprTuple { elts, .. }) => {
@@ -153,7 +158,7 @@ impl<'db> Unpacker<'db> {
153158
Type::LiteralString
154159
} else {
155160
ty.iterate(self.db())
156-
.unwrap_with_diagnostic(&self.context, AnyNodeRef::from(target))
161+
.unwrap_with_diagnostic(&self.context, value_expr)
157162
};
158163
for target_type in &mut target_types {
159164
target_type.push(ty);
@@ -167,7 +172,7 @@ impl<'db> Unpacker<'db> {
167172
[] => Type::unknown(),
168173
types => UnionType::from_elements(self.db(), types),
169174
};
170-
self.unpack_inner(element, element_ty);
175+
self.unpack_inner(element, value_expr, element_ty);
171176
}
172177
}
173178
_ => {}

0 commit comments

Comments
 (0)