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

Wrong fix for W605 in f-strings #11491

Open
FreeHarry opened this issue May 22, 2024 · 2 comments
Open

Wrong fix for W605 in f-strings #11491

FreeHarry opened this issue May 22, 2024 · 2 comments
Labels
bug Something isn't working

Comments

@FreeHarry
Copy link

Python: 3.12.3
Version: ruff 0.4.4
Command: ruff check --select W605 .\test.py --fix

And let's given the following code snippet in test.py:

total = 10
ok = 7
incomplete = 3
s = f"TOTAL: {total}\nOK: {ok}\INCOMPLETE: {incomplete}\n"

It is wrongly fixed as:

total = 10
ok = 7
incomplete = 3
s = rf"TOTAL: {total}\nOK: {ok}\INCOMPLETE: {incomplete}\n"

Now the string is a raw string. The newlines will then interpreted as literals and will not remain newlines.


If I do the same for a normal string:

s = "TOTAL: 10\nOK: 3\INCOMPLETE: 7\n"

It is correctly fixed as:

s = "TOTAL: 10\nOK: 3\\INCOMPLETE: 7\n"

The invalid escape sequence '\I' is correctly fixed as \\, and the newlines are still there.


Even if it is a f-string, but without placeholders, it is done correctly:

s = f"TOTAL: 10\nOK: 3\INCOMPLETE: 7\n"

Result is correct:

s = f"TOTAL: 10\nOK: 3\\INCOMPLETE: 7\n"
@dhruvmanila dhruvmanila added the bug Something isn't working label May 22, 2024
@dhruvmanila
Copy link
Member

Thank you for the report!

This is happening because each f-string element is analyzed individually. So, first TOTAL: is analyzed, then \nOK: and then \INCOMPLETE: , but the checker isn't aware that one of the element contains a newline character.

dylwil3 added a commit that referenced this issue Dec 4, 2024
…e (W605)` (#14748)

When fixing an invalid escape sequence in an f-string, each f-string
element is analyzed for valid escape characters prior to creating the
diagnostic and fix. This allows us to safely prefix with `r` to create a
raw string if no valid escape characters were found anywhere in the
f-string, and otherwise insert backslashes.

This fixes a bug in the original implementation: each "f-string part"
was treated separately, so it was not possible to tell whether a valid
escape character was or would be used elsewhere in the f-string.

Progress towards #11491 but format specifiers are not handled in this
PR.
@dylwil3
Copy link
Collaborator

dylwil3 commented Dec 4, 2024

Update: The issue in the original post has been resolved, but I'm leaving this open because invalid escapes in format specifiers are not handled yet (see #14748 (comment) and the surrounding discussion).

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
Development

No branches or pull requests

3 participants