Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

C419 introduce invalid python since v0.5.3 #12420

Closed
UnknownPlatypus opened this issue Jul 20, 2024 · 6 comments · Fixed by #12422
Closed

C419 introduce invalid python since v0.5.3 #12420

UnknownPlatypus opened this issue Jul 20, 2024 · 6 comments · Fixed by #12422
Labels
bug Something isn't working fixes Related to suggested fixes for violations

Comments

@UnknownPlatypus
Copy link
Contributor

HI,

Since v0.5.3 and the changes made in #12364, ruff autofix introduced breakage in my code.
The following was autofixed from this:

timedelta_list = [dt.timedelta(days=2),dt.timedelta(days=1)]
res = sum(
    [
        delta
        for delta in timedelta_list
        if delta
    ],
    dt.timedelta(),
).total_seconds()

to this

timedelta_list = [dt.timedelta(days=2),dt.timedelta(days=1)]
res = sum(
    delta
        for delta in timedelta_list
        if delta,
    dt.timedelta(),
).total_seconds()
@charliermarsh
Copy link
Member

Thanks, we need to parenthesize in those cases.

@charliermarsh charliermarsh added bug Something isn't working fixes Related to suggested fixes for violations labels Jul 20, 2024
@UnknownPlatypus
Copy link
Contributor Author

UnknownPlatypus commented Jul 20, 2024 •
edited
Loading

Thanks, we need to parenthesize in those cases.

Yes exactly! Thanks for the quick response!

@charliermarsh
Copy link
Member

Sorry about that -- will try to fix soon.

@UnknownPlatypus UnknownPlatypus changed the title C419 introduce invalid python since v0.5.3 C419 introduce invalid python since v0.5.3 Jul 20, 2024
@UnknownPlatypus
Copy link
Contributor Author

No worries, thanks for the great work on this tool all-together.
I don't know if it's enough to yank the v0.5.3 given that every user with this rule already selected might be affected automatically when upgrading ?

@charliermarsh charliermarsh mentioned this issue Jul 20, 2024
Merged
@charliermarsh
Copy link
Member

I think I'll just cut a new release today with the fix (#12422).

charliermarsh added a commit that referenced this issue Jul 20, 2024
@charliermarsh
Insert parentheses for multi-argument generators (#12422)
2c1926b 
## Summary

Closes #12420.
@charliermarsh
Copy link
Member

It's also a preview-only behavior I think, so it's less urgent.

@dhruvmanila dhruvmanila mentioned this issue Jul 22, 2024
Merged
dhruvmanila added a commit that referenced this issue Jul 22, 2024
@dhruvmanila
Raise syntax error for unparenthesized generator expr in multi-argume…
978909f 
…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
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug Something isn't working fixes Related to suggested fixes for violations
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Insert parentheses for multi-argument generators astral-sh/ruff
2 participants
@charliermarsh @UnknownPlatypus