diff --git a/crates/ruff_linter/src/checkers/ast/mod.rs b/crates/ruff_linter/src/checkers/ast/mod.rs index c253f39e3e..400b1142e0 100644 --- a/crates/ruff_linter/src/checkers/ast/mod.rs +++ b/crates/ruff_linter/src/checkers/ast/mod.rs @@ -549,6 +549,7 @@ impl SemanticSyntaxContext for Checker<'_> { } SemanticSyntaxErrorKind::ReboundComprehensionVariable | SemanticSyntaxErrorKind::DuplicateTypeParameter + | SemanticSyntaxErrorKind::MultipleCaseAssignment(_) | SemanticSyntaxErrorKind::IrrefutableCasePattern(_) if self.settings.preview.is_enabled() => { @@ -556,6 +557,7 @@ impl SemanticSyntaxContext for Checker<'_> { } SemanticSyntaxErrorKind::ReboundComprehensionVariable | SemanticSyntaxErrorKind::DuplicateTypeParameter + | SemanticSyntaxErrorKind::MultipleCaseAssignment(_) | SemanticSyntaxErrorKind::IrrefutableCasePattern(_) => {} } } diff --git a/crates/ruff_python_parser/resources/inline/err/multiple_assignment_in_case_pattern.py b/crates/ruff_python_parser/resources/inline/err/multiple_assignment_in_case_pattern.py new file mode 100644 index 0000000000..45323edd66 --- /dev/null +++ b/crates/ruff_python_parser/resources/inline/err/multiple_assignment_in_case_pattern.py @@ -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 diff --git a/crates/ruff_python_parser/resources/inline/ok/multiple_assignment_in_case_pattern.py b/crates/ruff_python_parser/resources/inline/ok/multiple_assignment_in_case_pattern.py new file mode 100644 index 0000000000..05b1908b24 --- /dev/null +++ b/crates/ruff_python_parser/resources/inline/ok/multiple_assignment_in_case_pattern.py @@ -0,0 +1,2 @@ +match 2: + case Class(x) | [x] | x: ... diff --git a/crates/ruff_python_parser/src/semantic_errors.rs b/crates/ruff_python_parser/src/semantic_errors.rs index 3d7918bee5..4e2351a983 100644 --- a/crates/ruff_python_parser/src/semantic_errors.rs +++ b/crates/ruff_python_parser/src/semantic_errors.rs @@ -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(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(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; diff --git a/crates/ruff_python_parser/tests/snapshots/invalid_syntax@multiple_assignment_in_case_pattern.py.snap b/crates/ruff_python_parser/tests/snapshots/invalid_syntax@multiple_assignment_in_case_pattern.py.snap new file mode 100644 index 0000000000..3484caa037 --- /dev/null +++ b/crates/ruff_python_parser/tests/snapshots/invalid_syntax@multiple_assignment_in_case_pattern.py.snap @@ -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 + | diff --git a/crates/ruff_python_parser/tests/snapshots/valid_syntax@multiple_assignment_in_case_pattern.py.snap b/crates/ruff_python_parser/tests/snapshots/valid_syntax@multiple_assignment_in_case_pattern.py.snap new file mode 100644 index 0000000000..a12c28b913 --- /dev/null +++ b/crates/ruff_python_parser/tests/snapshots/valid_syntax@multiple_assignment_in_case_pattern.py.snap @@ -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, + }, + ), + }, + ), + ], + }, + ], + }, + ), + ], + }, +) +```