Skip to content

Conversation

@dylwil3
Copy link
Collaborator

@dylwil3 dylwil3 commented Jul 7, 2025

Parsing the (invalid) expression f"{\t"i}" caused a panic because the TStringMiddle character was "unreachable" due the way the parser recovered from the line continuation (it ate the t-string start).

The cause of the issue is as follows:

The parser begins parsing the f-string and expects to see a list of objects, essentially alternating between interpolated elements and ordinary strings. It is happy to see the first left brace, but then there is a lexical error caused by the line-continuation character. So instead of the parser seeing a list of elements with just one member, it sees a list that starts like this:

  • Interpolated element with an invalid token, stored as a Name
  • Something else built from tokens beginning with TStringStart and TStringMiddle

When it sees the TStringStart error recovery says "that's a list element I don't know what to do with, let's skip it". When it sees TStringMiddle it says "oh, that looks like the middle of some interpolated string so let's try to parse it as one of the literal elements of my FString". Unfortunately, the function being used to parse individual list elements thinks (arguably correctly) that it's not possible to have a TStringMiddle sitting in your FString, and hits unreachable.

Two potential ways (among many) to solve this issue are:

  1. Allow a TStringMiddle as a valid "literal" part of an f-string during parsing (with the hope/understanding that this would only occur in an invalid context)
  2. Skip the TStringMiddle as an "unexpected/invalid list item" in the same way that we skipped TStringStart.

I have opted for the second approach since it seems somehow more morally correct, even though it loses more information. To implement this, the recovery context needs to know whether we are in an f-string or t-string - hence the changes to that enum. As a bonus we get slightly more specific error messages in some cases.

Closes #18860

@dylwil3 dylwil3 added bug Something isn't working parser Related to the parser labels Jul 7, 2025
@github-actions
Copy link
Contributor

github-actions bot commented Jul 7, 2025

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

✅ ecosystem check detected no linter changes.

Formatter (stable)

ℹ️ ecosystem check encountered format errors. (no format changes; 1 project error)

openai/openai-cookbook (error)

warning: Detected debug build without --no-cache.
error: Failed to parse examples/mcp/databricks_mcp_cookbook.ipynb:12:1:8: Simple statements must be separated by newlines or semicolons

Formatter (preview)

ℹ️ ecosystem check encountered format errors. (no format changes; 1 project error)

openai/openai-cookbook (error)

ruff format --preview

warning: Detected debug build without --no-cache.
error: Failed to parse examples/mcp/databricks_mcp_cookbook.ipynb:12:1:8: Simple statements must be separated by newlines or semicolons

@dylwil3
Copy link
Collaborator Author

dylwil3 commented Jul 18, 2025

@MichaReiser okay after a more thorough investigation I have a better understanding of what is going on here and have gone with a different fix. I've tried to summarize my reasoning in the updated PR summary. Let me know whether you agree, or whether you'd like me to solve the issue in a different way.

@dylwil3 dylwil3 marked this pull request as ready for review July 18, 2025 21:52
@dylwil3 dylwil3 requested a review from dhruvmanila as a code owner July 18, 2025 21:52
Copy link
Member

@MichaReiser MichaReiser left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense. Thank you

@dylwil3 dylwil3 enabled auto-merge (squash) July 20, 2025 22:01
@dylwil3 dylwil3 merged commit 53fc061 into astral-sh:main Jul 20, 2025
34 checks passed
dcreager added a commit that referenced this pull request Jul 21, 2025
* main: (25 commits)
  [ty] Sync vendored typeshed stubs (#19461)
  [ty] Extend tuple `__len__` and `__bool__` special casing to also cover tuple subclasses (#19289)
  [ty] bump docstring-adder pin (#19458)
  [ty] Disallow assignment to `Final` class attributes (#19457)
  Update dependency ruff to v0.12.4 (#19442)
  Update pre-commit hook astral-sh/ruff-pre-commit to v0.12.4 (#19443)
  Update rui314/setup-mold digest to 702b190 (#19441)
  Update taiki-e/install-action action to v2.56.19 (#19448)
  Update Rust crate strum_macros to v0.27.2 (#19447)
  Update Rust crate strum to v0.27.2 (#19446)
  Update Rust crate rand to v0.9.2 (#19444)
  Update Rust crate serde_json to v1.0.141 (#19445)
  Fix `unreachable` panic in parser (#19183)
  [`ruff`] Support byte strings (`RUF055`) (#18926)
  [ty] Avoid second lookup for `infer_maybe_standalone_expression` (#19439)
  [ty] Implemented "go to definition" support for import statements (#19428)
  [ty] Avoid secondary tree traversal to get call expression for keyword arguments (#19429)
  [ty] Add goto definition to playground (#19425)
  [ty] Add support for `@warnings.deprecated` (#19376)
  [ty] make `del x` force local resolution of `x` in the current scope (#19389)
  ...
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.

Panic entered unreachable code f-string: unexpected token 'TStringMiddle' at 6..7 in ruff_python_parser/src/parser/expression.rs

2 participants