[ruff] Detect syntax errors in individual notebook cells#26419
Conversation
Ruff concatenates all notebook cells into a single source before parsing, so a syntax error confined to one cell can be silently completed by a following cell. Parse each cell as its own module and combine the parsed modules into one module via parse_unchecked_module_ranges, using new_starts_at so nodes keep their offsets into the concatenated source. A cell is valid only if it parses standalone, while definitions from earlier cells stay visible to later cells because the combined module keeps every cell's statements in order. Regular files are unaffected (unchanged single-parse path); notebooks are parsed in a single linear pass, each cell once, with no second parse. ty still concatenates and is left as a follow-up.
Memory usage reportMemory usage unchanged ✅ |
|
|
MichaReiser
left a comment
There was a problem hiding this comment.
Thank you.
There are a few more places that need changing:
-
We need to search all references to
ruff_python_parser::parse_uncheckedand make sure they call the new API for notebooks -
It also seems that this new parsing now results in an infinite loop in the
ruff:ignorehandling, we should take a look at why# Cell 1 def f(): pass # Cell 2 # ruff: disable[F401] # Cell 3 import os
MichaReiser
left a comment
There was a problem hiding this comment.
Thank you. I think there's one more case that we need to handle to truly close the linked issue
Each cell is parsed from a slice that ends at the next cell's start, so an error anchored at that trailing EOF (an unclosed bracket, or a decorator whose def is in the next cell) was attributed to the following cell. Anchor such errors at the cell's last offset so they are reported in the cell that contains them. Route ty (ruff_db) through the same per-cell path so the type checker surfaces these errors too, and add a notebook mdtest covering the rendered output.
MichaReiser
left a comment
There was a problem hiding this comment.
Thank you. I'd prefer if we don't need to manually adjust the error ranges. I think we can accomplish this by using the \n separator that our notebooks insert.
|
@MichaReiser Sounds good, switching to the separator approach.
Passing those into I'll document the single-byte For the token stream, the content slice hides the Watching the boundary I'll commit the changes soon. |
|
@MichaReiser Done, switched to the separator approach. I documented the single-byte The per-cell parse no longer lexes that separator, so I bridge each internal cell boundary with a Full test suite for the touched crates is green. |
Parse each cell from its content range, which excludes the trailing `\n` separator, instead of relocating errors after the fact. A per-cell EOF error (an unclosed bracket, or a decorator whose `def` is in the next cell) then lands on the separator, the cell's own last line, and is attributed to the right cell without mutating the error range. Because the per-cell parse no longer lexes the separator, bridge each internal cell boundary with a `NonLogicalNewline` so token-based checks such as the blank-line rules don't misread it. Document the single-byte `\n` separator requirement on `parse_cells_unchecked`.
Summary
Fixes part of #23782.
Ruff concatenates all notebook cells into a single source before parsing, so a syntax error confined to one cell can be silently completed by the next cell's content.
Example:
if True:in one cell with its body in the next, an unclosed bracket, an unterminated triple-quoted string or f-string, a line continuation, or a decorator whosedefis in the following cell.This parses each cell as its own module and combines the parsed modules into a single module via
parse_cells_unchecked, usingnew_starts_atso the nodes keep their offsets into the concatenated source.A cell is therefore valid only if it parses standalone, matching how Jupyter executes cells, while definitions from earlier cells stay visible to later cells because the combined module keeps every cell's statements in order.
Each cell's range stops before the newline that Ruff inserts between cells, so a syntax error anchored at the cell's trailing end lands on that separator, the cell's own last line, and is reported where it occurs rather than in the following cell. This covers a syntax error at a cell boundary, an unclosed bracket, an unterminated string, or a decorator whose
defis in the next cell.Regular files are unaffected, they take the existing single-parse path, and notebooks are still parsed in a single linear pass. The linter, language server, and
ruff_dev print-tokensshare this path so editor and test output match the CLI.Because each cell is parsed without its trailing separator newline, the combined token stream bridges every internal cell boundary with a
NonLogicalNewlineso the boundary tokens don't produce spurious blank-line or whitespace diagnostics (E301-E303, E305, E306, W291/W293/W391); a notebook test asserts this.A boundary
Dedentcan also share its offset with a suppression comment opening the next cell, which made range-suppression handling (# ruff: disable/enable) loop indefinitely; it now splits the token stream onend <= offsetso the comment comes first, with a notebook test covering it.ty (
ruff_db) parses notebooks through the same per-cell path, so the type checker and its language server report these errors in the same place.Test plan
cargo test -p ruff_python_parser notebookcargo test -p ruff_linter notebookcargo test -p ty_python_semantic notebook./target/debug/ruff check <split-cell .ipynb>: the error is now reported