[`parser`] Fix panic when parsing IPython escape command expressions #21480

danparizher · 2025-11-16T17:40:27Z

Summary

Fixes a panic when parsing IPython escape commands with Help kind (?) in expression contexts. The parser now reports an error instead of panicking.

Fixes #21465.

Problem

The parser panicked with unreachable!() in parse_ipython_escape_command_expression when encountering escape commands with Help kind (?) in expression contexts, where only Magic (%) and Shell (!) are allowed.

Approach

Replaced the unreachable!() panic with error handling that adds a ParseErrorType::OtherError and continues parsing, returning a valid AST node with the error attached.

Test Plan

Added test_ipython_escape_command_in_with_statement and test_ipython_help_escape_command_as_expression to verify the fix.

astral-sh-bot · 2025-11-16T17:52:22Z

`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.

MichaReiser · 2025-11-16T18:04:17Z

crates/ruff_python_parser/src/parser/expression.rs

        };

-        if !matches!(kind, IpyEscapeKind::Magic | IpyEscapeKind::Shell) {
-            // This should never occur as the lexer won't allow it.


Can you explain to me why the lexer emits the Magic and Shell tokens contrary to what the comment says? Should we fix the lexer instead?

That makes more sense, yeah. I'm pushing a change to have it fixed at the lexer level by adding an in_expression_context parameter to lex_ipython_escape_command and preventing Help kind conversion when true (e.g., %foo? stays Magic in expression contexts).

I think it makes sense to keep the parser check as a defensive measure for edge cases like with a,?b, where Help tokens can still be lexed at the start of logical lines. Does this approach look right?

dhruvmanila · 2025-11-17T06:01:35Z

Thanks for putting up this PR!

I think the problem is in the way the parser is recovering. Considering your example:

with a, ?b
?

As there's a newline before the final ?, the lexer emits the IpyEscapeCommand token. Now, the parser is trying to parse a list of expressions that comprises of with items but (1) it tries to recover from an unexpected ? before the b and then it doesn't consider the newline token (\n) as the end of the with items given that this is unparenthesized. I think a (better?) fix would be to try to make the parser recover from this newline token and consider that the end of with items so that the second ? is parsed as statement which will then avoid this panic.

What do you think? This is what I'm referring to:

diff --git a/crates/ruff_python_parser/src/parser/mod.rs b/crates/ruff_python_parser/src/parser/mod.rs
index 5f35b84b04..59c1857152 100644
--- a/crates/ruff_python_parser/src/parser/mod.rs
+++ b/crates/ruff_python_parser/src/parser/mod.rs
@@ -1115,7 +1115,12 @@ impl RecoveryContextKind {
                     TokenKind::Colon => Some(ListTerminatorKind::ErrorRecovery),
                     _ => None,
                 },
-                WithItemKind::Unparenthesized | WithItemKind::ParenthesizedExpression => p
+                WithItemKind::Unparenthesized => matches!(
+                    p.current_token_kind(),
+                    TokenKind::Colon | TokenKind::Newline
+                )
+                .then_some(ListTerminatorKind::Regular),
+                WithItemKind::ParenthesizedExpression => p
                     .at(TokenKind::Colon)
                     .then_some(ListTerminatorKind::Regular),
             },

Edit: Oh, I think it should maybe be restricted to WithItemKind::Unparenthesized. I've updated the diff snippet.

dhruvmanila · 2025-11-17T06:15:06Z

What do you think? This is what I'm referring to:

Ok, I tried this and it seems to be working well and I think this is more correct approach. I've tried it on various code snippets to use parentheses in various places:

with (a, ?b)
?

with (a, ?b
?

with a, ?b
?

MichaReiser · 2025-11-20T09:17:46Z

crates/ruff_python_parser/src/parser/tests.rs

+#[test]
+fn test_ipython_escape_command_in_with_statement() {
+    // Regression test for https://github.com/astral-sh/ruff/issues/21465
+    // This should not panic when encountering a `?` escape command in a with statement
+    // The original issue was that parsing `with a,?b\n?` would panic when trying to
+    // parse `?b` as an expression. Now it should handle it gracefully with an error.
+    let source = "with a,?b\n?";
+
+    // This should not panic - it may return an error, but that's fine
+    let parsed = parse(source, ParseOptions::from(Mode::Ipython));
+
+    // Verify it doesn't panic - check if it parsed or has errors
+    match parsed {
+        Ok(result) => {
+            // If it parsed successfully, snapshot the result
+            insta::assert_debug_snapshot!(result.syntax());
+        }
+        Err(error) => {
+            // If it has errors, that's also fine - the important thing is it didn't panic
+            // Snapshot the error to verify it's the expected error type
+            insta::assert_debug_snapshot!(error);
+        }
+    }
+}
+
+#[test]
+fn test_ipython_help_escape_command_as_expression() {
+    // Regression test for https://github.com/astral-sh/ruff/issues/21465
+    // Test that parsing a `?` escape command as an expression doesn't panic.
+    // Only `%` and `!` escape commands are allowed as expressions.
+    let source = "?foo";
+
+    // Try to parse as an expression - this should not panic
+    let parsed = parse_expression(source);
+
+    // Should return an error, not panic
+    let error = parsed.unwrap_err();
+    insta::assert_debug_snapshot!(error);
+}


Can you convert those tests to inline tests instead

ruff/crates/ruff_python_parser/CONTRIBUTING.md

Lines 26 to 28 in c424007

// test_err this_is_the_test_name

// [1, 2

println!("some rust code");

MichaReiser · 2025-11-20T09:18:37Z

crates/ruff_python_parser/src/parser/tests.rs

+    // This should not panic when encountering a `?` escape command in a with statement
+    // The original issue was that parsing `with a,?b\n?` would panic when trying to
+    // parse `?b` as an expression. Now it should handle it gracefully with an error.
+    let source = "with a,?b\n?";


Can you add some case arms or statements coming after with so that it also demonstrates that the parser recovers properly?

dhruvmanila

Can you revert the original solution now that the parser recovers correctly? I'd also recommend to add some more test cases especially the ones that contains parentheses.

crates/ruff_python_parser/src/parser/expression.rs

crates/ruff_python_parser/src/parser/statement.rs

crates/ruff_python_parser/src/lexer.rs

dhruvmanila

Thank you!

dhruvmanila · 2025-11-24T05:06:38Z

Hmm, actually the test cases now don't really test the bug which, I think, requires the ? on the next line as mentioned here (#21480 (comment)). I'll update the test cases and merge.

* origin/main: (27 commits) [ty] Add hint about resolved Python version when a user attempts to import a member added on a newer version (#21615) Use release commit for actions/checkout (#21610) [ty] Add failing mdtest for known `Protocol` panic (#21594) [`parser`] Fix panic when parsing IPython escape command expressions (#21480) Fix cargo shear in CI (#21609) Update actions/checkout digest to c2d88d3 (#21601) Update dependency ruff to v0.14.6 (#21603) Update astral-sh/setup-uv action to v7.1.4 (#21602) Update Rust crate clap to v4.5.53 (#21604) Update taiki-e/install-action action to v2.62.56 (#21608) Update Rust crate hashbrown to v0.16.1 (#21605) Update Rust crate indexmap to v2.12.1 (#21606) Update Rust crate syn to v2.0.111 (#21607) [ty] Check method definitions on subclasses for Liskov violations (#21436) [ty] Fix panic for unclosed string literal in type annotation position (#21592) [ty] Fix rendering of unused suppression diagnostic (#21580) [ty] Improve lsp handling of hover/goto on imports (#21572) [ty] Improve diagnostics when a submodule is not available as an attribute on a module-literal type (#21561) [ty] Improve concise diagnostics for invalid exceptions when a user catches a tuple of objects (#21578) [ty] upgrade salsa (#21575) ...

…d-typevar * origin/main: (30 commits) [ty] Double click to insert inlay hint (#21600) [ty] Switch the error code from `unresolved-attribute` to `possibly-missing-attribute` for submodules that may not be available (#21618) [ty] Substitute for `typing.Self` when checking protocol members (#21569) [ty] Don't suggest things that aren't subclasses of `BaseException` after `raise` [ty] Add hint about resolved Python version when a user attempts to import a member added on a newer version (#21615) Use release commit for actions/checkout (#21610) [ty] Add failing mdtest for known `Protocol` panic (#21594) [`parser`] Fix panic when parsing IPython escape command expressions (#21480) Fix cargo shear in CI (#21609) Update actions/checkout digest to c2d88d3 (#21601) Update dependency ruff to v0.14.6 (#21603) Update astral-sh/setup-uv action to v7.1.4 (#21602) Update Rust crate clap to v4.5.53 (#21604) Update taiki-e/install-action action to v2.62.56 (#21608) Update Rust crate hashbrown to v0.16.1 (#21605) Update Rust crate indexmap to v2.12.1 (#21606) Update Rust crate syn to v2.0.111 (#21607) [ty] Check method definitions on subclasses for Liskov violations (#21436) [ty] Fix panic for unclosed string literal in type annotation position (#21592) [ty] Fix rendering of unused suppression diagnostic (#21580) ...

fix-21465

2c6ba72

danparizher requested review from MichaReiser and dhruvmanila as code owners November 16, 2025 17:40

AlexWaygood added bug Something isn't working parser Related to the parser labels Nov 16, 2025

MichaReiser reviewed Nov 16, 2025

View reviewed changes

Fix at lexer level

fb5b81a

Refactor recovery context handling

c318542

MichaReiser reviewed Nov 20, 2025

View reviewed changes

danparizher added 2 commits November 20, 2025 20:41

Rearrange tests, add case

ad7380b

Remove unreferenced snapshots

44f8526

danparizher requested a review from MichaReiser November 21, 2025 02:23

dhruvmanila requested changes Nov 21, 2025

View reviewed changes

Revert original changes

e987706

danparizher requested a review from dhruvmanila November 23, 2025 00:09

dhruvmanila approved these changes Nov 24, 2025

View reviewed changes

Update test cases

c33102f

dhruvmanila enabled auto-merge (squash) November 24, 2025 05:12

Delete snapshots

b3549d4

dhruvmanila merged commit 474b005 into astral-sh:main Nov 24, 2025
36 of 37 checks passed

danparizher deleted the fix-21465 branch November 24, 2025 06:01

	// test_err this_is_the_test_name
	// [1, 2
	println!("some rust code");

[parser] Fix panic when parsing IPython escape command expressions #21480

[parser] Fix panic when parsing IPython escape command expressions #21480

Uh oh!

Conversation

danparizher commented Nov 16, 2025

Summary

Problem

Approach

Test Plan

Uh oh!

astral-sh-bot bot commented Nov 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

ruff-ecosystem results

Linter (stable)

Linter (preview)

Formatter (stable)

Formatter (preview)

Uh oh!

MichaReiser Nov 16, 2025

Choose a reason for hiding this comment

Uh oh!

danparizher Nov 16, 2025

Choose a reason for hiding this comment

Uh oh!

dhruvmanila commented Nov 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dhruvmanila commented Nov 17, 2025

Uh oh!

MichaReiser Nov 20, 2025

Choose a reason for hiding this comment

Uh oh!

MichaReiser Nov 20, 2025

Choose a reason for hiding this comment

Uh oh!

dhruvmanila left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

dhruvmanila left a comment

Choose a reason for hiding this comment

Uh oh!

dhruvmanila commented Nov 24, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

[`parser`] Fix panic when parsing IPython escape command expressions #21480

[`parser`] Fix panic when parsing IPython escape command expressions #21480

astral-sh-bot bot commented Nov 16, 2025 •

edited

Loading

`ruff-ecosystem` results

dhruvmanila commented Nov 17, 2025 •

edited

Loading