mirrors/ruff
1
0
Fork 0
mirror of https://github.com/astral-sh/ruff.git synced 2025-07-12 07:35:07 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 3

Consider if expression for parenthesized with items parsing (#11010)

Browse source 
## Summary

This PR fixes the bug in parenthesized with items parsing where the `if`
expression would result into a syntax error.

The reason being that once we identify that the ambiguous left
parenthesis belongs to the context expression, the parser converts the
parsed with item into an equivalent expression. Then, the parser
continuous to parse any postfix expressions. Now, attribute, subscript,
and call are taken into account as they're grouped in
`parse_postfix_expression` but `if` expression has it's own parsing
function.

Use `parse_if_expression` once all postfix expressions have been parsed.
Ideally, I think that `if` could be included in postfix expression
parsing as they can be chained as well (`x if True else y if True else
z`).

## Test Plan

Add test cases and verified the snapshots.
This commit is contained in:
Dhruv Manilawala 2024-04-18 20:00:15 +05:30 • committed by GitHub
parent 8020d486f6
commit 6c4d779140
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
4 changed files with 297 additions and 3 deletions
Download patch file Download diff file Expand all files Collapse all files

4
crates/ruff_python_parser/resources/inline/ok/ambiguous_lpar_with_items_if_expr.py Normal file
View file

@ -0,0 +1,4 @@
with (x) if True else y: ...
with (x for x in iter) if True else y: ...
with (x async for x in iter) if True else y: ...
with (x)[0] if True else y: ...

2
crates/ruff_python_parser/src/parser/expression.rs
View file

@ -2370,7 +2370,7 @@ impl<'src> Parser<'src> {
/// If the parser isn't positioned at an `if` token.
///
/// See: <https://docs.python.org/3/reference/expressions.html#conditional-expressions>
fn parse_if_expression(&mut self, body: Expr, start: TextSize) -> ast::ExprIf {
pub(super) fn parse_if_expression(&mut self, body: Expr, start: TextSize) -> ast::ExprIf {
self.bump(TokenKind::If);
let test = self.parse_simple_expression(AllowStarredExpression::No);

15
crates/ruff_python_parser/src/parser/statement.rs
View file

@ -2085,7 +2085,7 @@ impl<'src> Parser<'src> {
self.expect(TokenKind::Rpar);
}
let lhs = if parsed_with_items.len() == 1 && !has_trailing_comma {
let mut lhs = if parsed_with_items.len() == 1 && !has_trailing_comma {
// SAFETY: We've checked that `items` has only one item.
let expr = parsed_with_items.pop().unwrap().item.context_expr;
@ -2145,7 +2145,18 @@ impl<'src> Parser<'src> {
// considered when parsing the with item in the case. So, the parser
// stops when it sees the `)` token and doesn't check for any postfix
// expressions.
let context_expr = self.parse_postfix_expression(lhs, start);
lhs = self.parse_postfix_expression(lhs, start);
// test_ok ambiguous_lpar_with_items_if_expr
// with (x) if True else y: ...
// with (x for x in iter) if True else y: ...
// with (x async for x in iter) if True else y: ...
// with (x)[0] if True else y: ...
let context_expr = if self.at(TokenKind::If) {
Expr::If(self.parse_if_expression(lhs, start))
} else {
lhs
};
let optional_vars = self
.at(TokenKind::As)

279
crates/ruff_python_parser/tests/snapshots/valid_syntax@ambiguous_lpar_with_items_if_expr.py.snap Normal file
View file

@ -0,0 +1,279 @@
---
source: crates/ruff_python_parser/tests/fixtures.rs
input_file: crates/ruff_python_parser/resources/inline/ok/ambiguous_lpar_with_items_if_expr.py
---
## AST
```
Module(
ModModule {
range: 0..153,
body: [
With(
StmtWith {
range: 0..28,
is_async: false,
items: [
WithItem {
range: 5..23,
context_expr: If(
ExprIf {
range: 5..23,
test: BooleanLiteral(
ExprBooleanLiteral {
range: 12..16,
value: true,
},
),
body: Name(
ExprName {
range: 6..7,
id: "x",
ctx: Load,
},
),
orelse: Name(
ExprName {
range: 22..23,
id: "y",
ctx: Load,
},
),
},
),
optional_vars: None,
},
],
body: [
Expr(
StmtExpr {
range: 25..28,
value: EllipsisLiteral(
ExprEllipsisLiteral {
range: 25..28,
},
),
},
),
],
},
),
With(
StmtWith {
range: 29..71,
is_async: false,
items: [
WithItem {
range: 34..66,
context_expr: If(
ExprIf {
range: 34..66,
test: BooleanLiteral(
ExprBooleanLiteral {
range: 55..59,
value: true,
},
),
body: Generator(
ExprGenerator {
range: 34..51,
elt: Name(
ExprName {
range: 35..36,
id: "x",
ctx: Load,
},
),
generators: [
Comprehension {
range: 37..50,
target: Name(
ExprName {
range: 41..42,
id: "x",
ctx: Store,
},
),
iter: Name(
ExprName {
range: 46..50,
id: "iter",
ctx: Load,
},
),
ifs: [],
is_async: false,
},
],
parenthesized: true,
},
),
orelse: Name(
ExprName {
range: 65..66,
id: "y",
ctx: Load,
},
),
},
),
optional_vars: None,
},
],
body: [
Expr(
StmtExpr {
range: 68..71,
value: EllipsisLiteral(
ExprEllipsisLiteral {
range: 68..71,
},
),
},
),
],
},
),
With(
StmtWith {
range: 72..120,
is_async: false,
items: [
WithItem {
range: 77..115,
context_expr: If(
ExprIf {
range: 77..115,
test: BooleanLiteral(
ExprBooleanLiteral {
range: 104..108,
value: true,
},
),
body: Generator(
ExprGenerator {
range: 77..100,
elt: Name(
ExprName {
range: 78..79,
id: "x",
ctx: Load,
},
),
generators: [
Comprehension {
range: 80..99,
target: Name(
ExprName {
range: 90..91,
id: "x",
ctx: Store,
},
),
iter: Name(
ExprName {
range: 95..99,
id: "iter",
ctx: Load,
},
),
ifs: [],
is_async: true,
},
],
parenthesized: true,
},
),
orelse: Name(
ExprName {
range: 114..115,
id: "y",
ctx: Load,
},
),
},
),
optional_vars: None,
},
],
body: [
Expr(
StmtExpr {
range: 117..120,
value: EllipsisLiteral(
ExprEllipsisLiteral {
range: 117..120,
},
),
},
),
],
},
),
With(
StmtWith {
range: 121..152,
is_async: false,
items: [
WithItem {
range: 126..147,
context_expr: If(
ExprIf {
range: 126..147,
test: BooleanLiteral(
ExprBooleanLiteral {
range: 136..140,
value: true,
},
),
body: Subscript(
ExprSubscript {
range: 126..132,
value: Name(
ExprName {
range: 127..128,
id: "x",
ctx: Load,
},
),
slice: NumberLiteral(
ExprNumberLiteral {
range: 130..131,
value: Int(
0,
),
},
),
ctx: Load,
},
),
orelse: Name(
ExprName {
range: 146..147,
id: "y",
ctx: Load,
},
),
},
),
optional_vars: None,
},
],
body: [
Expr(
StmtExpr {
range: 149..152,
value: EllipsisLiteral(
ExprEllipsisLiteral {
range: 149..152,
},
),
},
),
],
},
),
],
},
)
```