Skip to content

Commit

Permalink
Raise syntax error for unparenthesized generator expr in multi-argume…
Browse files Browse the repository at this point in the history 
…nt call (#12445)

## Summary

This PR fixes a bug to raise a syntax error when an unparenthesized
generator expression is used as an argument to a call when there are
more than one argument.

For reference, the grammar is:
```
primary:
    | ...
    | primary genexp 
    | primary '(' [arguments] ')' 
    | ...

genexp:
    | '(' ( assignment_expression | expression !':=') for_if_clauses ')' 
```

The `genexp` requires the parenthesis as mentioned in the grammar. So,
the grammar for a call expression is either a name followed by a
generator expression or a name followed by a list of argument. In the
former case, the parenthesis are excluded because the generator
expression provides them while in the later case, the parenthesis are
explicitly provided for a list of arguments which means that the
generator expression requires it's own parenthesis.

This was discovered in #12420.

## Test Plan

Add test cases for valid and invalid syntax.

Make sure that the parser from CPython also raises this at the parsing
step:
```console
$ python3.13 -m ast parser/_.py
  File "parser/_.py", line 1
    total(1, 2, x for x in range(5), 6)
                ^^^^^^^^^^^^^^^^^^^
SyntaxError: Generator expression must be parenthesized

$ python3.13 -m ast parser/_.py
  File "parser/_.py", line 1
    sum(x for x in range(10), 10)
        ^^^^^^^^^^^^^^^^^^^^
SyntaxError: Generator expression must be parenthesized
```
  • Loading branch information
@dhruvmanila
dhruvmanila authored Jul 22, 2024
1 parent f8735e1 commit 978909f
Show file tree
Hide file tree
Showing 7 changed files with 336 additions and 4 deletions.
2 changes: 1 addition & 1 deletion crates/ruff_linter/resources/test/fixtures/flake8_comprehensions/C419_1.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
sum(x.val for x in bar)
min(x.val for x in bar)
max(x.val for x in bar)
sum(x.val for x in bar, 0)
sum((x.val for x in bar), 0)

# Multi-line
sum(
Expand Down
2 changes: 2 additions & 0 deletions crates/ruff_python_parser/resources/inline/err/args_unparenthesized_generator.py
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
sum(x for x in range(10), 5)
total(1, 2, x for x in range(5), 6)
1 change: 1 addition & 0 deletions crates/ruff_python_parser/resources/inline/ok/args_unparenthesized_generator.py
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
sum(x for x in range(10))
5 changes: 5 additions & 0 deletions crates/ruff_python_parser/src/error.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,8 @@ pub enum ParseErrorType {
UnparenthesizedNamedExpression,
/// An unparenthesized tuple expression was found where it is not allowed.
UnparenthesizedTupleExpression,
/// An unparenthesized generator expression was found where it is not allowed.
UnparenthesizedGeneratorExpression,

/// An invalid usage of a lambda expression was found.
InvalidLambdaExpressionUsage,
Expand Down Expand Up @@ -216,6 +218,9 @@ impl std::fmt::Display for ParseErrorType {
ParseErrorType::UnparenthesizedTupleExpression => {
f.write_str("Unparenthesized tuple expression cannot be used here")
}
ParseErrorType::UnparenthesizedGeneratorExpression => {
f.write_str("Unparenthesized generator expression cannot be used here")
}
ParseErrorType::InvalidYieldExpressionUsage => {
f.write_str("Yield expression cannot be used here")
}
Expand Down
26 changes: 23 additions & 3 deletions crates/ruff_python_parser/src/parser/expression.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2272,9 +2272,10 @@ impl<'src> Parser<'src> {
command
}

/// Validate that the given arguments doesn't have any duplicate keyword argument.
///
/// Report errors for all the duplicate names found.
/// Performs the following validations on the function call arguments:
/// 1. There aren't any duplicate keyword argument
/// 2. If there are more than one argument (positional or keyword), all generator expressions
/// present should be parenthesized.
fn validate_arguments(&mut self, arguments: &ast::Arguments) {
let mut all_arg_names =
FxHashSet::with_capacity_and_hasher(arguments.keywords.len(), FxBuildHasher);
Expand All @@ -2292,6 +2293,25 @@ impl<'src> Parser<'src> {
);
}
}

if arguments.len() > 1 {
for arg in arguments.args.iter() {
if let Some(ast::ExprGenerator {
range,
parenthesized: false,
..
}) = arg.as_generator_expr()
{
// test_ok args_unparenthesized_generator
// sum(x for x in range(10))

// test_err args_unparenthesized_generator
// sum(x for x in range(10), 5)
// total(1, 2, x for x in range(5), 6)
self.add_error(ParseErrorType::UnparenthesizedGeneratorExpression, range);
}
}
}
}
}

Expand Down
213 changes: 213 additions & 0 deletions .../ruff_python_parser/tests/snapshots/invalid_syntax@args_unparenthesized_generator.py.snap
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,213 @@
---
source: crates/ruff_python_parser/tests/fixtures.rs
input_file: crates/ruff_python_parser/resources/inline/err/args_unparenthesized_generator.py
---
## AST

```
Module(
ModModule {
range: 0..65,
body: [
Expr(
StmtExpr {
range: 0..28,
value: Call(
ExprCall {
range: 0..28,
func: Name(
ExprName {
range: 0..3,
id: Name("sum"),
ctx: Load,
},
),
arguments: Arguments {
range: 3..28,
args: [
Generator(
ExprGenerator {
range: 4..24,
elt: Name(
ExprName {
range: 4..5,
id: Name("x"),
ctx: Load,
},
),
generators: [
Comprehension {
range: 6..24,
target: Name(
ExprName {
range: 10..11,
id: Name("x"),
ctx: Store,
},
),
iter: Call(
ExprCall {
range: 15..24,
func: Name(
ExprName {
range: 15..20,
id: Name("range"),
ctx: Load,
},
),
arguments: Arguments {
range: 20..24,
args: [
NumberLiteral(
ExprNumberLiteral {
range: 21..23,
value: Int(
10,
),
},
),
],
keywords: [],
},
},
),
ifs: [],
is_async: false,
},
],
parenthesized: false,
},
),
NumberLiteral(
ExprNumberLiteral {
range: 26..27,
value: Int(
5,
),
},
),
],
keywords: [],
},
},
),
},
),
Expr(
StmtExpr {
range: 29..64,
value: Call(
ExprCall {
range: 29..64,
func: Name(
ExprName {
range: 29..34,
id: Name("total"),
ctx: Load,
},
),
arguments: Arguments {
range: 34..64,
args: [
NumberLiteral(
ExprNumberLiteral {
range: 35..36,
value: Int(
1,
),
},
),
NumberLiteral(
ExprNumberLiteral {
range: 38..39,
value: Int(
2,
),
},
),
Generator(
ExprGenerator {
range: 41..60,
elt: Name(
ExprName {
range: 41..42,
id: Name("x"),
ctx: Load,
},
),
generators: [
Comprehension {
range: 43..60,
target: Name(
ExprName {
range: 47..48,
id: Name("x"),
ctx: Store,
},
),
iter: Call(
ExprCall {
range: 52..60,
func: Name(
ExprName {
range: 52..57,
id: Name("range"),
ctx: Load,
},
),
arguments: Arguments {
range: 57..60,
args: [
NumberLiteral(
ExprNumberLiteral {
range: 58..59,
value: Int(
5,
),
},
),
],
keywords: [],
},
},
),
ifs: [],
is_async: false,
},
],
parenthesized: false,
},
),
NumberLiteral(
ExprNumberLiteral {
range: 62..63,
value: Int(
6,
),
},
),
],
keywords: [],
},
},
),
},
),
],
},
)
```
## Errors

|
1 | sum(x for x in range(10), 5)
| ^^^^^^^^^^^^^^^^^^^^ Syntax Error: Unparenthesized generator expression cannot be used here
2 | total(1, 2, x for x in range(5), 6)
|


|
1 | sum(x for x in range(10), 5)
2 | total(1, 2, x for x in range(5), 6)
| ^^^^^^^^^^^^^^^^^^^ Syntax Error: Unparenthesized generator expression cannot be used here
|
91 changes: 91 additions & 0 deletions ...es/ruff_python_parser/tests/snapshots/valid_syntax@args_unparenthesized_generator.py.snap
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
---
source: crates/ruff_python_parser/tests/fixtures.rs
input_file: crates/ruff_python_parser/resources/inline/ok/args_unparenthesized_generator.py
---
## AST

```
Module(
ModModule {
range: 0..26,
body: [
Expr(
StmtExpr {
range: 0..25,
value: Call(
ExprCall {
range: 0..25,
func: Name(
ExprName {
range: 0..3,
id: Name("sum"),
ctx: Load,
},
),
arguments: Arguments {
range: 3..25,
args: [
Generator(
ExprGenerator {
range: 4..24,
elt: Name(
ExprName {
range: 4..5,
id: Name("x"),
ctx: Load,
},
),
generators: [
Comprehension {
range: 6..24,
target: Name(
ExprName {
range: 10..11,
id: Name("x"),
ctx: Store,
},
),
iter: Call(
ExprCall {
range: 15..24,
func: Name(
ExprName {
range: 15..20,
id: Name("range"),
ctx: Load,
},
),
arguments: Arguments {
range: 20..24,
args: [
NumberLiteral(
ExprNumberLiteral {
range: 21..23,
value: Int(
10,
),
},
),
],
keywords: [],
},
},
),
ifs: [],
is_async: false,
},
],
parenthesized: false,
},
),
],
keywords: [],
},
},
),
},
),
],
},
)
```

0 comments on commit 978909f

Please sign in to comment.