[syntax-errors] Irrefutable case pattern before final case (#16905)

Summary
--

Detects irrefutable `match` cases before the final case using a modified
version
of the existing `Pattern::is_irrefutable` method from the AST crate. The
modified method helps to retrieve a more precise diagnostic range to
match what
Python 3.13 shows in the REPL.

Test Plan
--

New inline tests, as well as some updates to existing tests that had
irrefutable
patterns before the last block.
This commit is contained in:
Brent Westbrook 2025-03-26 12:27:16 -04:00 committed by GitHub
parent 58350ec93b
commit 5697d21fca
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
17 changed files with 1420 additions and 594 deletions

View file

@ -0,0 +1,12 @@
match x:
case var: ... # capture pattern
case 2: ...
match x:
case _: ...
case 2: ... # wildcard pattern
match x:
case var1 as var2: ... # as pattern with irrefutable left-hand side
case 2: ...
match x:
case enum.variant | var: ... # or pattern with irrefutable part
case 2: ...

View file

@ -0,0 +1,9 @@
match x:
case 2: ...
case var: ...
match x:
case 2: ...
case _: ...
match x:
case var if True: ... # don't try to refute a guarded pattern
case 2: ...

View file

@ -1,3 +1,4 @@
match foo:
case foo_bar: ...
match foo:
case _: ...

View file

@ -1,4 +1,6 @@
match foo:
case case: ...
match foo:
case match: ...
match foo:
case type: ...

View file

@ -1,5 +1,5 @@
match subject:
case a: ...
case a if x: ...
case a, b: ...
case a, b if x: ...
case a: ...

View file

@ -243,18 +243,21 @@ match x:
match x:
case a:
...
match x:
case a as b:
...
match x:
case 1 | 2 as two:
...
case 1 + 3j as sum:
...
case a.b as ab:
...
case _:
...
case _ as x:
...
match x:
case _:
...
# PatternMatchSequence
match x:

View file

@ -1060,10 +1060,10 @@ impl RecoveryContextKind {
None => {
// test_ok match_sequence_pattern_terminator
// match subject:
// case a: ...
// case a if x: ...
// case a, b: ...
// case a, b if x: ...
// case a: ...
matches!(p.current_token_kind(), TokenKind::Colon | TokenKind::If)
.then_some(ListTerminatorKind::Regular)
}

View file

@ -488,13 +488,16 @@ impl Parser<'_> {
// test_ok match_as_pattern_soft_keyword
// match foo:
// case case: ...
// match foo:
// case match: ...
// match foo:
// case type: ...
let ident = self.parse_identifier();
// test_ok match_as_pattern
// match foo:
// case foo_bar: ...
// match foo:
// case _: ...
Pattern::MatchAs(ast::PatternMatchAs {
range: ident.range,

View file

@ -9,7 +9,7 @@ use std::fmt::Display;
use ruff_python_ast::{
self as ast,
visitor::{walk_expr, Visitor},
Expr, PythonVersion, Stmt, StmtExpr, StmtImportFrom,
Expr, IrrefutablePatternKind, PythonVersion, Stmt, StmtExpr, StmtImportFrom,
};
use ruff_text_size::{Ranged, TextRange};
@ -54,27 +54,30 @@ impl SemanticSyntaxChecker {
}
fn check_stmt<Ctx: SemanticSyntaxContext>(&mut self, stmt: &ast::Stmt, ctx: &Ctx) {
if let Stmt::ImportFrom(StmtImportFrom { range, module, .. }) = stmt {
if self.seen_futures_boundary && matches!(module.as_deref(), Some("__future__")) {
Self::add_error(ctx, SemanticSyntaxErrorKind::LateFutureImport, *range);
match stmt {
Stmt::ImportFrom(StmtImportFrom { range, module, .. }) => {
if self.seen_futures_boundary && matches!(module.as_deref(), Some("__future__")) {
Self::add_error(ctx, SemanticSyntaxErrorKind::LateFutureImport, *range);
}
}
Stmt::Match(match_stmt) => {
Self::irrefutable_match_case(match_stmt, ctx);
}
Stmt::FunctionDef(ast::StmtFunctionDef { type_params, .. })
| Stmt::ClassDef(ast::StmtClassDef { type_params, .. })
| Stmt::TypeAlias(ast::StmtTypeAlias { type_params, .. }) => {
if let Some(type_params) = type_params {
Self::duplicate_type_parameter_name(type_params, ctx);
}
}
_ => {}
}
Self::duplicate_type_parameter_name(stmt, ctx);
}
fn duplicate_type_parameter_name<Ctx: SemanticSyntaxContext>(stmt: &ast::Stmt, ctx: &Ctx) {
let (Stmt::FunctionDef(ast::StmtFunctionDef { type_params, .. })
| Stmt::ClassDef(ast::StmtClassDef { type_params, .. })
| Stmt::TypeAlias(ast::StmtTypeAlias { type_params, .. })) = stmt
else {
return;
};
let Some(type_params) = type_params else {
return;
};
fn duplicate_type_parameter_name<Ctx: SemanticSyntaxContext>(
type_params: &ast::TypeParams,
ctx: &Ctx,
) {
if type_params.len() < 2 {
return;
}
@ -109,6 +112,49 @@ impl SemanticSyntaxChecker {
}
}
fn irrefutable_match_case<Ctx: SemanticSyntaxContext>(stmt: &ast::StmtMatch, ctx: &Ctx) {
// test_ok irrefutable_case_pattern_at_end
// match x:
// case 2: ...
// case var: ...
// match x:
// case 2: ...
// case _: ...
// match x:
// case var if True: ... # don't try to refute a guarded pattern
// case 2: ...
// test_err irrefutable_case_pattern
// match x:
// case var: ... # capture pattern
// case 2: ...
// match x:
// case _: ...
// case 2: ... # wildcard pattern
// match x:
// case var1 as var2: ... # as pattern with irrefutable left-hand side
// case 2: ...
// match x:
// case enum.variant | var: ... # or pattern with irrefutable part
// case 2: ...
for case in stmt
.cases
.iter()
.rev()
.skip(1)
.filter_map(|case| match case.guard {
Some(_) => None,
None => case.pattern.irrefutable_pattern(),
})
{
Self::add_error(
ctx,
SemanticSyntaxErrorKind::IrrefutableCasePattern(case.kind),
case.range,
);
}
}
pub fn visit_stmt<Ctx: SemanticSyntaxContext>(&mut self, stmt: &ast::Stmt, ctx: &Ctx) {
// update internal state
match stmt {
@ -209,7 +255,7 @@ pub struct SemanticSyntaxError {
impl Display for SemanticSyntaxError {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
match self.kind {
match &self.kind {
SemanticSyntaxErrorKind::LateFutureImport => {
f.write_str("__future__ imports must be at the top of the file")
}
@ -219,11 +265,23 @@ impl Display for SemanticSyntaxError {
SemanticSyntaxErrorKind::DuplicateTypeParameter => {
f.write_str("duplicate type parameter")
}
SemanticSyntaxErrorKind::IrrefutableCasePattern(kind) => match kind {
// These error messages are taken from CPython's syntax errors
IrrefutablePatternKind::Name(name) => {
write!(
f,
"name capture `{name}` makes remaining patterns unreachable"
)
}
IrrefutablePatternKind::Wildcard => {
f.write_str("wildcard makes remaining patterns unreachable")
}
},
}
}
}
#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
#[derive(Debug, Clone, PartialEq, Eq, Hash)]
pub enum SemanticSyntaxErrorKind {
/// Represents the use of a `__future__` import after the beginning of a file.
///
@ -265,6 +323,26 @@ pub enum SemanticSyntaxErrorKind {
/// class C[T, T]: ...
/// ```
DuplicateTypeParameter,
/// 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
/// case block, and it must be last."
///
/// ## Examples
///
/// ```python
/// match x:
/// case value: ... # irrefutable capture pattern
/// case other: ...
///
/// match x:
/// case _: ... # irrefutable wildcard pattern
/// case other: ...
/// ```
///
/// [Python reference]: https://docs.python.org/3/reference/compound_stmts.html#irrefutable-case-blocks
IrrefutableCasePattern(IrrefutablePatternKind),
}
/// Searches for the first named expression (`x := y`) rebinding one of the `iteration_variables` in

View file

@ -0,0 +1,374 @@
---
source: crates/ruff_python_parser/tests/fixtures.rs
input_file: crates/ruff_python_parser/resources/inline/err/irrefutable_case_pattern.py
---
## AST
```
Module(
ModModule {
range: 0..317,
body: [
Match(
StmtMatch {
range: 0..61,
subject: Name(
ExprName {
range: 6..7,
id: Name("x"),
ctx: Load,
},
),
cases: [
MatchCase {
range: 13..26,
pattern: MatchAs(
PatternMatchAs {
range: 18..21,
pattern: None,
name: Some(
Identifier {
id: Name("var"),
range: 18..21,
},
),
},
),
guard: None,
body: [
Expr(
StmtExpr {
range: 23..26,
value: EllipsisLiteral(
ExprEllipsisLiteral {
range: 23..26,
},
),
},
),
],
},
MatchCase {
range: 50..61,
pattern: MatchValue(
PatternMatchValue {
range: 55..56,
value: NumberLiteral(
ExprNumberLiteral {
range: 55..56,
value: Int(
2,
),
},
),
},
),
guard: None,
body: [
Expr(
StmtExpr {
range: 58..61,
value: EllipsisLiteral(
ExprEllipsisLiteral {
range: 58..61,
},
),
},
),
],
},
],
},
),
Match(
StmtMatch {
range: 62..102,
subject: Name(
ExprName {
range: 68..69,
id: Name("x"),
ctx: Load,
},
),
cases: [
MatchCase {
range: 75..86,
pattern: MatchAs(
PatternMatchAs {
range: 80..81,
pattern: None,
name: None,
},
),
guard: None,
body: [
Expr(
StmtExpr {
range: 83..86,
value: EllipsisLiteral(
ExprEllipsisLiteral {
range: 83..86,
},
),
},
),
],
},
MatchCase {
range: 91..102,
pattern: MatchValue(
PatternMatchValue {
range: 96..97,
value: NumberLiteral(
ExprNumberLiteral {
range: 96..97,
value: Int(
2,
),
},
),
},
),
guard: None,
body: [
Expr(
StmtExpr {
range: 99..102,
value: EllipsisLiteral(
ExprEllipsisLiteral {
range: 99..102,
},
),
},
),
],
},
],
},
),
Match(
StmtMatch {
range: 125..222,
subject: Name(
ExprName {
range: 131..132,
id: Name("x"),
ctx: Load,
},
),
cases: [
MatchCase {
range: 138..160,
pattern: MatchAs(
PatternMatchAs {
range: 143..155,
pattern: Some(
MatchAs(
PatternMatchAs {
range: 143..147,
pattern: None,
name: Some(
Identifier {
id: Name("var1"),
range: 143..147,
},
),
},
),
),
name: Some(
Identifier {
id: Name("var2"),
range: 151..155,
},
),
},
),
guard: None,
body: [
Expr(
StmtExpr {
range: 157..160,
value: EllipsisLiteral(
ExprEllipsisLiteral {
range: 157..160,
},
),
},
),
],
},
MatchCase {
range: 211..222,
pattern: MatchValue(
PatternMatchValue {
range: 216..217,
value: NumberLiteral(
ExprNumberLiteral {
range: 216..217,
value: Int(
2,
),
},
),
},
),
guard: None,
body: [
Expr(
StmtExpr {
range: 219..222,
value: EllipsisLiteral(
ExprEllipsisLiteral {
range: 219..222,
},
),
},
),
],
},
],
},
),
Match(
StmtMatch {
range: 223..316,
subject: Name(
ExprName {
range: 229..230,
id: Name("x"),
ctx: Load,
},
),
cases: [
MatchCase {
range: 236..264,
pattern: MatchOr(
PatternMatchOr {
range: 241..259,
patterns: [
MatchValue(
PatternMatchValue {
range: 241..253,
value: Attribute(
ExprAttribute {
range: 241..253,
value: Name(
ExprName {
range: 241..245,
id: Name("enum"),
ctx: Load,
},
),
attr: Identifier {
id: Name("variant"),
range: 246..253,
},
ctx: Load,
},
),
},
),
MatchAs(
PatternMatchAs {
range: 256..259,
pattern: None,
name: Some(
Identifier {
id: Name("var"),
range: 256..259,
},
),
},
),
],
},
),
guard: None,
body: [
Expr(
StmtExpr {
range: 261..264,
value: EllipsisLiteral(
ExprEllipsisLiteral {
range: 261..264,
},
),
},
),
],
},
MatchCase {
range: 305..316,
pattern: MatchValue(
PatternMatchValue {
range: 310..311,
value: NumberLiteral(
ExprNumberLiteral {
range: 310..311,
value: Int(
2,
),
},
),
},
),
guard: None,
body: [
Expr(
StmtExpr {
range: 313..316,
value: EllipsisLiteral(
ExprEllipsisLiteral {
range: 313..316,
},
),
},
),
],
},
],
},
),
],
},
)
```
## Semantic Syntax Errors
|
1 | match x:
2 | case var: ... # capture pattern
| ^^^ Syntax Error: name capture `var` makes remaining patterns unreachable
3 | case 2: ...
4 | match x:
|
|
3 | case 2: ...
4 | match x:
5 | case _: ...
| ^ Syntax Error: wildcard makes remaining patterns unreachable
6 | case 2: ... # wildcard pattern
7 | match x:
|
|
6 | case 2: ... # wildcard pattern
7 | match x:
8 | case var1 as var2: ... # as pattern with irrefutable left-hand side
| ^^^^ Syntax Error: name capture `var1` makes remaining patterns unreachable
9 | case 2: ...
10 | match x:
|
|
9 | case 2: ...
10 | match x:
11 | case enum.variant | var: ... # or pattern with irrefutable part
| ^^^ Syntax Error: name capture `var` makes remaining patterns unreachable
12 | case 2: ...
|

View file

@ -0,0 +1,230 @@
---
source: crates/ruff_python_parser/tests/fixtures.rs
input_file: crates/ruff_python_parser/resources/inline/ok/irrefutable_case_pattern_at_end.py
---
## AST
```
Module(
ModModule {
range: 0..176,
body: [
Match(
StmtMatch {
range: 0..42,
subject: Name(
ExprName {
range: 6..7,
id: Name("x"),
ctx: Load,
},
),
cases: [
MatchCase {
range: 13..24,
pattern: MatchValue(
PatternMatchValue {
range: 18..19,
value: NumberLiteral(
ExprNumberLiteral {
range: 18..19,
value: Int(
2,
),
},
),
},
),
guard: None,
body: [
Expr(
StmtExpr {
range: 21..24,
value: EllipsisLiteral(
ExprEllipsisLiteral {
range: 21..24,
},
),
},
),
],
},
MatchCase {
range: 29..42,
pattern: MatchAs(
PatternMatchAs {
range: 34..37,
pattern: None,
name: Some(
Identifier {
id: Name("var"),
range: 34..37,
},
),
},
),
guard: None,
body: [
Expr(
StmtExpr {
range: 39..42,
value: EllipsisLiteral(
ExprEllipsisLiteral {
range: 39..42,
},
),
},
),
],
},
],
},
),
Match(
StmtMatch {
range: 43..83,
subject: Name(
ExprName {
range: 49..50,
id: Name("x"),
ctx: Load,
},
),
cases: [
MatchCase {
range: 56..67,
pattern: MatchValue(
PatternMatchValue {
range: 61..62,
value: NumberLiteral(
ExprNumberLiteral {
range: 61..62,
value: Int(
2,
),
},
),
},
),
guard: None,
body: [
Expr(
StmtExpr {
range: 64..67,
value: EllipsisLiteral(
ExprEllipsisLiteral {
range: 64..67,
},
),
},
),
],
},
MatchCase {
range: 72..83,
pattern: MatchAs(
PatternMatchAs {
range: 77..78,
pattern: None,
name: None,
},
),
guard: None,
body: [
Expr(
StmtExpr {
range: 80..83,
value: EllipsisLiteral(
ExprEllipsisLiteral {
range: 80..83,
},
),
},
),
],
},
],
},
),
Match(
StmtMatch {
range: 84..175,
subject: Name(
ExprName {
range: 90..91,
id: Name("x"),
ctx: Load,
},
),
cases: [
MatchCase {
range: 97..118,
pattern: MatchAs(
PatternMatchAs {
range: 102..105,
pattern: None,
name: Some(
Identifier {
id: Name("var"),
range: 102..105,
},
),
},
),
guard: Some(
BooleanLiteral(
ExprBooleanLiteral {
range: 109..113,
value: true,
},
),
),
body: [
Expr(
StmtExpr {
range: 115..118,
value: EllipsisLiteral(
ExprEllipsisLiteral {
range: 115..118,
},
),
},
),
],
},
MatchCase {
range: 164..175,
pattern: MatchValue(
PatternMatchValue {
range: 169..170,
value: NumberLiteral(
ExprNumberLiteral {
range: 169..170,
value: Int(
2,
),
},
),
},
),
guard: None,
body: [
Expr(
StmtExpr {
range: 172..175,
value: EllipsisLiteral(
ExprEllipsisLiteral {
range: 172..175,
},
),
},
),
],
},
],
},
),
],
},
)
```

View file

@ -1,18 +1,17 @@
---
source: crates/ruff_python_parser/tests/fixtures.rs
input_file: crates/ruff_python_parser/resources/inline/ok/match_as_pattern.py
snapshot_kind: text
---
## AST
```
Module(
ModModule {
range: 0..49,
range: 0..60,
body: [
Match(
StmtMatch {
range: 0..48,
range: 0..32,
subject: Name(
ExprName {
range: 6..9,
@ -49,11 +48,25 @@ Module(
),
],
},
],
},
),
Match(
StmtMatch {
range: 33..59,
subject: Name(
ExprName {
range: 39..42,
id: Name("foo"),
ctx: Load,
},
),
cases: [
MatchCase {
range: 37..48,
range: 48..59,
pattern: MatchAs(
PatternMatchAs {
range: 42..43,
range: 53..54,
pattern: None,
name: None,
},
@ -62,10 +75,10 @@ Module(
body: [
Expr(
StmtExpr {
range: 45..48,
range: 56..59,
value: EllipsisLiteral(
ExprEllipsisLiteral {
range: 45..48,
range: 56..59,
},
),
},

View file

@ -1,18 +1,17 @@
---
source: crates/ruff_python_parser/tests/fixtures.rs
input_file: crates/ruff_python_parser/resources/inline/ok/match_as_pattern_soft_keyword.py
snapshot_kind: text
---
## AST
```
Module(
ModModule {
range: 0..69,
range: 0..91,
body: [
Match(
StmtMatch {
range: 0..68,
range: 0..29,
subject: Name(
ExprName {
range: 6..9,
@ -49,16 +48,30 @@ Module(
),
],
},
],
},
),
Match(
StmtMatch {
range: 30..60,
subject: Name(
ExprName {
range: 36..39,
id: Name("foo"),
ctx: Load,
},
),
cases: [
MatchCase {
range: 34..49,
range: 45..60,
pattern: MatchAs(
PatternMatchAs {
range: 39..44,
range: 50..55,
pattern: None,
name: Some(
Identifier {
id: Name("match"),
range: 39..44,
range: 50..55,
},
),
},
@ -67,26 +80,40 @@ Module(
body: [
Expr(
StmtExpr {
range: 46..49,
range: 57..60,
value: EllipsisLiteral(
ExprEllipsisLiteral {
range: 46..49,
range: 57..60,
},
),
},
),
],
},
],
},
),
Match(
StmtMatch {
range: 61..90,
subject: Name(
ExprName {
range: 67..70,
id: Name("foo"),
ctx: Load,
},
),
cases: [
MatchCase {
range: 54..68,
range: 76..90,
pattern: MatchAs(
PatternMatchAs {
range: 59..63,
range: 81..85,
pattern: None,
name: Some(
Identifier {
id: Name("type"),
range: 59..63,
range: 81..85,
},
),
},
@ -95,10 +122,10 @@ Module(
body: [
Expr(
StmtExpr {
range: 65..68,
range: 87..90,
value: EllipsisLiteral(
ExprEllipsisLiteral {
range: 65..68,
range: 87..90,
},
),
},

View file

@ -1,7 +1,6 @@
---
source: crates/ruff_python_parser/tests/fixtures.rs
input_file: crates/ruff_python_parser/resources/inline/ok/match_sequence_pattern_terminator.py
snapshot_kind: text
---
## AST
@ -22,7 +21,7 @@ Module(
),
cases: [
MatchCase {
range: 19..30,
range: 19..35,
pattern: MatchAs(
PatternMatchAs {
range: 24..25,
@ -35,38 +34,10 @@ Module(
),
},
),
guard: None,
body: [
Expr(
StmtExpr {
range: 27..30,
value: EllipsisLiteral(
ExprEllipsisLiteral {
range: 27..30,
},
),
},
),
],
},
MatchCase {
range: 35..51,
pattern: MatchAs(
PatternMatchAs {
range: 40..41,
pattern: None,
name: Some(
Identifier {
id: Name("a"),
range: 40..41,
},
),
},
),
guard: Some(
Name(
ExprName {
range: 45..46,
range: 29..30,
id: Name("x"),
ctx: Load,
},
@ -75,10 +46,10 @@ Module(
body: [
Expr(
StmtExpr {
range: 48..51,
range: 32..35,
value: EllipsisLiteral(
ExprEllipsisLiteral {
range: 48..51,
range: 32..35,
},
),
},
@ -86,78 +57,78 @@ Module(
],
},
MatchCase {
range: 56..70,
range: 40..54,
pattern: MatchSequence(
PatternMatchSequence {
range: 61..65,
range: 45..49,
patterns: [
MatchAs(
PatternMatchAs {
range: 61..62,
range: 45..46,
pattern: None,
name: Some(
Identifier {
id: Name("a"),
range: 61..62,
range: 45..46,
},
),
},
),
MatchAs(
PatternMatchAs {
range: 48..49,
pattern: None,
name: Some(
Identifier {
id: Name("b"),
range: 48..49,
},
),
},
),
],
},
),
guard: None,
body: [
Expr(
StmtExpr {
range: 51..54,
value: EllipsisLiteral(
ExprEllipsisLiteral {
range: 51..54,
},
),
},
),
],
},
MatchCase {
range: 59..78,
pattern: MatchSequence(
PatternMatchSequence {
range: 64..68,
patterns: [
MatchAs(
PatternMatchAs {
range: 64..65,
pattern: None,
name: Some(
Identifier {
id: Name("b"),
id: Name("a"),
range: 64..65,
},
),
},
),
],
},
),
guard: None,
body: [
Expr(
StmtExpr {
range: 67..70,
value: EllipsisLiteral(
ExprEllipsisLiteral {
range: 67..70,
},
),
},
),
],
},
MatchCase {
range: 75..94,
pattern: MatchSequence(
PatternMatchSequence {
range: 80..84,
patterns: [
MatchAs(
PatternMatchAs {
range: 80..81,
pattern: None,
name: Some(
Identifier {
id: Name("a"),
range: 80..81,
},
),
},
),
MatchAs(
PatternMatchAs {
range: 83..84,
range: 67..68,
pattern: None,
name: Some(
Identifier {
id: Name("b"),
range: 83..84,
range: 67..68,
},
),
},
@ -168,12 +139,40 @@ Module(
guard: Some(
Name(
ExprName {
range: 88..89,
range: 72..73,
id: Name("x"),
ctx: Load,
},
),
),
body: [
Expr(
StmtExpr {
range: 75..78,
value: EllipsisLiteral(
ExprEllipsisLiteral {
range: 75..78,
},
),
},
),
],
},
MatchCase {
range: 83..94,
pattern: MatchAs(
PatternMatchAs {
range: 88..89,
pattern: None,
name: Some(
Identifier {
id: Name("a"),
range: 88..89,
},
),
},
),
guard: None,
body: [
Expr(
StmtExpr {