don't escape nested quotes inside t-string replacement fields - #5265
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Black emits a t-string it cannot parse when a replacement field contains a quote. I came
at this from the f-string side, checking that t-strings had inherited the same
protections, and they had not:
visit_tstringtestschild.type == syms.fstring_replacement_field, but atstringnode's children aretstring_replacement_field(symbol 356 against 299), so that bail-out has never firedfor a single t-string.
normalize_string_quotesthen gates its "do not introducebackslashes in interpolated expressions" check on
"f" in prefix.casefold(), which isfalse for
t,rtandtr. With both guards bypassed,t'\'{a["b"]}\''normalises tot"'{a[\"b\"]}'"and the forced second pass falls over on its own output withInvalidInput: Cannot parse: 1:9 ... TokenError: Failed to parse: UnexpectedCharacterAfterBackslash. One such literal makes a whole file unformattable andblackdanswers 400 for valid input, so it seems worth closing before 3.14 t-stringsturn up in real code.
Both checks belong where they already sit rather than at the call site, since the caller
only has the finished literal and cannot tell a quote inside a replacement field from one
in the literal part. They are not redundant either: the
visit_tstringsymbol covers afield that already carries a backslash, while the prefix check catches the backslashes
normalisation itself introduces, and reverting either one puts the committed case back
into failure (the first with a diff, the second with the parse error). On behaviour, I
compared 59k formatting results across a synthetic prefix/quote matrix and the repo's own
sources in stable and preview: 10 crashes fixed, nothing newly broken, and no literal
without a
tprefix moves at all. What does move is t-strings carrying a backslash in areplacement field, which are now left alone instead of being requoted; that is the
f-string behaviour they should have had from the start, and 20736 f/t pairs across raw
and non-raw prefixes now agree exactly. I read that as part of the same defect rather
than a style change, but say the word if you would rather see it split.
Checklist - did you ...
--previewstyle, following the stability policy?CHANGES.mdif necessary?