diff --git a/Cargo.lock b/Cargo.lock index 74123f7491db82..ebb3e2ab997a90 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2087,6 +2087,7 @@ dependencies = [ "ruff_cache", "ruff_diagnostics", "ruff_index", + "ruff_jupyter", "ruff_macros", "ruff_python_ast", "ruff_python_codegen", @@ -2103,7 +2104,6 @@ dependencies = [ "semver", "serde", "serde_json", - "serde_with", "similar", "smallvec", "strum", @@ -2115,7 +2115,6 @@ dependencies = [ "typed-arena", "unicode-width", "unicode_names2", - "uuid", "wsl", ] @@ -2184,6 +2183,7 @@ dependencies = [ "ruff_cache", "ruff_diagnostics", "ruff_formatter", + "ruff_jupyter", "ruff_macros", "ruff_python_ast", "ruff_python_formatter", @@ -2227,6 +2227,7 @@ dependencies = [ "ruff_cli", "ruff_diagnostics", "ruff_formatter", + "ruff_jupyter", "ruff_python_ast", "ruff_python_codegen", "ruff_python_formatter", @@ -2281,6 +2282,25 @@ dependencies = [ "static_assertions", ] +[[package]] +name = "ruff_jupyter" +version = "0.0.0" +dependencies = [ + "anyhow", + "insta", + "itertools", + "once_cell", + "ruff_diagnostics", + "ruff_source_file", + "ruff_text_size", + "serde", + "serde_json", + "serde_with", + "test-case", + "thiserror", + "uuid", +] + [[package]] name = "ruff_macros" version = "0.0.0" diff --git a/crates/ruff/Cargo.toml b/crates/ruff/Cargo.toml index 7b9d30ebfd33bd..d5148fc2c9084e 100644 --- a/crates/ruff/Cargo.toml +++ b/crates/ruff/Cargo.toml @@ -18,6 +18,7 @@ name = "ruff" ruff_cache = { path = "../ruff_cache" } ruff_diagnostics = { path = "../ruff_diagnostics", features = ["serde"] } ruff_index = { path = "../ruff_index" } +ruff_jupyter = { path = "../ruff_jupyter" } ruff_macros = { path = "../ruff_macros" } ruff_python_ast = { path = "../ruff_python_ast", features = ["serde"] } ruff_python_codegen = { path = "../ruff_python_codegen" } @@ -64,17 +65,15 @@ schemars = { workspace = true, optional = true } semver = { version = "1.0.16" } serde = { workspace = true } serde_json = { workspace = true } -serde_with = { version = "3.0.0" } similar = { workspace = true } smallvec = { workspace = true } strum = { workspace = true } strum_macros = { workspace = true } -thiserror = { version = "1.0.43" } +thiserror = { workspace = true } toml = { workspace = true } typed-arena = { version = "2.0.2" } unicode-width = { workspace = true } unicode_names2 = { version = "0.6.0", git = "https://github.com/youknowone/unicode_names2.git", rev = "4ce16aa85cbcdd9cc830410f1a72ef9a235f2fde" } -uuid = { workspace = true, features = ["v4", "fast-rng", "macro-diagnostics", "js"] } wsl = { version = "0.1.0" } [dev-dependencies] diff --git a/crates/ruff/src/autofix/mod.rs b/crates/ruff/src/autofix/mod.rs index b85f1b54d21b23..6e33cb99a1ed5f 100644 --- a/crates/ruff/src/autofix/mod.rs +++ b/crates/ruff/src/autofix/mod.rs @@ -4,17 +4,15 @@ use std::collections::BTreeSet; use ruff_text_size::{Ranged, TextLen, TextRange, TextSize}; use rustc_hash::{FxHashMap, FxHashSet}; -use ruff_diagnostics::{Diagnostic, Edit, Fix, IsolationLevel}; +use ruff_diagnostics::{Diagnostic, Edit, Fix, IsolationLevel, SourceMap}; use ruff_source_file::Locator; -use crate::autofix::source_map::SourceMap; use crate::linter::FixTable; use crate::registry::{AsRule, Rule}; pub(crate) mod codemods; pub(crate) mod edits; pub(crate) mod snippet; -pub(crate) mod source_map; pub(crate) struct FixResult { /// The resulting source code, after applying all fixes. @@ -140,10 +138,9 @@ fn cmp_fix(rule1: Rule, rule2: Rule, fix1: &Fix, fix2: &Fix) -> std::cmp::Orderi mod tests { use ruff_text_size::{Ranged, TextSize}; - use ruff_diagnostics::{Diagnostic, Edit, Fix}; + use ruff_diagnostics::{Diagnostic, Edit, Fix, SourceMarker}; use ruff_source_file::Locator; - use crate::autofix::source_map::SourceMarker; use crate::autofix::{apply_fixes, FixResult}; use crate::rules::pycodestyle::rules::MissingNewlineAtEndOfFile; @@ -207,14 +204,8 @@ print("hello world") assert_eq!( source_map.markers(), &[ - SourceMarker { - source: 10.into(), - dest: 10.into(), - }, - SourceMarker { - source: 10.into(), - dest: 21.into(), - }, + SourceMarker::new(10.into(), 10.into(),), + SourceMarker::new(10.into(), 21.into(),), ] ); } @@ -250,14 +241,8 @@ class A(Bar): assert_eq!( source_map.markers(), &[ - SourceMarker { - source: 8.into(), - dest: 8.into(), - }, - SourceMarker { - source: 14.into(), - dest: 11.into(), - }, + SourceMarker::new(8.into(), 8.into(),), + SourceMarker::new(14.into(), 11.into(),), ] ); } @@ -289,14 +274,8 @@ class A: assert_eq!( source_map.markers(), &[ - SourceMarker { - source: 7.into(), - dest: 7.into() - }, - SourceMarker { - source: 15.into(), - dest: 7.into() - } + SourceMarker::new(7.into(), 7.into()), + SourceMarker::new(15.into(), 7.into()), ] ); } @@ -332,22 +311,10 @@ class A(object): assert_eq!( source_map.markers(), &[ - SourceMarker { - source: 8.into(), - dest: 8.into() - }, - SourceMarker { - source: 16.into(), - dest: 8.into() - }, - SourceMarker { - source: 22.into(), - dest: 14.into(), - }, - SourceMarker { - source: 30.into(), - dest: 14.into(), - } + SourceMarker::new(8.into(), 8.into()), + SourceMarker::new(16.into(), 8.into()), + SourceMarker::new(22.into(), 14.into(),), + SourceMarker::new(30.into(), 14.into(),), ] ); } @@ -382,14 +349,8 @@ class A: assert_eq!( source_map.markers(), &[ - SourceMarker { - source: 7.into(), - dest: 7.into(), - }, - SourceMarker { - source: 15.into(), - dest: 7.into(), - } + SourceMarker::new(7.into(), 7.into(),), + SourceMarker::new(15.into(), 7.into(),), ] ); } diff --git a/crates/ruff/src/lib.rs b/crates/ruff/src/lib.rs index a623582ad86137..676e521dfeca99 100644 --- a/crates/ruff/src/lib.rs +++ b/crates/ruff/src/lib.rs @@ -20,7 +20,6 @@ mod doc_lines; mod docstrings; pub mod fs; mod importer; -pub mod jupyter; mod lex; pub mod line_width; pub mod linter; diff --git a/crates/ruff/src/linter.rs b/crates/ruff/src/linter.rs index e247377b0043b2..a47c2b7c698d3e 100644 --- a/crates/ruff/src/linter.rs +++ b/crates/ruff/src/linter.rs @@ -6,8 +6,6 @@ use anyhow::{anyhow, Result}; use colored::Colorize; use itertools::Itertools; use log::error; -use ruff_python_parser::lexer::LexResult; -use ruff_python_parser::{AsMode, ParseError}; use rustc_hash::FxHashMap; use ruff_diagnostics::Diagnostic; @@ -15,7 +13,8 @@ use ruff_python_ast::imports::ImportMap; use ruff_python_ast::PySourceType; use ruff_python_codegen::Stylist; use ruff_python_index::Indexer; - +use ruff_python_parser::lexer::LexResult; +use ruff_python_parser::{AsMode, ParseError}; use ruff_source_file::{Locator, SourceFileBuilder}; use ruff_text_size::Ranged; @@ -609,3 +608,133 @@ This indicates a bug in `{}`. If you could open an issue at: ); } } + +#[cfg(test)] +mod tests { + use std::path::Path; + + use anyhow::Result; + use test_case::test_case; + + use ruff_jupyter::{Notebook, NotebookError}; + + use crate::registry::Rule; + use crate::source_kind::SourceKind; + use crate::test::{test_contents, test_notebook_path, TestedNotebook}; + use crate::{assert_messages, settings}; + + /// Construct a path to a Jupyter notebook in the `resources/test/fixtures/jupyter` directory. + fn notebook_path(path: impl AsRef) -> std::path::PathBuf { + Path::new("../ruff_jupyter/resources/test/fixtures/jupyter").join(path) + } + + #[test] + fn test_import_sorting() -> Result<(), NotebookError> { + let actual = notebook_path("isort.ipynb"); + let expected = notebook_path("isort_expected.ipynb"); + let TestedNotebook { + messages, + source_notebook, + .. + } = test_notebook_path( + &actual, + expected, + &settings::Settings::for_rule(Rule::UnsortedImports), + )?; + assert_messages!(messages, actual, source_notebook); + Ok(()) + } + + #[test] + fn test_ipy_escape_command() -> Result<(), NotebookError> { + let actual = notebook_path("ipy_escape_command.ipynb"); + let expected = notebook_path("ipy_escape_command_expected.ipynb"); + let TestedNotebook { + messages, + source_notebook, + .. + } = test_notebook_path( + &actual, + expected, + &settings::Settings::for_rule(Rule::UnusedImport), + )?; + assert_messages!(messages, actual, source_notebook); + Ok(()) + } + + #[test] + fn test_unused_variable() -> Result<(), NotebookError> { + let actual = notebook_path("unused_variable.ipynb"); + let expected = notebook_path("unused_variable_expected.ipynb"); + let TestedNotebook { + messages, + source_notebook, + .. + } = test_notebook_path( + &actual, + expected, + &settings::Settings::for_rule(Rule::UnusedVariable), + )?; + assert_messages!(messages, actual, source_notebook); + Ok(()) + } + + #[test] + fn test_json_consistency() -> Result<()> { + let actual_path = notebook_path("before_fix.ipynb"); + let expected_path = notebook_path("after_fix.ipynb"); + + let TestedNotebook { + linted_notebook: fixed_notebook, + .. + } = test_notebook_path( + actual_path, + &expected_path, + &settings::Settings::for_rule(Rule::UnusedImport), + )?; + let mut writer = Vec::new(); + fixed_notebook.write(&mut writer)?; + let actual = String::from_utf8(writer)?; + let expected = std::fs::read_to_string(expected_path)?; + assert_eq!(actual, expected); + Ok(()) + } + + #[test_case(Path::new("before_fix.ipynb"), true; "trailing_newline")] + #[test_case(Path::new("no_trailing_newline.ipynb"), false; "no_trailing_newline")] + fn test_trailing_newline(path: &Path, trailing_newline: bool) -> Result<()> { + let notebook = Notebook::from_path(¬ebook_path(path))?; + assert_eq!(notebook.trailing_newline(), trailing_newline); + + let mut writer = Vec::new(); + notebook.write(&mut writer)?; + let string = String::from_utf8(writer)?; + assert_eq!(string.ends_with('\n'), trailing_newline); + + Ok(()) + } + + // Version <4.5, don't emit cell ids + #[test_case(Path::new("no_cell_id.ipynb"), false; "no_cell_id")] + // Version 4.5, cell ids are missing and need to be added + #[test_case(Path::new("add_missing_cell_id.ipynb"), true; "add_missing_cell_id")] + fn test_cell_id(path: &Path, has_id: bool) -> Result<()> { + let source_notebook = Notebook::from_path(¬ebook_path(path))?; + let source_kind = SourceKind::IpyNotebook(source_notebook); + let (_, transformed) = test_contents( + &source_kind, + path, + &settings::Settings::for_rule(Rule::UnusedImport), + ); + let linted_notebook = transformed.into_owned().expect_ipy_notebook(); + let mut writer = Vec::new(); + linted_notebook.write(&mut writer)?; + let actual = String::from_utf8(writer)?; + if has_id { + assert!(actual.contains(r#""id": ""#)); + } else { + assert!(!actual.contains(r#""id":"#)); + } + Ok(()) + } +} diff --git a/crates/ruff/src/logging.rs b/crates/ruff/src/logging.rs index 6645e38eae9a85..485f9bc5663da7 100644 --- a/crates/ruff/src/logging.rs +++ b/crates/ruff/src/logging.rs @@ -12,8 +12,8 @@ use ruff_python_parser::{ParseError, ParseErrorType}; use ruff_source_file::{OneIndexed, SourceCode, SourceLocation}; use crate::fs; -use crate::jupyter::Notebook; use crate::source_kind::SourceKind; +use ruff_jupyter::Notebook; pub static WARNINGS: Lazy>> = Lazy::new(Mutex::default); diff --git a/crates/ruff/src/message/grouped.rs b/crates/ruff/src/message/grouped.rs index 467a5b54665f9f..009e5ad1105584 100644 --- a/crates/ruff/src/message/grouped.rs +++ b/crates/ruff/src/message/grouped.rs @@ -7,12 +7,12 @@ use colored::Colorize; use ruff_source_file::OneIndexed; use crate::fs::relativize_path; -use crate::jupyter::{Notebook, NotebookIndex}; use crate::message::diff::calculate_print_width; use crate::message::text::{MessageCodeFrame, RuleCodeAndBody}; use crate::message::{ group_messages_by_filename, Emitter, EmitterContext, Message, MessageWithLocation, }; +use ruff_jupyter::{Notebook, NotebookIndex}; #[derive(Default)] pub struct GroupedEmitter { diff --git a/crates/ruff/src/message/mod.rs b/crates/ruff/src/message/mod.rs index ea89872600cd34..15d669bff1836c 100644 --- a/crates/ruff/src/message/mod.rs +++ b/crates/ruff/src/message/mod.rs @@ -18,7 +18,7 @@ use ruff_source_file::{SourceFile, SourceLocation}; use ruff_text_size::{Ranged, TextRange, TextSize}; pub use text::TextEmitter; -use crate::jupyter::Notebook; +use ruff_jupyter::Notebook; mod azure; mod diff; diff --git a/crates/ruff/src/message/text.rs b/crates/ruff/src/message/text.rs index 1daec724d3f599..5696b11f3604db 100644 --- a/crates/ruff/src/message/text.rs +++ b/crates/ruff/src/message/text.rs @@ -11,11 +11,11 @@ use ruff_source_file::{OneIndexed, SourceLocation}; use ruff_text_size::{Ranged, TextRange, TextSize}; use crate::fs::relativize_path; -use crate::jupyter::{Notebook, NotebookIndex}; use crate::line_width::{LineWidthBuilder, TabSize}; use crate::message::diff::Diff; use crate::message::{Emitter, EmitterContext, Message}; use crate::registry::AsRule; +use ruff_jupyter::{Notebook, NotebookIndex}; bitflags! { #[derive(Default)] diff --git a/crates/ruff/src/rules/isort/block.rs b/crates/ruff/src/rules/isort/block.rs index b4fdee6d447788..d918d8c9cd9268 100644 --- a/crates/ruff/src/rules/isort/block.rs +++ b/crates/ruff/src/rules/isort/block.rs @@ -7,9 +7,9 @@ use ruff_python_ast::statement_visitor::StatementVisitor; use ruff_source_file::Locator; use crate::directives::IsortDirectives; -use crate::jupyter::Notebook; use crate::rules::isort::helpers; use crate::source_kind::SourceKind; +use ruff_jupyter::Notebook; /// A block of imports within a Python module. #[derive(Debug, Default)] diff --git a/crates/ruff/src/jupyter/snapshots/ruff__jupyter__notebook__tests__import_sorting.snap b/crates/ruff/src/snapshots/ruff__linter__tests__import_sorting.snap similarity index 97% rename from crates/ruff/src/jupyter/snapshots/ruff__jupyter__notebook__tests__import_sorting.snap rename to crates/ruff/src/snapshots/ruff__linter__tests__import_sorting.snap index e0ab3572a84d4f..1bba6e10562c55 100644 --- a/crates/ruff/src/jupyter/snapshots/ruff__jupyter__notebook__tests__import_sorting.snap +++ b/crates/ruff/src/snapshots/ruff__linter__tests__import_sorting.snap @@ -1,5 +1,5 @@ --- -source: crates/ruff/src/jupyter/notebook.rs +source: crates/ruff/src/linter.rs --- isort.ipynb:cell 1:1:1: I001 [*] Import block is un-sorted or un-formatted | diff --git a/crates/ruff/src/jupyter/snapshots/ruff__jupyter__notebook__tests__ipy_escape_command.snap b/crates/ruff/src/snapshots/ruff__linter__tests__ipy_escape_command.snap similarity index 87% rename from crates/ruff/src/jupyter/snapshots/ruff__jupyter__notebook__tests__ipy_escape_command.snap rename to crates/ruff/src/snapshots/ruff__linter__tests__ipy_escape_command.snap index 57f92d184e1d13..c471bd56aa76d7 100644 --- a/crates/ruff/src/jupyter/snapshots/ruff__jupyter__notebook__tests__ipy_escape_command.snap +++ b/crates/ruff/src/snapshots/ruff__linter__tests__ipy_escape_command.snap @@ -1,5 +1,5 @@ --- -source: crates/ruff/src/jupyter/notebook.rs +source: crates/ruff/src/linter.rs --- ipy_escape_command.ipynb:cell 1:5:8: F401 [*] `os` imported but unused | diff --git a/crates/ruff/src/jupyter/snapshots/ruff__jupyter__notebook__tests__unused_variable.snap b/crates/ruff/src/snapshots/ruff__linter__tests__unused_variable.snap similarity index 97% rename from crates/ruff/src/jupyter/snapshots/ruff__jupyter__notebook__tests__unused_variable.snap rename to crates/ruff/src/snapshots/ruff__linter__tests__unused_variable.snap index 689e20e25ee184..55578997159a9c 100644 --- a/crates/ruff/src/jupyter/snapshots/ruff__jupyter__notebook__tests__unused_variable.snap +++ b/crates/ruff/src/snapshots/ruff__linter__tests__unused_variable.snap @@ -1,5 +1,5 @@ --- -source: crates/ruff/src/jupyter/notebook.rs +source: crates/ruff/src/linter.rs --- unused_variable.ipynb:cell 1:2:5: F841 [*] Local variable `foo1` is assigned to but never used | diff --git a/crates/ruff/src/source_kind.rs b/crates/ruff/src/source_kind.rs index ac2d650f37b3e7..40ecb8edf65e0e 100644 --- a/crates/ruff/src/source_kind.rs +++ b/crates/ruff/src/source_kind.rs @@ -1,5 +1,5 @@ -use crate::autofix::source_map::SourceMap; -use crate::jupyter::Notebook; +use ruff_diagnostics::SourceMap; +use ruff_jupyter::Notebook; #[derive(Clone, Debug, PartialEq, is_macro::Is)] pub enum SourceKind { diff --git a/crates/ruff/src/test.rs b/crates/ruff/src/test.rs index f03de7e22b2035..73633256022f82 100644 --- a/crates/ruff/src/test.rs +++ b/crates/ruff/src/test.rs @@ -21,7 +21,6 @@ use ruff_text_size::Ranged; use crate::autofix::{fix_file, FixResult}; use crate::directives; -use crate::jupyter::{Notebook, NotebookError}; use crate::linter::{check_path, LinterResult}; use crate::message::{Emitter, EmitterContext, Message, TextEmitter}; use crate::packaging::detect_package_root; @@ -29,6 +28,7 @@ use crate::registry::AsRule; use crate::rules::pycodestyle::rules::syntax_error; use crate::settings::{flags, Settings}; use crate::source_kind::SourceKind; +use ruff_jupyter::{Notebook, NotebookError}; #[cfg(not(fuzzing))] pub(crate) fn test_resource_path(path: impl AsRef) -> std::path::PathBuf { diff --git a/crates/ruff_cli/Cargo.toml b/crates/ruff_cli/Cargo.toml index 9f81c423897e4b..ebbb5123b445c6 100644 --- a/crates/ruff_cli/Cargo.toml +++ b/crates/ruff_cli/Cargo.toml @@ -25,6 +25,7 @@ ruff = { path = "../ruff", features = ["clap"] } ruff_cache = { path = "../ruff_cache" } ruff_diagnostics = { path = "../ruff_diagnostics" } ruff_formatter = { path = "../ruff_formatter" } +ruff_jupyter = { path = "../ruff_jupyter" } ruff_macros = { path = "../ruff_macros" } ruff_python_ast = { path = "../ruff_python_ast" } ruff_python_formatter = { path = "../ruff_python_formatter" } diff --git a/crates/ruff_cli/src/diagnostics.rs b/crates/ruff_cli/src/diagnostics.rs index e15ac3d21eecef..16801ec6c58cfc 100644 --- a/crates/ruff_cli/src/diagnostics.rs +++ b/crates/ruff_cli/src/diagnostics.rs @@ -1,8 +1,8 @@ #![cfg_attr(target_family = "wasm", allow(dead_code))] -use std::fs::write; +use std::fs::{write, File}; use std::io; -use std::io::Write; +use std::io::{BufWriter, Write}; use std::ops::AddAssign; #[cfg(unix)] use std::os::unix::fs::PermissionsExt; @@ -16,7 +16,6 @@ use rustc_hash::FxHashMap; use similar::TextDiff; use thiserror::Error; -use ruff::jupyter::{Cell, Notebook, NotebookError}; use ruff::linter::{lint_fix, lint_only, FixTable, FixerResult, LinterResult}; use ruff::logging::DisplayParseError; use ruff::message::Message; @@ -26,6 +25,7 @@ use ruff::settings::{flags, AllSettings, Settings}; use ruff::source_kind::SourceKind; use ruff::{fs, IOError, SyntaxError}; use ruff_diagnostics::Diagnostic; +use ruff_jupyter::{Cell, Notebook, NotebookError}; use ruff_macros::CacheKey; use ruff_python_ast::imports::ImportMap; use ruff_python_ast::{PySourceType, SourceType, TomlSourceType}; @@ -243,7 +243,8 @@ pub(crate) fn lint_path( write(path, transformed.as_bytes())?; } SourceKind::IpyNotebook(notebook) => { - notebook.write(path)?; + let mut writer = BufWriter::new(File::create(path)?); + notebook.write(&mut writer)?; } }, flags::FixMode::Diff => { diff --git a/crates/ruff_dev/Cargo.toml b/crates/ruff_dev/Cargo.toml index 901b0c1de9ad3f..1e38e8a43bf1ca 100644 --- a/crates/ruff_dev/Cargo.toml +++ b/crates/ruff_dev/Cargo.toml @@ -18,6 +18,7 @@ ruff_formatter = { path = "../ruff_formatter" } ruff_python_ast = { path = "../ruff_python_ast" } ruff_python_codegen = { path = "../ruff_python_codegen" } ruff_python_formatter = { path = "../ruff_python_formatter" } +ruff_jupyter = { path = "../ruff_jupyter" } ruff_python_literal = { path = "../ruff_python_literal" } ruff_python_parser = { path = "../ruff_python_parser" } ruff_python_stdlib = { path = "../ruff_python_stdlib" } diff --git a/crates/ruff_dev/src/round_trip.rs b/crates/ruff_dev/src/round_trip.rs index a85d57e8cba1b7..07bbac8375ba8a 100644 --- a/crates/ruff_dev/src/round_trip.rs +++ b/crates/ruff_dev/src/round_trip.rs @@ -6,7 +6,6 @@ use std::path::PathBuf; use anyhow::Result; -use ruff::jupyter; use ruff_python_codegen::round_trip; use ruff_python_stdlib::path::is_jupyter_notebook; @@ -20,7 +19,7 @@ pub(crate) struct Args { pub(crate) fn main(args: &Args) -> Result<()> { let path = args.file.as_path(); if is_jupyter_notebook(path) { - println!("{}", jupyter::round_trip(path)?); + println!("{}", ruff_jupyter::round_trip(path)?); } else { let contents = fs::read_to_string(&args.file)?; println!("{}", round_trip(&contents, &args.file.to_string_lossy())?); diff --git a/crates/ruff_diagnostics/src/lib.rs b/crates/ruff_diagnostics/src/lib.rs index c166e8dbd8bb45..f093861de7fbfa 100644 --- a/crates/ruff_diagnostics/src/lib.rs +++ b/crates/ruff_diagnostics/src/lib.rs @@ -1,9 +1,11 @@ pub use diagnostic::{Diagnostic, DiagnosticKind}; pub use edit::Edit; pub use fix::{Applicability, Fix, IsolationLevel}; +pub use source_map::{SourceMap, SourceMarker}; pub use violation::{AlwaysAutofixableViolation, AutofixKind, Violation}; mod diagnostic; mod edit; mod fix; +mod source_map; mod violation; diff --git a/crates/ruff/src/autofix/source_map.rs b/crates/ruff_diagnostics/src/source_map.rs similarity index 71% rename from crates/ruff/src/autofix/source_map.rs rename to crates/ruff_diagnostics/src/source_map.rs index 7871e51d9cdca8..161496eadc50dc 100644 --- a/crates/ruff/src/autofix/source_map.rs +++ b/crates/ruff_diagnostics/src/source_map.rs @@ -1,15 +1,29 @@ use ruff_text_size::{Ranged, TextSize}; -use ruff_diagnostics::Edit; +use crate::Edit; /// Lightweight sourcemap marker representing the source and destination /// position for an [`Edit`]. #[derive(Debug, PartialEq, Eq)] -pub(crate) struct SourceMarker { +pub struct SourceMarker { /// Position of the marker in the original source. - pub(crate) source: TextSize, + source: TextSize, /// Position of the marker in the transformed code. - pub(crate) dest: TextSize, + dest: TextSize, +} + +impl SourceMarker { + pub fn new(source: TextSize, dest: TextSize) -> Self { + Self { source, dest } + } + + pub const fn source(&self) -> TextSize { + self.source + } + + pub const fn dest(&self) -> TextSize { + self.dest + } } /// A collection of [`SourceMarker`]. @@ -18,12 +32,12 @@ pub(crate) struct SourceMarker { /// the transformed code. Here, only the boundaries of edits are tracked instead /// of every single character. #[derive(Default, PartialEq, Eq)] -pub(crate) struct SourceMap(Vec); +pub struct SourceMap(Vec); impl SourceMap { /// Returns a slice of all the markers in the sourcemap in the order they /// were added. - pub(crate) fn markers(&self) -> &[SourceMarker] { + pub fn markers(&self) -> &[SourceMarker] { &self.0 } @@ -31,7 +45,7 @@ impl SourceMap { /// /// The `output_length` is the length of the transformed string before the /// edit is applied. - pub(crate) fn push_start_marker(&mut self, edit: &Edit, output_length: TextSize) { + pub fn push_start_marker(&mut self, edit: &Edit, output_length: TextSize) { self.0.push(SourceMarker { source: edit.start(), dest: output_length, @@ -42,7 +56,7 @@ impl SourceMap { /// /// The `output_length` is the length of the transformed string after the /// edit has been applied. - pub(crate) fn push_end_marker(&mut self, edit: &Edit, output_length: TextSize) { + pub fn push_end_marker(&mut self, edit: &Edit, output_length: TextSize) { if edit.is_insertion() { self.0.push(SourceMarker { source: edit.start(), diff --git a/crates/ruff_jupyter/Cargo.toml b/crates/ruff_jupyter/Cargo.toml new file mode 100644 index 00000000000000..20e8180951195b --- /dev/null +++ b/crates/ruff_jupyter/Cargo.toml @@ -0,0 +1,32 @@ +[package] +name = "ruff_jupyter" +version = "0.0.0" +publish = false +authors = { workspace = true } +edition = { workspace = true } +rust-version = { workspace = true } +homepage = { workspace = true } +documentation = { workspace = true } +repository = { workspace = true } +license = { workspace = true } + +[lib] + +[dependencies] +ruff_diagnostics = { path = "../ruff_diagnostics" } +ruff_source_file = { path = "../ruff_source_file" } +ruff_text_size = { path = "../ruff_text_size" } + +anyhow = { workspace = true } +itertools = { workspace = true } +once_cell = { workspace = true } +serde = { workspace = true } +serde_json = { workspace = true } +serde_with = { version = "3.0.0" } +thiserror = { workspace = true } +uuid = { workspace = true } + +[dev-dependencies] +insta = { workspace = true } +test-case = { workspace = true } + diff --git a/crates/ruff/resources/test/fixtures/jupyter/R.ipynb b/crates/ruff_jupyter/resources/test/fixtures/jupyter/R.ipynb similarity index 100% rename from crates/ruff/resources/test/fixtures/jupyter/R.ipynb rename to crates/ruff_jupyter/resources/test/fixtures/jupyter/R.ipynb diff --git a/crates/ruff/resources/test/fixtures/jupyter/add_missing_cell_id.ipynb b/crates/ruff_jupyter/resources/test/fixtures/jupyter/add_missing_cell_id.ipynb similarity index 100% rename from crates/ruff/resources/test/fixtures/jupyter/add_missing_cell_id.ipynb rename to crates/ruff_jupyter/resources/test/fixtures/jupyter/add_missing_cell_id.ipynb diff --git a/crates/ruff/resources/test/fixtures/jupyter/after_fix.ipynb b/crates/ruff_jupyter/resources/test/fixtures/jupyter/after_fix.ipynb similarity index 100% rename from crates/ruff/resources/test/fixtures/jupyter/after_fix.ipynb rename to crates/ruff_jupyter/resources/test/fixtures/jupyter/after_fix.ipynb diff --git a/crates/ruff/resources/test/fixtures/jupyter/before_fix.ipynb b/crates/ruff_jupyter/resources/test/fixtures/jupyter/before_fix.ipynb similarity index 100% rename from crates/ruff/resources/test/fixtures/jupyter/before_fix.ipynb rename to crates/ruff_jupyter/resources/test/fixtures/jupyter/before_fix.ipynb diff --git a/crates/ruff/resources/test/fixtures/jupyter/cell/cell_magic.json b/crates/ruff_jupyter/resources/test/fixtures/jupyter/cell/cell_magic.json similarity index 100% rename from crates/ruff/resources/test/fixtures/jupyter/cell/cell_magic.json rename to crates/ruff_jupyter/resources/test/fixtures/jupyter/cell/cell_magic.json diff --git a/crates/ruff/resources/test/fixtures/jupyter/cell/code_and_magic.json b/crates/ruff_jupyter/resources/test/fixtures/jupyter/cell/code_and_magic.json similarity index 100% rename from crates/ruff/resources/test/fixtures/jupyter/cell/code_and_magic.json rename to crates/ruff_jupyter/resources/test/fixtures/jupyter/cell/code_and_magic.json diff --git a/crates/ruff/resources/test/fixtures/jupyter/cell/markdown.json b/crates/ruff_jupyter/resources/test/fixtures/jupyter/cell/markdown.json similarity index 100% rename from crates/ruff/resources/test/fixtures/jupyter/cell/markdown.json rename to crates/ruff_jupyter/resources/test/fixtures/jupyter/cell/markdown.json diff --git a/crates/ruff/resources/test/fixtures/jupyter/cell/only_code.json b/crates/ruff_jupyter/resources/test/fixtures/jupyter/cell/only_code.json similarity index 100% rename from crates/ruff/resources/test/fixtures/jupyter/cell/only_code.json rename to crates/ruff_jupyter/resources/test/fixtures/jupyter/cell/only_code.json diff --git a/crates/ruff/resources/test/fixtures/jupyter/cell/only_magic.json b/crates/ruff_jupyter/resources/test/fixtures/jupyter/cell/only_magic.json similarity index 100% rename from crates/ruff/resources/test/fixtures/jupyter/cell/only_magic.json rename to crates/ruff_jupyter/resources/test/fixtures/jupyter/cell/only_magic.json diff --git a/crates/ruff/resources/test/fixtures/jupyter/invalid_extension.ipynb b/crates/ruff_jupyter/resources/test/fixtures/jupyter/invalid_extension.ipynb similarity index 100% rename from crates/ruff/resources/test/fixtures/jupyter/invalid_extension.ipynb rename to crates/ruff_jupyter/resources/test/fixtures/jupyter/invalid_extension.ipynb diff --git a/crates/ruff/resources/test/fixtures/jupyter/ipy_escape_command.ipynb b/crates/ruff_jupyter/resources/test/fixtures/jupyter/ipy_escape_command.ipynb similarity index 100% rename from crates/ruff/resources/test/fixtures/jupyter/ipy_escape_command.ipynb rename to crates/ruff_jupyter/resources/test/fixtures/jupyter/ipy_escape_command.ipynb diff --git a/crates/ruff/resources/test/fixtures/jupyter/ipy_escape_command_expected.ipynb b/crates/ruff_jupyter/resources/test/fixtures/jupyter/ipy_escape_command_expected.ipynb similarity index 100% rename from crates/ruff/resources/test/fixtures/jupyter/ipy_escape_command_expected.ipynb rename to crates/ruff_jupyter/resources/test/fixtures/jupyter/ipy_escape_command_expected.ipynb diff --git a/crates/ruff/resources/test/fixtures/jupyter/isort.ipynb b/crates/ruff_jupyter/resources/test/fixtures/jupyter/isort.ipynb similarity index 100% rename from crates/ruff/resources/test/fixtures/jupyter/isort.ipynb rename to crates/ruff_jupyter/resources/test/fixtures/jupyter/isort.ipynb diff --git a/crates/ruff/resources/test/fixtures/jupyter/isort_expected.ipynb b/crates/ruff_jupyter/resources/test/fixtures/jupyter/isort_expected.ipynb similarity index 100% rename from crates/ruff/resources/test/fixtures/jupyter/isort_expected.ipynb rename to crates/ruff_jupyter/resources/test/fixtures/jupyter/isort_expected.ipynb diff --git a/crates/ruff/resources/test/fixtures/jupyter/no_cell_id.ipynb b/crates/ruff_jupyter/resources/test/fixtures/jupyter/no_cell_id.ipynb similarity index 100% rename from crates/ruff/resources/test/fixtures/jupyter/no_cell_id.ipynb rename to crates/ruff_jupyter/resources/test/fixtures/jupyter/no_cell_id.ipynb diff --git a/crates/ruff/resources/test/fixtures/jupyter/no_trailing_newline.ipynb b/crates/ruff_jupyter/resources/test/fixtures/jupyter/no_trailing_newline.ipynb similarity index 100% rename from crates/ruff/resources/test/fixtures/jupyter/no_trailing_newline.ipynb rename to crates/ruff_jupyter/resources/test/fixtures/jupyter/no_trailing_newline.ipynb diff --git a/crates/ruff/resources/test/fixtures/jupyter/not_json.ipynb b/crates/ruff_jupyter/resources/test/fixtures/jupyter/not_json.ipynb similarity index 100% rename from crates/ruff/resources/test/fixtures/jupyter/not_json.ipynb rename to crates/ruff_jupyter/resources/test/fixtures/jupyter/not_json.ipynb diff --git a/crates/ruff/resources/test/fixtures/jupyter/unused_variable.ipynb b/crates/ruff_jupyter/resources/test/fixtures/jupyter/unused_variable.ipynb similarity index 100% rename from crates/ruff/resources/test/fixtures/jupyter/unused_variable.ipynb rename to crates/ruff_jupyter/resources/test/fixtures/jupyter/unused_variable.ipynb diff --git a/crates/ruff/resources/test/fixtures/jupyter/unused_variable_expected.ipynb b/crates/ruff_jupyter/resources/test/fixtures/jupyter/unused_variable_expected.ipynb similarity index 100% rename from crates/ruff/resources/test/fixtures/jupyter/unused_variable_expected.ipynb rename to crates/ruff_jupyter/resources/test/fixtures/jupyter/unused_variable_expected.ipynb diff --git a/crates/ruff/resources/test/fixtures/jupyter/valid.ipynb b/crates/ruff_jupyter/resources/test/fixtures/jupyter/valid.ipynb similarity index 100% rename from crates/ruff/resources/test/fixtures/jupyter/valid.ipynb rename to crates/ruff_jupyter/resources/test/fixtures/jupyter/valid.ipynb diff --git a/crates/ruff/resources/test/fixtures/jupyter/wrong_schema.ipynb b/crates/ruff_jupyter/resources/test/fixtures/jupyter/wrong_schema.ipynb similarity index 100% rename from crates/ruff/resources/test/fixtures/jupyter/wrong_schema.ipynb rename to crates/ruff_jupyter/resources/test/fixtures/jupyter/wrong_schema.ipynb diff --git a/crates/ruff/src/jupyter/index.rs b/crates/ruff_jupyter/src/index.rs similarity index 100% rename from crates/ruff/src/jupyter/index.rs rename to crates/ruff_jupyter/src/index.rs diff --git a/crates/ruff/src/jupyter/mod.rs b/crates/ruff_jupyter/src/lib.rs similarity index 100% rename from crates/ruff/src/jupyter/mod.rs rename to crates/ruff_jupyter/src/lib.rs diff --git a/crates/ruff/src/jupyter/notebook.rs b/crates/ruff_jupyter/src/notebook.rs similarity index 73% rename from crates/ruff/src/jupyter/notebook.rs rename to crates/ruff_jupyter/src/notebook.rs index aa04d2a3c34f48..651bf29fbda6a3 100644 --- a/crates/ruff/src/jupyter/notebook.rs +++ b/crates/ruff_jupyter/src/notebook.rs @@ -1,7 +1,7 @@ use std::cmp::Ordering; use std::fmt::Display; use std::fs::File; -use std::io::{BufReader, BufWriter, Cursor, Read, Seek, SeekFrom, Write}; +use std::io::{BufReader, Cursor, Read, Seek, SeekFrom, Write}; use std::path::Path; use std::{io, iter}; @@ -12,14 +12,12 @@ use serde_json::error::Category; use thiserror::Error; use uuid::Uuid; -use ruff_python_parser::lexer::lex; -use ruff_python_parser::Mode; +use ruff_diagnostics::{SourceMap, SourceMarker}; use ruff_source_file::{NewlineWithTrailingNewline, UniversalNewlineIterator}; use ruff_text_size::TextSize; -use crate::autofix::source_map::{SourceMap, SourceMarker}; -use crate::jupyter::index::NotebookIndex; -use crate::jupyter::schema::{Cell, RawNotebook, SortAlphabetically, SourceValue}; +use crate::index::NotebookIndex; +use crate::schema::{Cell, RawNotebook, SortAlphabetically, SourceValue}; /// Run round-trip source code generation on a given Jupyter notebook file path. pub fn round_trip(path: &Path) -> anyhow::Result { @@ -33,7 +31,7 @@ pub fn round_trip(path: &Path) -> anyhow::Result { let code = notebook.source_code().to_string(); notebook.update_cell_content(&code); let mut writer = Vec::new(); - notebook.write_inner(&mut writer)?; + notebook.write(&mut writer)?; Ok(String::from_utf8(writer)?) } @@ -99,8 +97,6 @@ pub enum NotebookError { Io(#[from] io::Error), #[error(transparent)] Json(serde_json::Error), - #[error("Expected a Jupyter Notebook, which must be internally stored as JSON, but found a Python source file: {0}")] - PythonSource(serde_json::Error), #[error("Expected a Jupyter Notebook, which must be internally stored as JSON, but this file isn't valid JSON: {0}")] InvalidJson(serde_json::Error), #[error("This file does not match the schema expected of Jupyter Notebooks: {0}")] @@ -162,24 +158,10 @@ impl Notebook { // Translate the error into a diagnostic return Err(match err.classify() { Category::Io => NotebookError::Json(err), - Category::Syntax | Category::Eof => { - // Maybe someone saved the python sources (those with the `# %%` separator) - // as jupyter notebook instead. Let's help them. - let mut contents = String::new(); - reader - .rewind() - .and_then(|_| reader.read_to_string(&mut contents))?; - - // Check if tokenizing was successful and the file is non-empty - if lex(&contents, Mode::Module).any(|result| result.is_err()) { - NotebookError::InvalidJson(err) - } else { - NotebookError::PythonSource(err) - } - } + Category::Syntax | Category::Eof => NotebookError::InvalidJson(err), Category::Data => { // We could try to read the schema version here but if this fails it's - // a bug anyway + // a bug anyway. NotebookError::InvalidSchema(err) } }); @@ -256,13 +238,13 @@ impl Notebook { // The first offset is always going to be at 0, so skip it. for offset in self.cell_offsets.iter_mut().skip(1).rev() { let closest_marker = match last_marker { - Some(marker) if marker.source <= *offset => marker, + Some(marker) if marker.source() <= *offset => marker, _ => { let Some(marker) = source_map .markers() .iter() .rev() - .find(|m| m.source <= *offset) + .find(|marker| marker.source() <= *offset) else { // There are no markers above the current offset, so we can // stop here. @@ -273,9 +255,9 @@ impl Notebook { } }; - match closest_marker.source.cmp(&closest_marker.dest) { - Ordering::Less => *offset += closest_marker.dest - closest_marker.source, - Ordering::Greater => *offset -= closest_marker.source - closest_marker.dest, + match closest_marker.source().cmp(&closest_marker.dest()) { + Ordering::Less => *offset += closest_marker.dest() - closest_marker.source(), + Ordering::Greater => *offset -= closest_marker.source() - closest_marker.dest(), Ordering::Equal => (), } } @@ -383,18 +365,23 @@ impl Notebook { /// The index is built only once when required. This is only used to /// report diagnostics, so by that time all of the autofixes must have /// been applied if `--fix` was passed. - pub(crate) fn index(&self) -> &NotebookIndex { + pub fn index(&self) -> &NotebookIndex { self.index.get_or_init(|| self.build_index()) } /// Return the cell offsets for the concatenated source code corresponding /// the Jupyter notebook. - pub(crate) fn cell_offsets(&self) -> &[TextSize] { + pub fn cell_offsets(&self) -> &[TextSize] { &self.cell_offsets } + /// Return `true` if the notebook has a trailing newline, `false` otherwise. + pub fn trailing_newline(&self) -> bool { + self.trailing_newline + } + /// Update the notebook with the given sourcemap and transformed content. - pub(crate) fn update(&mut self, source_map: &SourceMap, transformed: String) { + pub fn update(&mut self, source_map: &SourceMap, transformed: String) { // Cell offsets must be updated before updating the cell content as // it depends on the offsets to extract the cell content. self.index.take(); @@ -417,7 +404,8 @@ impl Notebook { .map_or(true, |language| language.name == "python") } - fn write_inner(&self, writer: &mut impl Write) -> anyhow::Result<()> { + /// Write the notebook back to the given [`Write`] implementor. + pub fn write(&self, writer: &mut impl Write) -> anyhow::Result<()> { // https://github.com/psf/black/blob/69ca0a4c7a365c5f5eea519a90980bab72cab764/src/black/__init__.py#LL1041 let formatter = serde_json::ser::PrettyFormatter::with_indent(b" "); let mut serializer = serde_json::Serializer::with_formatter(writer, formatter); @@ -427,13 +415,6 @@ impl Notebook { } Ok(()) } - - /// Write back with an indent of 1, just like black - pub fn write(&self, path: &Path) -> anyhow::Result<()> { - let mut writer = BufWriter::new(File::create(path)?); - self.write_inner(&mut writer)?; - Ok(()) - } } #[cfg(test)] @@ -443,17 +424,11 @@ mod tests { use anyhow::Result; use test_case::test_case; - use crate::jupyter::index::NotebookIndex; - use crate::jupyter::schema::Cell; - use crate::jupyter::{Notebook, NotebookError}; - use crate::registry::Rule; - use crate::source_kind::SourceKind; - use crate::test::{test_contents, test_notebook_path, test_resource_path, TestedNotebook}; - use crate::{assert_messages, settings}; + use crate::{Cell, Notebook, NotebookError, NotebookIndex}; /// Construct a path to a Jupyter notebook in the `resources/test/fixtures/jupyter` directory. fn notebook_path(path: impl AsRef) -> std::path::PathBuf { - test_resource_path("fixtures/jupyter").join(path) + Path::new("./resources/test/fixtures/jupyter").join(path) } #[test] @@ -474,7 +449,7 @@ mod tests { fn test_invalid() { assert!(matches!( Notebook::from_path(¬ebook_path("invalid_extension.ipynb")), - Err(NotebookError::PythonSource(_)) + Err(NotebookError::InvalidJson(_)) )); assert!(matches!( Notebook::from_path(¬ebook_path("not_json.ipynb")), @@ -545,114 +520,4 @@ print("after empty cells") ); Ok(()) } - - #[test] - fn test_import_sorting() -> Result<(), NotebookError> { - let actual = notebook_path("isort.ipynb"); - let expected = notebook_path("isort_expected.ipynb"); - let TestedNotebook { - messages, - source_notebook, - .. - } = test_notebook_path( - &actual, - expected, - &settings::Settings::for_rule(Rule::UnsortedImports), - )?; - assert_messages!(messages, actual, source_notebook); - Ok(()) - } - - #[test] - fn test_ipy_escape_command() -> Result<(), NotebookError> { - let actual = notebook_path("ipy_escape_command.ipynb"); - let expected = notebook_path("ipy_escape_command_expected.ipynb"); - let TestedNotebook { - messages, - source_notebook, - .. - } = test_notebook_path( - &actual, - expected, - &settings::Settings::for_rule(Rule::UnusedImport), - )?; - assert_messages!(messages, actual, source_notebook); - Ok(()) - } - - #[test] - fn test_unused_variable() -> Result<(), NotebookError> { - let actual = notebook_path("unused_variable.ipynb"); - let expected = notebook_path("unused_variable_expected.ipynb"); - let TestedNotebook { - messages, - source_notebook, - .. - } = test_notebook_path( - &actual, - expected, - &settings::Settings::for_rule(Rule::UnusedVariable), - )?; - assert_messages!(messages, actual, source_notebook); - Ok(()) - } - - #[test] - fn test_json_consistency() -> Result<()> { - let actual_path = notebook_path("before_fix.ipynb"); - let expected_path = notebook_path("after_fix.ipynb"); - - let TestedNotebook { - linted_notebook: fixed_notebook, - .. - } = test_notebook_path( - actual_path, - &expected_path, - &settings::Settings::for_rule(Rule::UnusedImport), - )?; - let mut writer = Vec::new(); - fixed_notebook.write_inner(&mut writer)?; - let actual = String::from_utf8(writer)?; - let expected = std::fs::read_to_string(expected_path)?; - assert_eq!(actual, expected); - Ok(()) - } - - #[test_case(Path::new("before_fix.ipynb"), true; "trailing_newline")] - #[test_case(Path::new("no_trailing_newline.ipynb"), false; "no_trailing_newline")] - fn test_trailing_newline(path: &Path, trailing_newline: bool) -> Result<()> { - let notebook = Notebook::from_path(¬ebook_path(path))?; - assert_eq!(notebook.trailing_newline, trailing_newline); - - let mut writer = Vec::new(); - notebook.write_inner(&mut writer)?; - let string = String::from_utf8(writer)?; - assert_eq!(string.ends_with('\n'), trailing_newline); - - Ok(()) - } - - // Version <4.5, don't emit cell ids - #[test_case(Path::new("no_cell_id.ipynb"), false; "no_cell_id")] - // Version 4.5, cell ids are missing and need to be added - #[test_case(Path::new("add_missing_cell_id.ipynb"), true; "add_missing_cell_id")] - fn test_cell_id(path: &Path, has_id: bool) -> Result<()> { - let source_notebook = Notebook::from_path(¬ebook_path(path))?; - let source_kind = SourceKind::IpyNotebook(source_notebook); - let (_, transformed) = test_contents( - &source_kind, - path, - &settings::Settings::for_rule(Rule::UnusedImport), - ); - let linted_notebook = transformed.into_owned().expect_ipy_notebook(); - let mut writer = Vec::new(); - linted_notebook.write_inner(&mut writer)?; - let actual = String::from_utf8(writer)?; - if has_id { - assert!(actual.contains(r#""id": ""#)); - } else { - assert!(!actual.contains(r#""id":"#)); - } - Ok(()) - } } diff --git a/crates/ruff/src/jupyter/schema.rs b/crates/ruff_jupyter/src/schema.rs similarity index 99% rename from crates/ruff/src/jupyter/schema.rs rename to crates/ruff_jupyter/src/schema.rs index e466615feca520..0c9b64988b0481 100644 --- a/crates/ruff/src/jupyter/schema.rs +++ b/crates/ruff_jupyter/src/schema.rs @@ -46,7 +46,7 @@ fn sort_alphabetically( /// /// use serde::Serialize; /// -/// use ruff::jupyter::SortAlphabetically; +/// use ruff_jupyter::SortAlphabetically; /// /// #[derive(Serialize)] /// struct MyStruct {