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

Rules Q002,PTH116 causes autofix error #6785

Closed
qarmin opened this issue Aug 22, 2023 · 4 comments · Fixed by #10199
Closed

Rules Q002,PTH116 causes autofix error #6785

qarmin opened this issue Aug 22, 2023 · 4 comments · Fixed by #10199
Labels
accepted Ready for implementation bug Something isn't working fixes Related to suggested fixes for violations

Comments

@qarmin
Copy link

qarmin commented Aug 22, 2023 •
edited
Loading

Ruff 0.0.285 (latest changes from main branch)

ruff  *.py --select Q002,PTH116 --no-cache

file content:

''"assert" ' SAM macro definitions '''

error:

error: Autofix introduced a syntax error. Reverting all changes.

This indicates a bug in `ruff`. If you could open an issue at:

    https://github.com/astral-sh/ruff/issues/new?title=%5BAutofix%20error%5D

...quoting the contents of `869198225PY_FILE_TEST_7954205181857005371.py`, the rule codes Q000, Q002, along with the `pyproject.toml` settings and executed command, we'd be very appreciative!

869198225PY_FILE_TEST_7954205181857005371.py.zip
@zanieb
Copy link
Member

zanieb commented Aug 22, 2023 •
edited
Loading 

unexpected EOF while parsing at byte offset 39
---
"""assert" " SAM macro definitions "''

---

Since the beginning gets converted to """ we end up with an unterminated string literal

@zanieb zanieb added bug Something isn't working accepted Ready for implementation fixes Related to suggested fixes for violations labels Aug 22, 2023
@qarmin qarmin changed the title Rules Q000, Q002 causes autofix error Rules Q002,PTH116 causes autofix error Sep 2, 2023
@robincaloudis
Copy link
Contributor

The underlying classes and concepts handling this quote-fixing mechanism used for inline/multiline strings as well as docstrings look interesting. I give it a shot.

@robincaloudis robincaloudis mentioned this issue Mar 2, 2024
Merged
@robincaloudis
Copy link
Contributor

Hey @zanieb, in case you are interested in the fix, I'd highly appreciate a review. I got in the habit of tagging Charlie whenever I had PR's to review, but do not want to put the pressure of reviewing on him all the time as he has probably a lot of things to do. You are a maintainer as well, aren't you? Thanks!

@zanieb
Copy link
Member

zanieb commented Mar 17, 2024

I'll try to take a look this week.

charliermarsh pushed a commit that referenced this issue Mar 18, 2024
@robincaloudis
[flake8-quotes] Fix Autofix Error (Q000, Q002) (#10199)
2edd617 
## Summary
In issue #6785 it is reported
that a docstring in the form of `''"assert" ' SAM macro definitions '''`
is autocorrected to `"""assert" ' SAM macro definitions '''` (note the
triple quotes one only one side), which breaks the python program due
`undetermined string lateral`.

* `Q002`: Not only would docstrings in the form of `''"assert" ' SAM
macro definitions '''` (single quotes) be autofixed wrongly, but also
e.g. `""'assert' ' SAM macro definitions '''` (double quotes). The bug
is present for docstrings in all scopes (e.g. module docstrings, class
docstrings, function docstrings)

* `Q000`: The autofix error is not only present for `Q002` (docstrings),
but also for inline strings (`Q000`). Therefore `s = ''"assert" ' SAM
macro definitions '''` will also be wrongly autofixed.

Note that situation in which the first string is non-empty can be fixed,
e.g. `'123'"assert" ' SAM macro definitions '''` -> `"123""assert" ' SAM
macro definitions '''` is valid.

## What
* Change FixAvailability of `Q000` `Q002` to `Sometimes`
* Changed both rules such that docstrings/inline strings that cannot be
fixed are still reported as bad quotes via diagnostics, but no fix is
provided

## Test Plan
* For `Q000`: Add docstrings in different scopes that (partially) would
have been autofixed wrongly
* For `Q002`: Add inline strings that (partially) would have been
autofixed wrongly

Closes #6785
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
accepted Ready for implementation 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.

[flake8-quotes] Fix Autofix Error (Q000, Q002) robincaloudis/ruff
3 participants
@zanieb @qarmin @robincaloudis