[syntax-errors] Multiple assignments in case pattern (#16957)

Summary
--

This PR detects multiple assignments to the same name in `case` patterns
by recursively visiting each pattern.

Test Plan
--

New inline tests.
This commit is contained in:
Brent Westbrook 2025-03-26 13:02:42 -04:00 committed by GitHub
parent 5697d21fca
commit d70a3e6753
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
6 changed files with 966 additions and 1 deletions

View file

@ -0,0 +1,10 @@
match 2:
case [y, z, y]: ... # MatchSequence
case [y, z, *y]: ... # MatchSequence
case [y, y, y]: ... # MatchSequence multiple
case {1: x, 2: x}: ... # MatchMapping duplicate pattern
case {1: x, **x}: ... # MatchMapping duplicate in **rest
case Class(x, x): ... # MatchClass positional
case Class(x=1, x=2): ... # MatchClass keyword
case [x] | {1: x} | Class(x=1, x=2): ... # MatchOr
case x as x: ... # MatchAs

View file

@ -0,0 +1,2 @@
match 2:
case Class(x) | [x] | x: ...

View file

@ -9,9 +9,10 @@ use std::fmt::Display;
use ruff_python_ast::{
self as ast,
visitor::{walk_expr, Visitor},
Expr, IrrefutablePatternKind, PythonVersion, Stmt, StmtExpr, StmtImportFrom,
Expr, IrrefutablePatternKind, Pattern, PythonVersion, Stmt, StmtExpr, StmtImportFrom,
};
use ruff_text_size::{Ranged, TextRange};
use rustc_hash::FxHashSet;
#[derive(Debug)]
pub struct SemanticSyntaxChecker {
@ -62,6 +63,7 @@ impl SemanticSyntaxChecker {
}
Stmt::Match(match_stmt) => {
Self::irrefutable_match_case(match_stmt, ctx);
Self::multiple_case_assignment(match_stmt, ctx);
}
Stmt::FunctionDef(ast::StmtFunctionDef { type_params, .. })
| Stmt::ClassDef(ast::StmtClassDef { type_params, .. })
@ -112,6 +114,16 @@ impl SemanticSyntaxChecker {
}
}
fn multiple_case_assignment<Ctx: SemanticSyntaxContext>(stmt: &ast::StmtMatch, ctx: &Ctx) {
for case in &stmt.cases {
let mut visitor = MultipleCaseAssignmentVisitor {
names: FxHashSet::default(),
ctx,
};
visitor.visit_pattern(&case.pattern);
}
}
fn irrefutable_match_case<Ctx: SemanticSyntaxContext>(stmt: &ast::StmtMatch, ctx: &Ctx) {
// test_ok irrefutable_case_pattern_at_end
// match x:
@ -265,6 +277,9 @@ impl Display for SemanticSyntaxError {
SemanticSyntaxErrorKind::DuplicateTypeParameter => {
f.write_str("duplicate type parameter")
}
SemanticSyntaxErrorKind::MultipleCaseAssignment(name) => {
write!(f, "multiple assignments to name `{name}` in pattern")
}
SemanticSyntaxErrorKind::IrrefutableCasePattern(kind) => match kind {
// These error messages are taken from CPython's syntax errors
IrrefutablePatternKind::Name(name) => {
@ -324,6 +339,18 @@ pub enum SemanticSyntaxErrorKind {
/// ```
DuplicateTypeParameter,
/// Represents a duplicate binding in a `case` pattern of a `match` statement.
///
/// ## Examples
///
/// ```python
/// match x:
/// case [x, y, x]: ...
/// case x as x: ...
/// case Class(x=1, x=2): ...
/// ```
MultipleCaseAssignment(ast::name::Name),
/// Represents an irrefutable `case` pattern before the last `case` in a `match` statement.
///
/// According to the [Python reference], "a match statement may have at most one irrefutable
@ -369,6 +396,93 @@ impl Visitor<'_> for ReboundComprehensionVisitor<'_> {
}
}
struct MultipleCaseAssignmentVisitor<'a, Ctx> {
names: FxHashSet<&'a ast::name::Name>,
ctx: &'a Ctx,
}
impl<'a, Ctx: SemanticSyntaxContext> MultipleCaseAssignmentVisitor<'a, Ctx> {
fn visit_pattern(&mut self, pattern: &'a Pattern) {
// test_err multiple_assignment_in_case_pattern
// match 2:
// case [y, z, y]: ... # MatchSequence
// case [y, z, *y]: ... # MatchSequence
// case [y, y, y]: ... # MatchSequence multiple
// case {1: x, 2: x}: ... # MatchMapping duplicate pattern
// case {1: x, **x}: ... # MatchMapping duplicate in **rest
// case Class(x, x): ... # MatchClass positional
// case Class(x=1, x=2): ... # MatchClass keyword
// case [x] | {1: x} | Class(x=1, x=2): ... # MatchOr
// case x as x: ... # MatchAs
match pattern {
Pattern::MatchValue(_) | Pattern::MatchSingleton(_) => {}
Pattern::MatchStar(ast::PatternMatchStar { name, .. }) => {
if let Some(name) = name {
self.insert(name);
}
}
Pattern::MatchSequence(ast::PatternMatchSequence { patterns, .. }) => {
for pattern in patterns {
self.visit_pattern(pattern);
}
}
Pattern::MatchMapping(ast::PatternMatchMapping { patterns, rest, .. }) => {
for pattern in patterns {
self.visit_pattern(pattern);
}
if let Some(rest) = rest {
self.insert(rest);
}
}
Pattern::MatchClass(ast::PatternMatchClass { arguments, .. }) => {
for pattern in &arguments.patterns {
self.visit_pattern(pattern);
}
for keyword in &arguments.keywords {
self.insert(&keyword.attr);
self.visit_pattern(&keyword.pattern);
}
}
Pattern::MatchAs(ast::PatternMatchAs { pattern, name, .. }) => {
if let Some(pattern) = pattern {
self.visit_pattern(pattern);
}
if let Some(name) = name {
self.insert(name);
}
}
Pattern::MatchOr(ast::PatternMatchOr { patterns, .. }) => {
// each of these patterns should be visited separately because patterns can only be
// duplicated within a single arm of the or pattern. For example, the case below is
// a valid pattern.
// test_ok multiple_assignment_in_case_pattern
// match 2:
// case Class(x) | [x] | x: ...
for pattern in patterns {
let mut visitor = Self {
names: FxHashSet::default(),
ctx: self.ctx,
};
visitor.visit_pattern(pattern);
}
}
}
}
/// Add an identifier to the set of visited names in `self` and emit a [`SemanticSyntaxError`]
/// if `ident` has already been seen.
fn insert(&mut self, ident: &'a ast::Identifier) {
if !self.names.insert(&ident.id) {
SemanticSyntaxChecker::add_error(
self.ctx,
SemanticSyntaxErrorKind::MultipleCaseAssignment(ident.id.clone()),
ident.range(),
);
}
}
}
pub trait SemanticSyntaxContext {
/// Returns `true` if a module's docstring boundary has been passed.
fn seen_docstring_boundary(&self) -> bool;

View file

@ -0,0 +1,722 @@
---
source: crates/ruff_python_parser/tests/fixtures.rs
input_file: crates/ruff_python_parser/resources/inline/err/multiple_assignment_in_case_pattern.py
---
## AST
```
Module(
ModModule {
range: 0..456,
body: [
Match(
StmtMatch {
range: 0..444,
subject: NumberLiteral(
ExprNumberLiteral {
range: 6..7,
value: Int(
2,
),
},
),
cases: [
MatchCase {
range: 13..32,
pattern: MatchSequence(
PatternMatchSequence {
range: 18..27,
patterns: [
MatchAs(
PatternMatchAs {
range: 19..20,
pattern: None,
name: Some(
Identifier {
id: Name("y"),
range: 19..20,
},
),
},
),
MatchAs(
PatternMatchAs {
range: 22..23,
pattern: None,
name: Some(
Identifier {
id: Name("z"),
range: 22..23,
},
),
},
),
MatchAs(
PatternMatchAs {
range: 25..26,
pattern: None,
name: Some(
Identifier {
id: Name("y"),
range: 25..26,
},
),
},
),
],
},
),
guard: None,
body: [
Expr(
StmtExpr {
range: 29..32,
value: EllipsisLiteral(
ExprEllipsisLiteral {
range: 29..32,
},
),
},
),
],
},
MatchCase {
range: 54..74,
pattern: MatchSequence(
PatternMatchSequence {
range: 59..69,
patterns: [
MatchAs(
PatternMatchAs {
range: 60..61,
pattern: None,
name: Some(
Identifier {
id: Name("y"),
range: 60..61,
},
),
},
),
MatchAs(
PatternMatchAs {
range: 63..64,
pattern: None,
name: Some(
Identifier {
id: Name("z"),
range: 63..64,
},
),
},
),
MatchStar(
PatternMatchStar {
range: 66..68,
name: Some(
Identifier {
id: Name("y"),
range: 67..68,
},
),
},
),
],
},
),
guard: None,
body: [
Expr(
StmtExpr {
range: 71..74,
value: EllipsisLiteral(
ExprEllipsisLiteral {
range: 71..74,
},
),
},
),
],
},
MatchCase {
range: 96..115,
pattern: MatchSequence(
PatternMatchSequence {
range: 101..110,
patterns: [
MatchAs(
PatternMatchAs {
range: 102..103,
pattern: None,
name: Some(
Identifier {
id: Name("y"),
range: 102..103,
},
),
},
),
MatchAs(
PatternMatchAs {
range: 105..106,
pattern: None,
name: Some(
Identifier {
id: Name("y"),
range: 105..106,
},
),
},
),
MatchAs(
PatternMatchAs {
range: 108..109,
pattern: None,
name: Some(
Identifier {
id: Name("y"),
range: 108..109,
},
),
},
),
],
},
),
guard: None,
body: [
Expr(
StmtExpr {
range: 112..115,
value: EllipsisLiteral(
ExprEllipsisLiteral {
range: 112..115,
},
),
},
),
],
},
MatchCase {
range: 146..168,
pattern: MatchMapping(
PatternMatchMapping {
range: 151..163,
keys: [
NumberLiteral(
ExprNumberLiteral {
range: 152..153,
value: Int(
1,
),
},
),
NumberLiteral(
ExprNumberLiteral {
range: 158..159,
value: Int(
2,
),
},
),
],
patterns: [
MatchAs(
PatternMatchAs {
range: 155..156,
pattern: None,
name: Some(
Identifier {
id: Name("x"),
range: 155..156,
},
),
},
),
MatchAs(
PatternMatchAs {
range: 161..162,
pattern: None,
name: Some(
Identifier {
id: Name("x"),
range: 161..162,
},
),
},
),
],
rest: None,
},
),
guard: None,
body: [
Expr(
StmtExpr {
range: 165..168,
value: EllipsisLiteral(
ExprEllipsisLiteral {
range: 165..168,
},
),
},
),
],
},
MatchCase {
range: 207..228,
pattern: MatchMapping(
PatternMatchMapping {
range: 212..223,
keys: [
NumberLiteral(
ExprNumberLiteral {
range: 213..214,
value: Int(
1,
),
},
),
],
patterns: [
MatchAs(
PatternMatchAs {
range: 216..217,
pattern: None,
name: Some(
Identifier {
id: Name("x"),
range: 216..217,
},
),
},
),
],
rest: Some(
Identifier {
id: Name("x"),
range: 221..222,
},
),
},
),
guard: None,
body: [
Expr(
StmtExpr {
range: 225..228,
value: EllipsisLiteral(
ExprEllipsisLiteral {
range: 225..228,
},
),
},
),
],
},
MatchCase {
range: 269..290,
pattern: MatchClass(
PatternMatchClass {
range: 274..285,
cls: Name(
ExprName {
range: 274..279,
id: Name("Class"),
ctx: Load,
},
),
arguments: PatternArguments {
range: 279..285,
patterns: [
MatchAs(
PatternMatchAs {
range: 280..281,
pattern: None,
name: Some(
Identifier {
id: Name("x"),
range: 280..281,
},
),
},
),
MatchAs(
PatternMatchAs {
range: 283..284,
pattern: None,
name: Some(
Identifier {
id: Name("x"),
range: 283..284,
},
),
},
),
],
keywords: [],
},
},
),
guard: None,
body: [
Expr(
StmtExpr {
range: 287..290,
value: EllipsisLiteral(
ExprEllipsisLiteral {
range: 287..290,
},
),
},
),
],
},
MatchCase {
range: 320..345,
pattern: MatchClass(
PatternMatchClass {
range: 325..340,
cls: Name(
ExprName {
range: 325..330,
id: Name("Class"),
ctx: Load,
},
),
arguments: PatternArguments {
range: 330..340,
patterns: [],
keywords: [
PatternKeyword {
range: 331..334,
attr: Identifier {
id: Name("x"),
range: 331..332,
},
pattern: MatchValue(
PatternMatchValue {
range: 333..334,
value: NumberLiteral(
ExprNumberLiteral {
range: 333..334,
value: Int(
1,
),
},
),
},
),
},
PatternKeyword {
range: 336..339,
attr: Identifier {
id: Name("x"),
range: 336..337,
},
pattern: MatchValue(
PatternMatchValue {
range: 338..339,
value: NumberLiteral(
ExprNumberLiteral {
range: 338..339,
value: Int(
2,
),
},
),
},
),
},
],
},
},
),
guard: None,
body: [
Expr(
StmtExpr {
range: 342..345,
value: EllipsisLiteral(
ExprEllipsisLiteral {
range: 342..345,
},
),
},
),
],
},
MatchCase {
range: 372..412,
pattern: MatchOr(
PatternMatchOr {
range: 377..407,
patterns: [
MatchSequence(
PatternMatchSequence {
range: 377..380,
patterns: [
MatchAs(
PatternMatchAs {
range: 378..379,
pattern: None,
name: Some(
Identifier {
id: Name("x"),
range: 378..379,
},
),
},
),
],
},
),
MatchMapping(
PatternMatchMapping {
range: 383..389,
keys: [
NumberLiteral(
ExprNumberLiteral {
range: 384..385,
value: Int(
1,
),
},
),
],
patterns: [
MatchAs(
PatternMatchAs {
range: 387..388,
pattern: None,
name: Some(
Identifier {
id: Name("x"),
range: 387..388,
},
),
},
),
],
rest: None,
},
),
MatchClass(
PatternMatchClass {
range: 392..407,
cls: Name(
ExprName {
range: 392..397,
id: Name("Class"),
ctx: Load,
},
),
arguments: PatternArguments {
range: 397..407,
patterns: [],
keywords: [
PatternKeyword {
range: 398..401,
attr: Identifier {
id: Name("x"),
range: 398..399,
},
pattern: MatchValue(
PatternMatchValue {
range: 400..401,
value: NumberLiteral(
ExprNumberLiteral {
range: 400..401,
value: Int(
1,
),
},
),
},
),
},
PatternKeyword {
range: 403..406,
attr: Identifier {
id: Name("x"),
range: 403..404,
},
pattern: MatchValue(
PatternMatchValue {
range: 405..406,
value: NumberLiteral(
ExprNumberLiteral {
range: 405..406,
value: Int(
2,
),
},
),
},
),
},
],
},
},
),
],
},
),
guard: None,
body: [
Expr(
StmtExpr {
range: 409..412,
value: EllipsisLiteral(
ExprEllipsisLiteral {
range: 409..412,
},
),
},
),
],
},
MatchCase {
range: 428..444,
pattern: MatchAs(
PatternMatchAs {
range: 433..439,
pattern: Some(
MatchAs(
PatternMatchAs {
range: 433..434,
pattern: None,
name: Some(
Identifier {
id: Name("x"),
range: 433..434,
},
),
},
),
),
name: Some(
Identifier {
id: Name("x"),
range: 438..439,
},
),
},
),
guard: None,
body: [
Expr(
StmtExpr {
range: 441..444,
value: EllipsisLiteral(
ExprEllipsisLiteral {
range: 441..444,
},
),
},
),
],
},
],
},
),
],
},
)
```
## Semantic Syntax Errors
|
1 | match 2:
2 | case [y, z, y]: ... # MatchSequence
| ^ Syntax Error: multiple assignments to name `y` in pattern
3 | case [y, z, *y]: ... # MatchSequence
4 | case [y, y, y]: ... # MatchSequence multiple
|
|
1 | match 2:
2 | case [y, z, y]: ... # MatchSequence
3 | case [y, z, *y]: ... # MatchSequence
| ^ Syntax Error: multiple assignments to name `y` in pattern
4 | case [y, y, y]: ... # MatchSequence multiple
5 | case {1: x, 2: x}: ... # MatchMapping duplicate pattern
|
|
2 | case [y, z, y]: ... # MatchSequence
3 | case [y, z, *y]: ... # MatchSequence
4 | case [y, y, y]: ... # MatchSequence multiple
| ^ Syntax Error: multiple assignments to name `y` in pattern
5 | case {1: x, 2: x}: ... # MatchMapping duplicate pattern
6 | case {1: x, **x}: ... # MatchMapping duplicate in **rest
|
|
2 | case [y, z, y]: ... # MatchSequence
3 | case [y, z, *y]: ... # MatchSequence
4 | case [y, y, y]: ... # MatchSequence multiple
| ^ Syntax Error: multiple assignments to name `y` in pattern
5 | case {1: x, 2: x}: ... # MatchMapping duplicate pattern
6 | case {1: x, **x}: ... # MatchMapping duplicate in **rest
|
|
3 | case [y, z, *y]: ... # MatchSequence
4 | case [y, y, y]: ... # MatchSequence multiple
5 | case {1: x, 2: x}: ... # MatchMapping duplicate pattern
| ^ Syntax Error: multiple assignments to name `x` in pattern
6 | case {1: x, **x}: ... # MatchMapping duplicate in **rest
7 | case Class(x, x): ... # MatchClass positional
|
|
4 | case [y, y, y]: ... # MatchSequence multiple
5 | case {1: x, 2: x}: ... # MatchMapping duplicate pattern
6 | case {1: x, **x}: ... # MatchMapping duplicate in **rest
| ^ Syntax Error: multiple assignments to name `x` in pattern
7 | case Class(x, x): ... # MatchClass positional
8 | case Class(x=1, x=2): ... # MatchClass keyword
|
|
5 | case {1: x, 2: x}: ... # MatchMapping duplicate pattern
6 | case {1: x, **x}: ... # MatchMapping duplicate in **rest
7 | case Class(x, x): ... # MatchClass positional
| ^ Syntax Error: multiple assignments to name `x` in pattern
8 | case Class(x=1, x=2): ... # MatchClass keyword
9 | case [x] | {1: x} | Class(x=1, x=2): ... # MatchOr
|
|
6 | case {1: x, **x}: ... # MatchMapping duplicate in **rest
7 | case Class(x, x): ... # MatchClass positional
8 | case Class(x=1, x=2): ... # MatchClass keyword
| ^ Syntax Error: multiple assignments to name `x` in pattern
9 | case [x] | {1: x} | Class(x=1, x=2): ... # MatchOr
10 | case x as x: ... # MatchAs
|
|
7 | case Class(x, x): ... # MatchClass positional
8 | case Class(x=1, x=2): ... # MatchClass keyword
9 | case [x] | {1: x} | Class(x=1, x=2): ... # MatchOr
| ^ Syntax Error: multiple assignments to name `x` in pattern
10 | case x as x: ... # MatchAs
|
|
8 | case Class(x=1, x=2): ... # MatchClass keyword
9 | case [x] | {1: x} | Class(x=1, x=2): ... # MatchOr
10 | case x as x: ... # MatchAs
| ^ Syntax Error: multiple assignments to name `x` in pattern
|

View file

@ -0,0 +1,115 @@
---
source: crates/ruff_python_parser/tests/fixtures.rs
input_file: crates/ruff_python_parser/resources/inline/ok/multiple_assignment_in_case_pattern.py
snapshot_kind: text
---
## AST
```
Module(
ModModule {
range: 0..42,
body: [
Match(
StmtMatch {
range: 0..41,
subject: NumberLiteral(
ExprNumberLiteral {
range: 6..7,
value: Int(
2,
),
},
),
cases: [
MatchCase {
range: 13..41,
pattern: MatchOr(
PatternMatchOr {
range: 18..36,
patterns: [
MatchClass(
PatternMatchClass {
range: 18..26,
cls: Name(
ExprName {
range: 18..23,
id: Name("Class"),
ctx: Load,
},
),
arguments: PatternArguments {
range: 23..26,
patterns: [
MatchAs(
PatternMatchAs {
range: 24..25,
pattern: None,
name: Some(
Identifier {
id: Name("x"),
range: 24..25,
},
),
},
),
],
keywords: [],
},
},
),
MatchSequence(
PatternMatchSequence {
range: 29..32,
patterns: [
MatchAs(
PatternMatchAs {
range: 30..31,
pattern: None,
name: Some(
Identifier {
id: Name("x"),
range: 30..31,
},
),
},
),
],
},
),
MatchAs(
PatternMatchAs {
range: 35..36,
pattern: None,
name: Some(
Identifier {
id: Name("x"),
range: 35..36,
},
),
},
),
],
},
),
guard: None,
body: [
Expr(
StmtExpr {
range: 38..41,
value: EllipsisLiteral(
ExprEllipsisLiteral {
range: 38..41,
},
),
},
),
],
},
],
},
),
],
},
)
```