From 5e64863895728ed21163bc45d62c6beabb4202c2 Mon Sep 17 00:00:00 2001 From: David Peter Date: Wed, 13 Nov 2024 20:50:39 +0100 Subject: [PATCH] [red-knot] Handle invalid assignment targets (#14325) ## Summary This fixes several panics related to invalid assignment targets. All of these led to some a crash, previously: ```py (x.y := 1) # only name-expressions are valid targets of named expressions ([x, y] := [1, 2]) # same (x, y): tuple[int, int] = (2, 3) # tuples are not valid targets for annotated assignments (x, y) += 2 # tuples are not valid targets for augmented assignments ``` closes #14321 closes #14322 ## Test Plan I symlinked four files from `crates/ruff_python_parser/resources` into the red knot corpus, as they seemed like ideal test files for this exact scenario. I think eventually, it might be a good idea to simply include *all* invalid-syntax examples from the parser tests into red knots corpus (I believe we're actually not too far from that goal). Or expand the scope of the corpus test to this directory. Then we can get rid of these symlinks again. --- .../src/semantic_index/builder.rs | 42 +++++++++++++++---- .../src/types/infer.rs | 16 +++++-- .../test/corpus/04_assign_invalid_target.py | 1 + .../04_assign_named_expr_invalid_target.py | 1 + .../corpus/04_aug_assign_invalid_target.py | 1 + .../corpus/98_ann_assign_invalid_target.py | 1 + 6 files changed, 49 insertions(+), 13 deletions(-) create mode 120000 crates/red_knot_workspace/resources/test/corpus/04_assign_invalid_target.py create mode 120000 crates/red_knot_workspace/resources/test/corpus/04_assign_named_expr_invalid_target.py create mode 120000 crates/red_knot_workspace/resources/test/corpus/04_aug_assign_invalid_target.py create mode 120000 crates/red_knot_workspace/resources/test/corpus/98_ann_assign_invalid_target.py diff --git a/crates/red_knot_python_semantic/src/semantic_index/builder.rs b/crates/red_knot_python_semantic/src/semantic_index/builder.rs index fda1f07cbcdab..5ff8c3903c89a 100644 --- a/crates/red_knot_python_semantic/src/semantic_index/builder.rs +++ b/crates/red_knot_python_semantic/src/semantic_index/builder.rs @@ -676,9 +676,18 @@ where if let Some(value) = &node.value { self.visit_expr(value); } - self.push_assignment(node.into()); - self.visit_expr(&node.target); - self.pop_assignment(); + + // See https://docs.python.org/3/library/ast.html#ast.AnnAssign + if matches!( + *node.target, + ast::Expr::Attribute(_) | ast::Expr::Subscript(_) | ast::Expr::Name(_) + ) { + self.push_assignment(node.into()); + self.visit_expr(&node.target); + self.pop_assignment(); + } else { + self.visit_expr(&node.target); + } } ast::Stmt::AugAssign( aug_assign @ ast::StmtAugAssign { @@ -690,9 +699,18 @@ where ) => { debug_assert_eq!(&self.current_assignments, &[]); self.visit_expr(value); - self.push_assignment(aug_assign.into()); - self.visit_expr(target); - self.pop_assignment(); + + // See https://docs.python.org/3/library/ast.html#ast.AugAssign + if matches!( + **target, + ast::Expr::Attribute(_) | ast::Expr::Subscript(_) | ast::Expr::Name(_) + ) { + self.push_assignment(aug_assign.into()); + self.visit_expr(target); + self.pop_assignment(); + } else { + self.visit_expr(target); + } } ast::Stmt::If(node) => { self.visit_expr(&node.test); @@ -1072,9 +1090,15 @@ where ast::Expr::Named(node) => { // TODO walrus in comprehensions is implicitly nonlocal self.visit_expr(&node.value); - self.push_assignment(node.into()); - self.visit_expr(&node.target); - self.pop_assignment(); + + // See https://peps.python.org/pep-0572/#differences-between-assignment-expressions-and-assignment-statements + if node.target.is_name_expr() { + self.push_assignment(node.into()); + self.visit_expr(&node.target); + self.pop_assignment(); + } else { + self.visit_expr(&node.target); + } } ast::Expr::Lambda(lambda) => { if let Some(parameters) = &lambda.parameters { diff --git a/crates/red_knot_python_semantic/src/types/infer.rs b/crates/red_knot_python_semantic/src/types/infer.rs index b0de7ca5c48ff..5801da551945c 100644 --- a/crates/red_knot_python_semantic/src/types/infer.rs +++ b/crates/red_knot_python_semantic/src/types/infer.rs @@ -2518,10 +2518,18 @@ impl<'db> TypeInferenceBuilder<'db> { } fn infer_named_expression(&mut self, named: &ast::ExprNamed) -> Type<'db> { - let definition = self.index.definition(named); - let result = infer_definition_types(self.db, definition); - self.extend(result); - result.binding_ty(definition) + // See https://peps.python.org/pep-0572/#differences-between-assignment-expressions-and-assignment-statements + if named.target.is_name_expr() { + let definition = self.index.definition(named); + let result = infer_definition_types(self.db, definition); + self.extend(result); + result.binding_ty(definition) + } else { + // For syntactically invalid targets, we still need to run type inference: + self.infer_expression(&named.target); + self.infer_expression(&named.value); + Type::Unknown + } } fn infer_named_expression_definition( diff --git a/crates/red_knot_workspace/resources/test/corpus/04_assign_invalid_target.py b/crates/red_knot_workspace/resources/test/corpus/04_assign_invalid_target.py new file mode 120000 index 0000000000000..8d551b9f78d09 --- /dev/null +++ b/crates/red_knot_workspace/resources/test/corpus/04_assign_invalid_target.py @@ -0,0 +1 @@ +../../../../ruff_python_parser/resources/invalid/statements/invalid_assignment_targets.py \ No newline at end of file diff --git a/crates/red_knot_workspace/resources/test/corpus/04_assign_named_expr_invalid_target.py b/crates/red_knot_workspace/resources/test/corpus/04_assign_named_expr_invalid_target.py new file mode 120000 index 0000000000000..c751f9658700a --- /dev/null +++ b/crates/red_knot_workspace/resources/test/corpus/04_assign_named_expr_invalid_target.py @@ -0,0 +1 @@ +../../../../ruff_python_parser/resources/invalid/expressions/named/invalid_target.py \ No newline at end of file diff --git a/crates/red_knot_workspace/resources/test/corpus/04_aug_assign_invalid_target.py b/crates/red_knot_workspace/resources/test/corpus/04_aug_assign_invalid_target.py new file mode 120000 index 0000000000000..f3ebd5de4abc3 --- /dev/null +++ b/crates/red_knot_workspace/resources/test/corpus/04_aug_assign_invalid_target.py @@ -0,0 +1 @@ +../../../../ruff_python_parser/resources/invalid/statements/invalid_augmented_assignment_target.py \ No newline at end of file diff --git a/crates/red_knot_workspace/resources/test/corpus/98_ann_assign_invalid_target.py b/crates/red_knot_workspace/resources/test/corpus/98_ann_assign_invalid_target.py new file mode 120000 index 0000000000000..ebc265cfa2500 --- /dev/null +++ b/crates/red_knot_workspace/resources/test/corpus/98_ann_assign_invalid_target.py @@ -0,0 +1 @@ +../../../../ruff_python_parser/resources/inline/err/ann_assign_stmt_invalid_target.py \ No newline at end of file