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

W605 fix is unsafe #11461

Closed
njzjz opened this issue May 17, 2024 · 5 comments · Fixed by #11465
Closed

W605 fix is unsafe #11461

njzjz opened this issue May 17, 2024 · 5 comments · Fixed by #11465
Assignees
Labels
bug Something isn't working

Comments

@njzjz
Copy link

njzjz commented May 17, 2024

Before fix:

print("\
test\.")

The original output:

$ python test_w605.py
test\.

Fix it:

$ ruff check test_w605.py --select W605 --fix --isolated
Found 1 error (1 fixed, 0 remaining).

The file becomes

print(r"\
test\.") 

Run it again:

$ python test_w605.py 
\
test\.

The output changes. So the fix is unsafe.

@charliermarsh
Copy link
Member

I'm sorry, I don't understand the difference between the two snippets. I something being obscured by GitHub's formatting?

@njzjz
Copy link
Author

njzjz commented May 17, 2024

Sorry I made a mistake, I fixed it.

In short, the first backslash (\) gives different outputs with the raw string and the regular string.

>>> "\
... "
''
>>> r"\
... "
'\\\n'

@zanieb
Copy link
Member

zanieb commented May 18, 2024

Is this an unsafe fix or is this just a bug that we should fix?

@njzjz
Copy link
Author

njzjz commented May 19, 2024

Is this an unsafe fix or is this just a bug that we should fix?

I think it is a bug that should be fixed, and due to this bug, the current fix is unsafe.

@charliermarsh charliermarsh added the bug Something isn't working label May 19, 2024
@charliermarsh
Copy link
Member

Let's fix the bug rather than changing the safety.

@charliermarsh charliermarsh self-assigned this May 19, 2024
charliermarsh added a commit that referenced this issue May 19, 2024
## Summary

We weren't treating the escaped newline as a valid condition to trigger
the safer fix (add an extra backslash before each invalid escape
sequence).

Closes #11461.
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

Successfully merging a pull request may close this issue.

3 participants