Skip to content

Commit 8020d48

Browse files
dhruvmaniladhruvmanila
authored
Reset FOR_TARGET context for all kinds of parentheses (#11009)
## Summary This PR fixes a bug in the new parser which involves the parser context w.r.t. for statement. This is specifically around the `in` keyword which can be present in the target expression and shouldn't be considered to be part of the `for` statement header. Ideally it should use a context which is passed between functions, thus using a call stack to set / unset a specific variant which will be done in a follow-up PR as it requires some amount of refactor. ## Test Plan Add test cases and update the snapshots.
1 parent 13ffb5b commit 8020d48

7 files changed

+445
-27
lines changed

‎crates/ruff_python_parser/resources/inline/err/for_in_target_postfix_expr.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
for d(x in y) in target: ...

‎crates/ruff_python_parser/resources/inline/err/parenthesized_compare_expr_in_for.py

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,2 +1,5 @@
11
for (x in y)() in iter: ...
22
for (x in y) in iter: ...
3+
for (x in y, z) in iter: ...
4+
for [x in y, z] in iter: ...
5+
for {x in y, z} in iter: ...

‎crates/ruff_python_parser/resources/inline/ok/for_in_target_postfix_expr.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
for d[x in y] in target: ...

‎crates/ruff_python_parser/src/parser/expression.rs

Lines changed: 47 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -360,7 +360,7 @@ impl<'src> Parser<'src> {
360360
}
361361

362362
// Don't parse a `CompareExpr` if we are parsing a `Comprehension` or `ForStmt`
363-
if token.is_compare_operator() && self.has_ctx(ParserCtxFlags::FOR_TARGET) {
363+
if matches!(token, TokenKind::In) && self.has_ctx(ParserCtxFlags::FOR_TARGET) {
364364
break;
365365
}
366366

@@ -659,11 +659,38 @@ impl<'src> Parser<'src> {
659659
Expr::IpyEscapeCommand(self.parse_ipython_escape_command_expression())
660660
}
661661
TokenKind::String | TokenKind::FStringStart => self.parse_strings(),
662-
TokenKind::Lpar => {
663-
return self.parse_parenthesized_expression();
662+
tok @ (TokenKind::Lpar | TokenKind::Lsqb | TokenKind::Lbrace) => {
663+
// We need to unset the `FOR_TARGET` in the context when parsing an expression
664+
// inside a parentheses, curly brace or brackets otherwise the `in` operator of a
665+
// comparison expression will not be parsed in a `for` target.
666+
667+
// test_ok parenthesized_compare_expr_in_for
668+
// for (x in y)[0] in iter: ...
669+
// for (x in y).attr in iter: ...
670+
671+
// test_err parenthesized_compare_expr_in_for
672+
// for (x in y)() in iter: ...
673+
// for (x in y) in iter: ...
674+
// for (x in y, z) in iter: ...
675+
// for [x in y, z] in iter: ...
676+
// for {x in y, z} in iter: ...
677+
let current_context = self.ctx - ParserCtxFlags::FOR_TARGET;
678+
let saved_context = self.set_ctx(current_context);
679+
680+
let expr = match tok {
681+
TokenKind::Lpar => {
682+
let parsed_expr = self.parse_parenthesized_expression();
683+
self.restore_ctx(current_context, saved_context);
684+
return parsed_expr;
685+
}
686+
TokenKind::Lsqb => self.parse_list_like_expression(),
687+
TokenKind::Lbrace => self.parse_set_or_dict_like_expression(),
688+
_ => unreachable!(),
689+
};
690+
691+
self.restore_ctx(current_context, saved_context);
692+
expr
664693
}
665-
TokenKind::Lsqb => self.parse_list_like_expression(),
666-
TokenKind::Lbrace => self.parse_set_or_dict_like_expression(),
667694

668695
kind => {
669696
if kind.is_keyword() {
@@ -692,14 +719,25 @@ impl<'src> Parser<'src> {
692719
///
693720
/// This method does nothing if the current token is not a candidate for a postfix expression.
694721
pub(super) fn parse_postfix_expression(&mut self, mut lhs: Expr, start: TextSize) -> Expr {
695-
loop {
722+
// test_ok for_in_target_postfix_expr
723+
// for d[x in y] in target: ...
724+
725+
// test_err for_in_target_postfix_expr
726+
// for d(x in y) in target: ...
727+
let current_context = self.ctx - ParserCtxFlags::FOR_TARGET;
728+
let saved_context = self.set_ctx(current_context);
729+
730+
lhs = loop {
696731
lhs = match self.current_token_kind() {
697732
TokenKind::Lpar => Expr::Call(self.parse_call_expression(lhs, start)),
698733
TokenKind::Lsqb => Expr::Subscript(self.parse_subscript_expression(lhs, start)),
699734
TokenKind::Dot => Expr::Attribute(self.parse_attribute_expression(lhs, start)),
700735
_ => break lhs,
701736
};
702-
}
737+
};
738+
739+
self.restore_ctx(current_context, saved_context);
740+
lhs
703741
}
704742

705743
/// Parse a call expression.
@@ -1748,26 +1786,13 @@ impl<'src> Parser<'src> {
17481786
.into();
17491787
}
17501788

1751-
// We need to unset the `FOR_TARGET` in the context when parsing a parenthesized expression
1752-
// otherwise a parenthesized comparison expression will not be parsed in a `for` target.
1753-
1754-
// test_ok parenthesized_compare_expr_in_for
1755-
// for (x in y)[0] in iter: ...
1756-
// for (x in y).attr in iter: ...
1757-
1758-
// test_err parenthesized_compare_expr_in_for
1759-
// for (x in y)() in iter: ...
1760-
// for (x in y) in iter: ...
1761-
let current_context = self.ctx - ParserCtxFlags::FOR_TARGET;
1762-
let saved_context = self.set_ctx(current_context);
1763-
17641789
// Use the more general rule of the three to parse the first element
17651790
// and limit it later.
17661791
let mut parsed_expr = self.parse_yield_expression_or_else(|p| {
17671792
p.parse_star_expression_or_higher(AllowNamedExpression::Yes)
17681793
});
17691794

1770-
let parsed_expr = match self.current_token_kind() {
1795+
match self.current_token_kind() {
17711796
TokenKind::Comma => {
17721797
// grammar: `tuple`
17731798
let tuple = self.parse_tuple_expression(
@@ -1812,11 +1837,7 @@ impl<'src> Parser<'src> {
18121837
parsed_expr.is_parenthesized = true;
18131838
parsed_expr
18141839
}
1815-
};
1816-
1817-
self.restore_ctx(current_context, saved_context);
1818-
1819-
parsed_expr
1840+
}
18201841
}
18211842

18221843
/// Parses multiple items separated by a comma into a tuple expression.

‎crates/ruff_python_parser/tests/snapshots/invalid_syntax@for_in_target_postfix_expr.py.snap

Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,89 @@
1+
---
2+
source: crates/ruff_python_parser/tests/fixtures.rs
3+
input_file: crates/ruff_python_parser/resources/inline/err/for_in_target_postfix_expr.py
4+
---
5+
## AST
6+
7+
```
8+
Module(
9+
ModModule {
10+
range: 0..29,
11+
body: [
12+
For(
13+
StmtFor {
14+
range: 0..28,
15+
is_async: false,
16+
target: Call(
17+
ExprCall {
18+
range: 4..13,
19+
func: Name(
20+
ExprName {
21+
range: 4..5,
22+
id: "d",
23+
ctx: Load,
24+
},
25+
),
26+
arguments: Arguments {
27+
range: 5..13,
28+
args: [
29+
Compare(
30+
ExprCompare {
31+
range: 6..12,
32+
left: Name(
33+
ExprName {
34+
range: 6..7,
35+
id: "x",
36+
ctx: Load,
37+
},
38+
),
39+
ops: [
40+
In,
41+
],
42+
comparators: [
43+
Name(
44+
ExprName {
45+
range: 11..12,
46+
id: "y",
47+
ctx: Load,
48+
},
49+
),
50+
],
51+
},
52+
),
53+
],
54+
keywords: [],
55+
},
56+
},
57+
),
58+
iter: Name(
59+
ExprName {
60+
range: 17..23,
61+
id: "target",
62+
ctx: Load,
63+
},
64+
),
65+
body: [
66+
Expr(
67+
StmtExpr {
68+
range: 25..28,
69+
value: EllipsisLiteral(
70+
ExprEllipsisLiteral {
71+
range: 25..28,
72+
},
73+
),
74+
},
75+
),
76+
],
77+
orelse: [],
78+
},
79+
),
80+
],
81+
},
82+
)
83+
```
84+
## Errors
85+
86+
|
87+
1 | for d(x in y) in target: ...
88+
| ^^^^^^^^^ Syntax Error: Invalid assignment target
89+
|

0 commit comments

Comments
 (0)