From 8c7179ea6f6e202151496337e88ebf0e83d34748 Mon Sep 17 00:00:00 2001 From: Omar Yusuf Abdi Date: Fri, 26 Jun 2026 21:50:17 +0200 Subject: [PATCH 1/8] [ruff] Detect syntax errors in individual notebook cells 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. --- crates/ruff_linter/src/linter.rs | 69 +++++++++++++++--- .../jupyter/cell_boundary_syntax_error.ipynb | 32 +++++++++ .../fixtures/jupyter/valid_multicell.ipynb | 52 ++++++++++++++ crates/ruff_python_parser/src/lib.rs | 67 ++++++++++++++++- crates/ruff_python_parser/src/parser/tests.rs | 71 ++++++++++++++++++- 5 files changed, 277 insertions(+), 14 deletions(-) create mode 100644 crates/ruff_notebook/resources/test/fixtures/jupyter/cell_boundary_syntax_error.ipynb create mode 100644 crates/ruff_notebook/resources/test/fixtures/jupyter/valid_multicell.ipynb diff --git a/crates/ruff_linter/src/linter.rs b/crates/ruff_linter/src/linter.rs index 8e32dd809b23f..9da8a8e1c8795 100644 --- a/crates/ruff_linter/src/linter.rs +++ b/crates/ruff_linter/src/linter.rs @@ -769,7 +769,11 @@ impl ParseSource { /// Like [`ruff_python_parser::parse_unchecked_source`] but with an additional [`PythonVersion`] /// argument. -fn parse_unchecked_source( +/// +/// Jupyter notebooks are parsed cell by cell so that a syntax error confined to one cell isn't +/// masked by the source of a following cell +/// Definitions still resolve across cells because the per-cell modules are concatenated into a single module +pub(crate) fn parse_unchecked_source( source_kind: &SourceKind, source_type: PySourceType, target_version: PythonVersion, @@ -778,9 +782,16 @@ fn parse_unchecked_source( // SAFETY: Safe because `PySourceType` always parses to a `ModModule`. See // `ruff_python_parser::parse_unchecked_source`. We use `parse_unchecked` (and thus // have to unwrap) in order to pass the `PythonVersion` via `ParseOptions`. - ruff_python_parser::parse_unchecked(source_kind.source_code(), options) - .try_into_module() - .expect("PySourceType always parses into a module") + match source_kind.as_ipy_notebook() { + Some(notebook) => ruff_python_parser::parse_unchecked_module_ranges( + source_kind.source_code(), + notebook.cell_offsets().ranges(), + options, + ), + None => ruff_python_parser::parse_unchecked(source_kind.source_code(), options) + .try_into_module() + .expect("PySourceType always parses into a module"), + } } #[cfg(test)] @@ -791,14 +802,13 @@ mod tests { use ruff_python_ast::{PySourceType, PythonVersion}; use ruff_python_codegen::Stylist; use ruff_python_index::Indexer; - use ruff_python_parser::ParseOptions; use ruff_python_trivia::textwrap::dedent; use test_case::test_case; use ruff_db::diagnostic::Diagnostic; use ruff_notebook::{Notebook, NotebookError}; - use crate::linter::check_path; + use crate::linter::{check_path, parse_unchecked_source}; use crate::registry::Rule; use crate::settings::LinterSettings; use crate::source_kind::SourceKind; @@ -811,6 +821,45 @@ mod tests { Path::new("../ruff_notebook/resources/test/fixtures/jupyter").join(path) } + #[test] + fn test_cell_boundary_syntax_error() -> Result<(), NotebookError> { + // A decorator whose definition lives in the next cell parses cleanly once the cells are + // concatenated, but is invalid on its own. Per-cell parsing must report it. + let path = notebook_path("cell_boundary_syntax_error.ipynb"); + let source_kind = SourceKind::ipy_notebook(Notebook::from_path(&path)?); + let diagnostics = + test_contents_syntax_errors(&source_kind, &path, &LinterSettings::default()); + assert!( + diagnostics.iter().any(Diagnostic::is_invalid_syntax), + "expected a cell-boundary syntax error, got: {diagnostics:?}" + ); + Ok(()) + } + + #[test] + fn test_notebook_cell_boundary_no_spurious_diagnostics() -> Result<(), NotebookError> { + // Per-cell parsing injects synthetic boundary tokens (trailing `Newline`/`Dedent`) + // Ensuring they don't trigger spurious blank-line or whitespace diagnostics on a valid notebook. + let path = notebook_path("valid_multicell.ipynb"); + let source_kind = SourceKind::ipy_notebook(Notebook::from_path(&path)?); + let settings = LinterSettings::for_rules([ + Rule::BlankLineBetweenMethods, + Rule::BlankLinesTopLevel, + Rule::TooManyBlankLines, + Rule::BlankLinesAfterFunctionOrClass, + Rule::BlankLinesBeforeNestedDefinition, + Rule::TrailingWhitespace, + Rule::BlankLineWithWhitespace, + Rule::TooManyNewlinesAtEndOfFile, + ]); + let diagnostics = test_contents_syntax_errors(&source_kind, &path, &settings); + assert!( + diagnostics.is_empty(), + "per-cell parsing introduced spurious diagnostics: {diagnostics:?}" + ); + Ok(()) + } + #[test] fn test_import_sorting() -> Result<(), NotebookError> { let actual = notebook_path("isort.ipynb"); @@ -964,11 +1013,9 @@ mod tests { ) -> Vec { let source_type = PySourceType::from(path); let target_version = settings.resolve_target_version(path); - let options = - ParseOptions::from(source_type).with_target_version(target_version.parser_version()); - let parsed = ruff_python_parser::parse_unchecked(source_kind.source_code(), options) - .try_into_module() - .expect("PySourceType always parses into a module"); + // Mirror the production parse path so notebooks are validated cell by cell. + let parsed = + parse_unchecked_source(source_kind, source_type, target_version.parser_version()); let locator = Locator::new(source_kind.source_code()); let stylist = Stylist::from_tokens(parsed.tokens(), locator.contents()); let indexer = Indexer::from_tokens(parsed.tokens(), locator.contents()); diff --git a/crates/ruff_notebook/resources/test/fixtures/jupyter/cell_boundary_syntax_error.ipynb b/crates/ruff_notebook/resources/test/fixtures/jupyter/cell_boundary_syntax_error.ipynb new file mode 100644 index 0000000000000..4951ec97403e1 --- /dev/null +++ b/crates/ruff_notebook/resources/test/fixtures/jupyter/cell_boundary_syntax_error.ipynb @@ -0,0 +1,32 @@ +{ + "cells": [ + { + "cell_type": "code", + "execution_count": null, + "id": "cell0", + "metadata": {}, + "outputs": [], + "source": [ + "@deco" + ] + }, + { + "cell_type": "code", + "execution_count": null, + "id": "cell1", + "metadata": {}, + "outputs": [], + "source": [ + "def f():\n", + " return 1" + ] + } + ], + "metadata": { + "language_info": { + "name": "python" + } + }, + "nbformat": 4, + "nbformat_minor": 5 +} diff --git a/crates/ruff_notebook/resources/test/fixtures/jupyter/valid_multicell.ipynb b/crates/ruff_notebook/resources/test/fixtures/jupyter/valid_multicell.ipynb new file mode 100644 index 0000000000000..ff86a72eeb679 --- /dev/null +++ b/crates/ruff_notebook/resources/test/fixtures/jupyter/valid_multicell.ipynb @@ -0,0 +1,52 @@ +{ + "cells": [ + { + "cell_type": "code", + "execution_count": null, + "id": "cell0", + "metadata": {}, + "outputs": [], + "source": [ + "import os\n", + "import sys" + ] + }, + { + "cell_type": "markdown", + "id": "cellmd", + "metadata": {}, + "source": [ + "## A heading between code cells" + ] + }, + { + "cell_type": "code", + "execution_count": null, + "id": "cell1", + "metadata": {}, + "outputs": [], + "source": [ + "\n", + "values = [os.getcwd(), sys.platform]" + ] + }, + { + "cell_type": "code", + "execution_count": null, + "id": "cell2", + "metadata": {}, + "outputs": [], + "source": [ + "for value in values:\n", + " print(value)" + ] + } + ], + "metadata": { + "language_info": { + "name": "python" + } + }, + "nbformat": 4, + "nbformat_minor": 5 +} diff --git a/crates/ruff_python_parser/src/lib.rs b/crates/ruff_python_parser/src/lib.rs index 9ff1aab6a7e6b..a857808587350 100644 --- a/crates/ruff_python_parser/src/lib.rs +++ b/crates/ruff_python_parser/src/lib.rs @@ -73,7 +73,8 @@ use crate::parser::Parser; use ruff_python_ast::token::Tokens; use ruff_python_ast::{ - Expr, Mod, ModExpression, ModModule, PySourceType, StringFlags, StringLiteral, Suite, + AtomicNodeIndex, Expr, Mod, ModExpression, ModModule, PySourceType, StringFlags, StringLiteral, + Suite, }; use ruff_text_size::{Ranged, TextRange}; @@ -298,6 +299,70 @@ pub fn parse_unchecked_source(source: &str, source_type: PySourceType) -> Parsed .unwrap() } +/// Parses each `range` of `source` as an independent module and concatenates the results into a +/// single [`Parsed`] whose nodes keep their offsets into `source`. +/// +/// This validates sources such as Jupyter notebooks, where each cell must be syntactically valid on +/// its own while later cells can still reference earlier definitions. +/// The `ranges` must be ordered and non-overlapping. An empty `ranges` falls back to [`parse_unchecked`]. +pub fn parse_unchecked_module_ranges( + source: &str, + ranges: impl IntoIterator, + options: ParseOptions, +) -> Parsed { + let mut ranges = ranges.into_iter().peekable(); + if ranges.peek().is_none() { + return parse_unchecked(source, options) + .try_into_module() + .expect("module options should parse into a module"); + } + + let mut body = Suite::new(); + let mut tokens = Vec::new(); + let mut errors = Vec::new(); + let mut unsupported_syntax_errors = Vec::new(); + let mut module_range: Option = None; + + for range in ranges { + if let Some(previous) = module_range { + debug_assert!(previous.end() <= range.start()); + } + + // The cell is lexed from `range.start()`, so the slice must keep the leading text to + // preserve absolute offsets into the concatenated source. + let cell_source = &source[..range.end().to_usize()]; + let Parsed { + syntax, + tokens: cell_tokens, + errors: cell_errors, + unsupported_syntax_errors: cell_unsupported_syntax_errors, + } = Parser::new_starts_at(cell_source, range.start(), options.clone()) + .parse() + .try_into_module() + .expect("module options should parse into a module"); + + body.extend(syntax.body); + tokens.extend(cell_tokens.iter().copied()); + errors.extend(cell_errors); + unsupported_syntax_errors.extend(cell_unsupported_syntax_errors); + module_range = Some(match module_range { + Some(previous) => TextRange::new(previous.start(), range.end()), + None => range, + }); + } + + Parsed { + syntax: ModModule { + node_index: AtomicNodeIndex::NONE, + range: module_range.expect("at least one range must be present"), + body, + }, + tokens: Tokens::new(tokens), + errors, + unsupported_syntax_errors, + } +} + /// Represents the parsed source code. #[derive(Debug, PartialEq, Clone, get_size2::GetSize)] pub struct Parsed { diff --git a/crates/ruff_python_parser/src/parser/tests.rs b/crates/ruff_python_parser/src/parser/tests.rs index 0cd15515e0cf4..7475740c03051 100644 --- a/crates/ruff_python_parser/src/parser/tests.rs +++ b/crates/ruff_python_parser/src/parser/tests.rs @@ -1,6 +1,10 @@ -use ruff_python_ast::{Expr, InterpolatedStringElement, IpyEscapeKind, Number, Stmt}; +use ruff_python_ast::{Expr, InterpolatedStringElement, IpyEscapeKind, ModModule, Number, Stmt}; +use ruff_text_size::{Ranged, TextRange, TextSize}; -use crate::{Mode, ParseErrorType, ParseOptions, parse, parse_expression, parse_module}; +use crate::{ + Mode, ParseErrorType, ParseOptions, Parsed, parse, parse_expression, parse_module, + parse_unchecked_module_ranges, +}; #[test] fn test_modes() { @@ -547,3 +551,66 @@ fn recursion_limit_nested_lambda_chain() { err.error ); } + +/// Parses `cells` as consecutive IPython cells, mirroring how a notebook's concatenated source is +/// split into contiguous ranges by `CellOffsets`. +fn parse_ipy_cells(cells: &[&str]) -> Parsed { + let mut source = String::new(); + let mut ranges = Vec::new(); + for cell in cells { + let start = TextSize::of(&source); + source.push_str(cell); + ranges.push(TextRange::new(start, TextSize::of(&source))); + } + parse_unchecked_module_ranges(&source, ranges, ParseOptions::from(Mode::Ipython)) +} + +#[test] +fn notebook_cell_boundary_syntax_errors() { + // Each pair concatenates into valid source, but the first cell is invalid on its own. + // Concatenated parse masks the error; per-cell parsing must surface it. + // Decorator case is grammar-level: a lexer-only boundary fix cannot catch it. + let cases = [ + ["if True:\n", " pass\n"], + ["x = [\n", "1]\n"], + ["s = \"\"\"a\n", "b\"\"\"\n"], + ["y = f\"{x\n", "}\"\n"], + ["z = 1 + \\\n", "2\n"], + ["@deco\n", "def f(): pass\n"], + ]; + + for [first, second] in cases { + let concatenated = format!("{first}{second}"); + assert!( + parse_module(&concatenated).is_ok(), + "concatenated source should parse cleanly: {concatenated:?}" + ); + assert!( + parse_ipy_cells(&[first, second]).has_invalid_syntax(), + "per-cell parsing should report a syntax error: {concatenated:?}" + ); + } +} + +#[test] +fn notebook_cells_resolve_across_boundaries() { + // A definition in an earlier cell stays visible to a later cell: the combined module keeps both + // statements in order, and the second cell's node carries its concatenated-source offset. + let cells = ["import os\n", "os.getcwd()\n"]; + let parsed = parse_ipy_cells(&cells); + + assert!(parsed.has_valid_syntax()); + + let body = &parsed.syntax().body; + assert_eq!(body.len(), 2); + assert!(matches!(body[0], Stmt::Import(_))); + assert_eq!(body[1].start(), TextSize::of(cells[0])); +} + +#[test] +fn notebook_empty_ranges_falls_back_to_whole_source() { + // No cell ranges e.g a notebook with only magic/markdown cells, parses the whole source once. + let parsed = parse_unchecked_module_ranges("x = 1\n", [], ParseOptions::from(Mode::Ipython)); + assert!(parsed.has_valid_syntax()); + assert_eq!(parsed.syntax().body.len(), 1); +} From b6f1512e9c657a6a22dafcc5a3883c8dcc7aa0b3 Mon Sep 17 00:00:00 2001 From: Omar Yusuf Abdi Date: Mon, 29 Jun 2026 21:42:36 +0200 Subject: [PATCH 2/8] [ruff] Fix cell-boundary suppression loop and route notebook parsing --- crates/ruff_dev/src/print_tokens.rs | 6 +-- crates/ruff_linter/src/linter.rs | 24 +++++++++++- crates/ruff_linter/src/suppression.rs | 11 +++++- crates/ruff_linter/src/test.rs | 21 +++++----- .../jupyter/cell_boundary_suppression.ipynb | 39 +++++++++++++++++++ crates/ruff_python_parser/src/lib.rs | 2 +- crates/ruff_python_parser/src/parser/tests.rs | 8 ++-- crates/ruff_server/src/lint.rs | 13 ++----- 8 files changed, 93 insertions(+), 31 deletions(-) create mode 100644 crates/ruff_notebook/resources/test/fixtures/jupyter/cell_boundary_suppression.ipynb diff --git a/crates/ruff_dev/src/print_tokens.rs b/crates/ruff_dev/src/print_tokens.rs index 3e7c89b868c22..b2b921f7e0fe8 100644 --- a/crates/ruff_dev/src/print_tokens.rs +++ b/crates/ruff_dev/src/print_tokens.rs @@ -4,9 +4,9 @@ use std::path::PathBuf; use anyhow::Result; +use ruff_linter::linter::parse_unchecked_source; use ruff_linter::source_kind::SourceKind; -use ruff_python_ast::{PySourceType, SourceType}; -use ruff_python_parser::parse_unchecked_source; +use ruff_python_ast::{PySourceType, PythonVersion, SourceType}; #[derive(clap::Args)] pub(crate) struct Args { @@ -24,7 +24,7 @@ pub(crate) fn main(args: &Args) -> Result<()> { args.file.display() ) })?; - let parsed = parse_unchecked_source(source_kind.source_code(), source_type); + let parsed = parse_unchecked_source(&source_kind, source_type, PythonVersion::default()); for token in parsed.tokens() { println!("{token:#?}"); } diff --git a/crates/ruff_linter/src/linter.rs b/crates/ruff_linter/src/linter.rs index 9da8a8e1c8795..785e23efe7c1f 100644 --- a/crates/ruff_linter/src/linter.rs +++ b/crates/ruff_linter/src/linter.rs @@ -773,7 +773,7 @@ impl ParseSource { /// Jupyter notebooks are parsed cell by cell so that a syntax error confined to one cell isn't /// masked by the source of a following cell /// Definitions still resolve across cells because the per-cell modules are concatenated into a single module -pub(crate) fn parse_unchecked_source( +pub fn parse_unchecked_source( source_kind: &SourceKind, source_type: PySourceType, target_version: PythonVersion, @@ -783,7 +783,7 @@ pub(crate) fn parse_unchecked_source( // `ruff_python_parser::parse_unchecked_source`. We use `parse_unchecked` (and thus // have to unwrap) in order to pass the `PythonVersion` via `ParseOptions`. match source_kind.as_ipy_notebook() { - Some(notebook) => ruff_python_parser::parse_unchecked_module_ranges( + Some(notebook) => ruff_python_parser::parse_cells_unchecked( source_kind.source_code(), notebook.cell_offsets().ranges(), options, @@ -860,6 +860,26 @@ mod tests { Ok(()) } + #[test] + fn test_notebook_cell_boundary_suppression() -> Result<(), NotebookError> { + // A cell that closes an indented block emits a trailing `Dedent` at the cell boundary, + // which shares its offset with a `# ruff: disable` comment opening the next cell. Range + // suppression handling used to get stuck on that dedent and loop forever; it must finish + // and apply the suppression so the following `import os` isn't reported as F401. + let path = notebook_path("cell_boundary_suppression.ipynb"); + let source_kind = SourceKind::ipy_notebook(Notebook::from_path(&path)?); + let diagnostics = test_contents_syntax_errors( + &source_kind, + &path, + &LinterSettings::for_rule(Rule::UnusedImport), + ); + assert!( + diagnostics.is_empty(), + "expected the cell-boundary `# ruff: disable` to suppress F401, got: {diagnostics:?}" + ); + Ok(()) + } + #[test] fn test_import_sorting() -> Result<(), NotebookError> { let actual = notebook_path("isort.ipynb"); diff --git a/crates/ruff_linter/src/suppression.rs b/crates/ruff_linter/src/suppression.rs index 4ebf99d380f87..653559a50c650 100644 --- a/crates/ruff_linter/src/suppression.rs +++ b/crates/ruff_linter/src/suppression.rs @@ -662,8 +662,15 @@ impl<'a> SuppressionsBuilder<'a> { continue; } - // Matched suppression comments - let (before, after) = tokens.split_at(suppression.token_range.start()); + // Matched suppression comments. A notebook cell closing an indented block emits a + // zero-width `Dedent` at the cell boundary, sharing its offset with a comment opening + // the next cell. Split on `end <= offset` rather than `Tokens::split_at` so that + // `Dedent` stays in `before`; otherwise `after` starts with it and the loop below spins + // on the dedent's `continue 'comments` without ever consuming the comment. + let tokens_slice: &[Token] = tokens; + let comment_start = suppression.token_range.start(); + let split = tokens_slice.partition_point(|token| token.end() <= comment_start); + let (before, after) = tokens_slice.split_at(split); let mut count = 0; let last_indent = before diff --git a/crates/ruff_linter/src/test.rs b/crates/ruff_linter/src/test.rs index 65153eee484c8..cba6b200fe39f 100644 --- a/crates/ruff_linter/src/test.rs +++ b/crates/ruff_linter/src/test.rs @@ -18,7 +18,7 @@ use ruff_notebook::NotebookError; use ruff_python_ast::PySourceType; use ruff_python_codegen::Stylist; use ruff_python_index::Indexer; -use ruff_python_parser::{ParseError, ParseOptions}; +use ruff_python_parser::ParseError; use ruff_python_trivia::textwrap::dedent; use ruff_source_file::SourceFileBuilder; @@ -231,11 +231,11 @@ pub fn test_contents<'a>( ) -> (Vec, Cow<'a, SourceKind>) { let source_type = PySourceType::from(path); let target_version = settings.resolve_target_version(path); - let options = - ParseOptions::from(source_type).with_target_version(target_version.parser_version()); - let parsed = ruff_python_parser::parse_unchecked(source_kind.source_code(), options.clone()) - .try_into_module() - .expect("PySourceType always parses into a module"); + let parsed = crate::linter::parse_unchecked_source( + source_kind, + source_type, + target_version.parser_version(), + ); let locator = Locator::new(source_kind.source_code()); let stylist = Stylist::from_tokens(parsed.tokens(), locator.contents()); let indexer = Indexer::from_tokens(parsed.tokens(), locator.contents()); @@ -299,10 +299,11 @@ pub fn test_contents<'a>( transformed = Cow::Owned(transformed.updated(fixed_contents, &source_map)); - let parsed = - ruff_python_parser::parse_unchecked(transformed.source_code(), options.clone()) - .try_into_module() - .expect("PySourceType always parses into a module"); + let parsed = crate::linter::parse_unchecked_source( + &transformed, + source_type, + target_version.parser_version(), + ); let locator = Locator::new(transformed.source_code()); let stylist = Stylist::from_tokens(parsed.tokens(), locator.contents()); let indexer = Indexer::from_tokens(parsed.tokens(), locator.contents()); diff --git a/crates/ruff_notebook/resources/test/fixtures/jupyter/cell_boundary_suppression.ipynb b/crates/ruff_notebook/resources/test/fixtures/jupyter/cell_boundary_suppression.ipynb new file mode 100644 index 0000000000000..ad6c1366e6ab5 --- /dev/null +++ b/crates/ruff_notebook/resources/test/fixtures/jupyter/cell_boundary_suppression.ipynb @@ -0,0 +1,39 @@ +{ + "cells": [ + { + "cell_type": "code", + "execution_count": null, + "metadata": {}, + "outputs": [], + "source": [ + "def f():\n", + " pass" + ] + }, + { + "cell_type": "code", + "execution_count": null, + "metadata": {}, + "outputs": [], + "source": [ + "# ruff: disable[F401]" + ] + }, + { + "cell_type": "code", + "execution_count": null, + "metadata": {}, + "outputs": [], + "source": [ + "import os" + ] + } + ], + "metadata": { + "language_info": { + "name": "python" + } + }, + "nbformat": 4, + "nbformat_minor": 5 +} diff --git a/crates/ruff_python_parser/src/lib.rs b/crates/ruff_python_parser/src/lib.rs index a857808587350..79e9b6a8204ba 100644 --- a/crates/ruff_python_parser/src/lib.rs +++ b/crates/ruff_python_parser/src/lib.rs @@ -305,7 +305,7 @@ pub fn parse_unchecked_source(source: &str, source_type: PySourceType) -> Parsed /// This validates sources such as Jupyter notebooks, where each cell must be syntactically valid on /// its own while later cells can still reference earlier definitions. /// The `ranges` must be ordered and non-overlapping. An empty `ranges` falls back to [`parse_unchecked`]. -pub fn parse_unchecked_module_ranges( +pub fn parse_cells_unchecked( source: &str, ranges: impl IntoIterator, options: ParseOptions, diff --git a/crates/ruff_python_parser/src/parser/tests.rs b/crates/ruff_python_parser/src/parser/tests.rs index 7475740c03051..c78e3362b1a73 100644 --- a/crates/ruff_python_parser/src/parser/tests.rs +++ b/crates/ruff_python_parser/src/parser/tests.rs @@ -2,8 +2,8 @@ use ruff_python_ast::{Expr, InterpolatedStringElement, IpyEscapeKind, ModModule, use ruff_text_size::{Ranged, TextRange, TextSize}; use crate::{ - Mode, ParseErrorType, ParseOptions, Parsed, parse, parse_expression, parse_module, - parse_unchecked_module_ranges, + Mode, ParseErrorType, ParseOptions, Parsed, parse, parse_cells_unchecked, parse_expression, + parse_module, }; #[test] @@ -562,7 +562,7 @@ fn parse_ipy_cells(cells: &[&str]) -> Parsed { source.push_str(cell); ranges.push(TextRange::new(start, TextSize::of(&source))); } - parse_unchecked_module_ranges(&source, ranges, ParseOptions::from(Mode::Ipython)) + parse_cells_unchecked(&source, ranges, ParseOptions::from(Mode::Ipython)) } #[test] @@ -610,7 +610,7 @@ fn notebook_cells_resolve_across_boundaries() { #[test] fn notebook_empty_ranges_falls_back_to_whole_source() { // No cell ranges e.g a notebook with only magic/markdown cells, parses the whole source once. - let parsed = parse_unchecked_module_ranges("x = 1\n", [], ParseOptions::from(Mode::Ipython)); + let parsed = parse_cells_unchecked("x = 1\n", [], ParseOptions::from(Mode::Ipython)); assert!(parsed.has_valid_syntax()); assert_eq!(parsed.syntax().body.len(), 1); } diff --git a/crates/ruff_server/src/lint.rs b/crates/ruff_server/src/lint.rs index cefd17cf1a49c..fb0943f1330fd 100644 --- a/crates/ruff_server/src/lint.rs +++ b/crates/ruff_server/src/lint.rs @@ -19,7 +19,7 @@ use ruff_linter::{ Locator, directives::{Flags, extract_directives}, generate_noqa_edits, - linter::check_path, + linter::{check_path, parse_unchecked_source}, package::PackageRoot, packaging::detect_package_root, preview::is_human_readable_names_enabled, @@ -30,7 +30,6 @@ use ruff_linter::{ use ruff_notebook::Notebook; use ruff_python_codegen::Stylist; use ruff_python_index::Indexer; -use ruff_python_parser::ParseOptions; use ruff_source_file::LineIndex; use ruff_text_size::{Ranged, TextRange}; @@ -109,13 +108,9 @@ pub(crate) fn check( let target_version = settings.linter.resolve_target_version(&document_path); - let parse_options = - ParseOptions::from(source_type).with_target_version(target_version.parser_version()); - - // Parse once. - let parsed = ruff_python_parser::parse_unchecked(source_kind.source_code(), parse_options) - .try_into_module() - .expect("PySourceType always parses to a ModModule"); + // Parse once. Notebooks are parsed cell by cell so a syntax error confined to one cell isn't + // masked by a following cell; definitions still resolve across cells. + let parsed = parse_unchecked_source(&source_kind, source_type, target_version.parser_version()); // Map row and column locations to byte slices (lazily). let locator = Locator::new(source_kind.source_code()); From 95acae73225c97186a26eeac5a07d32f382bdd22 Mon Sep 17 00:00:00 2001 From: Omar Yusuf Abdi Date: Mon, 29 Jun 2026 22:05:06 +0200 Subject: [PATCH 3/8] [ruff] Cover cell-boundary dedent in blank-line test fixture --- crates/ruff_linter/src/linter.rs | 5 +++-- .../fixtures/jupyter/valid_multicell.ipynb | 19 +++++++++++++------ 2 files changed, 16 insertions(+), 8 deletions(-) diff --git a/crates/ruff_linter/src/linter.rs b/crates/ruff_linter/src/linter.rs index 785e23efe7c1f..b8fd32beda3f4 100644 --- a/crates/ruff_linter/src/linter.rs +++ b/crates/ruff_linter/src/linter.rs @@ -838,8 +838,9 @@ mod tests { #[test] fn test_notebook_cell_boundary_no_spurious_diagnostics() -> Result<(), NotebookError> { - // Per-cell parsing injects synthetic boundary tokens (trailing `Newline`/`Dedent`) - // Ensuring they don't trigger spurious blank-line or whitespace diagnostics on a valid notebook. + // `valid_multicell.ipynb` ends a cell with an indented `def` block right before the next + // cell, so per-cell parsing emits a trailing `Dedent` at that boundary. Ensure those + // synthetic boundary tokens don't trigger spurious blank-line or whitespace diagnostics. let path = notebook_path("valid_multicell.ipynb"); let source_kind = SourceKind::ipy_notebook(Notebook::from_path(&path)?); let settings = LinterSettings::for_rules([ diff --git a/crates/ruff_notebook/resources/test/fixtures/jupyter/valid_multicell.ipynb b/crates/ruff_notebook/resources/test/fixtures/jupyter/valid_multicell.ipynb index ff86a72eeb679..f8bff12342663 100644 --- a/crates/ruff_notebook/resources/test/fixtures/jupyter/valid_multicell.ipynb +++ b/crates/ruff_notebook/resources/test/fixtures/jupyter/valid_multicell.ipynb @@ -3,7 +3,6 @@ { "cell_type": "code", "execution_count": null, - "id": "cell0", "metadata": {}, "outputs": [], "source": [ @@ -13,7 +12,6 @@ }, { "cell_type": "markdown", - "id": "cellmd", "metadata": {}, "source": [ "## A heading between code cells" @@ -22,18 +20,27 @@ { "cell_type": "code", "execution_count": null, - "id": "cell1", + "id": "d5e478da", "metadata": {}, "outputs": [], "source": [ - "\n", - "values = [os.getcwd(), sys.platform]" + "def compute():\n", + " return [os.getcwd(), sys.platform]" + ] + }, + { + "cell_type": "code", + "execution_count": null, + "id": "9cc80d85", + "metadata": {}, + "outputs": [], + "source": [ + "values = compute()" ] }, { "cell_type": "code", "execution_count": null, - "id": "cell2", "metadata": {}, "outputs": [], "source": [ From 3b29092ac2b9ebe3b2c495d956895e63ff038c18 Mon Sep 17 00:00:00 2001 From: Omar Yusuf Abdi Date: Sat, 4 Jul 2026 18:06:16 +0200 Subject: [PATCH 4/8] [ruff] Report cell-boundary syntax errors in their own cell 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. --- crates/ruff_db/src/parsed.rs | 17 +++++-- crates/ruff_python_parser/src/lib.rs | 25 +++++++++- crates/ruff_python_parser/src/parser/tests.rs | 25 ++++++++-- .../resources/mdtest/notebook.md | 46 +++++++++++++++++++ ...e2e__notebook__diagnostic_end_of_file.snap | 4 +- 5 files changed, 104 insertions(+), 13 deletions(-) create mode 100644 crates/ty_python_semantic/resources/mdtest/notebook.md diff --git a/crates/ruff_db/src/parsed.rs b/crates/ruff_db/src/parsed.rs index 84b2bd01aaa66..5baec3567094c 100644 --- a/crates/ruff_db/src/parsed.rs +++ b/crates/ruff_db/src/parsed.rs @@ -8,7 +8,8 @@ use ruff_python_ast::{ StringLiteral, }; use ruff_python_parser::{ - ParseError, ParseErrorType, ParseOptions, Parsed, parse_string_annotation, parse_unchecked, + ParseError, ParseErrorType, ParseOptions, Parsed, parse_cells_unchecked, + parse_string_annotation, parse_unchecked, }; use crate::Db; @@ -49,9 +50,17 @@ pub fn parsed_module_impl(db: &dyn Db, file: File) -> Parsed { let target_version = db.python_version(); let options = ParseOptions::from(ty).with_target_version(target_version); - parse_unchecked(&source, options) - .try_into_module() - .expect("PySourceType always parses into a module") + + // Notebooks parse each cell as an independent module so a syntax error confined to one cell is + // surfaced instead of being masked by a later cell's content. Regular files take the existing + // single-parse path. + if let Some(notebook) = source.as_notebook() { + parse_cells_unchecked(&source, notebook.cell_offsets().ranges(), options) + } else { + parse_unchecked(&source, options) + .try_into_module() + .expect("PySourceType always parses into a module") + } } pub fn parsed_string_annotation( diff --git a/crates/ruff_python_parser/src/lib.rs b/crates/ruff_python_parser/src/lib.rs index 79e9b6a8204ba..6711293c05f31 100644 --- a/crates/ruff_python_parser/src/lib.rs +++ b/crates/ruff_python_parser/src/lib.rs @@ -76,7 +76,7 @@ use ruff_python_ast::{ AtomicNodeIndex, Expr, Mod, ModExpression, ModModule, PySourceType, StringFlags, StringLiteral, Suite, }; -use ruff_text_size::{Ranged, TextRange}; +use ruff_text_size::{Ranged, TextRange, TextSize}; mod error; pub mod lexer; @@ -305,6 +305,10 @@ pub fn parse_unchecked_source(source: &str, source_type: PySourceType) -> Parsed /// This validates sources such as Jupyter notebooks, where each cell must be syntactically valid on /// its own while later cells can still reference earlier definitions. /// The `ranges` must be ordered and non-overlapping. An empty `ranges` falls back to [`parse_unchecked`]. +/// +/// A syntax error anchored at a range's trailing offset is pulled one byte back into that range so +/// it is attributed there rather than to the following range. This assumes each range ends on a +/// single-byte separator, as notebook cells do with a synthetic newline. pub fn parse_cells_unchecked( source: &str, ranges: impl IntoIterator, @@ -343,7 +347,24 @@ pub fn parse_cells_unchecked( body.extend(syntax.body); tokens.extend(cell_tokens.iter().copied()); - errors.extend(cell_errors); + + // A cell is parsed from a slice ending at `range.end()`, which is also the next cell's + // start offset. An error anchored at that trailing EOF (e.g. an unclosed bracket, or a + // decorator with no following `def`) would otherwise be attributed to the next cell. + // Anchor it at the cell's last offset so it stays in the cell that contains the error. + let boundary = range.end(); + errors.extend(cell_errors.into_iter().map(|error| { + if range.is_empty() || error.location.start() != boundary { + error + } else { + ParseError { + location: TextRange::new(boundary - TextSize::from(1), boundary), + ..error + } + } + })); + // Unlike `errors`, `unsupported_syntax_errors` anchor at real syntax constructs rather than + // the trailing EOF, so they never sit on the boundary and need no remapping. unsupported_syntax_errors.extend(cell_unsupported_syntax_errors); module_range = Some(match module_range { Some(previous) => TextRange::new(previous.start(), range.end()), diff --git a/crates/ruff_python_parser/src/parser/tests.rs b/crates/ruff_python_parser/src/parser/tests.rs index c78e3362b1a73..797bf995bcaf0 100644 --- a/crates/ruff_python_parser/src/parser/tests.rs +++ b/crates/ruff_python_parser/src/parser/tests.rs @@ -553,11 +553,16 @@ fn recursion_limit_nested_lambda_chain() { } /// Parses `cells` as consecutive IPython cells, mirroring how a notebook's concatenated source is -/// split into contiguous ranges by `CellOffsets`. +/// split into contiguous ranges by `CellOffsets`. Each cell carries its trailing separator newline, +/// matching the synthetic newline `CellOffsets` appends after every cell's content. fn parse_ipy_cells(cells: &[&str]) -> Parsed { let mut source = String::new(); let mut ranges = Vec::new(); for cell in cells { + assert!( + cell.ends_with('\n'), + "each cell must end with the separator newline that `CellOffsets` appends: {cell:?}" + ); let start = TextSize::of(&source); source.push_str(cell); ranges.push(TextRange::new(start, TextSize::of(&source))); @@ -568,8 +573,8 @@ fn parse_ipy_cells(cells: &[&str]) -> Parsed { #[test] fn notebook_cell_boundary_syntax_errors() { // Each pair concatenates into valid source, but the first cell is invalid on its own. - // Concatenated parse masks the error; per-cell parsing must surface it. - // Decorator case is grammar-level: a lexer-only boundary fix cannot catch it. + // Concatenated parse masks the error; per-cell parsing must surface it and report it in the + // first cell (an EOF-anchored error must not leak across the boundary into the next cell). let cases = [ ["if True:\n", " pass\n"], ["x = [\n", "1]\n"], @@ -585,9 +590,19 @@ fn notebook_cell_boundary_syntax_errors() { parse_module(&concatenated).is_ok(), "concatenated source should parse cleanly: {concatenated:?}" ); + + let parsed = parse_ipy_cells(&[first, second]); + let first_cell = TextRange::up_to(TextSize::of(first)); + let Some(error) = parsed.errors().first() else { + panic!("per-cell parsing should report a syntax error: {concatenated:?}"); + }; + // Cell attribution keys off the start offset's row, and the boundary offset is the next + // cell's first row, so the error must start strictly before the boundary. A zero-width + // error left at the boundary (the pre-fix behavior) would be reported in the next cell. assert!( - parse_ipy_cells(&[first, second]).has_invalid_syntax(), - "per-cell parsing should report a syntax error: {concatenated:?}" + error.location.start() < first_cell.end(), + "error {:?} should start inside the first cell {first_cell:?}: {concatenated:?}", + error.location, ); } } diff --git a/crates/ty_python_semantic/resources/mdtest/notebook.md b/crates/ty_python_semantic/resources/mdtest/notebook.md new file mode 100644 index 0000000000000..09fd7dcf3b75d --- /dev/null +++ b/crates/ty_python_semantic/resources/mdtest/notebook.md @@ -0,0 +1,46 @@ +# Notebook cells + +Ruff and ty parse each notebook cell as its own module, so a syntax error confined to one cell is +surfaced instead of being masked by a later cell's content. The error is reported in the cell that +contains it, even when it is anchored at the cell's trailing end, the same offset at which the next +cell begins. + +## Decorator at the end of a cell + +A decorator must be immediately followed by a function or class definition. A cell ending in a +decorator is therefore invalid on its own, and the error is anchored at the end of the cell, the +same offset at which the next cell begins. It must still be reported in the decorator's cell rather +than leaking into the next one. + +```ipynb +{ + "cells": [ + { + "cell_type": "code", + "execution_count": null, + "metadata": {}, + "outputs": [], + "source": ["# snapshot\n", "@staticmethod"] + }, + { + "cell_type": "code", + "execution_count": null, + "metadata": {}, + "outputs": [], + "source": ["def f(): ..."] + } + ], + "metadata": {}, + "nbformat": 4, + "nbformat_minor": 4 +} +``` + +```snapshot +error[invalid-syntax]: Expected class, function definition or async function definition after decorator + --> src/mdtest_snippet.ipynb:cell 1:2:14 + | +2 | @staticmethod + | ^ + | +``` diff --git a/crates/ty_server/tests/e2e/snapshots/e2e__notebook__diagnostic_end_of_file.snap b/crates/ty_server/tests/e2e/snapshots/e2e__notebook__diagnostic_end_of_file.snap index eb204f299cb6a..e09b5f6c19fc6 100644 --- a/crates/ty_server/tests/e2e/snapshots/e2e__notebook__diagnostic_end_of_file.snap +++ b/crates/ty_server/tests/e2e/snapshots/e2e__notebook__diagnostic_end_of_file.snap @@ -185,8 +185,8 @@ expression: diagnostics { "range": { "start": { - "line": 3, - "character": 0 + "line": 2, + "character": 7 }, "end": { "line": 3, From 309081b8901dc324ebfc0b71bbffa938cb7faa37 Mon Sep 17 00:00:00 2001 From: Omar Yusuf Abdi Date: Mon, 6 Jul 2026 18:40:29 +0200 Subject: [PATCH 5/8] [ruff] Attribute cell-boundary errors via content ranges 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`. --- crates/ruff_db/src/parsed.rs | 2 +- crates/ruff_linter/src/linter.rs | 2 +- crates/ruff_python_parser/src/lib.rs | 48 +++++++++---------- crates/ruff_python_parser/src/parser/tests.rs | 10 ++-- ...e2e__notebook__diagnostic_end_of_file.snap | 4 +- 5 files changed, 34 insertions(+), 32 deletions(-) diff --git a/crates/ruff_db/src/parsed.rs b/crates/ruff_db/src/parsed.rs index 5baec3567094c..af918f69f73ed 100644 --- a/crates/ruff_db/src/parsed.rs +++ b/crates/ruff_db/src/parsed.rs @@ -55,7 +55,7 @@ pub fn parsed_module_impl(db: &dyn Db, file: File) -> Parsed { // surfaced instead of being masked by a later cell's content. Regular files take the existing // single-parse path. if let Some(notebook) = source.as_notebook() { - parse_cells_unchecked(&source, notebook.cell_offsets().ranges(), options) + parse_cells_unchecked(&source, notebook.cell_offsets().content_ranges(), options) } else { parse_unchecked(&source, options) .try_into_module() diff --git a/crates/ruff_linter/src/linter.rs b/crates/ruff_linter/src/linter.rs index b8fd32beda3f4..ae3fca895a123 100644 --- a/crates/ruff_linter/src/linter.rs +++ b/crates/ruff_linter/src/linter.rs @@ -785,7 +785,7 @@ pub fn parse_unchecked_source( match source_kind.as_ipy_notebook() { Some(notebook) => ruff_python_parser::parse_cells_unchecked( source_kind.source_code(), - notebook.cell_offsets().ranges(), + notebook.cell_offsets().content_ranges(), options, ), None => ruff_python_parser::parse_unchecked(source_kind.source_code(), options) diff --git a/crates/ruff_python_parser/src/lib.rs b/crates/ruff_python_parser/src/lib.rs index 6711293c05f31..b85ee0e3d075c 100644 --- a/crates/ruff_python_parser/src/lib.rs +++ b/crates/ruff_python_parser/src/lib.rs @@ -71,12 +71,12 @@ pub use crate::parser::ParseOptions; use crate::parser::Parser; -use ruff_python_ast::token::Tokens; +use ruff_python_ast::token::{Token, TokenFlags, TokenKind, Tokens}; use ruff_python_ast::{ AtomicNodeIndex, Expr, Mod, ModExpression, ModModule, PySourceType, StringFlags, StringLiteral, Suite, }; -use ruff_text_size::{Ranged, TextRange, TextSize}; +use ruff_text_size::{Ranged, TextRange}; mod error; pub mod lexer; @@ -306,9 +306,10 @@ pub fn parse_unchecked_source(source: &str, source_type: PySourceType) -> Parsed /// its own while later cells can still reference earlier definitions. /// The `ranges` must be ordered and non-overlapping. An empty `ranges` falls back to [`parse_unchecked`]. /// -/// A syntax error anchored at a range's trailing offset is pulled one byte back into that range so -/// it is attributed there rather than to the following range. This assumes each range ends on a -/// single-byte separator, as notebook cells do with a synthetic newline. +/// Consecutive ranges must be separated by a single-byte `\n`: each range ends just before the +/// separator and the next range starts just after it, as Ruff's notebook cells are. A syntax error +/// anchored at a cell's trailing offset then lands on that separator, the cell's own last line, so +/// it is attributed to that cell rather than to the following one. pub fn parse_cells_unchecked( source: &str, ranges: impl IntoIterator, @@ -327,7 +328,7 @@ pub fn parse_cells_unchecked( let mut unsupported_syntax_errors = Vec::new(); let mut module_range: Option = None; - for range in ranges { + while let Some(range) = ranges.next() { if let Some(previous) = module_range { debug_assert!(previous.end() <= range.start()); } @@ -347,25 +348,24 @@ pub fn parse_cells_unchecked( body.extend(syntax.body); tokens.extend(cell_tokens.iter().copied()); - - // A cell is parsed from a slice ending at `range.end()`, which is also the next cell's - // start offset. An error anchored at that trailing EOF (e.g. an unclosed bracket, or a - // decorator with no following `def`) would otherwise be attributed to the next cell. - // Anchor it at the cell's last offset so it stays in the cell that contains the error. - let boundary = range.end(); - errors.extend(cell_errors.into_iter().map(|error| { - if range.is_empty() || error.location.start() != boundary { - error - } else { - ParseError { - location: TextRange::new(boundary - TextSize::from(1), boundary), - ..error - } - } - })); - // Unlike `errors`, `unsupported_syntax_errors` anchor at real syntax constructs rather than - // the trailing EOF, so they never sit on the boundary and need no remapping. + errors.extend(cell_errors); unsupported_syntax_errors.extend(cell_unsupported_syntax_errors); + + // Each range excludes its trailing `\n` separator (see the doc comment above), so the + // per-cell parse never lexes it, leaving a one-byte gap in the token stream between cells. + // Bridge it with the `NonLogicalNewline` the lexer would have emitted for the separator, so + // token-based checks such as the blank-line rules don't misread the boundary. The final + // cell's separator is the file-final newline and is deliberately left uncovered. + if let Some(next) = ranges.peek() { + let separator = TextRange::new(range.end(), next.start()); + debug_assert_eq!(separator.len().to_usize(), 1); + tokens.push(Token::new( + TokenKind::NonLogicalNewline, + separator, + TokenFlags::empty(), + )); + } + module_range = Some(match module_range { Some(previous) => TextRange::new(previous.start(), range.end()), None => range, diff --git a/crates/ruff_python_parser/src/parser/tests.rs b/crates/ruff_python_parser/src/parser/tests.rs index 797bf995bcaf0..4960928452e58 100644 --- a/crates/ruff_python_parser/src/parser/tests.rs +++ b/crates/ruff_python_parser/src/parser/tests.rs @@ -552,9 +552,9 @@ fn recursion_limit_nested_lambda_chain() { ); } -/// Parses `cells` as consecutive IPython cells, mirroring how a notebook's concatenated source is -/// split into contiguous ranges by `CellOffsets`. Each cell carries its trailing separator newline, -/// matching the synthetic newline `CellOffsets` appends after every cell's content. +/// Parses `cells` as consecutive IPython cells, mirroring how `CellOffsets::content_ranges` splits a +/// notebook's concatenated source: each range covers a cell's content up to but not including the +/// synthetic `\n` separator, and the next range starts just after it. fn parse_ipy_cells(cells: &[&str]) -> Parsed { let mut source = String::new(); let mut ranges = Vec::new(); @@ -565,7 +565,9 @@ fn parse_ipy_cells(cells: &[&str]) -> Parsed { ); let start = TextSize::of(&source); source.push_str(cell); - ranges.push(TextRange::new(start, TextSize::of(&source))); + // Exclude the trailing separator newline, matching `CellOffsets::content_ranges`. + let end = TextSize::of(&source) - TextSize::from(1); + ranges.push(TextRange::new(start, end)); } parse_cells_unchecked(&source, ranges, ParseOptions::from(Mode::Ipython)) } diff --git a/crates/ty_server/tests/e2e/snapshots/e2e__notebook__diagnostic_end_of_file.snap b/crates/ty_server/tests/e2e/snapshots/e2e__notebook__diagnostic_end_of_file.snap index e09b5f6c19fc6..e959dc3c59aad 100644 --- a/crates/ty_server/tests/e2e/snapshots/e2e__notebook__diagnostic_end_of_file.snap +++ b/crates/ty_server/tests/e2e/snapshots/e2e__notebook__diagnostic_end_of_file.snap @@ -189,8 +189,8 @@ expression: diagnostics "character": 7 }, "end": { - "line": 3, - "character": 0 + "line": 2, + "character": 7 } }, "severity": 1, From ac3993b3c4306d87c64fe61a4df913dd79780415 Mon Sep 17 00:00:00 2001 From: Micha Reiser Date: Tue, 7 Jul 2026 07:16:19 +0000 Subject: [PATCH 6/8] [ruff] Simplify notebook cell parsing changes --- crates/ruff_linter/src/linter.rs | 33 ++++------------ crates/ruff_linter/src/suppression.rs | 11 +----- .../jupyter/cell_boundary_suppression.ipynb | 39 ------------------- .../fixtures/jupyter/valid_multicell.ipynb | 18 +++++++++ crates/ruff_python_parser/src/lib.rs | 5 +++ 5 files changed, 33 insertions(+), 73 deletions(-) delete mode 100644 crates/ruff_notebook/resources/test/fixtures/jupyter/cell_boundary_suppression.ipynb diff --git a/crates/ruff_linter/src/linter.rs b/crates/ruff_linter/src/linter.rs index ae3fca895a123..77d4a30ec81b5 100644 --- a/crates/ruff_linter/src/linter.rs +++ b/crates/ruff_linter/src/linter.rs @@ -822,7 +822,7 @@ mod tests { } #[test] - fn test_cell_boundary_syntax_error() -> Result<(), NotebookError> { + fn notebook_cell_boundary_syntax_error() -> Result<(), NotebookError> { // A decorator whose definition lives in the next cell parses cleanly once the cells are // concatenated, but is invalid on its own. Per-cell parsing must report it. let path = notebook_path("cell_boundary_syntax_error.ipynb"); @@ -837,10 +837,12 @@ mod tests { } #[test] - fn test_notebook_cell_boundary_no_spurious_diagnostics() -> Result<(), NotebookError> { + fn notebook_cell_boundary_tokens_preserve_linting() -> Result<(), NotebookError> { // `valid_multicell.ipynb` ends a cell with an indented `def` block right before the next - // cell, so per-cell parsing emits a trailing `Dedent` at that boundary. Ensure those - // synthetic boundary tokens don't trigger spurious blank-line or whitespace diagnostics. + // cell, so per-cell parsing emits a trailing `Dedent` at that boundary. It also places a + // range-suppression comment and its suppressed import in separate cells. Ensure the + // boundary tokens preserve suppression without introducing blank-line or whitespace + // diagnostics. let path = notebook_path("valid_multicell.ipynb"); let source_kind = SourceKind::ipy_notebook(Notebook::from_path(&path)?); let settings = LinterSettings::for_rules([ @@ -852,31 +854,12 @@ mod tests { Rule::TrailingWhitespace, Rule::BlankLineWithWhitespace, Rule::TooManyNewlinesAtEndOfFile, + Rule::UnusedImport, ]); let diagnostics = test_contents_syntax_errors(&source_kind, &path, &settings); assert!( diagnostics.is_empty(), - "per-cell parsing introduced spurious diagnostics: {diagnostics:?}" - ); - Ok(()) - } - - #[test] - fn test_notebook_cell_boundary_suppression() -> Result<(), NotebookError> { - // A cell that closes an indented block emits a trailing `Dedent` at the cell boundary, - // which shares its offset with a `# ruff: disable` comment opening the next cell. Range - // suppression handling used to get stuck on that dedent and loop forever; it must finish - // and apply the suppression so the following `import os` isn't reported as F401. - let path = notebook_path("cell_boundary_suppression.ipynb"); - let source_kind = SourceKind::ipy_notebook(Notebook::from_path(&path)?); - let diagnostics = test_contents_syntax_errors( - &source_kind, - &path, - &LinterSettings::for_rule(Rule::UnusedImport), - ); - assert!( - diagnostics.is_empty(), - "expected the cell-boundary `# ruff: disable` to suppress F401, got: {diagnostics:?}" + "cell-boundary tokens changed lint results: {diagnostics:?}" ); Ok(()) } diff --git a/crates/ruff_linter/src/suppression.rs b/crates/ruff_linter/src/suppression.rs index 653559a50c650..4ebf99d380f87 100644 --- a/crates/ruff_linter/src/suppression.rs +++ b/crates/ruff_linter/src/suppression.rs @@ -662,15 +662,8 @@ impl<'a> SuppressionsBuilder<'a> { continue; } - // Matched suppression comments. A notebook cell closing an indented block emits a - // zero-width `Dedent` at the cell boundary, sharing its offset with a comment opening - // the next cell. Split on `end <= offset` rather than `Tokens::split_at` so that - // `Dedent` stays in `before`; otherwise `after` starts with it and the loop below spins - // on the dedent's `continue 'comments` without ever consuming the comment. - let tokens_slice: &[Token] = tokens; - let comment_start = suppression.token_range.start(); - let split = tokens_slice.partition_point(|token| token.end() <= comment_start); - let (before, after) = tokens_slice.split_at(split); + // Matched suppression comments + let (before, after) = tokens.split_at(suppression.token_range.start()); let mut count = 0; let last_indent = before diff --git a/crates/ruff_notebook/resources/test/fixtures/jupyter/cell_boundary_suppression.ipynb b/crates/ruff_notebook/resources/test/fixtures/jupyter/cell_boundary_suppression.ipynb deleted file mode 100644 index ad6c1366e6ab5..0000000000000 --- a/crates/ruff_notebook/resources/test/fixtures/jupyter/cell_boundary_suppression.ipynb +++ /dev/null @@ -1,39 +0,0 @@ -{ - "cells": [ - { - "cell_type": "code", - "execution_count": null, - "metadata": {}, - "outputs": [], - "source": [ - "def f():\n", - " pass" - ] - }, - { - "cell_type": "code", - "execution_count": null, - "metadata": {}, - "outputs": [], - "source": [ - "# ruff: disable[F401]" - ] - }, - { - "cell_type": "code", - "execution_count": null, - "metadata": {}, - "outputs": [], - "source": [ - "import os" - ] - } - ], - "metadata": { - "language_info": { - "name": "python" - } - }, - "nbformat": 4, - "nbformat_minor": 5 -} diff --git a/crates/ruff_notebook/resources/test/fixtures/jupyter/valid_multicell.ipynb b/crates/ruff_notebook/resources/test/fixtures/jupyter/valid_multicell.ipynb index f8bff12342663..41a85a4eca21a 100644 --- a/crates/ruff_notebook/resources/test/fixtures/jupyter/valid_multicell.ipynb +++ b/crates/ruff_notebook/resources/test/fixtures/jupyter/valid_multicell.ipynb @@ -28,6 +28,24 @@ " return [os.getcwd(), sys.platform]" ] }, + { + "cell_type": "code", + "execution_count": null, + "metadata": {}, + "outputs": [], + "source": [ + "# ruff: disable[F401]" + ] + }, + { + "cell_type": "code", + "execution_count": null, + "metadata": {}, + "outputs": [], + "source": [ + "import json" + ] + }, { "cell_type": "code", "execution_count": null, diff --git a/crates/ruff_python_parser/src/lib.rs b/crates/ruff_python_parser/src/lib.rs index b85ee0e3d075c..846caef5841b9 100644 --- a/crates/ruff_python_parser/src/lib.rs +++ b/crates/ruff_python_parser/src/lib.rs @@ -372,6 +372,11 @@ pub fn parse_cells_unchecked( }); } + body.shrink_to_fit(); + tokens.shrink_to_fit(); + errors.shrink_to_fit(); + unsupported_syntax_errors.shrink_to_fit(); + Parsed { syntax: ModModule { node_index: AtomicNodeIndex::NONE, From 1893a888e7b96a7758d1a637e096035a34838518 Mon Sep 17 00:00:00 2001 From: Micha Reiser Date: Tue, 7 Jul 2026 08:12:44 +0000 Subject: [PATCH 7/8] [ruff] Move notebook parser coverage to mdtests --- .../mdtest/notebook/cell-boundaries.md | 122 ++++++++++++++++++ crates/ruff_linter/src/linter.rs | 51 +------- crates/ruff_mdtest/src/lib.rs | 15 ++- .../jupyter/cell_boundary_syntax_error.ipynb | 32 ----- .../fixtures/jupyter/valid_multicell.ipynb | 77 ----------- crates/ruff_python_ast/src/token/tokens.rs | 9 ++ crates/ruff_python_parser/src/lib.rs | 27 ++-- crates/ruff_python_parser/src/parser/tests.rs | 88 +------------ crates/ruff_server/src/lint.rs | 3 +- .../resources/mdtest/notebook.md | 8 +- 10 files changed, 160 insertions(+), 272 deletions(-) create mode 100644 crates/ruff_linter/resources/mdtest/notebook/cell-boundaries.md delete mode 100644 crates/ruff_notebook/resources/test/fixtures/jupyter/cell_boundary_syntax_error.ipynb delete mode 100644 crates/ruff_notebook/resources/test/fixtures/jupyter/valid_multicell.ipynb diff --git a/crates/ruff_linter/resources/mdtest/notebook/cell-boundaries.md b/crates/ruff_linter/resources/mdtest/notebook/cell-boundaries.md new file mode 100644 index 0000000000000..c504a4b86fa97 --- /dev/null +++ b/crates/ruff_linter/resources/mdtest/notebook/cell-boundaries.md @@ -0,0 +1,122 @@ +# Notebook cell parsing + +Ruff parses every notebook cell as its own module while retaining a single combined module for +linting. + +## Syntax errors stop at cell boundaries + +A decorator at the end of a cell would apply to a definition in the next cell if the sources were +concatenated. Parsing the cells independently must instead report the syntax error in the decorator's +cell. + +`syntax-error.ipynb`: + +```ipynb +{ + "cells": [ + { + "cell_type": "code", + "execution_count": null, + "metadata": {}, + "outputs": [], + "source": ["# snapshot\n", "@deco"] + }, + { + "cell_type": "code", + "execution_count": null, + "metadata": {}, + "outputs": [], + "source": ["def f(): pass"] + } + ], + "metadata": {}, + "nbformat": 4, + "nbformat_minor": 4 +} +``` + +```snapshot +error[invalid-syntax]: Expected class, function definition or async function definition after decorator + --> src/syntax-error.ipynb:cell 1:2:6 + | +2 | @deco + | ^ + | +``` + +## Notebooks without Python cells + +A notebook containing no Python code cells still parses successfully. + +`no-code-cells.ipynb`: + +```ipynb +{ + "cells": [ + { + "cell_type": "markdown", + "metadata": {}, + "source": ["# Nothing to check"] + } + ], + "metadata": {}, + "nbformat": 4, + "nbformat_minor": 4 +} +``` + +## Cell boundary tokens + +Per-cell parsing inserts tokens at each boundary before merging the cells into one module. This +notebook exercises a trailing `Dedent`, a definition referenced across cells, and a range suppression +whose directive and import are in separate cells. None of the selected rules should report a +diagnostic. + +```toml +[lint] +select = [ + "E301", + "E302", + "E303", + "E305", + "E306", + "F401", + "F821", + "W291", + "W293", + "W391", +] +``` + +`boundary-tokens.ipynb`: + +```ipynb +{ + "cells": [ + { + "cell_type": "code", + "execution_count": null, + "metadata": {}, + "outputs": [], + "source": ["def compute():\n", " return 1"] + }, + { + "cell_type": "code", + "execution_count": null, + "metadata": {}, + "outputs": [], + "source": ["# ruff: disable[F401]"] + }, + { + "cell_type": "code", + "execution_count": null, + "metadata": {}, + "outputs": [], + "source": ["import json\n", "print(compute())"] + } + ], + "metadata": {}, + "nbformat": 4, + "nbformat_minor": 4 +} +``` diff --git a/crates/ruff_linter/src/linter.rs b/crates/ruff_linter/src/linter.rs index 77d4a30ec81b5..df79eab6996a7 100644 --- a/crates/ruff_linter/src/linter.rs +++ b/crates/ruff_linter/src/linter.rs @@ -767,12 +767,10 @@ impl ParseSource { } } -/// Like [`ruff_python_parser::parse_unchecked_source`] but with an additional [`PythonVersion`] -/// argument. +/// Like [`ruff_python_parser::parse_unchecked_source`], but with an explicit [`PythonVersion`] and +/// per-cell notebook parsing. /// -/// Jupyter notebooks are parsed cell by cell so that a syntax error confined to one cell isn't -/// masked by the source of a following cell -/// Definitions still resolve across cells because the per-cell modules are concatenated into a single module +/// Per-cell modules are merged so definitions remain visible across cells. pub fn parse_unchecked_source( source_kind: &SourceKind, source_type: PySourceType, @@ -821,49 +819,6 @@ mod tests { Path::new("../ruff_notebook/resources/test/fixtures/jupyter").join(path) } - #[test] - fn notebook_cell_boundary_syntax_error() -> Result<(), NotebookError> { - // A decorator whose definition lives in the next cell parses cleanly once the cells are - // concatenated, but is invalid on its own. Per-cell parsing must report it. - let path = notebook_path("cell_boundary_syntax_error.ipynb"); - let source_kind = SourceKind::ipy_notebook(Notebook::from_path(&path)?); - let diagnostics = - test_contents_syntax_errors(&source_kind, &path, &LinterSettings::default()); - assert!( - diagnostics.iter().any(Diagnostic::is_invalid_syntax), - "expected a cell-boundary syntax error, got: {diagnostics:?}" - ); - Ok(()) - } - - #[test] - fn notebook_cell_boundary_tokens_preserve_linting() -> Result<(), NotebookError> { - // `valid_multicell.ipynb` ends a cell with an indented `def` block right before the next - // cell, so per-cell parsing emits a trailing `Dedent` at that boundary. It also places a - // range-suppression comment and its suppressed import in separate cells. Ensure the - // boundary tokens preserve suppression without introducing blank-line or whitespace - // diagnostics. - let path = notebook_path("valid_multicell.ipynb"); - let source_kind = SourceKind::ipy_notebook(Notebook::from_path(&path)?); - let settings = LinterSettings::for_rules([ - Rule::BlankLineBetweenMethods, - Rule::BlankLinesTopLevel, - Rule::TooManyBlankLines, - Rule::BlankLinesAfterFunctionOrClass, - Rule::BlankLinesBeforeNestedDefinition, - Rule::TrailingWhitespace, - Rule::BlankLineWithWhitespace, - Rule::TooManyNewlinesAtEndOfFile, - Rule::UnusedImport, - ]); - let diagnostics = test_contents_syntax_errors(&source_kind, &path, &settings); - assert!( - diagnostics.is_empty(), - "cell-boundary tokens changed lint results: {diagnostics:?}" - ); - Ok(()) - } - #[test] fn test_import_sorting() -> Result<(), NotebookError> { let actual = notebook_path("isort.ipynb"); diff --git a/crates/ruff_mdtest/src/lib.rs b/crates/ruff_mdtest/src/lib.rs index 32510be58040f..6f874962b3f5e 100644 --- a/crates/ruff_mdtest/src/lib.rs +++ b/crates/ruff_mdtest/src/lib.rs @@ -68,8 +68,8 @@ fn run_test( } assert!( - matches!(embedded.lang, "py" | "pyi" | "python"), - "Supported file types are: py (or python), pyi, and ignore" + matches!(embedded.lang, "py" | "pyi" | "python" | "ipynb"), + "Supported file types are: py (or python), pyi, ipynb, and ignore" ); let full_path = embedded.full_path(&project_root); @@ -106,9 +106,14 @@ fn run_test( .filter_map(|test_file| { let mdtest_result = attempt_test( |file| { - let source_kind = SourceKind::Python { - code: source_text(db, file).as_str().to_string(), - is_stub: file.is_stub(db), + let source = source_text(db, file); + let source_kind = if let Some(notebook) = source.as_notebook() { + SourceKind::ipy_notebook(notebook.clone()) + } else { + SourceKind::Python { + code: source.as_str().to_string(), + is_stub: file.is_stub(db), + } }; let path = file .path(db) diff --git a/crates/ruff_notebook/resources/test/fixtures/jupyter/cell_boundary_syntax_error.ipynb b/crates/ruff_notebook/resources/test/fixtures/jupyter/cell_boundary_syntax_error.ipynb deleted file mode 100644 index 4951ec97403e1..0000000000000 --- a/crates/ruff_notebook/resources/test/fixtures/jupyter/cell_boundary_syntax_error.ipynb +++ /dev/null @@ -1,32 +0,0 @@ -{ - "cells": [ - { - "cell_type": "code", - "execution_count": null, - "id": "cell0", - "metadata": {}, - "outputs": [], - "source": [ - "@deco" - ] - }, - { - "cell_type": "code", - "execution_count": null, - "id": "cell1", - "metadata": {}, - "outputs": [], - "source": [ - "def f():\n", - " return 1" - ] - } - ], - "metadata": { - "language_info": { - "name": "python" - } - }, - "nbformat": 4, - "nbformat_minor": 5 -} diff --git a/crates/ruff_notebook/resources/test/fixtures/jupyter/valid_multicell.ipynb b/crates/ruff_notebook/resources/test/fixtures/jupyter/valid_multicell.ipynb deleted file mode 100644 index 41a85a4eca21a..0000000000000 --- a/crates/ruff_notebook/resources/test/fixtures/jupyter/valid_multicell.ipynb +++ /dev/null @@ -1,77 +0,0 @@ -{ - "cells": [ - { - "cell_type": "code", - "execution_count": null, - "metadata": {}, - "outputs": [], - "source": [ - "import os\n", - "import sys" - ] - }, - { - "cell_type": "markdown", - "metadata": {}, - "source": [ - "## A heading between code cells" - ] - }, - { - "cell_type": "code", - "execution_count": null, - "id": "d5e478da", - "metadata": {}, - "outputs": [], - "source": [ - "def compute():\n", - " return [os.getcwd(), sys.platform]" - ] - }, - { - "cell_type": "code", - "execution_count": null, - "metadata": {}, - "outputs": [], - "source": [ - "# ruff: disable[F401]" - ] - }, - { - "cell_type": "code", - "execution_count": null, - "metadata": {}, - "outputs": [], - "source": [ - "import json" - ] - }, - { - "cell_type": "code", - "execution_count": null, - "id": "9cc80d85", - "metadata": {}, - "outputs": [], - "source": [ - "values = compute()" - ] - }, - { - "cell_type": "code", - "execution_count": null, - "metadata": {}, - "outputs": [], - "source": [ - "for value in values:\n", - " print(value)" - ] - } - ], - "metadata": { - "language_info": { - "name": "python" - } - }, - "nbformat": 4, - "nbformat_minor": 5 -} diff --git a/crates/ruff_python_ast/src/token/tokens.rs b/crates/ruff_python_ast/src/token/tokens.rs index c71793d624858..2b715cd463904 100644 --- a/crates/ruff_python_ast/src/token/tokens.rs +++ b/crates/ruff_python_ast/src/token/tokens.rs @@ -225,6 +225,15 @@ impl Tokens { } } +impl IntoIterator for Tokens { + type Item = Token; + type IntoIter = std::vec::IntoIter; + + fn into_iter(self) -> Self::IntoIter { + self.raw.into_iter() + } +} + impl<'a> IntoIterator for &'a Tokens { type Item = &'a Token; type IntoIter = std::slice::Iter<'a, Token>; diff --git a/crates/ruff_python_parser/src/lib.rs b/crates/ruff_python_parser/src/lib.rs index 846caef5841b9..2e92844aea402 100644 --- a/crates/ruff_python_parser/src/lib.rs +++ b/crates/ruff_python_parser/src/lib.rs @@ -304,7 +304,7 @@ pub fn parse_unchecked_source(source: &str, source_type: PySourceType) -> Parsed /// /// This validates sources such as Jupyter notebooks, where each cell must be syntactically valid on /// its own while later cells can still reference earlier definitions. -/// The `ranges` must be ordered and non-overlapping. An empty `ranges` falls back to [`parse_unchecked`]. +/// The `ranges` must be ordered and non-overlapping. /// /// Consecutive ranges must be separated by a single-byte `\n`: each range ends just before the /// separator and the next range starts just after it, as Ruff's notebook cells are. A syntax error @@ -316,12 +316,6 @@ pub fn parse_cells_unchecked( options: ParseOptions, ) -> Parsed { let mut ranges = ranges.into_iter().peekable(); - if ranges.peek().is_none() { - return parse_unchecked(source, options) - .try_into_module() - .expect("module options should parse into a module"); - } - let mut body = Suite::new(); let mut tokens = Vec::new(); let mut errors = Vec::new(); @@ -330,12 +324,12 @@ pub fn parse_cells_unchecked( while let Some(range) = ranges.next() { if let Some(previous) = module_range { - debug_assert!(previous.end() <= range.start()); + assert!(previous.end() <= range.start()); } // The cell is lexed from `range.start()`, so the slice must keep the leading text to // preserve absolute offsets into the concatenated source. - let cell_source = &source[..range.end().to_usize()]; + let cell_source = &source[TextRange::up_to(range.end())]; let Parsed { syntax, tokens: cell_tokens, @@ -347,18 +341,17 @@ pub fn parse_cells_unchecked( .expect("module options should parse into a module"); body.extend(syntax.body); - tokens.extend(cell_tokens.iter().copied()); + tokens.extend(cell_tokens); errors.extend(cell_errors); unsupported_syntax_errors.extend(cell_unsupported_syntax_errors); - // Each range excludes its trailing `\n` separator (see the doc comment above), so the - // per-cell parse never lexes it, leaving a one-byte gap in the token stream between cells. - // Bridge it with the `NonLogicalNewline` the lexer would have emitted for the separator, so - // token-based checks such as the blank-line rules don't misread the boundary. The final - // cell's separator is the file-final newline and is deliberately left uncovered. + // Each range excludes its trailing `\n` separator (see the doc comment above), leaving a + // one-byte gap in the token stream. Cover it with a `NonLogicalNewline` so token-based + // checks don't treat the separator as another logical line terminator. The final cell's + // separator is the file-final newline and is deliberately left uncovered. if let Some(next) = ranges.peek() { let separator = TextRange::new(range.end(), next.start()); - debug_assert_eq!(separator.len().to_usize(), 1); + assert_eq!(&source[separator], "\n"); tokens.push(Token::new( TokenKind::NonLogicalNewline, separator, @@ -380,7 +373,7 @@ pub fn parse_cells_unchecked( Parsed { syntax: ModModule { node_index: AtomicNodeIndex::NONE, - range: module_range.expect("at least one range must be present"), + range: module_range.unwrap_or_default(), body, }, tokens: Tokens::new(tokens), diff --git a/crates/ruff_python_parser/src/parser/tests.rs b/crates/ruff_python_parser/src/parser/tests.rs index 4960928452e58..0cd15515e0cf4 100644 --- a/crates/ruff_python_parser/src/parser/tests.rs +++ b/crates/ruff_python_parser/src/parser/tests.rs @@ -1,10 +1,6 @@ -use ruff_python_ast::{Expr, InterpolatedStringElement, IpyEscapeKind, ModModule, Number, Stmt}; -use ruff_text_size::{Ranged, TextRange, TextSize}; +use ruff_python_ast::{Expr, InterpolatedStringElement, IpyEscapeKind, Number, Stmt}; -use crate::{ - Mode, ParseErrorType, ParseOptions, Parsed, parse, parse_cells_unchecked, parse_expression, - parse_module, -}; +use crate::{Mode, ParseErrorType, ParseOptions, parse, parse_expression, parse_module}; #[test] fn test_modes() { @@ -551,83 +547,3 @@ fn recursion_limit_nested_lambda_chain() { err.error ); } - -/// Parses `cells` as consecutive IPython cells, mirroring how `CellOffsets::content_ranges` splits a -/// notebook's concatenated source: each range covers a cell's content up to but not including the -/// synthetic `\n` separator, and the next range starts just after it. -fn parse_ipy_cells(cells: &[&str]) -> Parsed { - let mut source = String::new(); - let mut ranges = Vec::new(); - for cell in cells { - assert!( - cell.ends_with('\n'), - "each cell must end with the separator newline that `CellOffsets` appends: {cell:?}" - ); - let start = TextSize::of(&source); - source.push_str(cell); - // Exclude the trailing separator newline, matching `CellOffsets::content_ranges`. - let end = TextSize::of(&source) - TextSize::from(1); - ranges.push(TextRange::new(start, end)); - } - parse_cells_unchecked(&source, ranges, ParseOptions::from(Mode::Ipython)) -} - -#[test] -fn notebook_cell_boundary_syntax_errors() { - // Each pair concatenates into valid source, but the first cell is invalid on its own. - // Concatenated parse masks the error; per-cell parsing must surface it and report it in the - // first cell (an EOF-anchored error must not leak across the boundary into the next cell). - let cases = [ - ["if True:\n", " pass\n"], - ["x = [\n", "1]\n"], - ["s = \"\"\"a\n", "b\"\"\"\n"], - ["y = f\"{x\n", "}\"\n"], - ["z = 1 + \\\n", "2\n"], - ["@deco\n", "def f(): pass\n"], - ]; - - for [first, second] in cases { - let concatenated = format!("{first}{second}"); - assert!( - parse_module(&concatenated).is_ok(), - "concatenated source should parse cleanly: {concatenated:?}" - ); - - let parsed = parse_ipy_cells(&[first, second]); - let first_cell = TextRange::up_to(TextSize::of(first)); - let Some(error) = parsed.errors().first() else { - panic!("per-cell parsing should report a syntax error: {concatenated:?}"); - }; - // Cell attribution keys off the start offset's row, and the boundary offset is the next - // cell's first row, so the error must start strictly before the boundary. A zero-width - // error left at the boundary (the pre-fix behavior) would be reported in the next cell. - assert!( - error.location.start() < first_cell.end(), - "error {:?} should start inside the first cell {first_cell:?}: {concatenated:?}", - error.location, - ); - } -} - -#[test] -fn notebook_cells_resolve_across_boundaries() { - // A definition in an earlier cell stays visible to a later cell: the combined module keeps both - // statements in order, and the second cell's node carries its concatenated-source offset. - let cells = ["import os\n", "os.getcwd()\n"]; - let parsed = parse_ipy_cells(&cells); - - assert!(parsed.has_valid_syntax()); - - let body = &parsed.syntax().body; - assert_eq!(body.len(), 2); - assert!(matches!(body[0], Stmt::Import(_))); - assert_eq!(body[1].start(), TextSize::of(cells[0])); -} - -#[test] -fn notebook_empty_ranges_falls_back_to_whole_source() { - // No cell ranges e.g a notebook with only magic/markdown cells, parses the whole source once. - let parsed = parse_cells_unchecked("x = 1\n", [], ParseOptions::from(Mode::Ipython)); - assert!(parsed.has_valid_syntax()); - assert_eq!(parsed.syntax().body.len(), 1); -} diff --git a/crates/ruff_server/src/lint.rs b/crates/ruff_server/src/lint.rs index fb0943f1330fd..f2af9a44b9382 100644 --- a/crates/ruff_server/src/lint.rs +++ b/crates/ruff_server/src/lint.rs @@ -108,8 +108,7 @@ pub(crate) fn check( let target_version = settings.linter.resolve_target_version(&document_path); - // Parse once. Notebooks are parsed cell by cell so a syntax error confined to one cell isn't - // masked by a following cell; definitions still resolve across cells. + // Parse once. let parsed = parse_unchecked_source(&source_kind, source_type, target_version.parser_version()); // Map row and column locations to byte slices (lazily). diff --git a/crates/ty_python_semantic/resources/mdtest/notebook.md b/crates/ty_python_semantic/resources/mdtest/notebook.md index 09fd7dcf3b75d..7d9f840af0833 100644 --- a/crates/ty_python_semantic/resources/mdtest/notebook.md +++ b/crates/ty_python_semantic/resources/mdtest/notebook.md @@ -2,15 +2,13 @@ Ruff and ty parse each notebook cell as its own module, so a syntax error confined to one cell is surfaced instead of being masked by a later cell's content. The error is reported in the cell that -contains it, even when it is anchored at the cell's trailing end, the same offset at which the next -cell begins. +contains it, even when it is anchored at the cell's trailing end. ## Decorator at the end of a cell A decorator must be immediately followed by a function or class definition. A cell ending in a -decorator is therefore invalid on its own, and the error is anchored at the end of the cell, the -same offset at which the next cell begins. It must still be reported in the decorator's cell rather -than leaking into the next one. +decorator is therefore invalid on its own, and the error is anchored at the separator after the +cell. It must still be reported in the decorator's cell rather than leaking into the next one. ```ipynb { From 2d8bc2b14a3fb2a8d322de75615d54fed6e22deb Mon Sep 17 00:00:00 2001 From: Micha Reiser Date: Tue, 7 Jul 2026 09:02:21 +0000 Subject: [PATCH 8/8] Fix Clippy needless pass by value --- crates/ruff_db/src/parsed.rs | 2 +- crates/ruff_linter/src/linter.rs | 2 +- crates/ruff_python_parser/src/lib.rs | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/crates/ruff_db/src/parsed.rs b/crates/ruff_db/src/parsed.rs index af918f69f73ed..9e1b2b62d8dc3 100644 --- a/crates/ruff_db/src/parsed.rs +++ b/crates/ruff_db/src/parsed.rs @@ -55,7 +55,7 @@ pub fn parsed_module_impl(db: &dyn Db, file: File) -> Parsed { // surfaced instead of being masked by a later cell's content. Regular files take the existing // single-parse path. if let Some(notebook) = source.as_notebook() { - parse_cells_unchecked(&source, notebook.cell_offsets().content_ranges(), options) + parse_cells_unchecked(&source, notebook.cell_offsets().content_ranges(), &options) } else { parse_unchecked(&source, options) .try_into_module() diff --git a/crates/ruff_linter/src/linter.rs b/crates/ruff_linter/src/linter.rs index df79eab6996a7..4157bd5113527 100644 --- a/crates/ruff_linter/src/linter.rs +++ b/crates/ruff_linter/src/linter.rs @@ -784,7 +784,7 @@ pub fn parse_unchecked_source( Some(notebook) => ruff_python_parser::parse_cells_unchecked( source_kind.source_code(), notebook.cell_offsets().content_ranges(), - options, + &options, ), None => ruff_python_parser::parse_unchecked(source_kind.source_code(), options) .try_into_module() diff --git a/crates/ruff_python_parser/src/lib.rs b/crates/ruff_python_parser/src/lib.rs index 2e92844aea402..208e6be77fdef 100644 --- a/crates/ruff_python_parser/src/lib.rs +++ b/crates/ruff_python_parser/src/lib.rs @@ -313,7 +313,7 @@ pub fn parse_unchecked_source(source: &str, source_type: PySourceType) -> Parsed pub fn parse_cells_unchecked( source: &str, ranges: impl IntoIterator, - options: ParseOptions, + options: &ParseOptions, ) -> Parsed { let mut ranges = ranges.into_iter().peekable(); let mut body = Suite::new();