From 835e31b3ff8b1ab7665a64c519632550ac54acf1 Mon Sep 17 00:00:00 2001 From: Brent Westbrook <36778786+ntBre@users.noreply.github.com> Date: Mon, 10 Nov 2025 10:51:51 -0500 Subject: [PATCH] Fix syntax error false positive on alternative `match` patterns (#21362) Summary -- Fixes #21360 by using the union of names instead of overwriting them, as Micha suggested originally on #21104. This avoids overwriting the `n` name in the `Subscript` by the empty set of names visited in the nested OR pattern before visiting the other arm of the outer OR pattern. Test Plan -- A new inline test case taken from the issue --- .../inline/ok/nested_alternative_patterns.py | 2 + .../ruff_python_parser/src/semantic_errors.rs | 4 +- ...syntax@nested_alternative_patterns.py.snap | 212 +++++++++++++++++- 3 files changed, 216 insertions(+), 2 deletions(-) diff --git a/crates/ruff_python_parser/resources/inline/ok/nested_alternative_patterns.py b/crates/ruff_python_parser/resources/inline/ok/nested_alternative_patterns.py index d322fa3899..a12be52ac3 100644 --- a/crates/ruff_python_parser/resources/inline/ok/nested_alternative_patterns.py +++ b/crates/ruff_python_parser/resources/inline/ok/nested_alternative_patterns.py @@ -5,3 +5,5 @@ match 42: case [[x] | [x]] | x: ... match 42: case [[x | x] | [x]] | x: ... +match 42: + case ast.Subscript(n, ast.Constant() | ast.Slice()) | ast.Attribute(n): ... diff --git a/crates/ruff_python_parser/src/semantic_errors.rs b/crates/ruff_python_parser/src/semantic_errors.rs index 2775aa9065..1577f80cb5 100644 --- a/crates/ruff_python_parser/src/semantic_errors.rs +++ b/crates/ruff_python_parser/src/semantic_errors.rs @@ -1868,6 +1868,8 @@ impl<'a, Ctx: SemanticSyntaxContext> MatchPatternVisitor<'a, Ctx> { // case [[x] | [x]] | x: ... // match 42: // case [[x | x] | [x]] | x: ... + // match 42: + // case ast.Subscript(n, ast.Constant() | ast.Slice()) | ast.Attribute(n): ... SemanticSyntaxChecker::add_error( self.ctx, SemanticSyntaxErrorKind::DifferentMatchPatternBindings, @@ -1875,7 +1877,7 @@ impl<'a, Ctx: SemanticSyntaxContext> MatchPatternVisitor<'a, Ctx> { ); break; } - self.names = visitor.names; + self.names.extend(visitor.names); } } } diff --git a/crates/ruff_python_parser/tests/snapshots/valid_syntax@nested_alternative_patterns.py.snap b/crates/ruff_python_parser/tests/snapshots/valid_syntax@nested_alternative_patterns.py.snap index 8d241b04ef..b685a0656d 100644 --- a/crates/ruff_python_parser/tests/snapshots/valid_syntax@nested_alternative_patterns.py.snap +++ b/crates/ruff_python_parser/tests/snapshots/valid_syntax@nested_alternative_patterns.py.snap @@ -8,7 +8,7 @@ input_file: crates/ruff_python_parser/resources/inline/ok/nested_alternative_pat Module( ModModule { node_index: NodeIndex(None), - range: 0..181, + range: 0..271, body: [ Match( StmtMatch { @@ -489,6 +489,216 @@ Module( ], }, ), + Match( + StmtMatch { + node_index: NodeIndex(None), + range: 181..270, + subject: NumberLiteral( + ExprNumberLiteral { + node_index: NodeIndex(None), + range: 187..189, + value: Int( + 42, + ), + }, + ), + cases: [ + MatchCase { + range: 195..270, + node_index: NodeIndex(None), + pattern: MatchOr( + PatternMatchOr { + node_index: NodeIndex(None), + range: 200..265, + patterns: [ + MatchClass( + PatternMatchClass { + node_index: NodeIndex(None), + range: 200..246, + cls: Attribute( + ExprAttribute { + node_index: NodeIndex(None), + range: 200..213, + value: Name( + ExprName { + node_index: NodeIndex(None), + range: 200..203, + id: Name("ast"), + ctx: Load, + }, + ), + attr: Identifier { + id: Name("Subscript"), + range: 204..213, + node_index: NodeIndex(None), + }, + ctx: Load, + }, + ), + arguments: PatternArguments { + range: 213..246, + node_index: NodeIndex(None), + patterns: [ + MatchAs( + PatternMatchAs { + node_index: NodeIndex(None), + range: 214..215, + pattern: None, + name: Some( + Identifier { + id: Name("n"), + range: 214..215, + node_index: NodeIndex(None), + }, + ), + }, + ), + MatchOr( + PatternMatchOr { + node_index: NodeIndex(None), + range: 217..245, + patterns: [ + MatchClass( + PatternMatchClass { + node_index: NodeIndex(None), + range: 217..231, + cls: Attribute( + ExprAttribute { + node_index: NodeIndex(None), + range: 217..229, + value: Name( + ExprName { + node_index: NodeIndex(None), + range: 217..220, + id: Name("ast"), + ctx: Load, + }, + ), + attr: Identifier { + id: Name("Constant"), + range: 221..229, + node_index: NodeIndex(None), + }, + ctx: Load, + }, + ), + arguments: PatternArguments { + range: 229..231, + node_index: NodeIndex(None), + patterns: [], + keywords: [], + }, + }, + ), + MatchClass( + PatternMatchClass { + node_index: NodeIndex(None), + range: 234..245, + cls: Attribute( + ExprAttribute { + node_index: NodeIndex(None), + range: 234..243, + value: Name( + ExprName { + node_index: NodeIndex(None), + range: 234..237, + id: Name("ast"), + ctx: Load, + }, + ), + attr: Identifier { + id: Name("Slice"), + range: 238..243, + node_index: NodeIndex(None), + }, + ctx: Load, + }, + ), + arguments: PatternArguments { + range: 243..245, + node_index: NodeIndex(None), + patterns: [], + keywords: [], + }, + }, + ), + ], + }, + ), + ], + keywords: [], + }, + }, + ), + MatchClass( + PatternMatchClass { + node_index: NodeIndex(None), + range: 249..265, + cls: Attribute( + ExprAttribute { + node_index: NodeIndex(None), + range: 249..262, + value: Name( + ExprName { + node_index: NodeIndex(None), + range: 249..252, + id: Name("ast"), + ctx: Load, + }, + ), + attr: Identifier { + id: Name("Attribute"), + range: 253..262, + node_index: NodeIndex(None), + }, + ctx: Load, + }, + ), + arguments: PatternArguments { + range: 262..265, + node_index: NodeIndex(None), + patterns: [ + MatchAs( + PatternMatchAs { + node_index: NodeIndex(None), + range: 263..264, + pattern: None, + name: Some( + Identifier { + id: Name("n"), + range: 263..264, + node_index: NodeIndex(None), + }, + ), + }, + ), + ], + keywords: [], + }, + }, + ), + ], + }, + ), + guard: None, + body: [ + Expr( + StmtExpr { + node_index: NodeIndex(None), + range: 267..270, + value: EllipsisLiteral( + ExprEllipsisLiteral { + node_index: NodeIndex(None), + range: 267..270, + }, + ), + }, + ), + ], + }, + ], + }, + ), ], }, )