Skip to content

Comments

Raise syntax error for unparenthesized generator expr in multi-argument call#12445

Merged
dhruvmanila merged 1 commit intomainfrom
dhruv/unparenthesized-genexp
Jul 22, 2024
Merged

Raise syntax error for unparenthesized generator expr in multi-argument call#12445
dhruvmanila merged 1 commit intomainfrom
dhruv/unparenthesized-genexp

Conversation

@dhruvmanila
Copy link
Member

@dhruvmanila dhruvmanila commented Jul 22, 2024 •
edited
Loading

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:

$ 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

@dhruvmanila dhruvmanila added the bug Something isn't working label Jul 22, 2024
@dhruvmanila dhruvmanila requested a review from MichaReiser as a code owner July 22, 2024 03:37
@dhruvmanila dhruvmanila force-pushed the dhruv/unparenthesized-genexp branch from fd11ae2 to b370b57 Compare July 22, 2024 04:12
@github-actions
Copy link
Contributor

github-actions bot commented Jul 22, 2024

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

✅ ecosystem check detected no linter changes.

Formatter (stable)

ℹ️ ecosystem check encountered format errors. (no format changes; 1 project error)

openai/openai-cookbook (error) 

warning: Detected debug build without --no-cache.
error: Failed to parse examples/chatgpt/gpt_actions_library/gpt_action_canvaslms.ipynb:15:1:1: Expected an expression
error: Failed to parse examples/chatgpt/gpt_actions_library/gpt_action_outlook.ipynb:13:1:1: Expected an expression
error: Failed to parse examples/chatgpt/gpt_actions_library/gpt_action_salesforce.ipynb:17:1:13: Simple statements must be separated by newlines or semicolons

Formatter (preview)

ℹ️ ecosystem check encountered format errors. (no format changes; 1 project error)

openai/openai-cookbook (error) 

ruff format --preview

warning: Detected debug build without --no-cache.
error: Failed to parse examples/chatgpt/gpt_actions_library/gpt_action_canvaslms.ipynb:15:1:1: Expected an expression
error: Failed to parse examples/chatgpt/gpt_actions_library/gpt_action_outlook.ipynb:13:1:1: Expected an expression
error: Failed to parse examples/chatgpt/gpt_actions_library/gpt_action_salesforce.ipynb:17:1:13: Simple statements must be separated by newlines or semicolons

@dhruvmanila dhruvmanila merged commit 978909f into main Jul 22, 2024
@dhruvmanila dhruvmanila deleted the dhruv/unparenthesized-genexp branch July 22, 2024 09:14
@BrewTestBot BrewTestBot mentioned this pull request Jul 25, 2024
Merged
@ntBre ntBre mentioned this pull request May 5, 2025
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@MichaReiser MichaReiser MichaReiser approved these changes

Assignees

No one assigned

Labels

bug Something isn't working

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@dhruvmanila @MichaReiser