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.

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

COM812 breaks conditionals inside multi-line f-strings #8556

Closed
parafoxia opened this issue Nov 8, 2023 · 4 comments · Fixed by #8574 or #8582
Closed

COM812 breaks conditionals inside multi-line f-strings #8556

parafoxia opened this issue Nov 8, 2023 · 4 comments · Fixed by #8574 or #8582
Assignees
Labels
bug Something isn't working

Comments

@parafoxia
Copy link

Synopsis

The COM812 rule is capable of breaking conditional statements within triple f-strings by converting the else-statement into a tuple (or a tuple containing the original tuple).

Of course you can use # noqa: COM812, though I think that linters should be incapable of breaking code.

MRE

(This example assumes the line should be split across multiple lines.)

Original mre.py:

x = f"""This is a test. {
    "Another sentence."
    if True else
    "Alternative route!"
}"""

print(x)
$ py test.py
This is a test. Another sentence.

$ ruff check mre.py
mre.py:4:25: COM812 [*] Trailing comma missing
Found 1 error.
[*] 1 fixable with the `--fix` option.

"Fixed" mre.py

x = f"""This is a test. {
    "Another sentence."
    if True else
    "Alternative route!",
}"""

print(x)
$ py test.py
This is a test. ('Another sentence.',)

$ ruff check mre.py
(Exit code 0)

Environment

  • Ruff version: 0.1.3
  • Python version: 3.9.11
  • OS: macOS Sonoma 14.0
@charliermarsh charliermarsh added the bug Something isn't working label Nov 9, 2023
@charliermarsh charliermarsh self-assigned this Nov 9, 2023
@charliermarsh
Copy link
Member

Thanks, I'll take a look at this!

@dhruvmanila
Copy link
Member

I think it must be considering the curly braces as ContextType::Dict. It needs to take into account the new f-string tokens and skip if inside a f-string.

@charliermarsh
Copy link
Member

@dhruvmanila - Feel free to fix if you're there, I won't get to it for a few hours.

@charliermarsh
Copy link
Member

Taking a look now.

@dhruvmanila dhruvmanila reopened this Nov 9, 2023
dhruvmanila added a commit that referenced this issue Nov 10, 2023
## Summary

This fixes the bug where the `flake8-commas` rules weren't taking the
new f-string tokens into account.

## Test Plan

Add new test cases around f-strings for all of `flake8-commas`'s rules.

fixes: #8556
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
3 participants