Suppress PLE2510 fix inside f-string interpolation for Python < 3.12#23910
Suppress PLE2510 fix inside f-string interpolation for Python < 3.12#23910omar-y-abdi wants to merge 3 commits into
Conversation
astral-sh#18816 caused the PLE2510 autofix to introduce syntax errors when replacing control characters with escape sequences inside f-string interpolation expressions. In Python < 3.12, backslash escapes are not allowed inside f-string `{...}` (PEP 701 lifted this in 3.12). Fixed by tracking f-string/t-string interpolation nesting via a brace-depth stack in the token loop, then suppressing the fix (not the diagnostic) when the token is inside an interpolation and the target version doesn't support PEP 701. Closes astral-sh#18816
ntBre
left a comment
ntBre
left a comment
There was a problem hiding this comment.
Thanks for working on this! The changes look good to me overall, I just had one minor suggestion for testing and an idea for allowing escapes in format specs like CPython does.
ntBre
added
bug
|
Address review feedback from ntBre: - Allow autofix for FStringMiddle/TStringMiddle tokens inside interpolation: format specs support backslash escapes in all Python versions, only expression bodies are restricted pre-3.12. - Replace two separate snapshot tests (py39/py312) with a single assert_diagnostics_diff! call since they only differ by target version. - Move interpolation/version logic to the call site in tokens.rs so invalid_string_characters receives a single suppress_fix bool instead of in_interpolation + target_version. - Remove redundant if-let after is_empty guard, skip function call for non-string tokens.
|
Thanks for the review! Addressed both suggestions:
Also simplified the function signature -- |
ntBre
left a comment
ntBre
left a comment
There was a problem hiding this comment.
Thanks, the tests look good now, but I'm a bit confused by some of the other changes in the last commit.
|
@ntBre Hey, regarding your comments: The unwrap change and the matches move were unnecessary refactoring that you never asked for in your first round of feedback. I restored the if let and moved the matches call back into the rule function. |
|
@omar-y-abdi are you interested in resolving the CI failures? I think you just need to merge the main branch and then update the snapshots because of a context window change in our output formatting. |
3 participants
Summary
Fixes #18816.
The PLE2510/PLE2512/PLE2513/PLE2514/PLE2515 autofix replaces raw control characters with escape sequences (e.g.,
\x08byte with\b). When the control character is inside an f-string interpolation expression, this introduces a backslash inside{...}, which is a syntax error in Python < 3.12 (before PEP 701).This PR suppresses the autofix (not the diagnostic) when the token is inside an f-string/t-string interpolation and the target version doesn't support PEP 701.
Approach
Track f-string/t-string interpolation nesting in the token loop using a brace-depth stack (
Vec<u32>):FStringStart/TStringStartpushes a new depth counterLbraceincrements the current depth (handles nested dict/set literals)Rbracedecrements the current depthFStringEnd/TStringEndpops the stackdepth > 0means we're inside an interpolation{...}The
in_interpolationflag and resolvedtarget_versionare passed toinvalid_string_characters, which suppresses the fix whenin_interpolation && !target_version.supports_pep_701().Test plan
invalid_characters_fstring.pyfixture with control characters in:invalid_characters_fstring_py39test: verifies fix is suppressed for interpolation casesinvalid_characters_fstring_py312test: verifies fix is applied for all cases