Skip to content
6 changes: 3 additions & 3 deletions crates/ruff_dev/src/print_tokens.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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:#?}");
}
Expand Down
90 changes: 79 additions & 11 deletions crates/ruff_linter/src/linter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 fn parse_unchecked_source(
source_kind: &SourceKind,
source_type: PySourceType,
target_version: PythonVersion,
Expand All @@ -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_cells_unchecked(
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)]
Expand All @@ -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;
Expand All @@ -811,6 +821,66 @@ 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> {
// `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([
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_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");
Expand Down Expand Up @@ -964,11 +1034,9 @@ mod tests {
) -> Vec<Diagnostic> {
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());
Expand Down
11 changes: 9 additions & 2 deletions crates/ruff_linter/src/suppression.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
21 changes: 11 additions & 10 deletions crates/ruff_linter/src/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -231,11 +231,11 @@ pub fn test_contents<'a>(
) -> (Vec<Diagnostic>, 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());
Expand Down Expand Up @@ -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());
Expand Down
Original file line number Diff line number Diff line change
@@ -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
}
Original file line number Diff line number Diff line change
@@ -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
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
{
"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,
"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
}
Loading