From 8b9ab48ac61b7c73df7d03ccda06085e61e62a5e Mon Sep 17 00:00:00 2001 From: Brent Westbrook <36778786+ntBre@users.noreply.github.com> Date: Wed, 15 Oct 2025 09:23:16 -0400 Subject: [PATCH] Fix syntax error false positives for escapes and quotes in f-strings (#20867) Summary -- Fixes #20844 by refining the unsupported syntax error check for [PEP 701] f-strings before Python 3.12 to allow backslash escapes and escaped outer quotes in the format spec part of f-strings. These are only disallowed within the f-string expression part on earlier versions. Using the examples from the PR: ```pycon >>> f"{1:\x64}" '1' >>> f"{1:\"d\"}" Traceback (most recent call last): File "", line 1, in ValueError: Invalid format specifier '"d"' for object of type 'int' ``` Note that the second case is a runtime error, but this is actually avoidable if you override `__format__`, so despite being pretty weird, this could actually be a valid use case. ```pycon >>> class C: ... def __format__(*args, **kwargs): return "" ... >>> f"{C():\"d\"}" '' ``` At first I thought narrowing the range we check to exclude the format spec would only work for escapes, but it turns out that cases like `f"{1:""}"` are already covered by an existing `ParseError`, so we can just narrow the range of both our escape and quote checks. Our comment check also seems to be working correctly because it's based on the actual tokens. A case like [this](https://play.ruff.rs/9f1c2ff2-cd8e-4ad7-9f40-56c0a524209f): ```python f"""{1:# }""" ``` doesn't include a comment token, instead the `#` is part of an `InterpolatedStringLiteralElement`. Test Plan -- New inline parser tests [PEP 701]: https://peps.python.org/pep-0701/ --- .../test/fixtures/ruff/expression/fstring.py | 2 - .../format@expression__fstring.py.snap | 54 -------- .../err/nested_quote_in_format_spec_py312.py | 2 + .../non_nested_quote_in_format_spec_py311.py | 2 + .../inline/ok/pep701_f_string_py311.py | 2 + .../src/parser/expression.rs | 26 +++- ...@nested_quote_in_format_spec_py312.py.snap | 90 +++++++++++++ ..._nested_quote_in_format_spec_py311.py.snap | 77 +++++++++++ ...valid_syntax@pep701_f_string_py311.py.snap | 124 +++++++++++++++++- 9 files changed, 317 insertions(+), 62 deletions(-) create mode 100644 crates/ruff_python_parser/resources/inline/err/nested_quote_in_format_spec_py312.py create mode 100644 crates/ruff_python_parser/resources/inline/ok/non_nested_quote_in_format_spec_py311.py create mode 100644 crates/ruff_python_parser/tests/snapshots/invalid_syntax@nested_quote_in_format_spec_py312.py.snap create mode 100644 crates/ruff_python_parser/tests/snapshots/valid_syntax@non_nested_quote_in_format_spec_py311.py.snap diff --git a/crates/ruff_python_formatter/resources/test/fixtures/ruff/expression/fstring.py b/crates/ruff_python_formatter/resources/test/fixtures/ruff/expression/fstring.py index 541af99baa..fd658cc5ce 100644 --- a/crates/ruff_python_formatter/resources/test/fixtures/ruff/expression/fstring.py +++ b/crates/ruff_python_formatter/resources/test/fixtures/ruff/expression/fstring.py @@ -706,8 +706,6 @@ f'{1:hy "user"}' f'{1: abcd "{1}" }' f'{1: abcd "{'aa'}" }' f'{1=: "abcd {'aa'}}' -# FIXME(brent) This should not be a syntax error on output. The escaped quotes are in the format -# spec, which is valid even before 3.12. f'{x:a{z:hy "user"}} \'\'\'' # Changing the outer quotes is fine because the format-spec is in a nested expression. diff --git a/crates/ruff_python_formatter/tests/snapshots/format@expression__fstring.py.snap b/crates/ruff_python_formatter/tests/snapshots/format@expression__fstring.py.snap index 2eb1e09a08..c9d90b1764 100644 --- a/crates/ruff_python_formatter/tests/snapshots/format@expression__fstring.py.snap +++ b/crates/ruff_python_formatter/tests/snapshots/format@expression__fstring.py.snap @@ -712,8 +712,6 @@ f'{1:hy "user"}' f'{1: abcd "{1}" }' f'{1: abcd "{'aa'}" }' f'{1=: "abcd {'aa'}}' -# FIXME(brent) This should not be a syntax error on output. The escaped quotes are in the format -# spec, which is valid even before 3.12. f'{x:a{z:hy "user"}} \'\'\'' # Changing the outer quotes is fine because the format-spec is in a nested expression. @@ -1536,8 +1534,6 @@ f'{1:hy "user"}' f'{1: abcd "{1}" }' f'{1: abcd "{"aa"}" }' f'{1=: "abcd {'aa'}}' -# FIXME(brent) This should not be a syntax error on output. The escaped quotes are in the format -# spec, which is valid even before 3.12. f"{x:a{z:hy \"user\"}} '''" # Changing the outer quotes is fine because the format-spec is in a nested expression. @@ -2365,8 +2361,6 @@ f'{1:hy "user"}' f'{1: abcd "{1}" }' f'{1: abcd "{"aa"}" }' f'{1=: "abcd {'aa'}}' -# FIXME(brent) This should not be a syntax error on output. The escaped quotes are in the format -# spec, which is valid even before 3.12. f"{x:a{z:hy \"user\"}} '''" # Changing the outer quotes is fine because the format-spec is in a nested expression. @@ -2418,30 +2412,6 @@ print(f"{ {}, 1 }") ### Unsupported Syntax Errors -error[invalid-syntax]: Cannot use an escape sequence (backslash) in f-strings on Python 3.10 (syntax was added in Python 3.12) - --> fstring.py:764:19 - | -762 | # FIXME(brent) This should not be a syntax error on output. The escaped quotes are in the format -763 | # spec, which is valid even before 3.12. -764 | f"{x:a{z:hy \"user\"}} '''" - | ^ -765 | -766 | # Changing the outer quotes is fine because the format-spec is in a nested expression. - | -warning: Only accept new syntax errors if they are also present in the input. The formatter should not introduce syntax errors. - -error[invalid-syntax]: Cannot use an escape sequence (backslash) in f-strings on Python 3.10 (syntax was added in Python 3.12) - --> fstring.py:764:13 - | -762 | # FIXME(brent) This should not be a syntax error on output. The escaped quotes are in the format -763 | # spec, which is valid even before 3.12. -764 | f"{x:a{z:hy \"user\"}} '''" - | ^ -765 | -766 | # Changing the outer quotes is fine because the format-spec is in a nested expression. - | -warning: Only accept new syntax errors if they are also present in the input. The formatter should not introduce syntax errors. - error[invalid-syntax]: Cannot reuse outer quote character in f-strings on Python 3.10 (syntax was added in Python 3.12) --> fstring.py:178:8 | @@ -2452,27 +2422,3 @@ error[invalid-syntax]: Cannot reuse outer quote character in f-strings on Python 179 | f"foo {'"bar"'}" | warning: Only accept new syntax errors if they are also present in the input. The formatter should not introduce syntax errors. - -error[invalid-syntax]: Cannot reuse outer quote character in f-strings on Python 3.10 (syntax was added in Python 3.12) - --> fstring.py:773:14 - | -771 | f'{1=: "abcd \'\'}' # Don't change the outer quotes, or it results in a syntax error -772 | f"{1=: abcd \'\'}" # Changing the quotes here is fine because the inner quotes aren't the opposite quotes -773 | f"{1=: abcd \"\"}" # Changing the quotes here is fine because the inner quotes are escaped - | ^ -774 | # Don't change the quotes in the following cases: -775 | f'{x=:hy "user"} \'\'\'' - | -warning: Only accept new syntax errors if they are also present in the input. The formatter should not introduce syntax errors. - -error[invalid-syntax]: Cannot reuse outer quote character in f-strings on Python 3.10 (syntax was added in Python 3.12) - --> fstring.py:764:14 - | -762 | # FIXME(brent) This should not be a syntax error on output. The escaped quotes are in the format -763 | # spec, which is valid even before 3.12. -764 | f"{x:a{z:hy \"user\"}} '''" - | ^ -765 | -766 | # Changing the outer quotes is fine because the format-spec is in a nested expression. - | -warning: Only accept new syntax errors if they are also present in the input. The formatter should not introduce syntax errors. diff --git a/crates/ruff_python_parser/resources/inline/err/nested_quote_in_format_spec_py312.py b/crates/ruff_python_parser/resources/inline/err/nested_quote_in_format_spec_py312.py new file mode 100644 index 0000000000..e11455f9c7 --- /dev/null +++ b/crates/ruff_python_parser/resources/inline/err/nested_quote_in_format_spec_py312.py @@ -0,0 +1,2 @@ +# parse_options: {"target-version": "3.12"} +f"{1:""}" # this is a ParseError on all versions diff --git a/crates/ruff_python_parser/resources/inline/ok/non_nested_quote_in_format_spec_py311.py b/crates/ruff_python_parser/resources/inline/ok/non_nested_quote_in_format_spec_py311.py new file mode 100644 index 0000000000..ce69b8518b --- /dev/null +++ b/crates/ruff_python_parser/resources/inline/ok/non_nested_quote_in_format_spec_py311.py @@ -0,0 +1,2 @@ +# parse_options: {"target-version": "3.11"} +f"{1:''}" # but this is okay on all versions diff --git a/crates/ruff_python_parser/resources/inline/ok/pep701_f_string_py311.py b/crates/ruff_python_parser/resources/inline/ok/pep701_f_string_py311.py index 40c0958df6..a50bc7593b 100644 --- a/crates/ruff_python_parser/resources/inline/ok/pep701_f_string_py311.py +++ b/crates/ruff_python_parser/resources/inline/ok/pep701_f_string_py311.py @@ -5,3 +5,5 @@ f"""{f'''{f'{"# not a comment"}'}'''}""" f"""{f'''# before expression {f'# aro{f"#{1+1}#"}und #'}'''} # after expression""" f"escape outside of \t {expr}\n" f"test\"abcd" +f"{1:\x64}" # escapes are valid in the format spec +f"{1:\"d\"}" # this also means that escaped outer quotes are valid diff --git a/crates/ruff_python_parser/src/parser/expression.rs b/crates/ruff_python_parser/src/parser/expression.rs index 043de1772c..6920e7cdd1 100644 --- a/crates/ruff_python_parser/src/parser/expression.rs +++ b/crates/ruff_python_parser/src/parser/expression.rs @@ -1571,6 +1571,8 @@ impl<'src> Parser<'src> { // f"""{f'''# before expression {f'# aro{f"#{1+1}#"}und #'}'''} # after expression""" // f"escape outside of \t {expr}\n" // f"test\"abcd" + // f"{1:\x64}" # escapes are valid in the format spec + // f"{1:\"d\"}" # this also means that escaped outer quotes are valid // test_err pep701_f_string_py311 // # parse_options: {"target-version": "3.11"} @@ -1586,6 +1588,13 @@ impl<'src> Parser<'src> { // f"""{f"""{x}"""}""" # mark the whole triple quote // f"{'\n'.join(['\t', '\v', '\r'])}" # multiple escape sequences, multiple errors + // test_err nested_quote_in_format_spec_py312 + // # parse_options: {"target-version": "3.12"} + // f"{1:""}" # this is a ParseError on all versions + + // test_ok non_nested_quote_in_format_spec_py311 + // # parse_options: {"target-version": "3.11"} + // f"{1:''}" # but this is okay on all versions let range = self.node_range(start); if !self.options.target_version.supports_pep_701() @@ -1594,22 +1603,29 @@ impl<'src> Parser<'src> { let quote_bytes = flags.quote_str().as_bytes(); let quote_len = flags.quote_len(); for expr in elements.interpolations() { - for slash_position in memchr::memchr_iter(b'\\', self.source[expr.range].as_bytes()) - { + // We need to check the whole expression range, including any leading or trailing + // debug text, but exclude the format spec, where escapes and escaped, reused quotes + // are allowed. + let range = expr + .format_spec + .as_ref() + .map(|format_spec| TextRange::new(expr.start(), format_spec.start())) + .unwrap_or(expr.range); + for slash_position in memchr::memchr_iter(b'\\', self.source[range].as_bytes()) { let slash_position = TextSize::try_from(slash_position).unwrap(); self.add_unsupported_syntax_error( UnsupportedSyntaxErrorKind::Pep701FString(FStringKind::Backslash), - TextRange::at(expr.range.start() + slash_position, '\\'.text_len()), + TextRange::at(range.start() + slash_position, '\\'.text_len()), ); } if let Some(quote_position) = - memchr::memmem::find(self.source[expr.range].as_bytes(), quote_bytes) + memchr::memmem::find(self.source[range].as_bytes(), quote_bytes) { let quote_position = TextSize::try_from(quote_position).unwrap(); self.add_unsupported_syntax_error( UnsupportedSyntaxErrorKind::Pep701FString(FStringKind::NestedQuote), - TextRange::at(expr.range.start() + quote_position, quote_len), + TextRange::at(range.start() + quote_position, quote_len), ); } } diff --git a/crates/ruff_python_parser/tests/snapshots/invalid_syntax@nested_quote_in_format_spec_py312.py.snap b/crates/ruff_python_parser/tests/snapshots/invalid_syntax@nested_quote_in_format_spec_py312.py.snap new file mode 100644 index 0000000000..ba7e665b79 --- /dev/null +++ b/crates/ruff_python_parser/tests/snapshots/invalid_syntax@nested_quote_in_format_spec_py312.py.snap @@ -0,0 +1,90 @@ +--- +source: crates/ruff_python_parser/tests/fixtures.rs +input_file: crates/ruff_python_parser/resources/inline/err/nested_quote_in_format_spec_py312.py +--- +## AST + +``` +Module( + ModModule { + node_index: NodeIndex(None), + range: 0..94, + body: [ + Expr( + StmtExpr { + node_index: NodeIndex(None), + range: 44..53, + value: FString( + ExprFString { + node_index: NodeIndex(None), + range: 44..53, + value: FStringValue { + inner: Concatenated( + [ + FString( + FString { + range: 44..50, + node_index: NodeIndex(None), + elements: [ + Interpolation( + InterpolatedElement { + range: 46..49, + node_index: NodeIndex(None), + expression: NumberLiteral( + ExprNumberLiteral { + node_index: NodeIndex(None), + range: 47..48, + value: Int( + 1, + ), + }, + ), + debug_text: None, + conversion: None, + format_spec: Some( + InterpolatedStringFormatSpec { + range: 49..49, + node_index: NodeIndex(None), + elements: [], + }, + ), + }, + ), + ], + flags: FStringFlags { + quote_style: Double, + prefix: Regular, + triple_quoted: false, + }, + }, + ), + Literal( + StringLiteral { + range: 50..53, + node_index: NodeIndex(None), + value: "}", + flags: StringLiteralFlags { + quote_style: Double, + prefix: Empty, + triple_quoted: false, + }, + }, + ), + ], + ), + }, + }, + ), + }, + ), + ], + }, +) +``` +## Errors + + | +1 | # parse_options: {"target-version": "3.12"} +2 | f"{1:""}" # this is a ParseError on all versions + | ^ Syntax Error: f-string: expecting '}' + | diff --git a/crates/ruff_python_parser/tests/snapshots/valid_syntax@non_nested_quote_in_format_spec_py311.py.snap b/crates/ruff_python_parser/tests/snapshots/valid_syntax@non_nested_quote_in_format_spec_py311.py.snap new file mode 100644 index 0000000000..eea96fe16f --- /dev/null +++ b/crates/ruff_python_parser/tests/snapshots/valid_syntax@non_nested_quote_in_format_spec_py311.py.snap @@ -0,0 +1,77 @@ +--- +source: crates/ruff_python_parser/tests/fixtures.rs +input_file: crates/ruff_python_parser/resources/inline/ok/non_nested_quote_in_format_spec_py311.py +--- +## AST + +``` +Module( + ModModule { + node_index: NodeIndex(None), + range: 0..90, + body: [ + Expr( + StmtExpr { + node_index: NodeIndex(None), + range: 44..53, + value: FString( + ExprFString { + node_index: NodeIndex(None), + range: 44..53, + value: FStringValue { + inner: Single( + FString( + FString { + range: 44..53, + node_index: NodeIndex(None), + elements: [ + Interpolation( + InterpolatedElement { + range: 46..52, + node_index: NodeIndex(None), + expression: NumberLiteral( + ExprNumberLiteral { + node_index: NodeIndex(None), + range: 47..48, + value: Int( + 1, + ), + }, + ), + debug_text: None, + conversion: None, + format_spec: Some( + InterpolatedStringFormatSpec { + range: 49..51, + node_index: NodeIndex(None), + elements: [ + Literal( + InterpolatedStringLiteralElement { + range: 49..51, + node_index: NodeIndex(None), + value: "''", + }, + ), + ], + }, + ), + }, + ), + ], + flags: FStringFlags { + quote_style: Double, + prefix: Regular, + triple_quoted: false, + }, + }, + ), + ), + }, + }, + ), + }, + ), + ], + }, +) +``` diff --git a/crates/ruff_python_parser/tests/snapshots/valid_syntax@pep701_f_string_py311.py.snap b/crates/ruff_python_parser/tests/snapshots/valid_syntax@pep701_f_string_py311.py.snap index 4b294d49ea..f9d61369a7 100644 --- a/crates/ruff_python_parser/tests/snapshots/valid_syntax@pep701_f_string_py311.py.snap +++ b/crates/ruff_python_parser/tests/snapshots/valid_syntax@pep701_f_string_py311.py.snap @@ -8,7 +8,7 @@ input_file: crates/ruff_python_parser/resources/inline/ok/pep701_f_string_py311. Module( ModModule { node_index: NodeIndex(None), - range: 0..278, + range: 0..398, body: [ Expr( StmtExpr { @@ -604,6 +604,128 @@ Module( ), }, ), + Expr( + StmtExpr { + node_index: NodeIndex(None), + range: 278..289, + value: FString( + ExprFString { + node_index: NodeIndex(None), + range: 278..289, + value: FStringValue { + inner: Single( + FString( + FString { + range: 278..289, + node_index: NodeIndex(None), + elements: [ + Interpolation( + InterpolatedElement { + range: 280..288, + node_index: NodeIndex(None), + expression: NumberLiteral( + ExprNumberLiteral { + node_index: NodeIndex(None), + range: 281..282, + value: Int( + 1, + ), + }, + ), + debug_text: None, + conversion: None, + format_spec: Some( + InterpolatedStringFormatSpec { + range: 283..287, + node_index: NodeIndex(None), + elements: [ + Literal( + InterpolatedStringLiteralElement { + range: 283..287, + node_index: NodeIndex(None), + value: "d", + }, + ), + ], + }, + ), + }, + ), + ], + flags: FStringFlags { + quote_style: Double, + prefix: Regular, + triple_quoted: false, + }, + }, + ), + ), + }, + }, + ), + }, + ), + Expr( + StmtExpr { + node_index: NodeIndex(None), + range: 330..342, + value: FString( + ExprFString { + node_index: NodeIndex(None), + range: 330..342, + value: FStringValue { + inner: Single( + FString( + FString { + range: 330..342, + node_index: NodeIndex(None), + elements: [ + Interpolation( + InterpolatedElement { + range: 332..341, + node_index: NodeIndex(None), + expression: NumberLiteral( + ExprNumberLiteral { + node_index: NodeIndex(None), + range: 333..334, + value: Int( + 1, + ), + }, + ), + debug_text: None, + conversion: None, + format_spec: Some( + InterpolatedStringFormatSpec { + range: 335..340, + node_index: NodeIndex(None), + elements: [ + Literal( + InterpolatedStringLiteralElement { + range: 335..340, + node_index: NodeIndex(None), + value: "\"d\"", + }, + ), + ], + }, + ), + }, + ), + ], + flags: FStringFlags { + quote_style: Double, + prefix: Regular, + triple_quoted: false, + }, + }, + ), + ), + }, + }, + ), + }, + ), ], }, )