Skip to content

Commit

Permalink
[red-knot] Handle invalid assignment targets (astral-sh#14325)
Browse files Browse the repository at this point in the history
## 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 astral-sh#14321
closes astral-sh#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.
  • Loading branch information
sharkdp authored Nov 13, 2024
1 parent 78e4753 commit 5e64863
Show file tree
Hide file tree
Showing 6 changed files with 49 additions and 13 deletions.
42 changes: 33 additions & 9 deletions crates/red_knot_python_semantic/src/semantic_index/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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);
Expand Down Expand Up @@ -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 {
Expand Down
16 changes: 12 additions & 4 deletions crates/red_knot_python_semantic/src/types/infer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down

0 comments on commit 5e64863

Please sign in to comment.