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

Preserve quotes in generated byte strings #15778

Merged
merged 10 commits into from
Jan 28, 2025
Merged

Preserve quotes in generated byte strings #15778

merged 10 commits into from
Jan 28, 2025

Conversation

ntBre
Copy link
Contributor

@ntBre ntBre commented Jan 27, 2025

Summary

This is a very closely related follow-up to #15726, adding the same quote-preserving behavior to bytestrings. Only one rule (UP018) was affected this time, and it was easy to mirror the plain string changes.

Test Plan

Existing tests

@ntBre ntBre added bug Something isn't working fixes Related to suggested fixes for violations labels Jan 27, 2025
@ntBre ntBre requested a review from MichaReiser as a code owner January 27, 2025 21:58
@ntBre ntBre requested review from AlexWaygood and removed request for MichaReiser January 27, 2025 21:59
Copy link
Contributor

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

✅ ecosystem check detected no linter changes.

Formatter (stable)

✅ ecosystem check detected no format changes.

Formatter (preview)

✅ ecosystem check detected no format changes.

@ntBre ntBre merged commit 98d20a8 into main Jan 28, 2025
21 checks passed
@ntBre ntBre deleted the brent/byte-quotes branch January 28, 2025 13:19
dcreager added a commit that referenced this pull request Jan 29, 2025
* main:
  [red-knot] Extend instance-attribute tests (#15808)
  Fix formatter warning message for `flake8-quotes` option (#15788)
  [`flake8-bugbear`] Exempt `NewType` calls where the original type is immutable (`B008`) (#15765)
  Add missing config docstrings (#15803)
  [`refurb`] Do not emit diagnostic when loop variables are used outside loop body (`FURB122`) (#15757)
  [`ruff`] Check for shadowed `map` before suggesting fix (`RUF058`) (#15790)
  [red-knot] Do not use explicit `knot_extensions.Unknown` declaration (#15787)
  Preserve quotes in generated byte strings (#15778)
  [minor] Simplify some `ExprStringLiteral` creation logic (#15775)
  Preserve quote style in generated code (#15726)
  Rename internal helper functions (#15771)
  [`airflow`] Extend airflow context parameter check for `BaseOperator.execute` (`AIR302`) (#15713)
  Implement tab autocomplete for `ruff config` (#15603)
ntBre added a commit that referenced this pull request Jan 29, 2025
## Summary

This is another follow-up to #15726 and #15778, extending the
quote-preserving behavior to f-strings and deleting the now-unused
`Generator::quote` field.

## Details
I also made one unrelated change to `rules/flynt/helpers.rs` to remove a
`to_string` call for making a `Box<str>` and tweaked some arguments to
some of the `Generator::unparse_f_string` methods to make the code
easier to follow, in my opinion. Happy to revert especially the latter
of these if needed.

Unfortunately this still does not fix the issue in #9660, which appears
to be more of an escaping issue than a quote-preservation issue. After
#15726, the result is now `a = f'# {"".join([])}' if 1 else ""` instead
of `a = f"# {''.join([])}" if 1 else ""` (single quotes on the outside
now), but we still don't have the desired behavior of double quotes
everywhere on Python 3.12+. I added a test for this but split it off
into another branch since it ended up being unaddressed here, but my
`dbg!` statements showed the correct preferred quotes going into
[`UnicodeEscape::with_preferred_quote`](https://github.com/astral-sh/ruff/blob/main/crates/ruff_python_literal/src/escape.rs#L54).

## Test Plan

Existing rule and `Generator` tests.

---------

Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>
ntBre added a commit that referenced this pull request Feb 4, 2025
## Summary

This is a follow-up to #15726, #15778, and #15794 to preserve the triple
quote and prefix flags in plain strings, bytestrings, and f-strings.

I also added a `StringLiteralFlags::without_triple_quotes` method to
avoid passing along triple quotes in rules like SIM905 where it might
not make sense, as discussed
[here](#15726 (comment)).

## Test Plan

Existing tests, plus many new cases in the `generator::tests::quote`
test that should cover all combinations of quotes and prefixes, at least
for simple string bodies.

Closes #7799 when combined with #15694, #15726, #15778, and #15794.

---------

Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working fixes Related to suggested fixes for violations
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants