diff --git a/crates/ruff_db/src/parsed.rs b/crates/ruff_db/src/parsed.rs index 84b2bd01aaa66..9e1b2b62d8dc3 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().content_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_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/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 8e32dd809b23f..4157bd5113527 100644 --- a/crates/ruff_linter/src/linter.rs +++ b/crates/ruff_linter/src/linter.rs @@ -767,9 +767,11 @@ impl ParseSource { } } -/// Like [`ruff_python_parser::parse_unchecked_source`] but with an additional [`PythonVersion`] -/// argument. -fn parse_unchecked_source( +/// Like [`ruff_python_parser::parse_unchecked_source`], but with an explicit [`PythonVersion`] and +/// per-cell notebook parsing. +/// +/// Per-cell modules are merged so definitions remain visible across cells. +pub fn parse_unchecked_source( source_kind: &SourceKind, source_type: PySourceType, target_version: PythonVersion, @@ -778,9 +780,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_cells_unchecked( + source_kind.source_code(), + notebook.cell_offsets().content_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 +800,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; @@ -964,11 +972,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_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_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_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 9ff1aab6a7e6b..208e6be77fdef 100644 --- a/crates/ruff_python_parser/src/lib.rs +++ b/crates/ruff_python_parser/src/lib.rs @@ -71,9 +71,10 @@ 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::{ - 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,89 @@ 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. +/// +/// 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, + options: &ParseOptions, +) -> Parsed { + let mut ranges = ranges.into_iter().peekable(); + 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; + + while let Some(range) = ranges.next() { + if let Some(previous) = module_range { + 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[TextRange::up_to(range.end())]; + 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); + errors.extend(cell_errors); + unsupported_syntax_errors.extend(cell_unsupported_syntax_errors); + + // 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()); + assert_eq!(&source[separator], "\n"); + 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, + }); + } + + 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, + range: module_range.unwrap_or_default(), + 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_server/src/lint.rs b/crates/ruff_server/src/lint.rs index cefd17cf1a49c..f2af9a44b9382 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,8 @@ 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"); + 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()); 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..7d9f840af0833 --- /dev/null +++ b/crates/ty_python_semantic/resources/mdtest/notebook.md @@ -0,0 +1,44 @@ +# 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. + +## 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 separator after the +cell. 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..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 @@ -185,12 +185,12 @@ expression: diagnostics { "range": { "start": { - "line": 3, - "character": 0 + "line": 2, + "character": 7 }, "end": { - "line": 3, - "character": 0 + "line": 2, + "character": 7 } }, "severity": 1,