Use speculative parsing for with-items (#11770)

## Summary

This PR updates the with-items parsing logic to use speculative parsing
instead.

### Existing logic

First, let's understand the previous logic:
1. The parser sees `(`, it doesn't know whether it's part of a
parenthesized with items or a parenthesized expression
2. Consider it a parenthesized with items and perform a hand-rolled
speculative parsing
3. Then, verify the assumption and if it's incorrect convert the parsed
with items into an appropriate expression which becomes part of the
first with item

Here, in (3) there are lots of edge cases which we've to deal with:
1. Trailing comma with a single element should be [converted to the
expression as
is](9b2cf569b2/crates/ruff_python_parser/src/parser/statement.rs (L2140-L2153))
2. Trailing comma with multiple elements should be [converted to a tuple
expression](9b2cf569b2/crates/ruff_python_parser/src/parser/statement.rs (L2155-L2178))
3. Limit the allowed expression based on whether it's
[(1)](9b2cf569b2/crates/ruff_python_parser/src/parser/statement.rs (L2144-L2152))
or
[(2)](9b2cf569b2/crates/ruff_python_parser/src/parser/statement.rs (L2157-L2171))
4. [Consider postfix
expressions](9b2cf569b2/crates/ruff_python_parser/src/parser/statement.rs (L2181-L2200))
after (3)
5. [Consider `if`
expressions](9b2cf569b2/crates/ruff_python_parser/src/parser/statement.rs (L2203-L2208))
after (3)
6. [Consider binary
expressions](9b2cf569b2/crates/ruff_python_parser/src/parser/statement.rs (L2210-L2228))
after (3)

Consider other cases like
* [Single generator
expression](9b2cf569b2/crates/ruff_python_parser/src/parser/statement.rs (L2020-L2035))
* [Expecting a
comma](9b2cf569b2/crates/ruff_python_parser/src/parser/statement.rs (L2122-L2130))

And, this is all possible only if we allow parsing these expressions in
the [with item parsing
logic](9b2cf569b2/crates/ruff_python_parser/src/parser/statement.rs (L2287-L2334)).

### Speculative parsing

With #11457 merged, we can simplify this logic by changing the step (3)
from above to just rewind the parser back to the `(` if our assumption
(parenthesized with-items) was incorrect and then continue parsing it
considering parenthesized expression.

This also behaves a lot similar to what a PEG parser does which is to
consider the first grammar rule and if it fails consider the second
grammar rule and so on.

resolves: #11639 

## Test Plan

- [x] Verify the updated snapshots
- [x] Run the fuzzer on around 3000 valid source code (locally)
This commit is contained in:
Dhruv Manilawala 2024-06-06 14:29:56 +05:30 committed by GitHub
parent 5a5a588a72
commit 6c1fa1d440
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
9 changed files with 556 additions and 731 deletions

View file

@ -375,10 +375,10 @@ Module(
is_async: false,
items: [
WithItem {
range: 364..384,
range: 363..384,
context_expr: Generator(
ExprGenerator {
range: 364..384,
range: 363..384,
elt: Name(
ExprName {
range: 364..365,
@ -426,7 +426,7 @@ Module(
is_async: false,
},
],
parenthesized: false,
parenthesized: true,
},
),
optional_vars: None,
@ -459,90 +459,89 @@ Module(
),
With(
StmtWith {
range: 397..435,
range: 397..410,
is_async: false,
items: [
WithItem {
range: 403..407,
context_expr: Name(
range: 402..410,
context_expr: Tuple(
ExprTuple {
range: 402..410,
elts: [
Name(
ExprName {
range: 403..407,
id: "item",
ctx: Load,
},
),
Name(
ExprName {
range: 409..410,
id: "x",
ctx: Load,
},
),
],
ctx: Load,
parenthesized: true,
},
),
optional_vars: None,
},
],
body: [],
},
),
For(
StmtFor {
range: 411..429,
is_async: false,
target: Name(
ExprName {
range: 415..416,
id: "x",
ctx: Store,
},
),
iter: Call(
ExprCall {
range: 420..429,
func: Name(
ExprName {
range: 403..407,
id: "item",
range: 420..425,
id: "range",
ctx: Load,
},
),
optional_vars: None,
},
WithItem {
range: 409..429,
context_expr: Generator(
ExprGenerator {
range: 409..429,
elt: Name(
ExprName {
range: 409..410,
id: "x",
ctx: Load,
arguments: Arguments {
range: 425..429,
args: [
NumberLiteral(
ExprNumberLiteral {
range: 426..428,
value: Int(
10,
),
},
),
generators: [
Comprehension {
range: 411..429,
target: Name(
ExprName {
range: 415..416,
id: "x",
ctx: Store,
},
),
iter: Call(
ExprCall {
range: 420..429,
func: Name(
ExprName {
range: 420..425,
id: "range",
ctx: Load,
},
),
arguments: Arguments {
range: 425..429,
args: [
NumberLiteral(
ExprNumberLiteral {
range: 426..428,
value: Int(
10,
),
},
),
],
keywords: [],
},
},
),
ifs: [],
is_async: false,
},
],
parenthesized: false,
},
),
optional_vars: None,
},
],
body: [
Expr(
StmtExpr {
range: 432..435,
value: EllipsisLiteral(
ExprEllipsisLiteral {
range: 432..435,
},
),
],
keywords: [],
},
),
],
},
),
body: [],
orelse: [],
},
),
Expr(
StmtExpr {
range: 432..435,
value: EllipsisLiteral(
ExprEllipsisLiteral {
range: 432..435,
},
),
},
),
With(
@ -588,10 +587,10 @@ Module(
is_async: false,
items: [
WithItem {
range: 523..539,
range: 522..539,
context_expr: Generator(
ExprGenerator {
range: 523..539,
range: 522..539,
elt: Starred(
ExprStarred {
range: 523..525,
@ -626,7 +625,7 @@ Module(
is_async: false,
},
],
parenthesized: false,
parenthesized: true,
},
),
optional_vars: None,
@ -659,88 +658,92 @@ Module(
),
With(
StmtWith {
range: 552..594,
range: 552..567,
is_async: false,
items: [
WithItem {
range: 558..563,
context_expr: Name(
ExprName {
range: 558..563,
id: "item1",
ctx: Load,
},
),
optional_vars: None,
},
WithItem {
range: 565..581,
context_expr: Generator(
ExprGenerator {
range: 565..581,
elt: Starred(
ExprStarred {
range: 565..567,
value: Name(
ExprName {
range: 566..567,
id: "x",
ctx: Load,
},
),
ctx: Load,
},
),
generators: [
Comprehension {
range: 568..581,
target: Name(
ExprName {
range: 572..573,
id: "x",
ctx: Store,
},
),
iter: Name(
ExprName {
range: 577..581,
id: "iter",
ctx: Load,
},
),
ifs: [],
is_async: false,
},
range: 557..567,
context_expr: Tuple(
ExprTuple {
range: 557..567,
elts: [
Name(
ExprName {
range: 558..563,
id: "item1",
ctx: Load,
},
),
Starred(
ExprStarred {
range: 565..567,
value: Name(
ExprName {
range: 566..567,
id: "x",
ctx: Load,
},
),
ctx: Load,
},
),
],
parenthesized: false,
},
),
optional_vars: None,
},
WithItem {
range: 583..588,
context_expr: Name(
ExprName {
range: 583..588,
id: "item2",
ctx: Load,
parenthesized: true,
},
),
optional_vars: None,
},
],
body: [
Expr(
StmtExpr {
range: 591..594,
value: EllipsisLiteral(
ExprEllipsisLiteral {
range: 591..594,
body: [],
},
),
For(
StmtFor {
range: 568..588,
is_async: false,
target: Name(
ExprName {
range: 572..573,
id: "x",
ctx: Store,
},
),
iter: Tuple(
ExprTuple {
range: 577..588,
elts: [
Name(
ExprName {
range: 577..581,
id: "iter",
ctx: Load,
},
),
},
),
],
Name(
ExprName {
range: 583..588,
id: "item2",
ctx: Load,
},
),
],
ctx: Load,
parenthesized: false,
},
),
body: [],
orelse: [],
},
),
Expr(
StmtExpr {
range: 591..594,
value: EllipsisLiteral(
ExprEllipsisLiteral {
range: 591..594,
},
),
},
),
With(
@ -1095,10 +1098,10 @@ Module(
is_async: false,
items: [
WithItem {
range: 751..771,
range: 750..771,
context_expr: Generator(
ExprGenerator {
range: 751..766,
range: 750..766,
elt: Name(
ExprName {
range: 751..752,
@ -1127,7 +1130,7 @@ Module(
is_async: false,
},
],
parenthesized: false,
parenthesized: true,
},
),
optional_vars: Some(
@ -1360,7 +1363,7 @@ Module(
2 | # These cases should raise the correct syntax error and recover properly.
3 |
4 | with (item1, item2),: ...
| ^ Syntax Error: Expected an expression
| ^ Syntax Error: Trailing comma not allowed
5 | with (item1, item2), as f: ...
6 | with (item1, item2), item3,: ...
|
@ -1369,7 +1372,7 @@ Module(
|
4 | with (item1, item2),: ...
5 | with (item1, item2), as f: ...
| ^^ Syntax Error: Expected an expression
| ^^ Syntax Error: Expected an expression or the end of the with item list
6 | with (item1, item2), item3,: ...
7 | with (*item): ...
|
@ -1429,7 +1432,16 @@ Module(
9 | with (item := 10 as f): ...
10 | with (item1, item2 := 10 as f): ...
11 | with (x for x in range(10), item): ...
| ^^^^^^^^^^^^^^^^^^^^ Syntax Error: Unparenthesized generator expression cannot be used here
| ^ Syntax Error: Expected ')', found ','
12 | with (item, x for x in range(10)): ...
|
|
9 | with (item := 10 as f): ...
10 | with (item1, item2 := 10 as f): ...
11 | with (x for x in range(10), item): ...
| ^ Syntax Error: Expected ',', found ')'
12 | with (item, x for x in range(10)): ...
|
@ -1438,7 +1450,27 @@ Module(
10 | with (item1, item2 := 10 as f): ...
11 | with (x for x in range(10), item): ...
12 | with (item, x for x in range(10)): ...
| ^^^^^^^^^^^^^^^^^^^^ Syntax Error: Unparenthesized generator expression cannot be used here
| ^^^ Syntax Error: Expected ',', found 'for'
13 |
14 | # Make sure the parser doesn't report the same error twice
|
|
10 | with (item1, item2 := 10 as f): ...
11 | with (x for x in range(10), item): ...
12 | with (item, x for x in range(10)): ...
| ^ Syntax Error: Expected ':', found ')'
13 |
14 | # Make sure the parser doesn't report the same error twice
|
|
10 | with (item1, item2 := 10 as f): ...
11 | with (x for x in range(10), item): ...
12 | with (item, x for x in range(10)): ...
| ^ Syntax Error: Expected a statement
13 |
14 | # Make sure the parser doesn't report the same error twice
|
@ -1463,10 +1495,48 @@ Module(
|
|
15 | with ((*item)): ...
16 |
17 | with (*x for x in iter, item): ...
| ^ Syntax Error: Expected ')', found ','
18 | with (item1, *x for x in iter, item2): ...
19 | with (x as f, *y): ...
|
|
15 | with ((*item)): ...
16 |
17 | with (*x for x in iter, item): ...
| ^ Syntax Error: Expected ',', found ')'
18 | with (item1, *x for x in iter, item2): ...
19 | with (x as f, *y): ...
|
|
17 | with (*x for x in iter, item): ...
18 | with (item1, *x for x in iter, item2): ...
| ^^ Syntax Error: Iterable unpacking cannot be used in a comprehension
| ^^^ Syntax Error: Expected ',', found 'for'
19 | with (x as f, *y): ...
20 | with (*x, y as f): ...
|
|
17 | with (*x for x in iter, item): ...
18 | with (item1, *x for x in iter, item2): ...
| ^ Syntax Error: Expected ':', found ')'
19 | with (x as f, *y): ...
20 | with (*x, y as f): ...
|
|
17 | with (*x for x in iter, item): ...
18 | with (item1, *x for x in iter, item2): ...
| ^ Syntax Error: Expected a statement
19 | with (x as f, *y): ...
20 | with (*x, y as f): ...
|
@ -1535,7 +1605,17 @@ Module(
23 | with (x, yield from y): ...
24 | with (x as f, y) as f: ...
25 | with (x for x in iter as y): ...
| ^^^^^^^^^^^^^^^ Syntax Error: Unparenthesized generator expression cannot be used here
| ^^ Syntax Error: Expected ')', found 'as'
26 |
27 | # The inner `(...)` is parsed as parenthesized expression
|
|
23 | with (x, yield from y): ...
24 | with (x as f, y) as f: ...
25 | with (x for x in iter as y): ...
| ^ Syntax Error: Expected ',', found ')'
26 |
27 | # The inner `(...)` is parsed as parenthesized expression
|

View file

@ -15,7 +15,7 @@ Module(
is_async: false,
items: [
WithItem {
range: 6..6,
range: 5..6,
context_expr: Name(
ExprName {
range: 6..6,
@ -25,32 +25,34 @@ Module(
),
optional_vars: None,
},
WithItem {
range: 9..14,
context_expr: BinOp(
ExprBinOp {
range: 9..14,
left: Name(
ExprName {
range: 9..10,
id: "x",
ctx: Load,
},
),
op: Add,
right: Name(
ExprName {
range: 13..14,
id: "y",
ctx: Load,
},
),
},
),
optional_vars: None,
},
],
body: [],
body: [
Expr(
StmtExpr {
range: 9..14,
value: BinOp(
ExprBinOp {
range: 9..14,
left: Name(
ExprName {
range: 9..10,
id: "x",
ctx: Load,
},
),
op: Add,
right: Name(
ExprName {
range: 13..14,
id: "y",
ctx: Load,
},
),
},
),
},
),
],
},
),
],

View file

@ -15,7 +15,7 @@ Module(
is_async: false,
items: [
WithItem {
range: 6..6,
range: 5..6,
context_expr: Name(
ExprName {
range: 6..6,

View file

@ -0,0 +1,60 @@
---
source: crates/ruff_python_parser/tests/fixtures.rs
input_file: crates/ruff_python_parser/resources/inline/err/with_items_parenthesized_missing_colon.py
---
## AST
```
Module(
ModModule {
range: 0..57,
body: [
With(
StmtWith {
range: 28..56,
is_async: false,
items: [
WithItem {
range: 34..39,
context_expr: Name(
ExprName {
range: 34..39,
id: "item1",
ctx: Load,
},
),
optional_vars: None,
},
WithItem {
range: 41..46,
context_expr: Name(
ExprName {
range: 41..46,
id: "item2",
ctx: Load,
},
),
optional_vars: None,
},
],
body: [
Pass(
StmtPass {
range: 52..56,
},
),
],
},
),
],
},
)
```
## Errors
|
1 | # `)` followed by a newline
2 | with (item1, item2)
| ^ Syntax Error: Expected ':', found newline
3 | pass
|

View file

@ -239,42 +239,49 @@ Module(
),
With(
StmtWith {
range: 136..160,
range: 136..159,
is_async: false,
items: [
WithItem {
range: 142..147,
context_expr: Name(
ExprName {
range: 142..147,
id: "item1",
range: 141..154,
context_expr: Tuple(
ExprTuple {
range: 141..154,
elts: [
Name(
ExprName {
range: 142..147,
id: "item1",
ctx: Load,
},
),
Name(
ExprName {
range: 149..154,
id: "item2",
ctx: Load,
},
),
],
ctx: Load,
},
),
optional_vars: None,
},
WithItem {
range: 149..154,
context_expr: Name(
ExprName {
range: 149..154,
id: "item2",
ctx: Load,
},
),
optional_vars: None,
},
WithItem {
range: 156..159,
context_expr: EllipsisLiteral(
ExprEllipsisLiteral {
range: 156..159,
parenthesized: true,
},
),
optional_vars: None,
},
],
body: [],
body: [
Expr(
StmtExpr {
range: 156..159,
value: EllipsisLiteral(
ExprEllipsisLiteral {
range: 156..159,
},
),
},
),
],
},
),
],