Skip to content

Conversation

@ntBre
Copy link
Contributor

@ntBre ntBre commented Oct 17, 2025

Summary

This PR fixes the issue I added in #20867 and noticed in #20930. Cases like this
cause an error on any Python version:

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:

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

ntBre added 3 commits October 17, 2025 15:16
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
@ntBre ntBre added bug Something isn't working parser Related to the parser labels Oct 17, 2025
@github-actions
Copy link
Contributor

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

✅ ecosystem check detected no linter changes.

Formatter (stable)

✅ ecosystem check detected no format changes.

Formatter (preview)

✅ ecosystem check detected no format changes.

@ntBre ntBre marked this pull request as ready for review October 17, 2025 19:55
@ntBre ntBre merged commit 38c074e into main Oct 20, 2025
40 checks passed
@ntBre ntBre deleted the brent/fix-nested-quotes-again branch October 20, 2025 13:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working parser Related to the parser

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants