From e345307260569be55a5a6c512b2bfb1cca7f2f7a Mon Sep 17 00:00:00 2001 From: David Peter Date: Thu, 6 Feb 2025 15:36:22 +0100 Subject: [PATCH] [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. --- .../resources/mdtest/diagnostics/unpacking.md | 21 ++++++++++++++ ...acking_-_Right_hand_side_not_iterable.snap | 28 +++++++++++++++++++ ..._Unpacking_-_Too_few_values_to_unpack.snap | 28 +++++++++++++++++++ ...Unpacking_-_Too_many_values_to_unpack.snap | 28 +++++++++++++++++++ .../src/types/unpacker.rs | 15 ++++++---- 5 files changed, 115 insertions(+), 5 deletions(-) create mode 100644 crates/red_knot_python_semantic/resources/mdtest/diagnostics/unpacking.md create mode 100644 crates/red_knot_python_semantic/resources/mdtest/snapshots/unpacking.md_-_Unpacking_-_Right_hand_side_not_iterable.snap create mode 100644 crates/red_knot_python_semantic/resources/mdtest/snapshots/unpacking.md_-_Unpacking_-_Too_few_values_to_unpack.snap create mode 100644 crates/red_knot_python_semantic/resources/mdtest/snapshots/unpacking.md_-_Unpacking_-_Too_many_values_to_unpack.snap diff --git a/crates/red_knot_python_semantic/resources/mdtest/diagnostics/unpacking.md b/crates/red_knot_python_semantic/resources/mdtest/diagnostics/unpacking.md new file mode 100644 index 0000000000..481d55a151 --- /dev/null +++ b/crates/red_knot_python_semantic/resources/mdtest/diagnostics/unpacking.md @@ -0,0 +1,21 @@ +# Unpacking + + + +## 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] +``` diff --git a/crates/red_knot_python_semantic/resources/mdtest/snapshots/unpacking.md_-_Unpacking_-_Right_hand_side_not_iterable.snap b/crates/red_knot_python_semantic/resources/mdtest/snapshots/unpacking.md_-_Unpacking_-_Right_hand_side_not_iterable.snap new file mode 100644 index 0000000000..d4fd66b20c --- /dev/null +++ b/crates/red_knot_python_semantic/resources/mdtest/snapshots/unpacking.md_-_Unpacking_-_Right_hand_side_not_iterable.snap @@ -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 + | + +``` diff --git a/crates/red_knot_python_semantic/resources/mdtest/snapshots/unpacking.md_-_Unpacking_-_Too_few_values_to_unpack.snap b/crates/red_knot_python_semantic/resources/mdtest/snapshots/unpacking.md_-_Unpacking_-_Too_few_values_to_unpack.snap new file mode 100644 index 0000000000..52ded77098 --- /dev/null +++ b/crates/red_knot_python_semantic/resources/mdtest/snapshots/unpacking.md_-_Unpacking_-_Too_few_values_to_unpack.snap @@ -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) + | + +``` diff --git a/crates/red_knot_python_semantic/resources/mdtest/snapshots/unpacking.md_-_Unpacking_-_Too_many_values_to_unpack.snap b/crates/red_knot_python_semantic/resources/mdtest/snapshots/unpacking.md_-_Unpacking_-_Too_many_values_to_unpack.snap new file mode 100644 index 0000000000..f1f5bb30d1 --- /dev/null +++ b/crates/red_knot_python_semantic/resources/mdtest/snapshots/unpacking.md_-_Unpacking_-_Too_many_values_to_unpack.snap @@ -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) + | + +``` diff --git a/crates/red_knot_python_semantic/src/types/unpacker.rs b/crates/red_knot_python_semantic/src/types/unpacker.rs index f895bb8cbc..d1b288ed18 100644 --- a/crates/red_knot_python_semantic/src/types/unpacker.rs +++ b/crates/red_knot_python_semantic/src/types/unpacker.rs @@ -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( @@ -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, .. }) => { @@ -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); @@ -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); } } _ => {}