-
Notifications
You must be signed in to change notification settings - Fork 1.6k
Fix syntax error false positives for escapes and quotes in f-strings #20867
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
Conversation
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 "<stdin>", line 1, in <module> 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 "<C>" ... >>> f"{C():\"d\"}" '<C>' ``` 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/
this resolves the case I had annotated with FIXME, as well as a separate case
that I didn't annotate, but these both parse fine on 3.11:
```pycon
>>> f'{x:a{z:hy "user"}} \'\'\''
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
NameError: name 'x' is not defined
>>> f"{1=: abcd \"\"}"
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
ValueError: Invalid format specifier ' abcd ""' for object of type 'int'
```
|
amyreese
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
MichaReiser
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you
…rable * origin/main: Don't use codspeed or depot runners in CI jobs on forks (#20894) [ty] cache Type::is_redundant_with (#20477) Fix run-away for mutually referential instance attributes (#20645) [ty] Limit shown import paths to at most 5 unless ty runs with `-v` (#20912) [ty] Use field-specifier return type as the default type for the field (#20915) [ty] Do not assume that `field`s have a default value (#20914) [ty] Fix match pattern value narrowing to use equality semantics (#20882) Update setup instructions for Zed 0.208.0+ (#20902) Move TOML indent size config (#20905) [syntax-errors]: implement F702 as semantic syntax error (#20869) [ty] Heterogeneous unpacking support for unions (#20377) [ty] refactor `Place` (#20871) Auto-accept snapshot changes as part of typeshed-sync PRs (#20892) [`airflow`] Add warning to `airflow.datasets.DatasetEvent` usage (`AIR301`) (#20551) [`flake8-pyi`] Fix operator precedence by adding parentheses when needed (`PYI061`) (#20508) [`pyupgrade`] Fix false negative for `TypeVar` with default argument in `non-pep695-generic-class` (`UP046`) (#20660) Update parser snapshots (#20893) Fix syntax error false positives for escapes and quotes in f-strings (#20867)
…nt-sets * dcreager/non-non-inferable: Don't use codspeed or depot runners in CI jobs on forks (#20894) [ty] cache Type::is_redundant_with (#20477) Fix run-away for mutually referential instance attributes (#20645) [ty] Limit shown import paths to at most 5 unless ty runs with `-v` (#20912) [ty] Use field-specifier return type as the default type for the field (#20915) [ty] Do not assume that `field`s have a default value (#20914) [ty] Fix match pattern value narrowing to use equality semantics (#20882) Update setup instructions for Zed 0.208.0+ (#20902) Move TOML indent size config (#20905) [syntax-errors]: implement F702 as semantic syntax error (#20869) [ty] Heterogeneous unpacking support for unions (#20377) [ty] refactor `Place` (#20871) Auto-accept snapshot changes as part of typeshed-sync PRs (#20892) [`airflow`] Add warning to `airflow.datasets.DatasetEvent` usage (`AIR301`) (#20551) [`flake8-pyi`] Fix operator precedence by adding parentheses when needed (`PYI061`) (#20508) [`pyupgrade`] Fix false negative for `TypeVar` with default argument in `non-pep695-generic-class` (`UP046`) (#20660) Update parser snapshots (#20893) Fix syntax error false positives for escapes and quotes in f-strings (#20867)
## Summary Fixes #20774 by tracking whether an `InterpolatedStringState` element is nested inside of another interpolated element. This feels like kind of a naive fix, so I'm welcome to other ideas. But it resolves the problem in the issue and clears up the syntax error in the black compatibility test, without affecting many other cases. The other affected case is actually interesting too because the [input](https://github.com/astral-sh/ruff/blob/96b156303b81c5114e8375a6ffd467fb638c3963/crates/ruff_python_formatter/resources/test/fixtures/ruff/expression/fstring.py#L707) is invalid, but the previous quote selection fixed the invalid syntax: ```pycon Python 3.11.13 (main, Sep 2 2025, 14:20:25) [Clang 20.1.4 ] on linux Type "help", "copyright", "credits" or "license" for more information. >>> f'{1: abcd "{'aa'}" }' # input File "<stdin>", line 1 f'{1: abcd "{'aa'}" }' ^^ SyntaxError: f-string: expecting '}' >>> f'{1: abcd "{"aa"}" }' # old output Traceback (most recent call last): File "<stdin>", line 1, in <module> ValueError: Invalid format specifier ' abcd "aa" ' for object of type 'int' >>> f'{1: abcd "{'aa'}" }' # new output File "<stdin>", line 1 f'{1: abcd "{'aa'}" }' ^^ SyntaxError: f-string: expecting '}' ``` We now preserve the invalid syntax in the input. Unfortunately, this also seems to be another edge case I didn't consider in #20867 because we don't flag this as a syntax error after 0.14.1: <details><summary>Shell output</summary> <p> ``` > uvx ruff@0.14.0 check --ignore ALL --target-version py311 - <<EOF f'{1: abcd "{'aa'}" }' EOF invalid-syntax: Cannot reuse outer quote character in f-strings on Python 3.11 (syntax was added in Python 3.12) --> -:1:14 | 1 | f'{1: abcd "{'aa'}" }' | ^ | Found 1 error. > uvx ruff@0.14.1 check --ignore ALL --target-version py311 - <<EOF f'{1: abcd "{'aa'}" }' EOF All checks passed! > uvx python@3.11 -m ast <<EOF f'{1: abcd "{'aa'}" }' EOF Traceback (most recent call last): File "<frozen runpy>", line 198, in _run_module_as_main File "<frozen runpy>", line 88, in _run_code File "/home/brent/.local/share/uv/python/cpython-3.11.13-linux-x86_64-gnu/lib/python3.11/ast.py", line 1752, in <module> main() File "/home/brent/.local/share/uv/python/cpython-3.11.13-linux-x86_64-gnu/lib/python3.11/ast.py", line 1748, in main tree = parse(source, args.infile.name, args.mode, type_comments=args.no_type_comments) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/home/brent/.local/share/uv/python/cpython-3.11.13-linux-x86_64-gnu/lib/python3.11/ast.py", line 50, in parse return compile(source, filename, mode, flags, ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "<stdin>", line 1 f'{1: abcd "{'aa'}" }' ^^ SyntaxError: f-string: expecting '}' ``` </p> </details> I assumed that was the same `ParseError` as the one caused by `f"{1:""}"`, but this is a nested interpolation inside of the format spec. ## Test Plan New test copied from the black compatibility test. I guess this is a duplicate now, I started working on this branch before the new black tests were imported, so I could delete the separate test in our fixtures if that's preferable.
Summary -- This PR fixes the issue I added in #20867 and noticed in #20930. Cases like this cause an error on any Python version: ```py f"{1:""}" ``` which gave me a false sense of security before. Cases like this are still invalid only before 3.12 and weren't flagged after the changes in #20867: ```py f'{1: abcd "{'aa'}" }' # ^ reused quote f'{1: abcd "{"\n"}" }' # ^ backslash ``` I didn't recognize these as nested interpolations that also need to be checked for invalid expressions, so filtering out the whole format spec wasn't quite right. There's basically no code change in this PR, I just moved the existing check from `parse_interpolated_string`, which parses the entire string, to `parse_interpolated_element`. This kind of seems more natural anyway and avoids having to try to recursively visit nested elements after the fact in `parse_interpolated_string`. So viewing the diff with something like ``` git diff --color-moved --ignore-space-change --color-moved-ws=allow-indentation-change main...HEAD ``` should make this more clear. Test Plan -- New tests
Summary -- This PR fixes the issue I added in #20867 and noticed in #20930. Cases like this cause an error on any Python version: ```py f"{1:""}" ``` which gave me a false sense of security before. Cases like this are still invalid only before 3.12 and weren't flagged after the changes in #20867: ```py f'{1: abcd "{'aa'}" }' # ^ reused quote f'{1: abcd "{"\n"}" }' # ^ backslash ``` I didn't recognize these as nested interpolations that also need to be checked for invalid expressions, so filtering out the whole format spec wasn't quite right. And `elements.interpolations()` only iterates over the outermost interpolations, not the nested ones. There's basically no code change in this PR, I just moved the existing check from `parse_interpolated_string`, which parses the entire string, to `parse_interpolated_element`. This kind of seems more natural anyway and avoids having to try to recursively visit nested elements after the fact in `parse_interpolated_string`. So viewing the diff with something like ``` git diff --color-moved --ignore-space-change --color-moved-ws=allow-indentation-change main ``` should make this more clear. Test Plan -- New tests
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:
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 bea valid use case.
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 alreadycovered by an existing
ParseError, so we can just narrow the range of both ourescape and quote checks.
Our comment check also seems to be working correctly because it's based on the
actual tokens. A case like this:
f"""{1:# }"""doesn't include a comment token, instead the
#is part of anInterpolatedStringLiteralElement.Test Plan
New inline parser tests