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

D300: prevent autofix when both triples are in body #8462

Merged
merged 4 commits into from Nov 3, 2023

Conversation

T-256
Copy link
Contributor

@T-256 T-256 commented Nov 3, 2023

Summary

Addresses #8402 (comment)

Test Plan

Added associated test

Copy link

github-actions bot commented Nov 3, 2023

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

✅ ecosystem check detected no linter changes.

@charliermarsh charliermarsh added the bug Something isn't working label Nov 3, 2023
@charliermarsh charliermarsh enabled auto-merge (squash) November 3, 2023 16:22
@T-256 T-256 disabled auto-merge November 3, 2023 16:26
@T-256
Copy link
Contributor Author

T-256 commented Nov 3, 2023

Sorry for disabling auto-merge, but I think this should have autofix:

def contains_triples(t):
    '''(\'''|\""")'''

Into

def contains_triples(t):
    """(\'''|\""")"""

@charliermarsh
Copy link
Member

I think this is fine to merge as-is because we already get that "wrong" today for the simpler case, I think:

def contains_triples(t):
    '''(\""")'''

Getting it right requires that we parse the entire docstring to track escapes.

@T-256
Copy link
Contributor Author

T-256 commented Nov 3, 2023

Getting it right requires that we parse the entire docstring to track escapes

Correct, I was looking for escape-processed of docstring content to implement it, but didn't find a proper way.
BTW I added a todo example, I also think it's safe to merge for now.

@charliermarsh charliermarsh merged commit 4982694 into astral-sh:main Nov 3, 2023
16 checks passed
@charliermarsh
Copy link
Member

@T-256 - We have some stuff like that in crates/ruff_linter/src/rules/flake8_quotes/rules/avoidable_escaped_quote.rs.

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 this pull request may close these issues.

None yet

2 participants