Skip to content

[pylint] Avoid syntax errors in invalid character replacements in f-strings before Python 3.12 (PLE2510, PLE2512, PLE2513, PLE2514, PLE2515)#25544

Merged
ntBre merged 3 commits into
astral-sh:mainfrom
koriyoshi2041:explore-ple2510-fstring-fix
Jun 2, 2026
Merged

Conversation

@koriyoshi2041
Copy link
Copy Markdown
Contributor

@koriyoshi2041 koriyoshi2041 commented Jun 2, 2026

Summary

Fixes #18816.

When a raw control character appears inside an f-string replacement field or format spec, replacing it with an escape sequence can introduce invalid pre-3.12 syntax. This keeps the diagnostic, but suppresses the fix for interpolated f-string/t-string elements when the target version is older than Python 3.12. Literal f-string text and Python 3.12+ targets keep the existing fix behavior.

Test plan

  • cargo test -p ruff_linter --features test-rules invalidcharacter
  • cargo test -p ruff_linter --features test-rules rules::pylint::tests::invalid_characters_pre_py312
  • cargo fmt --check
  • git diff --check

@astral-sh-bot astral-sh-bot Bot requested a review from ntBre June 2, 2026 08:11
ntBre
ntBre reviewed Jun 2, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

@ntBre ntBre left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, this looks nearly identical to #23910, so I just had a few questions about the differences.

Comment thread crates/ruff_linter/src/linter.rs Outdated
stylist,
source_type,
source_kind.as_ipy_notebook().map(Notebook::cell_offsets),
target_version.linter_version(),
Copy link
Copy Markdown
Contributor

@ntBre ntBre Jun 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We shouldn't need to pass this separately. We can retrieve the version from the context inside of check_tokens.

Comment thread crates/ruff_linter/src/rules/pylint/rules/invalid_string_characters.rs Outdated
if !token.unwrap_string_flags().is_raw_string() && !is_escaped {
if !token.unwrap_string_flags().is_raw_string()
&& !is_escaped
&& !(target_version < PythonVersion::PY312 && inside_interpolation)
Copy link
Copy Markdown
Contributor

@ntBre ntBre Jun 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should use target_version.supports_pep_701() instead of a direct comparison to PY312.

Comment thread crates/ruff_linter/src/checkers/tokens.rs
Comment thread crates/ruff_linter/resources/test/fixtures/pylint/invalid_characters_pre_py312.py Outdated
Copy link
Copy Markdown
Contributor

@ntBre ntBre Jun 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we add any of the other test cases from https://github.com/astral-sh/ruff/pull/23910/changes#diff-233fb41e62597c60b28d2c5a042798417a33955c75feeb9b23e129809767960c?

@astral-sh-bot
Copy link
Copy Markdown

astral-sh-bot Bot commented Jun 2, 2026 •
edited
Loading

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

✅ ecosystem check detected no linter changes.

@ntBre ntBre added bug Something isn't working fixes Related to suggested fixes for violations labels Jun 2, 2026
@koriyoshi2041
Copy link
Copy Markdown
Contributor Author

koriyoshi2041 commented Jun 2, 2026

Thanks, pushed 8c1df70ef2 with the review changes:

  • get the target version from context inside check_tokens instead of threading it through linter.rs;
  • use supports_pep_701() for the fix gate;
  • update the f/t-string interpolation depth before calling invalid_string_characters;
  • add the extra ordinary f-string literal / regular string cases, plus a Python 3.12 snapshot so the pre-3.12 and 3.12 behavior are both covered.

Local checks now pass:

  • cargo test -p ruff_linter --features test-rules invalid_characters
  • cargo fmt --check
  • cargo clippy -p ruff_linter --all-targets --all-features --locked -- -D warnings
  • git diff --check

ntBre
ntBre approved these changes Jun 2, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

@ntBre ntBre left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. I went ahead and converted the tests to mdtests and restored the FStringMiddle/TStringMiddle change from my review of #23910.

@ntBre ntBre changed the title [pylint] Avoid unsafe PLE2510 fixes in pre-3.12 f-strings [pylint] Avoid syntax errors in invalid character replacements in f-strings before Python 3.12 (PLE2510, PLE2512, PLE2513, PLE2514, PLE2515) Jun 2, 2026
@ntBre ntBre merged commit f83e48c into astral-sh:main Jun 2, 2026
45 checks passed
@BrewTestBot BrewTestBot mentioned this pull request Jun 4, 2026
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ntBre ntBre ntBre approved these changes

Assignees

@ntBre ntBre

Labels

bug Something isn't working fixes Related to suggested fixes for violations

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[Pylint] PLE2510 fix causes syntax error inside pre-3.12 f-strings

2 participants

@koriyoshi2041 @ntBre