From 4620604a207e252fbf20458426e8d89086b52b39 Mon Sep 17 00:00:00 2001 From: Brent Westbrook Date: Tue, 16 Sep 2025 13:38:41 -0400 Subject: [PATCH 01/36] add DiagnosticId::Unformatted --- crates/ruff_db/src/diagnostic/mod.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/crates/ruff_db/src/diagnostic/mod.rs b/crates/ruff_db/src/diagnostic/mod.rs index 99b8b9d83bc1e6..f8830645f9549e 100644 --- a/crates/ruff_db/src/diagnostic/mod.rs +++ b/crates/ruff_db/src/diagnostic/mod.rs @@ -1016,6 +1016,9 @@ pub enum DiagnosticId { /// Use of a deprecated setting. DeprecatedSetting, + + /// The code needs to be formatted. + Unformatted, } impl DiagnosticId { @@ -1055,6 +1058,7 @@ impl DiagnosticId { DiagnosticId::UnnecessaryOverridesSection => "unnecessary-overrides-section", DiagnosticId::UselessOverridesSection => "useless-overrides-section", DiagnosticId::DeprecatedSetting => "deprecated-setting", + DiagnosticId::Unformatted => "unformatted", } } From 92208abd5539ab27240f64e012fb74724b779553 Mon Sep 17 00:00:00 2001 From: Brent Westbrook Date: Wed, 24 Sep 2025 14:57:15 -0400 Subject: [PATCH 02/36] add --output-format to the ruff format cli --- crates/ruff/src/args.rs | 6 ++++++ docs/configuration.md | 5 +++++ 2 files changed, 11 insertions(+) diff --git a/crates/ruff/src/args.rs b/crates/ruff/src/args.rs index aef5280955e06c..2e4817836e5d3f 100644 --- a/crates/ruff/src/args.rs +++ b/crates/ruff/src/args.rs @@ -537,6 +537,11 @@ pub struct FormatCommand { /// Exit with a non-zero status code if any files were modified via format, even if all files were formatted successfully. #[arg(long, help_heading = "Miscellaneous", alias = "exit-non-zero-on-fix")] pub exit_non_zero_on_format: bool, + + /// Output serialization format for violations. + /// The default serialization format is "full". + #[arg(long, value_enum, env = "RUFF_OUTPUT_FORMAT")] + pub output_format: Option, } #[derive(Copy, Clone, Debug, clap::Parser)] @@ -784,6 +789,7 @@ impl FormatCommand { target_version: self.target_version.map(ast::PythonVersion::from), cache_dir: self.cache_dir, extension: self.extension, + output_format: self.output_format, ..ExplicitConfigOverrides::default() }; diff --git a/docs/configuration.md b/docs/configuration.md index 0b79a20de94371..13aee26da96ab0 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -727,6 +727,11 @@ Options: --preview Enable preview mode; enables unstable formatting. Use `--no-preview` to disable + --output-format + Output serialization format for violations. The default serialization + format is "full" [env: RUFF_OUTPUT_FORMAT=] [possible values: + concise, full, json, json-lines, junit, grouped, github, gitlab, + pylint, rdjson, azure, sarif] -h, --help Print help (see more with '--help') From 041b35dafc02a114851937065c5c241e15cab1be Mon Sep 17 00:00:00 2001 From: Brent Westbrook Date: Wed, 24 Sep 2025 14:52:47 -0400 Subject: [PATCH 03/36] allow tempdir_filter to take any AsRef this is convenient for passing in the result of tempdir.join calls and matches the version in lint.rs --- crates/ruff/tests/format.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/ruff/tests/format.rs b/crates/ruff/tests/format.rs index f1369048f7326a..3b9f266862ff0b 100644 --- a/crates/ruff/tests/format.rs +++ b/crates/ruff/tests/format.rs @@ -12,8 +12,8 @@ use tempfile::TempDir; const BIN_NAME: &str = "ruff"; -fn tempdir_filter(tempdir: &TempDir) -> String { - format!(r"{}\\?/?", escape(tempdir.path().to_str().unwrap())) +fn tempdir_filter(path: impl AsRef) -> String { + format!(r"{}\\?/?", escape(path.as_ref().to_str().unwrap())) } #[test] From 9bb4374fd7fc630ccfad8e7ffa3172b7156e806d Mon Sep 17 00:00:00 2001 From: Brent Westbrook Date: Wed, 24 Sep 2025 15:43:11 -0400 Subject: [PATCH 04/36] respect show_fix_status for `full` output this wasn't a problem for the linter because we always want to show the fix status, but we don't need the visual clutter of marking every formatter diagnostic as fixable --- crates/ruff_db/src/diagnostic/render.rs | 2 +- crates/ruff_db/src/diagnostic/render/full.rs | 7 ++++++- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/crates/ruff_db/src/diagnostic/render.rs b/crates/ruff_db/src/diagnostic/render.rs index 86b04fef002f68..2ab885cdc59459 100644 --- a/crates/ruff_db/src/diagnostic/render.rs +++ b/crates/ruff_db/src/diagnostic/render.rs @@ -258,7 +258,7 @@ impl<'a> ResolvedDiagnostic<'a> { id, message: diag.inner.message.as_str().to_string(), annotations, - is_fixable: diag.has_applicable_fix(config), + is_fixable: config.show_fix_status && diag.has_applicable_fix(config), } } diff --git a/crates/ruff_db/src/diagnostic/render/full.rs b/crates/ruff_db/src/diagnostic/render/full.rs index c6a5bc13fa83eb..eb782e72c3a60b 100644 --- a/crates/ruff_db/src/diagnostic/render/full.rs +++ b/crates/ruff_db/src/diagnostic/render/full.rs @@ -366,6 +366,7 @@ mod tests { fn hide_severity_output() { let (mut env, diagnostics) = create_diagnostics(DiagnosticFormat::Full); env.hide_severity(true); + env.show_fix_status(true); env.fix_applicability(Applicability::DisplayOnly); insta::assert_snapshot!(env.render_diagnostics(&diagnostics), @r#" @@ -584,7 +585,8 @@ print() /// Check that ranges in notebooks are remapped relative to the cells. #[test] fn notebook_output() { - let (env, diagnostics) = create_notebook_diagnostics(DiagnosticFormat::Full); + let (mut env, diagnostics) = create_notebook_diagnostics(DiagnosticFormat::Full); + env.show_fix_status(true); insta::assert_snapshot!(env.render_diagnostics(&diagnostics), @r" error[unused-import][*]: `os` imported but unused --> notebook.ipynb:cell 1:2:8 @@ -698,6 +700,7 @@ print() fn notebook_output_with_diff() { let (mut env, diagnostics) = create_notebook_diagnostics(DiagnosticFormat::Full); env.show_fix_diff(true); + env.show_fix_status(true); env.fix_applicability(Applicability::DisplayOnly); insta::assert_snapshot!(env.render_diagnostics(&diagnostics), @r" @@ -752,6 +755,7 @@ print() fn notebook_output_with_diff_spanning_cells() { let (mut env, mut diagnostics) = create_notebook_diagnostics(DiagnosticFormat::Full); env.show_fix_diff(true); + env.show_fix_status(true); env.fix_applicability(Applicability::DisplayOnly); // Move all of the edits from the later diagnostics to the first diagnostic to simulate a @@ -928,6 +932,7 @@ line 10 env.add("example.py", contents); env.format(DiagnosticFormat::Full); env.show_fix_diff(true); + env.show_fix_status(true); env.fix_applicability(Applicability::DisplayOnly); let mut diagnostic = env.err().primary("example.py", "3", "3", "label").build(); From 9cffdd808e4847ab7c7d1e149d39087c3c36d053 Mon Sep 17 00:00:00 2001 From: Brent Westbrook Date: Wed, 24 Sep 2025 15:44:47 -0400 Subject: [PATCH 05/36] suppress the URL for all non-lint DiagnosticIds instead of just syntax errors --- crates/ruff_db/src/diagnostic/mod.rs | 23 +++++++++++++++-------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/crates/ruff_db/src/diagnostic/mod.rs b/crates/ruff_db/src/diagnostic/mod.rs index f8830645f9549e..b2100403a22379 100644 --- a/crates/ruff_db/src/diagnostic/mod.rs +++ b/crates/ruff_db/src/diagnostic/mod.rs @@ -432,14 +432,21 @@ impl Diagnostic { /// Returns the URL for the rule documentation, if it exists. pub fn to_ruff_url(&self) -> Option { - if self.is_invalid_syntax() { - None - } else { - Some(format!( - "{}/rules/{}", - env!("CARGO_PKG_HOMEPAGE"), - self.name() - )) + match self.id() { + DiagnosticId::Panic + | DiagnosticId::Io + | DiagnosticId::InvalidSyntax + | DiagnosticId::RevealedType + | DiagnosticId::UnknownRule + | DiagnosticId::InvalidGlob + | DiagnosticId::EmptyInclude + | DiagnosticId::UnnecessaryOverridesSection + | DiagnosticId::UselessOverridesSection + | DiagnosticId::DeprecatedSetting + | DiagnosticId::Unformatted => None, + DiagnosticId::Lint(lint_name) => { + Some(format!("{}/rules/{lint_name}", env!("CARGO_PKG_HOMEPAGE"))) + } } } From 5bfe3f244034a4081eb3951e43fc8cc3c4a5b5b3 Mon Sep 17 00:00:00 2001 From: Brent Westbrook Date: Wed, 24 Sep 2025 11:29:14 -0400 Subject: [PATCH 06/36] fix a couple of Rome references --- crates/ruff_formatter/src/diagnostics.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/ruff_formatter/src/diagnostics.rs b/crates/ruff_formatter/src/diagnostics.rs index eb2f1435b3af9a..08964496aded8a 100644 --- a/crates/ruff_formatter/src/diagnostics.rs +++ b/crates/ruff_formatter/src/diagnostics.rs @@ -38,12 +38,12 @@ impl std::fmt::Display for FormatError { ), FormatError::InvalidDocument(error) => std::write!( fmt, - "Invalid document: {error}\n\n This is an internal Rome error. Please report if necessary." + "Invalid document: {error}\n\n This is an internal Ruff error. Please report if necessary." ), FormatError::PoorLayout => { std::write!( fmt, - "Poor layout: The formatter wasn't able to pick a good layout for your document. This is an internal Rome error. Please report if necessary." + "Poor layout: The formatter wasn't able to pick a good layout for your document. This is an internal Ruff error. Please report if necessary." ) } } From 76e63b36bd1ab3420c3cf61a07bc217ae827c5cd Mon Sep 17 00:00:00 2001 From: Brent Westbrook Date: Mon, 22 Sep 2025 16:17:41 -0400 Subject: [PATCH 07/36] add `CellOffsets::ranges` helper --- crates/ruff_notebook/src/cell.rs | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/crates/ruff_notebook/src/cell.rs b/crates/ruff_notebook/src/cell.rs index 56c1822341f798..0bcd61337018a6 100644 --- a/crates/ruff_notebook/src/cell.rs +++ b/crates/ruff_notebook/src/cell.rs @@ -308,6 +308,13 @@ impl CellOffsets { }) .is_ok() } + + /// Returns an iterator over [`TextRange`]s covered by each cell. + pub fn ranges(&self) -> impl Iterator { + self.iter() + .tuple_windows() + .map(|(start, end)| TextRange::new(*start, *end)) + } } impl Deref for CellOffsets { From 7e48156af0cf212afd8c14ecb72547912dd17d51 Mon Sep 17 00:00:00 2001 From: Brent Westbrook Date: Tue, 16 Sep 2025 13:38:59 -0400 Subject: [PATCH 08/36] convert FormatPathResults to Diagnostics just before rendering --- crates/ruff/src/commands/format.rs | 194 ++++++++++++++++-- crates/ruff/src/commands/format_stdin.rs | 2 +- crates/ruff/tests/format.rs | 55 +++++ .../format__output_format_azure.snap | 18 ++ .../format__output_format_concise.snap | 19 ++ .../snapshots/format__output_format_full.snap | 26 +++ .../format__output_format_github.snap | 18 ++ .../format__output_format_gitlab.snap | 37 ++++ .../format__output_format_grouped.snap | 21 ++ .../format__output_format_json-lines.snap | 18 ++ .../snapshots/format__output_format_json.snap | 51 +++++ .../format__output_format_junit.snap | 25 +++ .../format__output_format_pylint.snap | 18 ++ .../format__output_format_rdjson.snap | 49 +++++ .../format__output_format_sarif.snap | 83 ++++++++ crates/ruff_linter/src/settings/types.rs | 14 ++ 16 files changed, 630 insertions(+), 18 deletions(-) create mode 100644 crates/ruff/tests/snapshots/format__output_format_azure.snap create mode 100644 crates/ruff/tests/snapshots/format__output_format_concise.snap create mode 100644 crates/ruff/tests/snapshots/format__output_format_full.snap create mode 100644 crates/ruff/tests/snapshots/format__output_format_github.snap create mode 100644 crates/ruff/tests/snapshots/format__output_format_gitlab.snap create mode 100644 crates/ruff/tests/snapshots/format__output_format_grouped.snap create mode 100644 crates/ruff/tests/snapshots/format__output_format_json-lines.snap create mode 100644 crates/ruff/tests/snapshots/format__output_format_json.snap create mode 100644 crates/ruff/tests/snapshots/format__output_format_junit.snap create mode 100644 crates/ruff/tests/snapshots/format__output_format_pylint.snap create mode 100644 crates/ruff/tests/snapshots/format__output_format_rdjson.snap create mode 100644 crates/ruff/tests/snapshots/format__output_format_sarif.snap diff --git a/crates/ruff/src/commands/format.rs b/crates/ruff/src/commands/format.rs index 1006346008d315..7bf80200f8f521 100644 --- a/crates/ruff/src/commands/format.rs +++ b/crates/ruff/src/commands/format.rs @@ -11,13 +11,19 @@ use itertools::Itertools; use log::{error, warn}; use rayon::iter::Either::{Left, Right}; use rayon::iter::{IntoParallelRefIterator, ParallelIterator}; +use ruff_db::diagnostic::{ + Annotation, Diagnostic, DiagnosticFormat, DiagnosticId, DisplayDiagnosticConfig, + DisplayDiagnostics, DisplayGithubDiagnostics, GithubRenderer, Severity, Span, +}; +use ruff_linter::message::{Emitter, EmitterContext, GroupedEmitter, SarifEmitter}; +use ruff_linter::settings::types::OutputFormat; use ruff_python_parser::ParseError; -use rustc_hash::FxHashSet; +use rustc_hash::{FxHashMap, FxHashSet}; use thiserror::Error; use tracing::debug; use ruff_db::panic::{PanicError, catch_unwind}; -use ruff_diagnostics::SourceMap; +use ruff_diagnostics::{Edit, Fix, SourceMap}; use ruff_linter::fs; use ruff_linter::logging::{DisplayParseError, LogLevel}; use ruff_linter::package::PackageRoot; @@ -27,7 +33,7 @@ use ruff_linter::source_kind::{SourceError, SourceKind}; use ruff_linter::warn_user_once; use ruff_python_ast::{PySourceType, SourceType}; use ruff_python_formatter::{FormatModuleError, QuoteStyle, format_module_source, format_range}; -use ruff_source_file::LineIndex; +use ruff_source_file::{LineIndex, SourceFileBuilder}; use ruff_text_size::{TextLen, TextRange, TextSize}; use ruff_workspace::FormatterSettings; use ruff_workspace::resolver::{ResolvedFile, Resolver, match_exclusion, python_files_in_path}; @@ -69,6 +75,9 @@ pub(crate) fn format( let files = resolve_default_files(cli.files, false); let (paths, resolver) = python_files_in_path(&files, &pyproject_config, config_arguments)?; + let output_format = pyproject_config.settings.output_format; + let preview = pyproject_config.settings.formatter.preview; + if paths.is_empty() { warn_user_once!("No Python files found under the given path(s)"); return Ok(ExitStatus::Success); @@ -194,7 +203,11 @@ pub(crate) fn format( match mode { FormatMode::Write => {} FormatMode::Check => { - results.write_changed(&mut stdout().lock())?; + if preview.is_enabled() { + results.write_changed_preview(&mut stdout().lock(), output_format)?; + } else { + results.write_changed(&mut stdout().lock())?; + } } FormatMode::Diff => { results.write_diff(&mut stdout().lock())?; @@ -206,7 +219,7 @@ pub(crate) fn format( if mode.is_diff() { // Allow piping the diff to e.g. a file by writing the summary to stderr results.write_summary(&mut stderr().lock())?; - } else { + } else if !preview.is_enabled() || output_format.is_human_readable() { results.write_summary(&mut stdout().lock())?; } } @@ -275,7 +288,10 @@ pub(crate) fn format_path( // Format the source. let format_result = match format_source(&unformatted, source_type, Some(path), settings, range)? { - FormattedSource::Formatted(formatted) => match mode { + FormattedSource::Formatted { + formatted, + unformatted, + } => match mode { FormatMode::Write => { let mut writer = File::create(path).map_err(|err| { FormatCommandError::Write(Some(path.to_path_buf()), err.into()) @@ -293,9 +309,15 @@ pub(crate) fn format_path( } } - FormatResult::Formatted + FormatResult::Formatted { + unformatted, + formatted, + } } - FormatMode::Check => FormatResult::Formatted, + FormatMode::Check => FormatResult::Formatted { + unformatted, + formatted, + }, FormatMode::Diff => FormatResult::Diff { unformatted, formatted, @@ -321,7 +343,10 @@ pub(crate) fn format_path( #[derive(Debug)] pub(crate) enum FormattedSource { /// The source was formatted, and the [`SourceKind`] contains the transformed source code. - Formatted(SourceKind), + Formatted { + formatted: SourceKind, + unformatted: SourceKind, + }, /// The source was unchanged. Unchanged, } @@ -329,7 +354,13 @@ pub(crate) enum FormattedSource { impl From for FormatResult { fn from(value: FormattedSource) -> Self { match value { - FormattedSource::Formatted(_) => FormatResult::Formatted, + FormattedSource::Formatted { + formatted, + unformatted, + } => FormatResult::Formatted { + formatted, + unformatted, + }, FormattedSource::Unchanged => FormatResult::Unchanged, } } @@ -382,7 +413,10 @@ pub(crate) fn format_source( if formatted.len() == unformatted.len() && formatted == *unformatted { Ok(FormattedSource::Unchanged) } else { - Ok(FormattedSource::Formatted(SourceKind::Python(formatted))) + Ok(FormattedSource::Formatted { + formatted: SourceKind::Python(formatted), + unformatted: SourceKind::Python(unformatted.to_string()), + }) } } SourceKind::IpyNotebook(notebook) => { @@ -467,9 +501,10 @@ pub(crate) fn format_source( let mut formatted = notebook.clone(); formatted.update(&source_map, output); - Ok(FormattedSource::Formatted(SourceKind::IpyNotebook( - formatted, - ))) + Ok(FormattedSource::Formatted { + formatted: SourceKind::IpyNotebook(formatted), + unformatted: SourceKind::IpyNotebook(notebook.clone()), + }) } } } @@ -478,7 +513,10 @@ pub(crate) fn format_source( #[derive(Debug, Clone, is_macro::Is)] pub(crate) enum FormatResult { /// The file was formatted. - Formatted, + Formatted { + unformatted: SourceKind, + formatted: SourceKind, + }, /// The file was formatted, [`SourceKind`] contains the formatted code Diff { @@ -517,7 +555,7 @@ impl<'a> FormatResults<'a> { /// Returns `true` if any of the files require formatting. fn any_formatted(&self) -> bool { self.results.iter().any(|result| match result.result { - FormatResult::Formatted | FormatResult::Diff { .. } => true, + FormatResult::Formatted { .. } | FormatResult::Diff { .. } => true, FormatResult::Unchanged | FormatResult::Skipped => false, }) } @@ -566,6 +604,90 @@ impl<'a> FormatResults<'a> { Ok(()) } + /// Write a list of the files that would be changed to the given writer. + fn write_changed_preview( + &self, + f: &mut impl Write, + output_format: OutputFormat, + ) -> io::Result<()> { + let notebook_index = FxHashMap::default(); + let diagnostics: Vec<_> = self.to_diagnostics().collect(); + + let context = EmitterContext::new(¬ebook_index); + let config = DisplayDiagnosticConfig::default(); + match output_format { + OutputFormat::Concise => { + let config = config + .format(DiagnosticFormat::Concise) + .hide_severity(true) + .color(!cfg!(test) && colored::control::SHOULD_COLORIZE.should_colorize()); + let value = DisplayDiagnostics::new(&context, &config, &diagnostics); + write!(f, "{value}")?; + } + OutputFormat::Full => { + let config = config + .format(DiagnosticFormat::Full) + .hide_severity(true) + .show_fix_diff(true) + .color(!cfg!(test) && colored::control::SHOULD_COLORIZE.should_colorize()); + let value = DisplayDiagnostics::new(&context, &config, &diagnostics); + write!(f, "{value}")?; + } + OutputFormat::Json => { + let config = config.format(DiagnosticFormat::Json); + let value = DisplayDiagnostics::new(&context, &config, &diagnostics); + write!(f, "{value}")?; + } + OutputFormat::JsonLines => { + let config = config.format(DiagnosticFormat::JsonLines); + let value = DisplayDiagnostics::new(&context, &config, &diagnostics); + write!(f, "{value}")?; + } + OutputFormat::Junit => { + let config = config.format(DiagnosticFormat::Junit); + let value = DisplayDiagnostics::new(&context, &config, &diagnostics); + write!(f, "{value}")?; + } + OutputFormat::Grouped => { + GroupedEmitter::default() + .emit(f, &diagnostics, &context) + .map_err(std::io::Error::other)?; + } + OutputFormat::Github => { + let renderer = GithubRenderer::new(&context, "Ruff"); + let value = DisplayGithubDiagnostics::new(&renderer, &diagnostics); + write!(f, "{value}")?; + } + OutputFormat::Gitlab => { + let config = config.format(DiagnosticFormat::Gitlab); + let value = DisplayDiagnostics::new(&context, &config, &diagnostics); + write!(f, "{value}")?; + } + OutputFormat::Pylint => { + let config = config.format(DiagnosticFormat::Pylint); + let value = DisplayDiagnostics::new(&context, &config, &diagnostics); + write!(f, "{value}")?; + } + OutputFormat::Rdjson => { + let config = config.format(DiagnosticFormat::Rdjson); + let value = DisplayDiagnostics::new(&context, &config, &diagnostics); + write!(f, "{value}")?; + } + OutputFormat::Azure => { + let config = config.format(DiagnosticFormat::Azure); + let value = DisplayDiagnostics::new(&context, &config, &diagnostics); + write!(f, "{value}")?; + } + OutputFormat::Sarif => { + SarifEmitter + .emit(f, &diagnostics, &context) + .map_err(std::io::Error::other)?; + } + } + + Ok(()) + } + /// Write a summary of the formatting results to the given writer. fn write_summary(&self, f: &mut impl Write) -> io::Result<()> { // Compute the number of changed and unchanged files. @@ -573,7 +695,7 @@ impl<'a> FormatResults<'a> { let mut unchanged = 0u32; for result in self.results { match &result.result { - FormatResult::Formatted => { + FormatResult::Formatted { .. } => { changed += 1; } FormatResult::Unchanged => unchanged += 1, @@ -628,6 +750,44 @@ impl<'a> FormatResults<'a> { Ok(()) } } + + /// Convert formatted files into [`Diagnostic`]s. + fn to_diagnostics(&self) -> impl Iterator { + self.results + .iter() + .filter_map(|result| { + let FormatResult::Formatted { + unformatted, + formatted, + } = &result.result + else { + return None; + }; + + let mut diagnostic = Diagnostic::new( + DiagnosticId::Unformatted, + Severity::Error, + "File would be reformatted", + ); + + let path = result.path.to_string_lossy(); + // For now, report the edit as a replacement of the whole file's contents. + let fix = Fix::safe_edit(Edit::range_replacement( + formatted.source_code().to_string(), + TextRange::up_to(unformatted.source_code().text_len()), + )); + + let source_file = SourceFileBuilder::new(path, unformatted.source_code()).finish(); + let span = Span::from(source_file); + let mut annotation = Annotation::primary(span); + annotation.set_file_level(true); + diagnostic.annotate(annotation); + diagnostic.set_fix(fix); + + Some(diagnostic) + }) + .sorted_unstable_by(Diagnostic::ruff_start_ordering) + } } /// An error that can occur while formatting a set of files. diff --git a/crates/ruff/src/commands/format_stdin.rs b/crates/ruff/src/commands/format_stdin.rs index f5b8ea6c603c35..6615cad0336ee5 100644 --- a/crates/ruff/src/commands/format_stdin.rs +++ b/crates/ruff/src/commands/format_stdin.rs @@ -109,7 +109,7 @@ fn format_source_code( let formatted = format_source(&source_kind, source_type, path, settings, range)?; match &formatted { - FormattedSource::Formatted(formatted) => match mode { + FormattedSource::Formatted { formatted, .. } => match mode { FormatMode::Write => { let mut writer = stdout().lock(); formatted diff --git a/crates/ruff/tests/format.rs b/crates/ruff/tests/format.rs index 3b9f266862ff0b..1ddabded60ee9e 100644 --- a/crates/ruff/tests/format.rs +++ b/crates/ruff/tests/format.rs @@ -609,6 +609,61 @@ if __name__ == "__main__": Ok(()) } +#[test_case::test_case("concise")] +#[test_case::test_case("full")] +#[test_case::test_case("json")] +#[test_case::test_case("json-lines")] +#[test_case::test_case("junit")] +#[test_case::test_case("grouped")] +#[test_case::test_case("github")] +#[test_case::test_case("gitlab")] +#[test_case::test_case("pylint")] +#[test_case::test_case("rdjson")] +#[test_case::test_case("azure")] +#[test_case::test_case("sarif")] +fn output_format(output_format: &str) -> Result<()> { + const CONTENT: &str = r#" +from test import say_hy + +if __name__ == "__main__": + say_hy("dear Ruff contributor") +"#; + + let tempdir = TempDir::new()?; + let input = tempdir.path().join("input.py"); + fs::write(&input, CONTENT)?; + + let snapshot = format!("output_format_{output_format}"); + + let project_dir = dunce::canonicalize(tempdir.path())?; + + insta::with_settings!({ + filters => vec![ + (tempdir_filter(&project_dir).as_str(), "[TMP]/"), + (tempdir_filter(&tempdir).as_str(), "[TMP]/"), + (r#""[^"]+\\?/?input.py"#, r#""[TMP]/input.py"#), + (ruff_linter::VERSION, "[VERSION]"), + ] + }, { + assert_cmd_snapshot!( + snapshot, + Command::new(get_cargo_bin(BIN_NAME)) + .args([ + "format", + "--no-cache", + "--output-format", + output_format, + "--preview", + "--check", + "input.py", + ]) + .current_dir(&tempdir), + ); + }); + + Ok(()) +} + #[test] fn exit_non_zero_on_format() -> Result<()> { let tempdir = TempDir::new()?; diff --git a/crates/ruff/tests/snapshots/format__output_format_azure.snap b/crates/ruff/tests/snapshots/format__output_format_azure.snap new file mode 100644 index 00000000000000..4a19f9d50f327e --- /dev/null +++ b/crates/ruff/tests/snapshots/format__output_format_azure.snap @@ -0,0 +1,18 @@ +--- +source: crates/ruff/tests/format.rs +info: + program: ruff + args: + - format + - "--no-cache" + - "--output-format" + - azure + - "--check" + - input.py +--- +success: false +exit_code: 1 +----- stdout ----- +##vso[task.logissue type=error;sourcepath=[TMP]/input.py;code=unformatted;]File would be reformatted + +----- stderr ----- diff --git a/crates/ruff/tests/snapshots/format__output_format_concise.snap b/crates/ruff/tests/snapshots/format__output_format_concise.snap new file mode 100644 index 00000000000000..d377a4abb7f600 --- /dev/null +++ b/crates/ruff/tests/snapshots/format__output_format_concise.snap @@ -0,0 +1,19 @@ +--- +source: crates/ruff/tests/format.rs +info: + program: ruff + args: + - format + - "--no-cache" + - "--output-format" + - concise + - "--check" + - input.py +--- +success: false +exit_code: 1 +----- stdout ----- +input.py: unformatted: File would be reformatted +1 file would be reformatted + +----- stderr ----- diff --git a/crates/ruff/tests/snapshots/format__output_format_full.snap b/crates/ruff/tests/snapshots/format__output_format_full.snap new file mode 100644 index 00000000000000..08341f30f215e3 --- /dev/null +++ b/crates/ruff/tests/snapshots/format__output_format_full.snap @@ -0,0 +1,26 @@ +--- +source: crates/ruff/tests/format.rs +info: + program: ruff + args: + - format + - "--no-cache" + - "--output-format" + - full + - "--preview" + - "--check" + - input.py +--- +success: false +exit_code: 1 +----- stdout ----- +unformatted: File would be reformatted +--> input.py:1:1 + - +1 | from test import say_hy +2 | +3 | if __name__ == "__main__": + +1 file would be reformatted + +----- stderr ----- diff --git a/crates/ruff/tests/snapshots/format__output_format_github.snap b/crates/ruff/tests/snapshots/format__output_format_github.snap new file mode 100644 index 00000000000000..881ec1a7f40be2 --- /dev/null +++ b/crates/ruff/tests/snapshots/format__output_format_github.snap @@ -0,0 +1,18 @@ +--- +source: crates/ruff/tests/format.rs +info: + program: ruff + args: + - format + - "--no-cache" + - "--output-format" + - github + - "--check" + - input.py +--- +success: false +exit_code: 1 +----- stdout ----- +::error title=Ruff (unformatted),file=[TMP]/input.py,line=1,col=1,endLine=1,endColumn=1::input.py:1:1: unformatted: File would be reformatted + +----- stderr ----- diff --git a/crates/ruff/tests/snapshots/format__output_format_gitlab.snap b/crates/ruff/tests/snapshots/format__output_format_gitlab.snap new file mode 100644 index 00000000000000..7008a4aec16b58 --- /dev/null +++ b/crates/ruff/tests/snapshots/format__output_format_gitlab.snap @@ -0,0 +1,37 @@ +--- +source: crates/ruff/tests/format.rs +info: + program: ruff + args: + - format + - "--no-cache" + - "--output-format" + - gitlab + - "--check" + - input.py +--- +success: false +exit_code: 1 +----- stdout ----- +[ + { + "check_name": "unformatted", + "description": "unformatted: File would be reformatted", + "severity": "major", + "fingerprint": "d868d7da11a65fcf", + "location": { + "path": "input.py", + "positions": { + "begin": { + "line": 1, + "column": 1 + }, + "end": { + "line": 1, + "column": 1 + } + } + } + } +] +----- stderr ----- diff --git a/crates/ruff/tests/snapshots/format__output_format_grouped.snap b/crates/ruff/tests/snapshots/format__output_format_grouped.snap new file mode 100644 index 00000000000000..bd9ec0e7e493a2 --- /dev/null +++ b/crates/ruff/tests/snapshots/format__output_format_grouped.snap @@ -0,0 +1,21 @@ +--- +source: crates/ruff/tests/format.rs +info: + program: ruff + args: + - format + - "--no-cache" + - "--output-format" + - grouped + - "--check" + - input.py +--- +success: false +exit_code: 1 +----- stdout ----- +input.py: + 1:1 unformatted: File would be reformatted + +1 file would be reformatted + +----- stderr ----- diff --git a/crates/ruff/tests/snapshots/format__output_format_json-lines.snap b/crates/ruff/tests/snapshots/format__output_format_json-lines.snap new file mode 100644 index 00000000000000..1c915c1d359e4e --- /dev/null +++ b/crates/ruff/tests/snapshots/format__output_format_json-lines.snap @@ -0,0 +1,18 @@ +--- +source: crates/ruff/tests/format.rs +info: + program: ruff + args: + - format + - "--no-cache" + - "--output-format" + - json-lines + - "--check" + - input.py +--- +success: false +exit_code: 1 +----- stdout ----- +{"cell":null,"code":"unformatted","end_location":{"column":1,"row":1},"filename":"[TMP]/input.py","fix":{"applicability":"safe","edits":[{"content":"from test import say_hy\n\nif __name__ == \"__main__\":\n say_hy(\"dear Ruff contributor\")\n","end_location":{"column":1,"row":6},"location":{"column":1,"row":1}}],"message":null},"location":{"column":1,"row":1},"message":"File would be reformatted","noqa_row":null,"url":null} + +----- stderr ----- diff --git a/crates/ruff/tests/snapshots/format__output_format_json.snap b/crates/ruff/tests/snapshots/format__output_format_json.snap new file mode 100644 index 00000000000000..c35d663f5de7d4 --- /dev/null +++ b/crates/ruff/tests/snapshots/format__output_format_json.snap @@ -0,0 +1,51 @@ +--- +source: crates/ruff/tests/format.rs +info: + program: ruff + args: + - format + - "--no-cache" + - "--output-format" + - json + - "--check" + - input.py +--- +success: false +exit_code: 1 +----- stdout ----- +[ + { + "cell": null, + "code": "unformatted", + "end_location": { + "column": 1, + "row": 1 + }, + "filename": "[TMP]/input.py", + "fix": { + "applicability": "safe", + "edits": [ + { + "content": "from test import say_hy\n\nif __name__ == \"__main__\":\n say_hy(\"dear Ruff contributor\")\n", + "end_location": { + "column": 1, + "row": 6 + }, + "location": { + "column": 1, + "row": 1 + } + } + ], + "message": null + }, + "location": { + "column": 1, + "row": 1 + }, + "message": "File would be reformatted", + "noqa_row": null, + "url": null + } +] +----- stderr ----- diff --git a/crates/ruff/tests/snapshots/format__output_format_junit.snap b/crates/ruff/tests/snapshots/format__output_format_junit.snap new file mode 100644 index 00000000000000..f9e48486b0f0d5 --- /dev/null +++ b/crates/ruff/tests/snapshots/format__output_format_junit.snap @@ -0,0 +1,25 @@ +--- +source: crates/ruff/tests/format.rs +info: + program: ruff + args: + - format + - "--no-cache" + - "--output-format" + - junit + - "--check" + - input.py +--- +success: false +exit_code: 1 +----- stdout ----- + + + + + File would be reformatted + + + + +----- stderr ----- diff --git a/crates/ruff/tests/snapshots/format__output_format_pylint.snap b/crates/ruff/tests/snapshots/format__output_format_pylint.snap new file mode 100644 index 00000000000000..7d5f80fed4bf04 --- /dev/null +++ b/crates/ruff/tests/snapshots/format__output_format_pylint.snap @@ -0,0 +1,18 @@ +--- +source: crates/ruff/tests/format.rs +info: + program: ruff + args: + - format + - "--no-cache" + - "--output-format" + - pylint + - "--check" + - input.py +--- +success: false +exit_code: 1 +----- stdout ----- +input.py:1: [unformatted] File would be reformatted + +----- stderr ----- diff --git a/crates/ruff/tests/snapshots/format__output_format_rdjson.snap b/crates/ruff/tests/snapshots/format__output_format_rdjson.snap new file mode 100644 index 00000000000000..52ad2e1e200769 --- /dev/null +++ b/crates/ruff/tests/snapshots/format__output_format_rdjson.snap @@ -0,0 +1,49 @@ +--- +source: crates/ruff/tests/format.rs +info: + program: ruff + args: + - format + - "--no-cache" + - "--output-format" + - rdjson + - "--check" + - input.py +--- +success: false +exit_code: 1 +----- stdout ----- +{ + "diagnostics": [ + { + "code": { + "value": "unformatted" + }, + "location": { + "path": "[TMP]/input.py" + }, + "message": "File would be reformatted", + "suggestions": [ + { + "range": { + "end": { + "column": 1, + "line": 6 + }, + "start": { + "column": 1, + "line": 1 + } + }, + "text": "from test import say_hy\n\nif __name__ == \"__main__\":\n say_hy(\"dear Ruff contributor\")\n" + } + ] + } + ], + "severity": "WARNING", + "source": { + "name": "ruff", + "url": "https://docs.astral.sh/ruff" + } +} +----- stderr ----- diff --git a/crates/ruff/tests/snapshots/format__output_format_sarif.snap b/crates/ruff/tests/snapshots/format__output_format_sarif.snap new file mode 100644 index 00000000000000..5900afc5c8f79f --- /dev/null +++ b/crates/ruff/tests/snapshots/format__output_format_sarif.snap @@ -0,0 +1,83 @@ +--- +source: crates/ruff/tests/format.rs +info: + program: ruff + args: + - format + - "--no-cache" + - "--output-format" + - sarif + - "--check" + - input.py +--- +success: false +exit_code: 1 +----- stdout ----- +{ + "$schema": "https://json.schemastore.org/sarif-2.1.0.json", + "runs": [ + { + "results": [ + { + "fixes": [ + { + "artifactChanges": [ + { + "artifactLocation": { + "uri": "[TMP]/input.py" + }, + "replacements": [ + { + "deletedRegion": { + "endColumn": 1, + "endLine": 6, + "startColumn": 1, + "startLine": 1 + }, + "insertedContent": { + "text": "from test import say_hy\n\nif __name__ == \"__main__\":\n say_hy(\"dear Ruff contributor\")\n" + } + } + ] + } + ], + "description": { + "text": null + } + } + ], + "level": "error", + "locations": [ + { + "physicalLocation": { + "artifactLocation": { + "uri": "[TMP]/input.py" + }, + "region": { + "endColumn": 1, + "endLine": 1, + "startColumn": 1, + "startLine": 1 + } + } + } + ], + "message": { + "text": "File would be reformatted" + }, + "ruleId": "unformatted" + } + ], + "tool": { + "driver": { + "informationUri": "https://github.com/astral-sh/ruff", + "name": "ruff", + "rules": [], + "version": "[VERSION]" + } + } + } + ], + "version": "2.1.0" +} +----- stderr ----- diff --git a/crates/ruff_linter/src/settings/types.rs b/crates/ruff_linter/src/settings/types.rs index a72e80284ad408..a397886e4a92ac 100644 --- a/crates/ruff_linter/src/settings/types.rs +++ b/crates/ruff_linter/src/settings/types.rs @@ -534,6 +534,20 @@ pub enum OutputFormat { Sarif, } +impl OutputFormat { + /// Returns `true` if this format is intended for users to read directly, in contrast to + /// machine-readable or structured formats. + /// + /// This can be used to check whether information beyond the diagnostics, such as a header or + /// `Found N diagnostics` footer, should be included. + pub const fn is_human_readable(&self) -> bool { + matches!( + self, + OutputFormat::Full | OutputFormat::Concise | OutputFormat::Grouped + ) + } +} + impl Display for OutputFormat { fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { match self { From a5ec195c532559dad9d6b8236ac2ec048ec7547e Mon Sep 17 00:00:00 2001 From: Brent Westbrook Date: Wed, 24 Sep 2025 16:16:00 -0400 Subject: [PATCH 09/36] build a real notebook index and render cell diffs --- crates/ruff/src/commands/format.rs | 44 +++++++++++++++++++++----- crates/ruff/tests/format.rs | 51 ++++++++++++++++++++++++++++++ 2 files changed, 87 insertions(+), 8 deletions(-) diff --git a/crates/ruff/src/commands/format.rs b/crates/ruff/src/commands/format.rs index 7bf80200f8f521..94c37c621e36ef 100644 --- a/crates/ruff/src/commands/format.rs +++ b/crates/ruff/src/commands/format.rs @@ -17,6 +17,7 @@ use ruff_db::diagnostic::{ }; use ruff_linter::message::{Emitter, EmitterContext, GroupedEmitter, SarifEmitter}; use ruff_linter::settings::types::OutputFormat; +use ruff_notebook::NotebookIndex; use ruff_python_parser::ParseError; use rustc_hash::{FxHashMap, FxHashSet}; use thiserror::Error; @@ -610,8 +611,8 @@ impl<'a> FormatResults<'a> { f: &mut impl Write, output_format: OutputFormat, ) -> io::Result<()> { - let notebook_index = FxHashMap::default(); - let diagnostics: Vec<_> = self.to_diagnostics().collect(); + let mut notebook_index = FxHashMap::default(); + let diagnostics: Vec<_> = self.to_diagnostics(&mut notebook_index).collect(); let context = EmitterContext::new(¬ebook_index); let config = DisplayDiagnosticConfig::default(); @@ -752,7 +753,10 @@ impl<'a> FormatResults<'a> { } /// Convert formatted files into [`Diagnostic`]s. - fn to_diagnostics(&self) -> impl Iterator { + fn to_diagnostics( + &self, + notebook_index: &mut FxHashMap, + ) -> impl Iterator { self.results .iter() .filter_map(|result| { @@ -771,11 +775,35 @@ impl<'a> FormatResults<'a> { ); let path = result.path.to_string_lossy(); - // For now, report the edit as a replacement of the whole file's contents. - let fix = Fix::safe_edit(Edit::range_replacement( - formatted.source_code().to_string(), - TextRange::up_to(unformatted.source_code().text_len()), - )); + // For now, report the edit as a replacement of the whole file's contents. For + // scripts, this is a single `Edit`, but notebook edits must be split by cell in + // order to render them as diffs. + let fix = if let SourceKind::IpyNotebook(formatted) = formatted + && let SourceKind::IpyNotebook(unformatted) = unformatted + { + notebook_index.insert(path.to_string(), unformatted.index().clone()); + + let mut edits = formatted + .cell_offsets() + .ranges() + .zip(unformatted.cell_offsets().ranges()) + .map(|(formatted_range, unformatted_range)| { + let formatted = &formatted.source_code()[formatted_range]; + Edit::range_replacement(formatted.to_string(), unformatted_range) + }); + + Fix::safe_edits( + edits + .next() + .expect("Formatted files must have at least one edit"), + edits, + ) + } else { + Fix::safe_edit(Edit::range_replacement( + formatted.source_code().to_string(), + TextRange::up_to(unformatted.source_code().text_len()), + )) + }; let source_file = SourceFileBuilder::new(path, unformatted.source_code()).finish(); let span = Span::from(source_file); diff --git a/crates/ruff/tests/format.rs b/crates/ruff/tests/format.rs index 1ddabded60ee9e..b94bf98e948d28 100644 --- a/crates/ruff/tests/format.rs +++ b/crates/ruff/tests/format.rs @@ -664,6 +664,57 @@ if __name__ == "__main__": Ok(()) } +#[test] +fn output_format_notebook() { + let args = ["format", "--no-cache", "--isolated", "--preview", "--check"]; + let fixtures = Path::new("resources").join("test").join("fixtures"); + let path = fixtures.join("unformatted.ipynb"); + insta::with_settings!({filters => vec![ + // Replace windows paths + (r"\\", "/"), + ]}, { + assert_cmd_snapshot!( + Command::new(get_cargo_bin(BIN_NAME)).args(args).arg(path), + @r" + success: false + exit_code: 1 + ----- stdout ----- + unformatted: File would be reformatted + --> resources/test/fixtures/unformatted.ipynb:cell 1:1:1 + ::: cell 1 + 1 | import numpy + - maths = (numpy.arange(100)**2).sum() + - stats= numpy.asarray([1,2,3,4]).median() + 2 + + 3 + maths = (numpy.arange(100) ** 2).sum() + 4 + stats = numpy.asarray([1, 2, 3, 4]).median() + ::: cell 3 + 1 | # A cell with IPython escape command + 2 | def some_function(foo, bar): + 3 | pass + 4 + + 5 + + 6 | %matplotlib inline + ::: cell 4 + 1 | foo = %pwd + - def some_function(foo,bar,): + 2 + + 3 + + 4 + def some_function( + 5 + foo, + 6 + bar, + 7 + ): + 8 | # Another cell with IPython escape command + 9 | foo = %pwd + 10 | print(foo) + + 1 file would be reformatted + + ----- stderr ----- + "); + }); +} + #[test] fn exit_non_zero_on_format() -> Result<()> { let tempdir = TempDir::new()?; From 4ea16b7dd6a0b0f9c2983b27f9cb68fc98e03bc0 Mon Sep 17 00:00:00 2001 From: Brent Westbrook Date: Wed, 24 Sep 2025 11:59:31 -0400 Subject: [PATCH 10/36] convert errors to diagnostics too --- crates/ruff/src/commands/format.rs | 94 +++++++++++++++++++++++++--- crates/ruff_db/src/diagnostic/mod.rs | 14 ++++- 2 files changed, 100 insertions(+), 8 deletions(-) diff --git a/crates/ruff/src/commands/format.rs b/crates/ruff/src/commands/format.rs index 94c37c621e36ef..756e8c3584fb74 100644 --- a/crates/ruff/src/commands/format.rs +++ b/crates/ruff/src/commands/format.rs @@ -194,10 +194,15 @@ pub(crate) fn format( caches.persist()?; // Report on any errors. - errors.sort_unstable_by(|a, b| a.path().cmp(&b.path())); - - for error in &errors { - error!("{error}"); + // + // We only convert errors to `Diagnostic`s in `Check` mode with preview enabled, otherwise we + // fall back on printing simple messages. + if !(preview.is_enabled() && mode.is_check()) { + errors.sort_unstable_by(|a, b| a.path().cmp(&b.path())); + + for error in &errors { + error!("{error}"); + } } let results = FormatResults::new(results.as_slice(), mode); @@ -205,7 +210,7 @@ pub(crate) fn format( FormatMode::Write => {} FormatMode::Check => { if preview.is_enabled() { - results.write_changed_preview(&mut stdout().lock(), output_format)?; + results.write_changed_preview(&mut stdout().lock(), output_format, &errors)?; } else { results.write_changed(&mut stdout().lock())?; } @@ -605,14 +610,22 @@ impl<'a> FormatResults<'a> { Ok(()) } - /// Write a list of the files that would be changed to the given writer. + /// Write a list of the files that would be changed and any errors to the given writer. + /// + /// Errors are reported first in the order they are provided, followed by the remaining + /// diagnostics sorted by file name. fn write_changed_preview( &self, f: &mut impl Write, output_format: OutputFormat, + errors: &[FormatCommandError], ) -> io::Result<()> { let mut notebook_index = FxHashMap::default(); - let diagnostics: Vec<_> = self.to_diagnostics(&mut notebook_index).collect(); + let diagnostics: Vec<_> = errors + .iter() + .map(Diagnostic::from) + .chain(self.to_diagnostics(&mut notebook_index)) + .collect(); let context = EmitterContext::new(¬ebook_index); let config = DisplayDiagnosticConfig::default(); @@ -852,6 +865,73 @@ impl FormatCommandError { } } +impl From<&FormatCommandError> for Diagnostic { + fn from(error: &FormatCommandError) -> Self { + let annotation = error.path().map(|path| { + let file = SourceFileBuilder::new(path.to_string_lossy(), "").finish(); + let span = Span::from(file); + Annotation::primary(span) + }); + + let mut diagnostic = match error { + // TODO(brent) also not sure about this one. Most of the ignore::Error variants + // do fit pretty well under Io errors, but we could match and be even more + // specific if we wanted. We already have a `DiagnosticId::InvalidGlob`, which + // could map to Error::Glob, for example. + FormatCommandError::Ignore(error) => { + Diagnostic::new(DiagnosticId::Io, Severity::Error, error) + } + // TODO(brent) not sure if this is correct, DisplayParseError includes some + // color and other formatting. I think we'd rather have access to the `path` and + // underlying ParseError directly, like in other variants here. + FormatCommandError::Parse(display_parse_error) => Diagnostic::new( + DiagnosticId::InvalidSyntax, + Severity::Error, + display_parse_error, + ), + FormatCommandError::Panic(_, panic_error) => { + Diagnostic::new(DiagnosticId::Panic, Severity::Fatal, panic_error) + } + + // I/O errors + FormatCommandError::Read(_, source_error) + | FormatCommandError::Write(_, source_error) => { + Diagnostic::new(DiagnosticId::Io, Severity::Error, source_error) + } + FormatCommandError::Diff(_, error) => { + Diagnostic::new(DiagnosticId::Io, Severity::Error, error) + } + + FormatCommandError::Format(_, format_module_error) => match format_module_error { + FormatModuleError::ParseError(parse_error) => Diagnostic::new( + DiagnosticId::InvalidSyntax, + Severity::Error, + &parse_error.error, + ), + FormatModuleError::FormatError(format_error) => { + Diagnostic::new(DiagnosticId::FormatError, Severity::Error, format_error) + } + // TODO(brent) this might need a new DiagnosticId, or we could reuse + // `FormatError`, which also has an InvalidDocument variant, like PrintError + FormatModuleError::PrintError(print_error) => { + Diagnostic::new(DiagnosticId::InvalidSyntax, Severity::Error, print_error) + } + }, + FormatCommandError::RangeFormatNotebook(_) => Diagnostic::new( + DiagnosticId::RangeFormatNotebook, + Severity::Error, + "Range formatting isn't supported for notebooks.", + ), + }; + + if let Some(annotation) = annotation { + diagnostic.annotate(annotation); + } + + diagnostic + } +} + impl Display for FormatCommandError { fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { match self { diff --git a/crates/ruff_db/src/diagnostic/mod.rs b/crates/ruff_db/src/diagnostic/mod.rs index b2100403a22379..16484564899658 100644 --- a/crates/ruff_db/src/diagnostic/mod.rs +++ b/crates/ruff_db/src/diagnostic/mod.rs @@ -443,7 +443,9 @@ impl Diagnostic { | DiagnosticId::UnnecessaryOverridesSection | DiagnosticId::UselessOverridesSection | DiagnosticId::DeprecatedSetting - | DiagnosticId::Unformatted => None, + | DiagnosticId::Unformatted + | DiagnosticId::RangeFormatNotebook + | DiagnosticId::FormatError => None, DiagnosticId::Lint(lint_name) => { Some(format!("{}/rules/{lint_name}", env!("CARGO_PKG_HOMEPAGE"))) } @@ -1026,6 +1028,14 @@ pub enum DiagnosticId { /// The code needs to be formatted. Unformatted, + + /// Range formatting isn't supported for notebooks. + RangeFormatNotebook, + + /// Something went wrong while formatting. + /// + /// This often indicates a bug in the formatter. + FormatError, } impl DiagnosticId { @@ -1066,6 +1076,8 @@ impl DiagnosticId { DiagnosticId::UselessOverridesSection => "useless-overrides-section", DiagnosticId::DeprecatedSetting => "deprecated-setting", DiagnosticId::Unformatted => "unformatted", + DiagnosticId::RangeFormatNotebook => "range-format-notebook", + DiagnosticId::FormatError => "format-error", } } From 39f96cc9f006a26c3563c862525dfc21c9bdc09f Mon Sep 17 00:00:00 2001 From: Brent Westbrook Date: Thu, 25 Sep 2025 12:10:46 -0400 Subject: [PATCH 11/36] allow passing a header offset into annotate-snippets as shown in the snapshot changes, this allows us to manually align what annotate-snippets calls the header sigil (the `-->` arrow in the diagnostic header) with our diff line number separators. these were aligned automatically in the linter because we were also emitting snippets with annotate-snippets with the same line numbers, but since the formatter diagnostics only have diffs and not snippets, the default line number width was zero. note that this still isn't perfect because we align with the highest line number in the entire file, not necessarily the highest _rendered_ line number, as shown in the updated notebook snapshot. still, it should get us closer to aligned than not having an offset at all and also end up being correct in most(?) cases. --- crates/ruff/src/commands/format.rs | 35 +++++++++++++++---- crates/ruff/tests/format.rs | 2 +- .../snapshots/format__output_format_full.snap | 2 +- .../src/renderer/display_list.rs | 19 ++++++---- crates/ruff_annotate_snippets/src/snippet.rs | 12 +++++++ crates/ruff_db/src/diagnostic/mod.rs | 7 ++++ crates/ruff_db/src/diagnostic/render.rs | 15 +++++++- 7 files changed, 76 insertions(+), 16 deletions(-) diff --git a/crates/ruff/src/commands/format.rs b/crates/ruff/src/commands/format.rs index 756e8c3584fb74..420a67092603d3 100644 --- a/crates/ruff/src/commands/format.rs +++ b/crates/ruff/src/commands/format.rs @@ -34,7 +34,7 @@ use ruff_linter::source_kind::{SourceError, SourceKind}; use ruff_linter::warn_user_once; use ruff_python_ast::{PySourceType, SourceType}; use ruff_python_formatter::{FormatModuleError, QuoteStyle, format_module_source, format_range}; -use ruff_source_file::{LineIndex, SourceFileBuilder}; +use ruff_source_file::{LineIndex, LineRanges, OneIndexed, SourceFileBuilder}; use ruff_text_size::{TextLen, TextRange, TextSize}; use ruff_workspace::FormatterSettings; use ruff_workspace::resolver::{ResolvedFile, Resolver, match_exclusion, python_files_in_path}; @@ -791,7 +791,15 @@ impl<'a> FormatResults<'a> { // For now, report the edit as a replacement of the whole file's contents. For // scripts, this is a single `Edit`, but notebook edits must be split by cell in // order to render them as diffs. - let fix = if let SourceKind::IpyNotebook(formatted) = formatted + // + // We also attempt to estimate the line number width for aligning the + // annotate-snippets header. This is only an estimate because we don't actually know + // if the maximum line number present in the document will be rendered as part of + // the diff, either as a changed line or as an unchanged context line. For + // notebooks, we refine our estimate by checking the number of lines in each cell + // individually, otherwise we could use `formatted.source_code().count_lines(...)` + // in both cases. + let (fix, line_count) = if let SourceKind::IpyNotebook(formatted) = formatted && let SourceKind::IpyNotebook(unformatted) = unformatted { notebook_index.insert(path.to_string(), unformatted.index().clone()); @@ -805,17 +813,29 @@ impl<'a> FormatResults<'a> { Edit::range_replacement(formatted.to_string(), unformatted_range) }); - Fix::safe_edits( + let fix = Fix::safe_edits( edits .next() .expect("Formatted files must have at least one edit"), edits, - ) + ); + let source = formatted.source_code(); + let line_count = formatted + .cell_offsets() + .ranges() + .map(|range| source.count_lines(range)) + .max() + .unwrap_or_default(); + (fix, line_count) } else { - Fix::safe_edit(Edit::range_replacement( + let fix = Fix::safe_edit(Edit::range_replacement( formatted.source_code().to_string(), TextRange::up_to(unformatted.source_code().text_len()), - )) + )); + let line_count = formatted + .source_code() + .count_lines(TextRange::up_to(formatted.source_code().text_len())); + (fix, line_count) }; let source_file = SourceFileBuilder::new(path, unformatted.source_code()).finish(); @@ -825,6 +845,9 @@ impl<'a> FormatResults<'a> { diagnostic.annotate(annotation); diagnostic.set_fix(fix); + let lines = OneIndexed::new(line_count as usize).unwrap_or_default(); + diagnostic.set_header_offset(lines.digits().get()); + Some(diagnostic) }) .sorted_unstable_by(Diagnostic::ruff_start_ordering) diff --git a/crates/ruff/tests/format.rs b/crates/ruff/tests/format.rs index b94bf98e948d28..39fe3bdd898c85 100644 --- a/crates/ruff/tests/format.rs +++ b/crates/ruff/tests/format.rs @@ -680,7 +680,7 @@ fn output_format_notebook() { exit_code: 1 ----- stdout ----- unformatted: File would be reformatted - --> resources/test/fixtures/unformatted.ipynb:cell 1:1:1 + --> resources/test/fixtures/unformatted.ipynb:cell 1:1:1 ::: cell 1 1 | import numpy - maths = (numpy.arange(100)**2).sum() diff --git a/crates/ruff/tests/snapshots/format__output_format_full.snap b/crates/ruff/tests/snapshots/format__output_format_full.snap index 08341f30f215e3..30eb3a0a51d64a 100644 --- a/crates/ruff/tests/snapshots/format__output_format_full.snap +++ b/crates/ruff/tests/snapshots/format__output_format_full.snap @@ -15,7 +15,7 @@ success: false exit_code: 1 ----- stdout ----- unformatted: File would be reformatted ---> input.py:1:1 + --> input.py:1:1 - 1 | from test import say_hy 2 | diff --git a/crates/ruff_annotate_snippets/src/renderer/display_list.rs b/crates/ruff_annotate_snippets/src/renderer/display_list.rs index 2728cfba7bfb98..fb88a262231fb0 100644 --- a/crates/ruff_annotate_snippets/src/renderer/display_list.rs +++ b/crates/ruff_annotate_snippets/src/renderer/display_list.rs @@ -56,6 +56,7 @@ pub(crate) struct DisplayList<'a> { pub(crate) stylesheet: &'a Stylesheet, pub(crate) anonymized_line_numbers: bool, pub(crate) cut_indicator: &'static str, + pub(crate) lineno_offset: usize, } impl PartialEq for DisplayList<'_> { @@ -81,13 +82,14 @@ impl Display for DisplayList<'_> { _ => max, }) }); - let lineno_width = if lineno_width == 0 { - lineno_width - } else if self.anonymized_line_numbers { - ANONYMIZED_LINE_NUM.len() - } else { - ((lineno_width as f64).log10().floor() as usize) + 1 - }; + let lineno_width = self.lineno_offset + + if lineno_width == 0 { + lineno_width + } else if self.anonymized_line_numbers { + ANONYMIZED_LINE_NUM.len() + } else { + ((lineno_width as f64).log10().floor() as usize) + 1 + }; let multiline_depth = self.body.iter().fold(0, |max, set| { set.display_lines.iter().fold(max, |max2, line| match line { @@ -124,6 +126,7 @@ impl<'a> DisplayList<'a> { term_width: usize, cut_indicator: &'static str, ) -> DisplayList<'a> { + let lineno_offset = message.lineno_offset; let body = format_message( message, term_width, @@ -137,6 +140,7 @@ impl<'a> DisplayList<'a> { stylesheet, anonymized_line_numbers, cut_indicator, + lineno_offset, } } @@ -1088,6 +1092,7 @@ fn format_message<'m>( footer, snippets, is_fixable, + lineno_offset: _, } = message; let mut sets = vec![]; diff --git a/crates/ruff_annotate_snippets/src/snippet.rs b/crates/ruff_annotate_snippets/src/snippet.rs index 339cd3a8505eae..4fca75c571742b 100644 --- a/crates/ruff_annotate_snippets/src/snippet.rs +++ b/crates/ruff_annotate_snippets/src/snippet.rs @@ -23,6 +23,7 @@ pub struct Message<'a> { pub(crate) snippets: Vec>, pub(crate) footer: Vec>, pub(crate) is_fixable: bool, + pub(crate) lineno_offset: usize, } impl<'a> Message<'a> { @@ -59,6 +60,16 @@ impl<'a> Message<'a> { self.is_fixable = yes; self } + + /// Add an offset used for aligning the header sigil (`-->`) with the line number separators. + /// + /// For normal diagnostics this is computed automatically based on the lines to be rendered. + /// This is intended only for use in the formatter, where we don't render a snippet directly but + /// still want the header to align with the diff. + pub fn lineno_offset(mut self, offset: usize) -> Self { + self.lineno_offset = offset; + self + } } /// Structure containing the slice of text to be annotated and @@ -173,6 +184,7 @@ impl Level { snippets: vec![], footer: vec![], is_fixable: false, + lineno_offset: 0, } } diff --git a/crates/ruff_db/src/diagnostic/mod.rs b/crates/ruff_db/src/diagnostic/mod.rs index 16484564899658..1eab12d03634f4 100644 --- a/crates/ruff_db/src/diagnostic/mod.rs +++ b/crates/ruff_db/src/diagnostic/mod.rs @@ -69,6 +69,7 @@ impl Diagnostic { parent: None, noqa_offset: None, secondary_code: None, + header_offset: 0, }); Diagnostic { inner } } @@ -521,6 +522,11 @@ impl Diagnostic { a.cmp(&b) } + + /// Add an offset for aligning the header sigil with the line number separators in a diff. + pub fn set_header_offset(&mut self, offset: usize) { + Arc::make_mut(&mut self.inner).header_offset = offset; + } } #[derive(Debug, Clone, Eq, PartialEq, Hash, get_size2::GetSize)] @@ -534,6 +540,7 @@ struct DiagnosticInner { parent: Option, noqa_offset: Option, secondary_code: Option, + header_offset: usize, } struct RenderingSortKey<'a> { diff --git a/crates/ruff_db/src/diagnostic/render.rs b/crates/ruff_db/src/diagnostic/render.rs index 2ab885cdc59459..785329af9cf350 100644 --- a/crates/ruff_db/src/diagnostic/render.rs +++ b/crates/ruff_db/src/diagnostic/render.rs @@ -208,6 +208,7 @@ struct ResolvedDiagnostic<'a> { message: String, annotations: Vec>, is_fixable: bool, + header_offset: usize, } impl<'a> ResolvedDiagnostic<'a> { @@ -259,6 +260,7 @@ impl<'a> ResolvedDiagnostic<'a> { message: diag.inner.message.as_str().to_string(), annotations, is_fixable: config.show_fix_status && diag.has_applicable_fix(config), + header_offset: diag.inner.header_offset, } } @@ -288,6 +290,7 @@ impl<'a> ResolvedDiagnostic<'a> { message: diag.inner.message.as_str().to_string(), annotations, is_fixable: false, + header_offset: 0, } } @@ -385,6 +388,7 @@ impl<'a> ResolvedDiagnostic<'a> { message: &self.message, snippets_by_input, is_fixable: self.is_fixable, + header_offset: self.header_offset, } } } @@ -492,6 +496,11 @@ struct RenderableDiagnostic<'r> { /// /// This is rendered as a `[*]` indicator after the diagnostic ID. is_fixable: bool, + /// Offset to align the header sigil (`-->`) with the subsequent line number separators. + /// + /// This is only needed for formatter diagnostics where we don't render a snippet via + /// `annotate-snippets` and thus the alignment isn't computed automatically. + header_offset: usize, } impl RenderableDiagnostic<'_> { @@ -504,7 +513,11 @@ impl RenderableDiagnostic<'_> { .iter() .map(|snippet| snippet.to_annotate(path)) }); - let mut message = self.level.title(self.message).is_fixable(self.is_fixable); + let mut message = self + .level + .title(self.message) + .is_fixable(self.is_fixable) + .lineno_offset(self.header_offset); if let Some(id) = self.id { message = message.id(id); } From a43fa9324925c4c44515eff9568ba7cf73a5f253 Mon Sep 17 00:00:00 2001 From: Brent Westbrook Date: Thu, 25 Sep 2025 13:30:26 -0400 Subject: [PATCH 12/36] factor out PanicError::to_diagnostic_message --- crates/ruff_db/src/panic.rs | 23 +++++++++++++++++++++++ crates/ty_project/src/lib.rs | 19 +------------------ 2 files changed, 24 insertions(+), 18 deletions(-) diff --git a/crates/ruff_db/src/panic.rs b/crates/ruff_db/src/panic.rs index a6b67f1f1de0b9..d91c1f7fe7a847 100644 --- a/crates/ruff_db/src/panic.rs +++ b/crates/ruff_db/src/panic.rs @@ -31,6 +31,29 @@ impl Payload { } } +impl PanicError { + pub fn to_diagnostic_message(&self, path: Option) -> String { + use std::fmt::Write; + + let mut message = String::new(); + message.push_str("Panicked"); + + if let Some(location) = &self.location { + let _ = write!(&mut message, " at {location}"); + } + + if let Some(path) = path { + let _ = write!(&mut message, " when checking `{path}`"); + } + + if let Some(payload) = self.payload.as_str() { + let _ = write!(&mut message, ": `{payload}`"); + } + + message + } +} + impl std::fmt::Display for PanicError { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { write!(f, "panicked at")?; diff --git a/crates/ty_project/src/lib.rs b/crates/ty_project/src/lib.rs index b98917dbf5c4af..f5ef2201da3f0a 100644 --- a/crates/ty_project/src/lib.rs +++ b/crates/ty_project/src/lib.rs @@ -666,24 +666,7 @@ where }) { Ok(result) => Ok(result), Err(error) => { - use std::fmt::Write; - let mut message = String::new(); - message.push_str("Panicked"); - - if let Some(location) = error.location { - let _ = write!(&mut message, " at {location}"); - } - - let _ = write!( - &mut message, - " when checking `{file}`", - file = file.path(db) - ); - - if let Some(payload) = error.payload.as_str() { - let _ = write!(&mut message, ": `{payload}`"); - } - + let message = error.to_diagnostic_message(Some(file.path(db))); let mut diagnostic = Diagnostic::new(DiagnosticId::Panic, Severity::Fatal, message); diagnostic.sub(SubDiagnostic::new( SubDiagnosticSeverity::Info, From 959f55ae057843f8b3fe59a23577f5ae3948a903 Mon Sep 17 00:00:00 2001 From: Brent Westbrook Date: Thu, 25 Sep 2025 13:31:21 -0400 Subject: [PATCH 13/36] add error tests, set_file_level, and improve panic formatting --- crates/ruff/src/commands/format.rs | 140 ++++++++++++++++++++++++++++- 1 file changed, 136 insertions(+), 4 deletions(-) diff --git a/crates/ruff/src/commands/format.rs b/crates/ruff/src/commands/format.rs index 420a67092603d3..3650219eeffe36 100644 --- a/crates/ruff/src/commands/format.rs +++ b/crates/ruff/src/commands/format.rs @@ -13,7 +13,8 @@ use rayon::iter::Either::{Left, Right}; use rayon::iter::{IntoParallelRefIterator, ParallelIterator}; use ruff_db::diagnostic::{ Annotation, Diagnostic, DiagnosticFormat, DiagnosticId, DisplayDiagnosticConfig, - DisplayDiagnostics, DisplayGithubDiagnostics, GithubRenderer, Severity, Span, + DisplayDiagnostics, DisplayGithubDiagnostics, GithubRenderer, Severity, Span, SubDiagnostic, + SubDiagnosticSeverity, }; use ruff_linter::message::{Emitter, EmitterContext, GroupedEmitter, SarifEmitter}; use ruff_linter::settings::types::OutputFormat; @@ -893,7 +894,9 @@ impl From<&FormatCommandError> for Diagnostic { let annotation = error.path().map(|path| { let file = SourceFileBuilder::new(path.to_string_lossy(), "").finish(); let span = Span::from(file); - Annotation::primary(span) + let mut annotation = Annotation::primary(span); + annotation.set_file_level(true); + annotation }); let mut diagnostic = match error { @@ -912,8 +915,24 @@ impl From<&FormatCommandError> for Diagnostic { Severity::Error, display_parse_error, ), - FormatCommandError::Panic(_, panic_error) => { - Diagnostic::new(DiagnosticId::Panic, Severity::Fatal, panic_error) + FormatCommandError::Panic(path, panic_error) => { + let mut diagnostic = Diagnostic::new( + DiagnosticId::Panic, + Severity::Fatal, + panic_error.to_diagnostic_message(path.as_ref().map(|path| path.display())), + ); + diagnostic.sub(SubDiagnostic::new( + SubDiagnosticSeverity::Info, + "This indicates a bug in Ruff.", + )); + let report_message = "If you could open an issue at \ + https://github.com/astral-sh/ruff/issues/new?title=%5Bpanic%5D, \ + we'd be very appreciative!"; + diagnostic.sub(SubDiagnostic::new( + SubDiagnosticSeverity::Info, + report_message, + )); + diagnostic } // I/O errors @@ -1254,3 +1273,116 @@ pub(super) fn warn_incompatible_formatter_settings(resolver: &Resolver) { } } } + +#[cfg(test)] +mod tests { + use std::io; + use std::path::PathBuf; + + use ignore::Error; + use insta::assert_snapshot; + + use ruff_db::panic::catch_unwind; + use ruff_linter::logging::DisplayParseError; + use ruff_linter::source_kind::{SourceError, SourceKind}; + use ruff_python_formatter::FormatModuleError; + use ruff_python_parser::{ParseError, ParseErrorType}; + use ruff_text_size::TextRange; + + use crate::commands::format::{FormatCommandError, FormatMode, FormatResults}; + + #[test] + fn error_diagnostics() -> anyhow::Result<()> { + let path = PathBuf::from("test.py"); + let source_kind = SourceKind::Python("1".to_string()); + + let panic_error = catch_unwind(|| { + panic!("Test panic for FormatCommandError"); + }) + .unwrap_err(); + + let errors = [ + FormatCommandError::Ignore(Error::WithPath { + path: path.clone(), + err: Box::new(Error::Io(io::Error::new( + io::ErrorKind::PermissionDenied, + "Permission denied", + ))), + }), + FormatCommandError::Parse(DisplayParseError::from_source_kind( + ParseError { + error: ParseErrorType::UnexpectedIndentation, + location: TextRange::default(), + }, + Some(path.clone()), + &source_kind, + )), + FormatCommandError::Panic(Some(path.clone()), Box::new(panic_error)), + FormatCommandError::Read( + Some(path.clone()), + SourceError::Io(io::Error::new(io::ErrorKind::NotFound, "File not found")), + ), + FormatCommandError::Format( + Some(path.clone()), + FormatModuleError::ParseError(ParseError { + error: ParseErrorType::EmptySlice, + location: TextRange::default(), + }), + ), + FormatCommandError::Write( + Some(path.clone()), + SourceError::Io(io::Error::new( + io::ErrorKind::PermissionDenied, + "Cannot write to file", + )), + ), + FormatCommandError::Diff( + Some(path.clone()), + io::Error::other("Failed to generate diff"), + ), + FormatCommandError::RangeFormatNotebook(Some(path)), + ]; + + let results = FormatResults::new(&[], FormatMode::Check); + let mut buf = Vec::new(); + results.write_changed_preview( + &mut buf, + ruff_linter::settings::types::OutputFormat::Full, + &errors, + )?; + + let mut settings = insta::Settings::clone_current(); + settings.add_filter(r"(Panicked at) [^:]+:\d+:\d+", "$1 "); + let _s = settings.bind_to_scope(); + + assert_snapshot!(str::from_utf8(&buf)?, @r" + io: test.py: Permission denied + --> test.py:1:1 + + invalid-syntax: Failed to parse test.py:1:1: Unexpected indentation + --> test.py:1:1 + + panic: Panicked at when checking `test.py`: `Test panic for FormatCommandError` + --> test.py:1:1 + info: This indicates a bug in Ruff. + info: If you could open an issue at https://github.com/astral-sh/ruff/issues/new?title=%5Bpanic%5D, we'd be very appreciative! + + io: File not found + --> test.py:1:1 + + invalid-syntax: Expected index or slice expression + --> test.py:1:1 + + io: Cannot write to file + --> test.py:1:1 + + io: Failed to generate diff + --> test.py:1:1 + + range-format-notebook: Range formatting isn't supported for notebooks. + --> test.py:1:1 + "); + + Ok(()) + } +} From 2ce8f808770d64591f76f25fc1e5f5728599ffe9 Mon Sep 17 00:00:00 2001 From: Brent Westbrook Date: Fri, 26 Sep 2025 14:00:00 -0400 Subject: [PATCH 14/36] use FormatResult::diff for checking, revert other changes to FormatResult and to FormattedSource --- crates/ruff/src/commands/format.rs | 58 +++++++----------------- crates/ruff/src/commands/format_stdin.rs | 2 +- 2 files changed, 17 insertions(+), 43 deletions(-) diff --git a/crates/ruff/src/commands/format.rs b/crates/ruff/src/commands/format.rs index 3650219eeffe36..023470a614bfc5 100644 --- a/crates/ruff/src/commands/format.rs +++ b/crates/ruff/src/commands/format.rs @@ -295,10 +295,7 @@ pub(crate) fn format_path( // Format the source. let format_result = match format_source(&unformatted, source_type, Some(path), settings, range)? { - FormattedSource::Formatted { - formatted, - unformatted, - } => match mode { + FormattedSource::Formatted(formatted) => match mode { FormatMode::Write => { let mut writer = File::create(path).map_err(|err| { FormatCommandError::Write(Some(path.to_path_buf()), err.into()) @@ -316,16 +313,9 @@ pub(crate) fn format_path( } } - FormatResult::Formatted { - unformatted, - formatted, - } + FormatResult::Formatted } - FormatMode::Check => FormatResult::Formatted { - unformatted, - formatted, - }, - FormatMode::Diff => FormatResult::Diff { + FormatMode::Check | FormatMode::Diff => FormatResult::Diff { unformatted, formatted, }, @@ -350,10 +340,7 @@ pub(crate) fn format_path( #[derive(Debug)] pub(crate) enum FormattedSource { /// The source was formatted, and the [`SourceKind`] contains the transformed source code. - Formatted { - formatted: SourceKind, - unformatted: SourceKind, - }, + Formatted(SourceKind), /// The source was unchanged. Unchanged, } @@ -361,13 +348,7 @@ pub(crate) enum FormattedSource { impl From for FormatResult { fn from(value: FormattedSource) -> Self { match value { - FormattedSource::Formatted { - formatted, - unformatted, - } => FormatResult::Formatted { - formatted, - unformatted, - }, + FormattedSource::Formatted { .. } => FormatResult::Formatted, FormattedSource::Unchanged => FormatResult::Unchanged, } } @@ -420,10 +401,7 @@ pub(crate) fn format_source( if formatted.len() == unformatted.len() && formatted == *unformatted { Ok(FormattedSource::Unchanged) } else { - Ok(FormattedSource::Formatted { - formatted: SourceKind::Python(formatted), - unformatted: SourceKind::Python(unformatted.to_string()), - }) + Ok(FormattedSource::Formatted(SourceKind::Python(formatted))) } } SourceKind::IpyNotebook(notebook) => { @@ -508,10 +486,9 @@ pub(crate) fn format_source( let mut formatted = notebook.clone(); formatted.update(&source_map, output); - Ok(FormattedSource::Formatted { - formatted: SourceKind::IpyNotebook(formatted), - unformatted: SourceKind::IpyNotebook(notebook.clone()), - }) + Ok(FormattedSource::Formatted(SourceKind::IpyNotebook( + formatted, + ))) } } } @@ -519,13 +496,10 @@ pub(crate) fn format_source( /// The result of an individual formatting operation. #[derive(Debug, Clone, is_macro::Is)] pub(crate) enum FormatResult { - /// The file was formatted. - Formatted { - unformatted: SourceKind, - formatted: SourceKind, - }, + /// The file was formatted and written back to disk. + Formatted, - /// The file was formatted, [`SourceKind`] contains the formatted code + /// The file needs to be formatted, as the `formatted` and `unformatted` contents differ. Diff { unformatted: SourceKind, formatted: SourceKind, @@ -562,7 +536,7 @@ impl<'a> FormatResults<'a> { /// Returns `true` if any of the files require formatting. fn any_formatted(&self) -> bool { self.results.iter().any(|result| match result.result { - FormatResult::Formatted { .. } | FormatResult::Diff { .. } => true, + FormatResult::Formatted | FormatResult::Diff { .. } => true, FormatResult::Unchanged | FormatResult::Skipped => false, }) } @@ -597,7 +571,7 @@ impl<'a> FormatResults<'a> { .results .iter() .filter_map(|result| { - if result.result.is_formatted() { + if result.result.is_diff() { Some(result.path.as_path()) } else { None @@ -710,7 +684,7 @@ impl<'a> FormatResults<'a> { let mut unchanged = 0u32; for result in self.results { match &result.result { - FormatResult::Formatted { .. } => { + FormatResult::Formatted => { changed += 1; } FormatResult::Unchanged => unchanged += 1, @@ -774,7 +748,7 @@ impl<'a> FormatResults<'a> { self.results .iter() .filter_map(|result| { - let FormatResult::Formatted { + let FormatResult::Diff { unformatted, formatted, } = &result.result diff --git a/crates/ruff/src/commands/format_stdin.rs b/crates/ruff/src/commands/format_stdin.rs index 6615cad0336ee5..f5b8ea6c603c35 100644 --- a/crates/ruff/src/commands/format_stdin.rs +++ b/crates/ruff/src/commands/format_stdin.rs @@ -109,7 +109,7 @@ fn format_source_code( let formatted = format_source(&source_kind, source_type, path, settings, range)?; match &formatted { - FormattedSource::Formatted { formatted, .. } => match mode { + FormattedSource::Formatted(formatted) => match mode { FormatMode::Write => { let mut writer = stdout().lock(); formatted From 007a22f1c02fac304aa9f5451c4489de1a271cc0 Mon Sep 17 00:00:00 2001 From: Brent Westbrook Date: Fri, 26 Sep 2025 14:05:45 -0400 Subject: [PATCH 15/36] point out preview restriction on format --output-format --- crates/ruff/src/args.rs | 5 ++++- docs/configuration.md | 8 ++++---- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/crates/ruff/src/args.rs b/crates/ruff/src/args.rs index 2e4817836e5d3f..160d6a19b8f255 100644 --- a/crates/ruff/src/args.rs +++ b/crates/ruff/src/args.rs @@ -538,8 +538,11 @@ pub struct FormatCommand { #[arg(long, help_heading = "Miscellaneous", alias = "exit-non-zero-on-fix")] pub exit_non_zero_on_format: bool, - /// Output serialization format for violations. + /// Output serialization format for violations, when used with `--check`. /// The default serialization format is "full". + /// + /// Note that this option is currently only respected in preview mode. A warning will be emitted + /// if this flag is used on stable. #[arg(long, value_enum, env = "RUFF_OUTPUT_FORMAT")] pub output_format: Option, } diff --git a/docs/configuration.md b/docs/configuration.md index 13aee26da96ab0..8d3297fbcab8e1 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -728,10 +728,10 @@ Options: Enable preview mode; enables unstable formatting. Use `--no-preview` to disable --output-format - Output serialization format for violations. The default serialization - format is "full" [env: RUFF_OUTPUT_FORMAT=] [possible values: - concise, full, json, json-lines, junit, grouped, github, gitlab, - pylint, rdjson, azure, sarif] + Output serialization format for violations, when used with `--check`. + The default serialization format is "full" [env: RUFF_OUTPUT_FORMAT=] + [possible values: concise, full, json, json-lines, junit, grouped, + github, gitlab, pylint, rdjson, azure, sarif] -h, --help Print help (see more with '--help') From 451dc0c432cab342c5ab09f8d6aedb46786cedc7 Mon Sep 17 00:00:00 2001 From: Brent Westbrook Date: Fri, 26 Sep 2025 14:26:46 -0400 Subject: [PATCH 16/36] emit a warning for non-preview --output-format usage as a global option, preview comes from the resolved settings, so I just moved the pyproject.toml resolution outside of `format` and `format_stdin` since it was called immediately in both cases anyway. note that this won't warn for usage of the `output-format` setting within a pyproject.toml because that option will already have been `unwrap_or_default`ed by the time we could check it here. --- crates/ruff/src/commands/format.rs | 9 +++++---- crates/ruff/src/commands/format_stdin.rs | 8 +++----- crates/ruff/src/lib.rs | 12 +++++++++--- crates/ruff/tests/format.rs | 18 ++++++++++++++++++ 4 files changed, 35 insertions(+), 12 deletions(-) diff --git a/crates/ruff/src/commands/format.rs b/crates/ruff/src/commands/format.rs index 023470a614bfc5..7a1788b595ff92 100644 --- a/crates/ruff/src/commands/format.rs +++ b/crates/ruff/src/commands/format.rs @@ -38,11 +38,12 @@ use ruff_python_formatter::{FormatModuleError, QuoteStyle, format_module_source, use ruff_source_file::{LineIndex, LineRanges, OneIndexed, SourceFileBuilder}; use ruff_text_size::{TextLen, TextRange, TextSize}; use ruff_workspace::FormatterSettings; -use ruff_workspace::resolver::{ResolvedFile, Resolver, match_exclusion, python_files_in_path}; +use ruff_workspace::resolver::{ + PyprojectConfig, ResolvedFile, Resolver, match_exclusion, python_files_in_path, +}; use crate::args::{ConfigArguments, FormatArguments, FormatRange}; use crate::cache::{Cache, FileCacheKey, PackageCacheMap, PackageCaches}; -use crate::resolve::resolve; use crate::{ExitStatus, resolve_default_files}; #[derive(Debug, Copy, Clone, is_macro::Is)] @@ -71,11 +72,11 @@ impl FormatMode { pub(crate) fn format( cli: FormatArguments, config_arguments: &ConfigArguments, + pyproject_config: &PyprojectConfig, ) -> Result { - let pyproject_config = resolve(config_arguments, cli.stdin_filename.as_deref())?; let mode = FormatMode::from_cli(&cli); let files = resolve_default_files(cli.files, false); - let (paths, resolver) = python_files_in_path(&files, &pyproject_config, config_arguments)?; + let (paths, resolver) = python_files_in_path(&files, pyproject_config, config_arguments)?; let output_format = pyproject_config.settings.output_format; let preview = pyproject_config.settings.formatter.preview; diff --git a/crates/ruff/src/commands/format_stdin.rs b/crates/ruff/src/commands/format_stdin.rs index f5b8ea6c603c35..d5b16f1b3f5d70 100644 --- a/crates/ruff/src/commands/format_stdin.rs +++ b/crates/ruff/src/commands/format_stdin.rs @@ -7,7 +7,7 @@ use log::error; use ruff_linter::source_kind::SourceKind; use ruff_python_ast::{PySourceType, SourceType}; use ruff_workspace::FormatterSettings; -use ruff_workspace::resolver::{Resolver, match_exclusion, python_file_at_path}; +use ruff_workspace::resolver::{PyprojectConfig, Resolver, match_exclusion, python_file_at_path}; use crate::ExitStatus; use crate::args::{ConfigArguments, FormatArguments, FormatRange}; @@ -15,17 +15,15 @@ use crate::commands::format::{ FormatCommandError, FormatMode, FormatResult, FormattedSource, format_source, warn_incompatible_formatter_settings, }; -use crate::resolve::resolve; use crate::stdin::{parrot_stdin, read_from_stdin}; /// Run the formatter over a single file, read from `stdin`. pub(crate) fn format_stdin( cli: &FormatArguments, config_arguments: &ConfigArguments, + pyproject_config: &PyprojectConfig, ) -> Result { - let pyproject_config = resolve(config_arguments, cli.stdin_filename.as_deref())?; - - let mut resolver = Resolver::new(&pyproject_config); + let mut resolver = Resolver::new(pyproject_config); warn_incompatible_formatter_settings(&resolver); let mode = FormatMode::from_cli(cli); diff --git a/crates/ruff/src/lib.rs b/crates/ruff/src/lib.rs index 07b33fb71c5575..3bd457de8c9425 100644 --- a/crates/ruff/src/lib.rs +++ b/crates/ruff/src/lib.rs @@ -205,12 +205,18 @@ pub fn run( } fn format(args: FormatCommand, global_options: GlobalConfigArgs) -> Result { + let cli_output_format_set = args.output_format.is_some(); let (cli, config_arguments) = args.partition(global_options)?; - + let pyproject_config = resolve::resolve(&config_arguments, cli.stdin_filename.as_deref())?; + if cli_output_format_set && !pyproject_config.settings.formatter.preview.is_enabled() { + warn_user_once!( + "The --output-format flag for the formatter is unstable and requires preview mode to use." + ); + } if is_stdin(&cli.files, cli.stdin_filename.as_deref()) { - commands::format_stdin::format_stdin(&cli, &config_arguments) + commands::format_stdin::format_stdin(&cli, &config_arguments, &pyproject_config) } else { - commands::format::format(cli, &config_arguments) + commands::format::format(cli, &config_arguments, &pyproject_config) } } diff --git a/crates/ruff/tests/format.rs b/crates/ruff/tests/format.rs index 39fe3bdd898c85..06112d92f281ab 100644 --- a/crates/ruff/tests/format.rs +++ b/crates/ruff/tests/format.rs @@ -2461,3 +2461,21 @@ fn cookiecutter_globbing() -> Result<()> { Ok(()) } + +#[test] +fn stable_output_format_warning() { + assert_cmd_snapshot!( + Command::new(get_cargo_bin(BIN_NAME)) + .args(["format", "--output-format=full", "-"]) + .pass_stdin("1"), + @r" + success: true + exit_code: 0 + ----- stdout ----- + 1 + + ----- stderr ----- + warning: The --output-format flag for the formatter is unstable and requires preview mode to use. + ", + ) +} From 78fdc60d5c760c5fe22cdf1cd5c10cdb4fc9ac0b Mon Sep 17 00:00:00 2001 From: Brent Westbrook Date: Fri, 26 Sep 2025 14:36:16 -0400 Subject: [PATCH 17/36] sort errors and diagnostics together --- crates/ruff/src/commands/format.rs | 167 ++++++++++++++--------------- 1 file changed, 81 insertions(+), 86 deletions(-) diff --git a/crates/ruff/src/commands/format.rs b/crates/ruff/src/commands/format.rs index 7a1788b595ff92..7e0ecee87488b6 100644 --- a/crates/ruff/src/commands/format.rs +++ b/crates/ruff/src/commands/format.rs @@ -587,9 +587,6 @@ impl<'a> FormatResults<'a> { } /// Write a list of the files that would be changed and any errors to the given writer. - /// - /// Errors are reported first in the order they are provided, followed by the remaining - /// diagnostics sorted by file name. fn write_changed_preview( &self, f: &mut impl Write, @@ -601,6 +598,7 @@ impl<'a> FormatResults<'a> { .iter() .map(Diagnostic::from) .chain(self.to_diagnostics(&mut notebook_index)) + .sorted_unstable_by(Diagnostic::ruff_start_ordering) .collect(); let context = EmitterContext::new(¬ebook_index); @@ -746,87 +744,84 @@ impl<'a> FormatResults<'a> { &self, notebook_index: &mut FxHashMap, ) -> impl Iterator { - self.results - .iter() - .filter_map(|result| { - let FormatResult::Diff { - unformatted, - formatted, - } = &result.result - else { - return None; - }; + self.results.iter().filter_map(|result| { + let FormatResult::Diff { + unformatted, + formatted, + } = &result.result + else { + return None; + }; - let mut diagnostic = Diagnostic::new( - DiagnosticId::Unformatted, - Severity::Error, - "File would be reformatted", + let mut diagnostic = Diagnostic::new( + DiagnosticId::Unformatted, + Severity::Error, + "File would be reformatted", + ); + + let path = result.path.to_string_lossy(); + // For now, report the edit as a replacement of the whole file's contents. For + // scripts, this is a single `Edit`, but notebook edits must be split by cell in + // order to render them as diffs. + // + // We also attempt to estimate the line number width for aligning the + // annotate-snippets header. This is only an estimate because we don't actually know + // if the maximum line number present in the document will be rendered as part of + // the diff, either as a changed line or as an unchanged context line. For + // notebooks, we refine our estimate by checking the number of lines in each cell + // individually, otherwise we could use `formatted.source_code().count_lines(...)` + // in both cases. + let (fix, line_count) = if let SourceKind::IpyNotebook(formatted) = formatted + && let SourceKind::IpyNotebook(unformatted) = unformatted + { + notebook_index.insert(path.to_string(), unformatted.index().clone()); + + let mut edits = formatted + .cell_offsets() + .ranges() + .zip(unformatted.cell_offsets().ranges()) + .map(|(formatted_range, unformatted_range)| { + let formatted = &formatted.source_code()[formatted_range]; + Edit::range_replacement(formatted.to_string(), unformatted_range) + }); + + let fix = Fix::safe_edits( + edits + .next() + .expect("Formatted files must have at least one edit"), + edits, ); + let source = formatted.source_code(); + let line_count = formatted + .cell_offsets() + .ranges() + .map(|range| source.count_lines(range)) + .max() + .unwrap_or_default(); + (fix, line_count) + } else { + let fix = Fix::safe_edit(Edit::range_replacement( + formatted.source_code().to_string(), + TextRange::up_to(unformatted.source_code().text_len()), + )); + let line_count = formatted + .source_code() + .count_lines(TextRange::up_to(formatted.source_code().text_len())); + (fix, line_count) + }; - let path = result.path.to_string_lossy(); - // For now, report the edit as a replacement of the whole file's contents. For - // scripts, this is a single `Edit`, but notebook edits must be split by cell in - // order to render them as diffs. - // - // We also attempt to estimate the line number width for aligning the - // annotate-snippets header. This is only an estimate because we don't actually know - // if the maximum line number present in the document will be rendered as part of - // the diff, either as a changed line or as an unchanged context line. For - // notebooks, we refine our estimate by checking the number of lines in each cell - // individually, otherwise we could use `formatted.source_code().count_lines(...)` - // in both cases. - let (fix, line_count) = if let SourceKind::IpyNotebook(formatted) = formatted - && let SourceKind::IpyNotebook(unformatted) = unformatted - { - notebook_index.insert(path.to_string(), unformatted.index().clone()); - - let mut edits = formatted - .cell_offsets() - .ranges() - .zip(unformatted.cell_offsets().ranges()) - .map(|(formatted_range, unformatted_range)| { - let formatted = &formatted.source_code()[formatted_range]; - Edit::range_replacement(formatted.to_string(), unformatted_range) - }); - - let fix = Fix::safe_edits( - edits - .next() - .expect("Formatted files must have at least one edit"), - edits, - ); - let source = formatted.source_code(); - let line_count = formatted - .cell_offsets() - .ranges() - .map(|range| source.count_lines(range)) - .max() - .unwrap_or_default(); - (fix, line_count) - } else { - let fix = Fix::safe_edit(Edit::range_replacement( - formatted.source_code().to_string(), - TextRange::up_to(unformatted.source_code().text_len()), - )); - let line_count = formatted - .source_code() - .count_lines(TextRange::up_to(formatted.source_code().text_len())); - (fix, line_count) - }; - - let source_file = SourceFileBuilder::new(path, unformatted.source_code()).finish(); - let span = Span::from(source_file); - let mut annotation = Annotation::primary(span); - annotation.set_file_level(true); - diagnostic.annotate(annotation); - diagnostic.set_fix(fix); - - let lines = OneIndexed::new(line_count as usize).unwrap_or_default(); - diagnostic.set_header_offset(lines.digits().get()); - - Some(diagnostic) - }) - .sorted_unstable_by(Diagnostic::ruff_start_ordering) + let source_file = SourceFileBuilder::new(path, unformatted.source_code()).finish(); + let span = Span::from(source_file); + let mut annotation = Annotation::primary(span); + annotation.set_file_level(true); + diagnostic.annotate(annotation); + diagnostic.set_fix(fix); + + let lines = OneIndexed::new(line_count as usize).unwrap_or_default(); + diagnostic.set_header_offset(lines.digits().get()); + + Some(diagnostic) + }) } } @@ -1337,11 +1332,6 @@ mod tests { invalid-syntax: Failed to parse test.py:1:1: Unexpected indentation --> test.py:1:1 - panic: Panicked at when checking `test.py`: `Test panic for FormatCommandError` - --> test.py:1:1 - info: This indicates a bug in Ruff. - info: If you could open an issue at https://github.com/astral-sh/ruff/issues/new?title=%5Bpanic%5D, we'd be very appreciative! - io: File not found --> test.py:1:1 @@ -1356,6 +1346,11 @@ mod tests { range-format-notebook: Range formatting isn't supported for notebooks. --> test.py:1:1 + + panic: Panicked at when checking `test.py`: `Test panic for FormatCommandError` + --> test.py:1:1 + info: This indicates a bug in Ruff. + info: If you could open an issue at https://github.com/astral-sh/ruff/issues/new?title=%5Bpanic%5D, we'd be very appreciative! "); Ok(()) From 28ebcd555e6477b653182ed7dd98b0d5481840ee Mon Sep 17 00:00:00 2001 From: Brent Westbrook Date: Fri, 26 Sep 2025 14:57:22 -0400 Subject: [PATCH 18/36] factor out create_panic_diagnostic, reuse in the linter I also added the filter I was using in the formatter to the linter tests. This isn't strictly necessary but will be a bit more convenient if the source of the panic ever shifts around in the future. --- crates/ruff/src/commands/check.rs | 23 ++--------- crates/ruff/src/commands/format.rs | 26 +++---------- crates/ruff/tests/format.rs | 2 +- crates/ruff/tests/lint.rs | 13 ++++--- crates/ruff_linter/src/message/mod.rs | 56 ++++++++++++++++++++++++++- 5 files changed, 71 insertions(+), 49 deletions(-) diff --git a/crates/ruff/src/commands/check.rs b/crates/ruff/src/commands/check.rs index f6e7c57ddbfed7..d7d15dba350079 100644 --- a/crates/ruff/src/commands/check.rs +++ b/crates/ruff/src/commands/check.rs @@ -9,11 +9,10 @@ use ignore::Error; use log::{debug, warn}; #[cfg(not(target_family = "wasm"))] use rayon::prelude::*; +use ruff_linter::message::create_panic_diagnostic; use rustc_hash::FxHashMap; -use ruff_db::diagnostic::{ - Annotation, Diagnostic, DiagnosticId, Span, SubDiagnostic, SubDiagnosticSeverity, -}; +use ruff_db::diagnostic::Diagnostic; use ruff_db::panic::catch_unwind; use ruff_linter::package::PackageRoot; use ruff_linter::registry::Rule; @@ -195,23 +194,7 @@ fn lint_path( match result { Ok(inner) => inner, Err(error) => { - let message = match error.payload.as_str() { - Some(summary) => format!("Fatal error while linting: {summary}"), - _ => "Fatal error while linting".to_owned(), - }; - let mut diagnostic = Diagnostic::new( - DiagnosticId::Panic, - ruff_db::diagnostic::Severity::Fatal, - message, - ); - let span = Span::from(SourceFileBuilder::new(path.to_string_lossy(), "").finish()); - let mut annotation = Annotation::primary(span); - annotation.set_file_level(true); - diagnostic.annotate(annotation); - diagnostic.sub(SubDiagnostic::new( - SubDiagnosticSeverity::Info, - format!("{error}"), - )); + let diagnostic = create_panic_diagnostic(&error, Some(path)); Ok(Diagnostics::new(vec![diagnostic], FxHashMap::default())) } } diff --git a/crates/ruff/src/commands/format.rs b/crates/ruff/src/commands/format.rs index 7e0ecee87488b6..35fb3041465806 100644 --- a/crates/ruff/src/commands/format.rs +++ b/crates/ruff/src/commands/format.rs @@ -13,10 +13,11 @@ use rayon::iter::Either::{Left, Right}; use rayon::iter::{IntoParallelRefIterator, ParallelIterator}; use ruff_db::diagnostic::{ Annotation, Diagnostic, DiagnosticFormat, DiagnosticId, DisplayDiagnosticConfig, - DisplayDiagnostics, DisplayGithubDiagnostics, GithubRenderer, Severity, Span, SubDiagnostic, - SubDiagnosticSeverity, + DisplayDiagnostics, DisplayGithubDiagnostics, GithubRenderer, Severity, Span, +}; +use ruff_linter::message::{ + Emitter, EmitterContext, GroupedEmitter, SarifEmitter, create_panic_diagnostic, }; -use ruff_linter::message::{Emitter, EmitterContext, GroupedEmitter, SarifEmitter}; use ruff_linter::settings::types::OutputFormat; use ruff_notebook::NotebookIndex; use ruff_python_parser::ParseError; @@ -886,23 +887,7 @@ impl From<&FormatCommandError> for Diagnostic { display_parse_error, ), FormatCommandError::Panic(path, panic_error) => { - let mut diagnostic = Diagnostic::new( - DiagnosticId::Panic, - Severity::Fatal, - panic_error.to_diagnostic_message(path.as_ref().map(|path| path.display())), - ); - diagnostic.sub(SubDiagnostic::new( - SubDiagnosticSeverity::Info, - "This indicates a bug in Ruff.", - )); - let report_message = "If you could open an issue at \ - https://github.com/astral-sh/ruff/issues/new?title=%5Bpanic%5D, \ - we'd be very appreciative!"; - diagnostic.sub(SubDiagnostic::new( - SubDiagnosticSeverity::Info, - report_message, - )); - diagnostic + return create_panic_diagnostic(panic_error, path.as_deref()); } // I/O errors @@ -1351,6 +1336,7 @@ mod tests { --> test.py:1:1 info: This indicates a bug in Ruff. info: If you could open an issue at https://github.com/astral-sh/ruff/issues/new?title=%5Bpanic%5D, we'd be very appreciative! + info: run with `RUST_BACKTRACE=1` environment variable to show the full backtrace information "); Ok(()) diff --git a/crates/ruff/tests/format.rs b/crates/ruff/tests/format.rs index 06112d92f281ab..6205fa98e8b30e 100644 --- a/crates/ruff/tests/format.rs +++ b/crates/ruff/tests/format.rs @@ -2477,5 +2477,5 @@ fn stable_output_format_warning() { ----- stderr ----- warning: The --output-format flag for the formatter is unstable and requires preview mode to use. ", - ) + ); } diff --git a/crates/ruff/tests/lint.rs b/crates/ruff/tests/lint.rs index 75e267e1b4467e..d1500acc54b910 100644 --- a/crates/ruff/tests/lint.rs +++ b/crates/ruff/tests/lint.rs @@ -6272,6 +6272,7 @@ fn rule_panic_mixed_results_concise() -> Result<()> { filters => vec![ (tempdir_filter(&tempdir).as_str(), "[TMP]/"), (r"\\", r"/"), + (r"(Panicked at) [^:]+:\d+:\d+", "$1 ") ] }, { assert_cmd_snapshot!( @@ -6288,7 +6289,7 @@ fn rule_panic_mixed_results_concise() -> Result<()> { [TMP]/normal.py:1:1: RUF903 Hey this is a stable test rule with a display only fix. [TMP]/normal.py:1:1: RUF911 Hey this is a preview test rule. [TMP]/normal.py:1:1: RUF950 Hey this is a test rule that was redirected from another. - [TMP]/panic.py: panic: Fatal error while linting: This is a fake panic for testing. + [TMP]/panic.py: panic: Panicked at when checking `[TMP]/panic.py`: `This is a fake panic for testing.` Found 7 errors. [*] 1 fixable with the `--fix` option (1 hidden fix can be enabled with the `--unsafe-fixes` option). @@ -6317,6 +6318,7 @@ fn rule_panic_mixed_results_full() -> Result<()> { filters => vec![ (tempdir_filter(&tempdir).as_str(), "[TMP]/"), (r"\\", r"/"), + (r"(Panicked at) [^:]+:\d+:\d+", "$1 "), ] }, { assert_cmd_snapshot!( @@ -6347,12 +6349,11 @@ fn rule_panic_mixed_results_full() -> Result<()> { RUF950 Hey this is a test rule that was redirected from another. --> [TMP]/normal.py:1:1 - panic: Fatal error while linting: This is a fake panic for testing. + panic: Panicked at when checking `[TMP]/panic.py`: `This is a fake panic for testing.` --> [TMP]/panic.py:1:1 - info: panicked at crates/ruff_linter/src/rules/ruff/rules/test_rules.rs:511:9: - This is a fake panic for testing. - run with `RUST_BACKTRACE=1` environment variable to display a backtrace - + info: This indicates a bug in Ruff. + info: If you could open an issue at https://github.com/astral-sh/ruff/issues/new?title=%5Bpanic%5D, we'd be very appreciative! + info: run with `RUST_BACKTRACE=1` environment variable to show the full backtrace information Found 7 errors. [*] 1 fixable with the `--fix` option (1 hidden fix can be enabled with the `--unsafe-fixes` option). diff --git a/crates/ruff_linter/src/message/mod.rs b/crates/ruff_linter/src/message/mod.rs index 994dc81296d419..68a8d6257f5d20 100644 --- a/crates/ruff_linter/src/message/mod.rs +++ b/crates/ruff_linter/src/message/mod.rs @@ -1,17 +1,20 @@ +use std::backtrace::BacktraceStatus; use std::fmt::Display; use std::io::Write; +use std::path::Path; +use ruff_db::panic::PanicError; use rustc_hash::FxHashMap; use ruff_db::diagnostic::{ Annotation, Diagnostic, DiagnosticId, FileResolver, Input, LintName, SecondaryCode, Severity, - Span, UnifiedFile, + Span, SubDiagnostic, SubDiagnosticSeverity, UnifiedFile, }; use ruff_db::files::File; pub use grouped::GroupedEmitter; use ruff_notebook::NotebookIndex; -use ruff_source_file::SourceFile; +use ruff_source_file::{SourceFile, SourceFileBuilder}; use ruff_text_size::{Ranged, TextRange, TextSize}; pub use sarif::SarifEmitter; pub use text::TextEmitter; @@ -41,6 +44,55 @@ pub fn create_syntax_error_diagnostic( diag } +/// Create a `Diagnostic` from a panic. +pub fn create_panic_diagnostic(error: &PanicError, path: Option<&Path>) -> Diagnostic { + let mut diagnostic = Diagnostic::new( + DiagnosticId::Panic, + Severity::Fatal, + error.to_diagnostic_message(path.as_ref().map(|path| path.display())), + ); + + diagnostic.sub(SubDiagnostic::new( + SubDiagnosticSeverity::Info, + "This indicates a bug in Ruff.", + )); + let report_message = "If you could open an issue at \ + https://github.com/astral-sh/ruff/issues/new?title=%5Bpanic%5D, \ + we'd be very appreciative!"; + diagnostic.sub(SubDiagnostic::new( + SubDiagnosticSeverity::Info, + report_message, + )); + + if let Some(backtrace) = &error.backtrace { + match backtrace.status() { + BacktraceStatus::Disabled => { + diagnostic.sub(SubDiagnostic::new( + SubDiagnosticSeverity::Info, + "run with `RUST_BACKTRACE=1` environment variable to show the full backtrace information", + )); + } + BacktraceStatus::Captured => { + diagnostic.sub(SubDiagnostic::new( + SubDiagnosticSeverity::Info, + format!("Backtrace:\n{backtrace}"), + )); + } + _ => {} + } + } + + if let Some(path) = path { + let file = SourceFileBuilder::new(path.to_string_lossy(), "").finish(); + let span = Span::from(file); + let mut annotation = Annotation::primary(span); + annotation.set_file_level(true); + diagnostic.annotate(annotation); + } + + diagnostic +} + #[expect(clippy::too_many_arguments)] pub fn create_lint_diagnostic( body: B, From 090468d961361211e062126599254687a25e6e81 Mon Sep 17 00:00:00 2001 From: Brent Westbrook Date: Fri, 26 Sep 2025 15:23:32 -0400 Subject: [PATCH 19/36] delete TODO for ignore::Errors --- crates/ruff/src/commands/format.rs | 4 ---- 1 file changed, 4 deletions(-) diff --git a/crates/ruff/src/commands/format.rs b/crates/ruff/src/commands/format.rs index 35fb3041465806..42b52fbe44a15b 100644 --- a/crates/ruff/src/commands/format.rs +++ b/crates/ruff/src/commands/format.rs @@ -871,10 +871,6 @@ impl From<&FormatCommandError> for Diagnostic { }); let mut diagnostic = match error { - // TODO(brent) also not sure about this one. Most of the ignore::Error variants - // do fit pretty well under Io errors, but we could match and be even more - // specific if we wanted. We already have a `DiagnosticId::InvalidGlob`, which - // could map to Error::Glob, for example. FormatCommandError::Ignore(error) => { Diagnostic::new(DiagnosticId::Io, Severity::Error, error) } From 723d1d56ea2e31201cc65e4e3b475a9a4baf2e1a Mon Sep 17 00:00:00 2001 From: Brent Westbrook Date: Fri, 26 Sep 2025 15:39:14 -0400 Subject: [PATCH 20/36] use FormatError for PrintError --- crates/ruff/src/commands/format.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/crates/ruff/src/commands/format.rs b/crates/ruff/src/commands/format.rs index 42b52fbe44a15b..a965be24652644 100644 --- a/crates/ruff/src/commands/format.rs +++ b/crates/ruff/src/commands/format.rs @@ -904,10 +904,8 @@ impl From<&FormatCommandError> for Diagnostic { FormatModuleError::FormatError(format_error) => { Diagnostic::new(DiagnosticId::FormatError, Severity::Error, format_error) } - // TODO(brent) this might need a new DiagnosticId, or we could reuse - // `FormatError`, which also has an InvalidDocument variant, like PrintError FormatModuleError::PrintError(print_error) => { - Diagnostic::new(DiagnosticId::InvalidSyntax, Severity::Error, print_error) + Diagnostic::new(DiagnosticId::FormatError, Severity::Error, print_error) } }, FormatCommandError::RangeFormatNotebook(_) => Diagnostic::new( From 05faf43ca6740da7e7719352c6414cdf657648b8 Mon Sep 17 00:00:00 2001 From: Brent Westbrook Date: Fri, 26 Sep 2025 16:20:51 -0400 Subject: [PATCH 21/36] add DisplayParseError::error and render the ParseErrorType directly --- crates/ruff/src/commands/format.rs | 7 ++----- crates/ruff_linter/src/logging.rs | 5 +++++ 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/crates/ruff/src/commands/format.rs b/crates/ruff/src/commands/format.rs index a965be24652644..a97448de59fcc2 100644 --- a/crates/ruff/src/commands/format.rs +++ b/crates/ruff/src/commands/format.rs @@ -874,13 +874,10 @@ impl From<&FormatCommandError> for Diagnostic { FormatCommandError::Ignore(error) => { Diagnostic::new(DiagnosticId::Io, Severity::Error, error) } - // TODO(brent) not sure if this is correct, DisplayParseError includes some - // color and other formatting. I think we'd rather have access to the `path` and - // underlying ParseError directly, like in other variants here. FormatCommandError::Parse(display_parse_error) => Diagnostic::new( DiagnosticId::InvalidSyntax, Severity::Error, - display_parse_error, + &display_parse_error.error().error, ), FormatCommandError::Panic(path, panic_error) => { return create_panic_diagnostic(panic_error, path.as_deref()); @@ -1308,7 +1305,7 @@ mod tests { io: test.py: Permission denied --> test.py:1:1 - invalid-syntax: Failed to parse test.py:1:1: Unexpected indentation + invalid-syntax: Unexpected indentation --> test.py:1:1 io: File not found diff --git a/crates/ruff_linter/src/logging.rs b/crates/ruff_linter/src/logging.rs index 5b293c797bab18..9dba2228b08910 100644 --- a/crates/ruff_linter/src/logging.rs +++ b/crates/ruff_linter/src/logging.rs @@ -223,6 +223,11 @@ impl DisplayParseError { pub fn path(&self) -> Option<&Path> { self.path.as_deref() } + + /// Return the underlying [`ParseError`]. + pub fn error(&self) -> &ParseError { + &self.error + } } impl std::error::Error for DisplayParseError {} From ed63b7e4959b51044c30896b3c4c03b0929ffd8c Mon Sep 17 00:00:00 2001 From: Brent Westbrook Date: Fri, 26 Sep 2025 16:35:46 -0400 Subject: [PATCH 22/36] calculate a real range for the diagnostics note that several of the tests in format.rs still start at 1:1. This is because the newlines in the `say_hy` examples with raw strings are actually part of the code passed to the formatter, which removes these initial newlines. --- crates/ruff/src/commands/format.rs | 28 ++++++++++++++++++- .../format__output_format_azure.snap | 3 +- .../format__output_format_concise.snap | 3 +- .../format__output_format_github.snap | 3 +- .../format__output_format_gitlab.snap | 5 ++-- .../format__output_format_json-lines.snap | 3 +- .../snapshots/format__output_format_json.snap | 5 ++-- .../format__output_format_junit.snap | 5 ++-- .../format__output_format_rdjson.snap | 13 ++++++++- .../format__output_format_sarif.snap | 5 ++-- 10 files changed, 59 insertions(+), 14 deletions(-) diff --git a/crates/ruff/src/commands/format.rs b/crates/ruff/src/commands/format.rs index a97448de59fcc2..ec4ed6ba9996a0 100644 --- a/crates/ruff/src/commands/format.rs +++ b/crates/ruff/src/commands/format.rs @@ -811,8 +811,34 @@ impl<'a> FormatResults<'a> { (fix, line_count) }; + // Locate the first and last characters that differ to use as the diagnostic + // range. + let range = { + let mut start = None; + let mut end = None; + for ((offset, old), new) in unformatted + .source_code() + .char_indices() + .zip(formatted.source_code().chars()) + { + if old != new { + if start.is_none() { + start = Some(offset); + } else { + end = Some(offset); + } + } + } + + let start = + start.map_or(TextSize::ZERO, |start| TextSize::try_from(start).unwrap()); + let end = end.map_or(start, |end| TextSize::try_from(end).unwrap()); + + TextRange::new(start, end) + }; + let source_file = SourceFileBuilder::new(path, unformatted.source_code()).finish(); - let span = Span::from(source_file); + let span = Span::from(source_file).with_range(range); let mut annotation = Annotation::primary(span); annotation.set_file_level(true); diagnostic.annotate(annotation); diff --git a/crates/ruff/tests/snapshots/format__output_format_azure.snap b/crates/ruff/tests/snapshots/format__output_format_azure.snap index 4a19f9d50f327e..f27a3277272b03 100644 --- a/crates/ruff/tests/snapshots/format__output_format_azure.snap +++ b/crates/ruff/tests/snapshots/format__output_format_azure.snap @@ -7,12 +7,13 @@ info: - "--no-cache" - "--output-format" - azure + - "--preview" - "--check" - input.py --- success: false exit_code: 1 ----- stdout ----- -##vso[task.logissue type=error;sourcepath=[TMP]/input.py;code=unformatted;]File would be reformatted +##vso[task.logissue type=error;sourcepath=[TMP]/input.py;linenumber=1;columnnumber=1;code=unformatted;]File would be reformatted ----- stderr ----- diff --git a/crates/ruff/tests/snapshots/format__output_format_concise.snap b/crates/ruff/tests/snapshots/format__output_format_concise.snap index d377a4abb7f600..8a246337681a3f 100644 --- a/crates/ruff/tests/snapshots/format__output_format_concise.snap +++ b/crates/ruff/tests/snapshots/format__output_format_concise.snap @@ -7,13 +7,14 @@ info: - "--no-cache" - "--output-format" - concise + - "--preview" - "--check" - input.py --- success: false exit_code: 1 ----- stdout ----- -input.py: unformatted: File would be reformatted +input.py:1:1: unformatted: File would be reformatted 1 file would be reformatted ----- stderr ----- diff --git a/crates/ruff/tests/snapshots/format__output_format_github.snap b/crates/ruff/tests/snapshots/format__output_format_github.snap index 881ec1a7f40be2..71a635ea1faf76 100644 --- a/crates/ruff/tests/snapshots/format__output_format_github.snap +++ b/crates/ruff/tests/snapshots/format__output_format_github.snap @@ -7,12 +7,13 @@ info: - "--no-cache" - "--output-format" - github + - "--preview" - "--check" - input.py --- success: false exit_code: 1 ----- stdout ----- -::error title=Ruff (unformatted),file=[TMP]/input.py,line=1,col=1,endLine=1,endColumn=1::input.py:1:1: unformatted: File would be reformatted +::error title=Ruff (unformatted),file=[TMP]/input.py,line=1,col=1,endLine=5,endColumn=35::input.py:1:1: unformatted: File would be reformatted ----- stderr ----- diff --git a/crates/ruff/tests/snapshots/format__output_format_gitlab.snap b/crates/ruff/tests/snapshots/format__output_format_gitlab.snap index 7008a4aec16b58..5098854caf46c8 100644 --- a/crates/ruff/tests/snapshots/format__output_format_gitlab.snap +++ b/crates/ruff/tests/snapshots/format__output_format_gitlab.snap @@ -7,6 +7,7 @@ info: - "--no-cache" - "--output-format" - gitlab + - "--preview" - "--check" - input.py --- @@ -27,8 +28,8 @@ exit_code: 1 "column": 1 }, "end": { - "line": 1, - "column": 1 + "line": 5, + "column": 35 } } } diff --git a/crates/ruff/tests/snapshots/format__output_format_json-lines.snap b/crates/ruff/tests/snapshots/format__output_format_json-lines.snap index 1c915c1d359e4e..ed2fdadfb6d5a9 100644 --- a/crates/ruff/tests/snapshots/format__output_format_json-lines.snap +++ b/crates/ruff/tests/snapshots/format__output_format_json-lines.snap @@ -7,12 +7,13 @@ info: - "--no-cache" - "--output-format" - json-lines + - "--preview" - "--check" - input.py --- success: false exit_code: 1 ----- stdout ----- -{"cell":null,"code":"unformatted","end_location":{"column":1,"row":1},"filename":"[TMP]/input.py","fix":{"applicability":"safe","edits":[{"content":"from test import say_hy\n\nif __name__ == \"__main__\":\n say_hy(\"dear Ruff contributor\")\n","end_location":{"column":1,"row":6},"location":{"column":1,"row":1}}],"message":null},"location":{"column":1,"row":1},"message":"File would be reformatted","noqa_row":null,"url":null} +{"cell":null,"code":"unformatted","end_location":{"column":35,"row":5},"filename":"[TMP]/input.py","fix":{"applicability":"safe","edits":[{"content":"from test import say_hy\n\nif __name__ == \"__main__\":\n say_hy(\"dear Ruff contributor\")\n","end_location":{"column":1,"row":6},"location":{"column":1,"row":1}}],"message":null},"location":{"column":1,"row":1},"message":"File would be reformatted","noqa_row":null,"url":null} ----- stderr ----- diff --git a/crates/ruff/tests/snapshots/format__output_format_json.snap b/crates/ruff/tests/snapshots/format__output_format_json.snap index c35d663f5de7d4..657ca20514e0d7 100644 --- a/crates/ruff/tests/snapshots/format__output_format_json.snap +++ b/crates/ruff/tests/snapshots/format__output_format_json.snap @@ -7,6 +7,7 @@ info: - "--no-cache" - "--output-format" - json + - "--preview" - "--check" - input.py --- @@ -18,8 +19,8 @@ exit_code: 1 "cell": null, "code": "unformatted", "end_location": { - "column": 1, - "row": 1 + "column": 35, + "row": 5 }, "filename": "[TMP]/input.py", "fix": { diff --git a/crates/ruff/tests/snapshots/format__output_format_junit.snap b/crates/ruff/tests/snapshots/format__output_format_junit.snap index f9e48486b0f0d5..518c836729944b 100644 --- a/crates/ruff/tests/snapshots/format__output_format_junit.snap +++ b/crates/ruff/tests/snapshots/format__output_format_junit.snap @@ -7,6 +7,7 @@ info: - "--no-cache" - "--output-format" - junit + - "--preview" - "--check" - input.py --- @@ -16,8 +17,8 @@ exit_code: 1 - - File would be reformatted + + line 1, col 1, File would be reformatted diff --git a/crates/ruff/tests/snapshots/format__output_format_rdjson.snap b/crates/ruff/tests/snapshots/format__output_format_rdjson.snap index 52ad2e1e200769..bae7a8706b7a84 100644 --- a/crates/ruff/tests/snapshots/format__output_format_rdjson.snap +++ b/crates/ruff/tests/snapshots/format__output_format_rdjson.snap @@ -7,6 +7,7 @@ info: - "--no-cache" - "--output-format" - rdjson + - "--preview" - "--check" - input.py --- @@ -20,7 +21,17 @@ exit_code: 1 "value": "unformatted" }, "location": { - "path": "[TMP]/input.py" + "path": "[TMP]/input.py", + "range": { + "end": { + "column": 35, + "line": 5 + }, + "start": { + "column": 1, + "line": 1 + } + } }, "message": "File would be reformatted", "suggestions": [ diff --git a/crates/ruff/tests/snapshots/format__output_format_sarif.snap b/crates/ruff/tests/snapshots/format__output_format_sarif.snap index 5900afc5c8f79f..658d6f19ae6667 100644 --- a/crates/ruff/tests/snapshots/format__output_format_sarif.snap +++ b/crates/ruff/tests/snapshots/format__output_format_sarif.snap @@ -7,6 +7,7 @@ info: - "--no-cache" - "--output-format" - sarif + - "--preview" - "--check" - input.py --- @@ -54,8 +55,8 @@ exit_code: 1 "uri": "[TMP]/input.py" }, "region": { - "endColumn": 1, - "endLine": 1, + "endColumn": 35, + "endLine": 5, "startColumn": 1, "startLine": 1 } From c1a3b44a059a896535c396d4c9eed5665ac35e48 Mon Sep 17 00:00:00 2001 From: Brent Westbrook Date: Fri, 26 Sep 2025 17:15:51 -0400 Subject: [PATCH 23/36] reuse FormatCommandError::Write instead of Diff the diff variant was only used for reporting a failure writing the diff to stdout --- crates/ruff/src/commands/format.rs | 32 ------------------------ crates/ruff/src/commands/format_stdin.rs | 6 +++-- 2 files changed, 4 insertions(+), 34 deletions(-) diff --git a/crates/ruff/src/commands/format.rs b/crates/ruff/src/commands/format.rs index ec4ed6ba9996a0..8bc4dfa8a7cfac 100644 --- a/crates/ruff/src/commands/format.rs +++ b/crates/ruff/src/commands/format.rs @@ -861,7 +861,6 @@ pub(crate) enum FormatCommandError { Read(Option, SourceError), Format(Option, FormatModuleError), Write(Option, SourceError), - Diff(Option, io::Error), RangeFormatNotebook(Option), } @@ -880,7 +879,6 @@ impl FormatCommandError { | Self::Read(path, _) | Self::Format(path, _) | Self::Write(path, _) - | Self::Diff(path, _) | Self::RangeFormatNotebook(path) => path.as_deref(), } } @@ -908,16 +906,10 @@ impl From<&FormatCommandError> for Diagnostic { FormatCommandError::Panic(path, panic_error) => { return create_panic_diagnostic(panic_error, path.as_deref()); } - - // I/O errors FormatCommandError::Read(_, source_error) | FormatCommandError::Write(_, source_error) => { Diagnostic::new(DiagnosticId::Io, Severity::Error, source_error) } - FormatCommandError::Diff(_, error) => { - Diagnostic::new(DiagnosticId::Io, Severity::Error, error) - } - FormatCommandError::Format(_, format_module_error) => match format_module_error { FormatModuleError::ParseError(parse_error) => Diagnostic::new( DiagnosticId::InvalidSyntax, @@ -1013,23 +1005,6 @@ impl Display for FormatCommandError { write!(f, "{header} {err}", header = "Failed to format:".bold()) } } - Self::Diff(path, err) => { - if let Some(path) = path { - write!( - f, - "{}{}{} {err}", - "Failed to generate diff for ".bold(), - fs::relativize_path(path).bold(), - ":".bold() - ) - } else { - write!( - f, - "{header} {err}", - header = "Failed to generate diff:".bold(), - ) - } - } Self::RangeFormatNotebook(path) => { if let Some(path) = path { write!( @@ -1308,10 +1283,6 @@ mod tests { "Cannot write to file", )), ), - FormatCommandError::Diff( - Some(path.clone()), - io::Error::other("Failed to generate diff"), - ), FormatCommandError::RangeFormatNotebook(Some(path)), ]; @@ -1343,9 +1314,6 @@ mod tests { io: Cannot write to file --> test.py:1:1 - io: Failed to generate diff - --> test.py:1:1 - range-format-notebook: Range formatting isn't supported for notebooks. --> test.py:1:1 diff --git a/crates/ruff/src/commands/format_stdin.rs b/crates/ruff/src/commands/format_stdin.rs index d5b16f1b3f5d70..015f32fba91206 100644 --- a/crates/ruff/src/commands/format_stdin.rs +++ b/crates/ruff/src/commands/format_stdin.rs @@ -4,7 +4,7 @@ use std::path::Path; use anyhow::Result; use log::error; -use ruff_linter::source_kind::SourceKind; +use ruff_linter::source_kind::{SourceError, SourceKind}; use ruff_python_ast::{PySourceType, SourceType}; use ruff_workspace::FormatterSettings; use ruff_workspace::resolver::{PyprojectConfig, Resolver, match_exclusion, python_file_at_path}; @@ -122,7 +122,9 @@ fn format_source_code( "{}", source_kind.diff(formatted, path).unwrap() ) - .map_err(|err| FormatCommandError::Diff(path.map(Path::to_path_buf), err))?; + .map_err(|err| { + FormatCommandError::Write(path.map(Path::to_path_buf), SourceError::Io(err)) + })?; } }, FormattedSource::Unchanged => { From 73eb86d0f53d9a2c3f7ef0a4766e68980ef56fc8 Mon Sep 17 00:00:00 2001 From: Brent Westbrook Date: Fri, 26 Sep 2025 17:18:39 -0400 Subject: [PATCH 24/36] DiagnosticId::RangeFormatNotebook -> InvalidCliOption --- crates/ruff/src/commands/format.rs | 4 ++-- crates/ruff_db/src/diagnostic/mod.rs | 8 ++++---- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/crates/ruff/src/commands/format.rs b/crates/ruff/src/commands/format.rs index 8bc4dfa8a7cfac..55f9b5568c1b84 100644 --- a/crates/ruff/src/commands/format.rs +++ b/crates/ruff/src/commands/format.rs @@ -924,7 +924,7 @@ impl From<&FormatCommandError> for Diagnostic { } }, FormatCommandError::RangeFormatNotebook(_) => Diagnostic::new( - DiagnosticId::RangeFormatNotebook, + DiagnosticId::InvalidCliOption, Severity::Error, "Range formatting isn't supported for notebooks.", ), @@ -1314,7 +1314,7 @@ mod tests { io: Cannot write to file --> test.py:1:1 - range-format-notebook: Range formatting isn't supported for notebooks. + invalid-cli-option: Range formatting isn't supported for notebooks. --> test.py:1:1 panic: Panicked at when checking `test.py`: `Test panic for FormatCommandError` diff --git a/crates/ruff_db/src/diagnostic/mod.rs b/crates/ruff_db/src/diagnostic/mod.rs index 1eab12d03634f4..9039c8e6e8d361 100644 --- a/crates/ruff_db/src/diagnostic/mod.rs +++ b/crates/ruff_db/src/diagnostic/mod.rs @@ -445,7 +445,7 @@ impl Diagnostic { | DiagnosticId::UselessOverridesSection | DiagnosticId::DeprecatedSetting | DiagnosticId::Unformatted - | DiagnosticId::RangeFormatNotebook + | DiagnosticId::InvalidCliOption | DiagnosticId::FormatError => None, DiagnosticId::Lint(lint_name) => { Some(format!("{}/rules/{lint_name}", env!("CARGO_PKG_HOMEPAGE"))) @@ -1036,8 +1036,8 @@ pub enum DiagnosticId { /// The code needs to be formatted. Unformatted, - /// Range formatting isn't supported for notebooks. - RangeFormatNotebook, + /// Use of an invalid command-line option. + InvalidCliOption, /// Something went wrong while formatting. /// @@ -1083,7 +1083,7 @@ impl DiagnosticId { DiagnosticId::UselessOverridesSection => "useless-overrides-section", DiagnosticId::DeprecatedSetting => "deprecated-setting", DiagnosticId::Unformatted => "unformatted", - DiagnosticId::RangeFormatNotebook => "range-format-notebook", + DiagnosticId::InvalidCliOption => "invalid-cli-option", DiagnosticId::FormatError => "format-error", } } From aee8fd3659a84cfbd8c88dff94b1801732281173 Mon Sep 17 00:00:00 2001 From: Brent Westbrook Date: Fri, 26 Sep 2025 17:22:51 -0400 Subject: [PATCH 25/36] DiagnosticId::FormatError -> InternalError --- crates/ruff/src/commands/format.rs | 4 ++-- crates/ruff_db/src/diagnostic/mod.rs | 10 +++++----- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/crates/ruff/src/commands/format.rs b/crates/ruff/src/commands/format.rs index 55f9b5568c1b84..b76317612ec89a 100644 --- a/crates/ruff/src/commands/format.rs +++ b/crates/ruff/src/commands/format.rs @@ -917,10 +917,10 @@ impl From<&FormatCommandError> for Diagnostic { &parse_error.error, ), FormatModuleError::FormatError(format_error) => { - Diagnostic::new(DiagnosticId::FormatError, Severity::Error, format_error) + Diagnostic::new(DiagnosticId::InternalError, Severity::Error, format_error) } FormatModuleError::PrintError(print_error) => { - Diagnostic::new(DiagnosticId::FormatError, Severity::Error, print_error) + Diagnostic::new(DiagnosticId::InternalError, Severity::Error, print_error) } }, FormatCommandError::RangeFormatNotebook(_) => Diagnostic::new( diff --git a/crates/ruff_db/src/diagnostic/mod.rs b/crates/ruff_db/src/diagnostic/mod.rs index 9039c8e6e8d361..90564c84a99ca5 100644 --- a/crates/ruff_db/src/diagnostic/mod.rs +++ b/crates/ruff_db/src/diagnostic/mod.rs @@ -446,7 +446,7 @@ impl Diagnostic { | DiagnosticId::DeprecatedSetting | DiagnosticId::Unformatted | DiagnosticId::InvalidCliOption - | DiagnosticId::FormatError => None, + | DiagnosticId::InternalError => None, DiagnosticId::Lint(lint_name) => { Some(format!("{}/rules/{lint_name}", env!("CARGO_PKG_HOMEPAGE"))) } @@ -1039,10 +1039,10 @@ pub enum DiagnosticId { /// Use of an invalid command-line option. InvalidCliOption, - /// Something went wrong while formatting. + /// An internal assumption was violated. /// - /// This often indicates a bug in the formatter. - FormatError, + /// This indicates a bug in the program rather than a user error. + InternalError, } impl DiagnosticId { @@ -1084,7 +1084,7 @@ impl DiagnosticId { DiagnosticId::DeprecatedSetting => "deprecated-setting", DiagnosticId::Unformatted => "unformatted", DiagnosticId::InvalidCliOption => "invalid-cli-option", - DiagnosticId::FormatError => "format-error", + DiagnosticId::InternalError => "internal-error", } } From 0668704fcc75c669b13c45cfdb4166c84365ccba Mon Sep 17 00:00:00 2001 From: Brent Westbrook Date: Fri, 26 Sep 2025 17:33:20 -0400 Subject: [PATCH 26/36] add a todo about the offset hack --- crates/ruff/src/commands/format.rs | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/crates/ruff/src/commands/format.rs b/crates/ruff/src/commands/format.rs index b76317612ec89a..8525e3765638a6 100644 --- a/crates/ruff/src/commands/format.rs +++ b/crates/ruff/src/commands/format.rs @@ -844,6 +844,16 @@ impl<'a> FormatResults<'a> { diagnostic.annotate(annotation); diagnostic.set_fix(fix); + // TODO(brent) this offset is a hack to get the header of the diagnostic message, which + // is rendered by our fork of `annotate-snippets`, to align with our manually-rendered + // diff. `annotate-snippets` computes the alignment of the arrow in the header based on + // the maximum line number width in its rendered snippet. However, we don't have a + // reasonable range to underline in an annotation, so we don't send `annotate-snippets` + // a snippet to measure. If we commit to staying on our fork, a more robust way of + // handling this would be to move the diff rendering in + // `ruff_db::diagnostic::render::full` into `annotate-snippets`, likely as another + // `DisplayLine` variant and update the `lineno_width` calculation in + // `DisplayList::fmt`. That would handle this offset "automatically." let lines = OneIndexed::new(line_count as usize).unwrap_or_default(); diagnostic.set_header_offset(lines.digits().get()); From 519b6fa85079c5569621fb969037d55bf524b297 Mon Sep 17 00:00:00 2001 From: Brent Westbrook Date: Fri, 26 Sep 2025 17:40:48 -0400 Subject: [PATCH 27/36] file_level -> hide_snippet --- crates/ruff/src/commands/format.rs | 4 ++-- crates/ruff_annotate_snippets/src/snippet.rs | 2 +- crates/ruff_db/src/diagnostic/mod.rs | 21 ++++++++++---------- crates/ruff_db/src/diagnostic/render.rs | 12 +++++------ crates/ruff_db/src/diagnostic/render/full.rs | 2 +- crates/ruff_linter/src/message/mod.rs | 4 ++-- 6 files changed, 23 insertions(+), 22 deletions(-) diff --git a/crates/ruff/src/commands/format.rs b/crates/ruff/src/commands/format.rs index 8525e3765638a6..554fe17788a670 100644 --- a/crates/ruff/src/commands/format.rs +++ b/crates/ruff/src/commands/format.rs @@ -840,7 +840,7 @@ impl<'a> FormatResults<'a> { let source_file = SourceFileBuilder::new(path, unformatted.source_code()).finish(); let span = Span::from(source_file).with_range(range); let mut annotation = Annotation::primary(span); - annotation.set_file_level(true); + annotation.hide_snippet(true); diagnostic.annotate(annotation); diagnostic.set_fix(fix); @@ -900,7 +900,7 @@ impl From<&FormatCommandError> for Diagnostic { let file = SourceFileBuilder::new(path.to_string_lossy(), "").finish(); let span = Span::from(file); let mut annotation = Annotation::primary(span); - annotation.set_file_level(true); + annotation.hide_snippet(true); annotation }); diff --git a/crates/ruff_annotate_snippets/src/snippet.rs b/crates/ruff_annotate_snippets/src/snippet.rs index 4fca75c571742b..76d615189daf21 100644 --- a/crates/ruff_annotate_snippets/src/snippet.rs +++ b/crates/ruff_annotate_snippets/src/snippet.rs @@ -155,7 +155,7 @@ impl<'a> Annotation<'a> { self } - pub fn is_file_level(mut self, yes: bool) -> Self { + pub fn hide_snippet(mut self, yes: bool) -> Self { self.is_file_level = yes; self } diff --git a/crates/ruff_db/src/diagnostic/mod.rs b/crates/ruff_db/src/diagnostic/mod.rs index 90564c84a99ca5..dd32f61c7b38af 100644 --- a/crates/ruff_db/src/diagnostic/mod.rs +++ b/crates/ruff_db/src/diagnostic/mod.rs @@ -758,11 +758,11 @@ pub struct Annotation { is_primary: bool, /// The diagnostic tags associated with this annotation. tags: Vec, - /// Whether this annotation is a file-level or full-file annotation. + /// Whether the snippet for this annotation should be hidden. /// /// When set, rendering will only include the file's name and (optional) range. Everything else /// is omitted, including any file snippet or message. - is_file_level: bool, + hide_snippet: bool, } impl Annotation { @@ -781,7 +781,7 @@ impl Annotation { message: None, is_primary: true, tags: Vec::new(), - is_file_level: false, + hide_snippet: false, } } @@ -798,7 +798,7 @@ impl Annotation { message: None, is_primary: false, tags: Vec::new(), - is_file_level: false, + hide_snippet: false, } } @@ -865,19 +865,20 @@ impl Annotation { self.tags.push(tag); } - /// Set whether or not this annotation is file-level. + /// Set whether or not the snippet on this annotation should be suppressed when rendering. /// - /// File-level annotations are only rendered with their file name and range, if available. This - /// is intended for backwards compatibility with Ruff diagnostics, which historically used + /// Such annotations are only rendered with their file name and range, if available. This is + /// intended for backwards compatibility with Ruff diagnostics, which historically used /// `TextRange::default` to indicate a file-level diagnostic. In the new diagnostic model, a /// [`Span`] with a range of `None` should be used instead, as mentioned in the `Span` /// documentation. /// /// TODO(brent) update this usage in Ruff and remove `is_file_level` entirely. See /// , especially my first comment, for more - /// details. - pub fn set_file_level(&mut self, yes: bool) { - self.is_file_level = yes; + /// details. As of 2025-09-26 we also use this to suppress snippet rendering for formatter + /// diagnostics, which also need to have a range, so we probably can't eliminate this entirely. + pub fn hide_snippet(&mut self, yes: bool) { + self.hide_snippet = yes; } } diff --git a/crates/ruff_db/src/diagnostic/render.rs b/crates/ruff_db/src/diagnostic/render.rs index 785329af9cf350..51caed471b2a99 100644 --- a/crates/ruff_db/src/diagnostic/render.rs +++ b/crates/ruff_db/src/diagnostic/render.rs @@ -408,7 +408,7 @@ struct ResolvedAnnotation<'a> { line_end: OneIndexed, message: Option<&'a str>, is_primary: bool, - is_file_level: bool, + hide_snippet: bool, notebook_index: Option, } @@ -456,7 +456,7 @@ impl<'a> ResolvedAnnotation<'a> { line_end, message: ann.get_message(), is_primary: ann.is_primary, - is_file_level: ann.is_file_level, + hide_snippet: ann.hide_snippet, notebook_index: resolver.notebook_index(&ann.span.file), }) } @@ -722,8 +722,8 @@ struct RenderableAnnotation<'r> { message: Option<&'r str>, /// Whether this annotation is considered "primary" or not. is_primary: bool, - /// Whether this annotation applies to an entire file, rather than a snippet within it. - is_file_level: bool, + /// Whether the snippet for this annotation should be hidden instead of rendered. + hide_snippet: bool, } impl<'r> RenderableAnnotation<'r> { @@ -745,7 +745,7 @@ impl<'r> RenderableAnnotation<'r> { range, message: ann.message, is_primary: ann.is_primary, - is_file_level: ann.is_file_level, + hide_snippet: ann.hide_snippet, } } @@ -771,7 +771,7 @@ impl<'r> RenderableAnnotation<'r> { if let Some(message) = self.message { ann = ann.label(message); } - ann.is_file_level(self.is_file_level) + ann.hide_snippet(self.hide_snippet) } } diff --git a/crates/ruff_db/src/diagnostic/render/full.rs b/crates/ruff_db/src/diagnostic/render/full.rs index eb782e72c3a60b..c87413a84e169a 100644 --- a/crates/ruff_db/src/diagnostic/render/full.rs +++ b/crates/ruff_db/src/diagnostic/render/full.rs @@ -573,7 +573,7 @@ print() let mut diagnostic = env.err().build(); let span = env.path("example.py").with_range(TextRange::default()); let mut annotation = Annotation::primary(span); - annotation.set_file_level(true); + annotation.hide_snippet(true); diagnostic.annotate(annotation); insta::assert_snapshot!(env.render(&diagnostic), @r" diff --git a/crates/ruff_linter/src/message/mod.rs b/crates/ruff_linter/src/message/mod.rs index 68a8d6257f5d20..66b6020332a542 100644 --- a/crates/ruff_linter/src/message/mod.rs +++ b/crates/ruff_linter/src/message/mod.rs @@ -86,7 +86,7 @@ pub fn create_panic_diagnostic(error: &PanicError, path: Option<&Path>) -> Diagn let file = SourceFileBuilder::new(path.to_string_lossy(), "").finish(); let span = Span::from(file); let mut annotation = Annotation::primary(span); - annotation.set_file_level(true); + annotation.hide_snippet(true); diagnostic.annotate(annotation); } @@ -122,7 +122,7 @@ where // actually need it, but we need to be able to cache the new diagnostic model first. See // https://github.com/astral-sh/ruff/issues/19688. if range == TextRange::default() { - annotation.set_file_level(true); + annotation.hide_snippet(true); } diagnostic.annotate(annotation); From 18640189a4ce7e5f74f690d400d9502d23863fcd Mon Sep 17 00:00:00 2001 From: Brent Westbrook Date: Mon, 29 Sep 2025 09:01:14 -0400 Subject: [PATCH 28/36] debug assert FormatResult::Diff --- crates/ruff/src/commands/format.rs | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/crates/ruff/src/commands/format.rs b/crates/ruff/src/commands/format.rs index 554fe17788a670..f70c21860bc8e8 100644 --- a/crates/ruff/src/commands/format.rs +++ b/crates/ruff/src/commands/format.rs @@ -746,12 +746,19 @@ impl<'a> FormatResults<'a> { notebook_index: &mut FxHashMap, ) -> impl Iterator { self.results.iter().filter_map(|result| { - let FormatResult::Diff { - unformatted, - formatted, - } = &result.result - else { - return None; + let (unformatted, formatted) = match &result.result { + FormatResult::Skipped | FormatResult::Unchanged => return None, + FormatResult::Diff { + unformatted, + formatted, + } => (unformatted, formatted), + FormatResult::Formatted => { + debug_assert!( + false, + "Expected `FormatResult::Diff` for changed files in check mode" + ); + return None; + } }; let mut diagnostic = Diagnostic::new( From aa532d78079c03dac786eb74f855c41381596d34 Mon Sep 17 00:00:00 2001 From: Brent Westbrook Date: Mon, 29 Sep 2025 09:15:22 -0400 Subject: [PATCH 29/36] narrow edit range to modified lines --- crates/ruff/src/commands/format.rs | 168 +++++++++++++----- crates/ruff/tests/format.rs | 2 +- .../format__output_format_github.snap | 2 +- .../format__output_format_gitlab.snap | 4 +- .../format__output_format_json-lines.snap | 2 +- .../snapshots/format__output_format_json.snap | 8 +- .../format__output_format_rdjson.snap | 8 +- .../format__output_format_sarif.snap | 9 +- 8 files changed, 141 insertions(+), 62 deletions(-) diff --git a/crates/ruff/src/commands/format.rs b/crates/ruff/src/commands/format.rs index f70c21860bc8e8..1681d2e810ec84 100644 --- a/crates/ruff/src/commands/format.rs +++ b/crates/ruff/src/commands/format.rs @@ -767,10 +767,13 @@ impl<'a> FormatResults<'a> { "File would be reformatted", ); + // Locate the first and last characters that differ to use as the diagnostic + // range and to narrow the `Edit` range. + let range = ModifiedRange::new(unformatted, formatted); + let path = result.path.to_string_lossy(); - // For now, report the edit as a replacement of the whole file's contents. For - // scripts, this is a single `Edit`, but notebook edits must be split by cell in - // order to render them as diffs. + // For scripts, this is a single `Edit` using the `ModifiedRange` above, but notebook + // edits must be split by cell in order to render them as diffs. // // We also attempt to estimate the line number width for aligning the // annotate-snippets header. This is only an estimate because we don't actually know @@ -788,9 +791,28 @@ impl<'a> FormatResults<'a> { .cell_offsets() .ranges() .zip(unformatted.cell_offsets().ranges()) - .map(|(formatted_range, unformatted_range)| { - let formatted = &formatted.source_code()[formatted_range]; - Edit::range_replacement(formatted.to_string(), unformatted_range) + .filter_map(|(formatted_range, unformatted_range)| { + // Filter out cells that weren't modified. We use `intersect` instead of + // `contains_range` because the full modified range might start or end in + // the middle of a cell: + // + // ``` + // | cell 1 | cell 2 | cell 3 | + // |----------------| modified range + // ``` + // + // The intersection will be `Some` for all three cells in this case. + if range.unformatted.intersect(unformatted_range).is_some() { + let formatted = &formatted.source_code()[formatted_range]; + let edit = if formatted.is_empty() { + Edit::range_deletion(unformatted_range) + } else { + Edit::range_replacement(formatted.to_string(), unformatted_range) + }; + Some(edit) + } else { + None + } }); let fix = Fix::safe_edits( @@ -803,49 +825,30 @@ impl<'a> FormatResults<'a> { let line_count = formatted .cell_offsets() .ranges() - .map(|range| source.count_lines(range)) + .filter_map(|r| { + if range.formatted.contains_range(r) { + Some(source.count_lines(r)) + } else { + None + } + }) .max() .unwrap_or_default(); (fix, line_count) } else { - let fix = Fix::safe_edit(Edit::range_replacement( - formatted.source_code().to_string(), - TextRange::up_to(unformatted.source_code().text_len()), - )); - let line_count = formatted - .source_code() - .count_lines(TextRange::up_to(formatted.source_code().text_len())); + let formatted_code = &formatted.source_code()[range.formatted]; + let edit = if formatted_code.is_empty() { + Edit::range_deletion(range.unformatted) + } else { + Edit::range_replacement(formatted_code.to_string(), range.unformatted) + }; + let fix = Fix::safe_edit(edit); + let line_count = formatted.source_code().count_lines(range.formatted); (fix, line_count) }; - // Locate the first and last characters that differ to use as the diagnostic - // range. - let range = { - let mut start = None; - let mut end = None; - for ((offset, old), new) in unformatted - .source_code() - .char_indices() - .zip(formatted.source_code().chars()) - { - if old != new { - if start.is_none() { - start = Some(offset); - } else { - end = Some(offset); - } - } - } - - let start = - start.map_or(TextSize::ZERO, |start| TextSize::try_from(start).unwrap()); - let end = end.map_or(start, |end| TextSize::try_from(end).unwrap()); - - TextRange::new(start, end) - }; - let source_file = SourceFileBuilder::new(path, unformatted.source_code()).finish(); - let span = Span::from(source_file).with_range(range); + let span = Span::from(source_file).with_range(range.unformatted); let mut annotation = Annotation::primary(span); annotation.hide_snippet(true); diagnostic.annotate(annotation); @@ -1066,6 +1069,54 @@ impl Display for FormatCommandError { } } +struct ModifiedRange { + unformatted: TextRange, + formatted: TextRange, +} + +impl ModifiedRange { + /// Determine the range that differs between `unformatted` and `formatted`. + /// + /// # Panics + /// + /// This function panics if the there are no differences between the two inputs. + fn new(unformatted: &SourceKind, formatted: &SourceKind) -> Self { + let unformatted = unformatted.source_code(); + let formatted = formatted.source_code(); + + let start = unformatted + .char_indices() + .zip(formatted.chars()) + .find_map(|((offset, old), new)| (old != new).then_some(offset)) + .expect("Expected at least one difference"); + + let start_size = TextSize::try_from(start).unwrap(); + + // This finds the position of the first character where the two suffixes differ in + // `unformatted`. + let mut offset = TextSize::ZERO; + for (old, new) in unformatted[start..] + .chars() + .rev() + .zip(formatted[start..].chars().rev()) + { + if old == new { + offset += old.text_len(); + } else { + break; + } + } + + let unformatted_range = TextRange::new(start_size, unformatted.text_len() - offset); + let formatted_range = TextRange::new(start_size, formatted.text_len() - offset); + + Self { + unformatted: unformatted_range, + formatted: formatted_range, + } + } +} + pub(super) fn warn_incompatible_formatter_settings(resolver: &Resolver) { // First, collect all rules that are incompatible regardless of the linter-specific settings. let mut incompatible_rules = FxHashSet::default(); @@ -1241,6 +1292,7 @@ pub(super) fn warn_incompatible_formatter_settings(resolver: &Resolver) { #[cfg(test)] mod tests { use std::io; + use std::ops::Range; use std::path::PathBuf; use ignore::Error; @@ -1251,9 +1303,10 @@ mod tests { use ruff_linter::source_kind::{SourceError, SourceKind}; use ruff_python_formatter::FormatModuleError; use ruff_python_parser::{ParseError, ParseErrorType}; - use ruff_text_size::TextRange; + use ruff_text_size::{TextRange, TextSize}; + use test_case::test_case; - use crate::commands::format::{FormatCommandError, FormatMode, FormatResults}; + use crate::commands::format::{FormatCommandError, FormatMode, FormatResults, ModifiedRange}; #[test] fn error_diagnostics() -> anyhow::Result<()> { @@ -1343,4 +1396,33 @@ mod tests { Ok(()) } + + #[test_case("abcdef", "abcXYdef", 3..3, 3..5; "insertion")] + #[test_case("abcXYdef", "abcdef", 3..5, 3..3; "deletion")] + #[test_case("abcXdef", "abcYdef", 3..4, 3..4; "modification")] + fn modified_range( + unformatted: &str, + formatted: &str, + expect_unformatted: Range, + expect_formatted: Range, + ) { + let mr = ModifiedRange::new( + &SourceKind::Python(unformatted.to_string()), + &SourceKind::Python(formatted.to_string()), + ); + assert_eq!( + mr.unformatted, + TextRange::new( + TextSize::new(expect_unformatted.start), + TextSize::new(expect_unformatted.end) + ) + ); + assert_eq!( + mr.formatted, + TextRange::new( + TextSize::new(expect_formatted.start), + TextSize::new(expect_formatted.end) + ) + ); + } } diff --git a/crates/ruff/tests/format.rs b/crates/ruff/tests/format.rs index 6205fa98e8b30e..53f3296cff17b2 100644 --- a/crates/ruff/tests/format.rs +++ b/crates/ruff/tests/format.rs @@ -680,7 +680,7 @@ fn output_format_notebook() { exit_code: 1 ----- stdout ----- unformatted: File would be reformatted - --> resources/test/fixtures/unformatted.ipynb:cell 1:1:1 + --> resources/test/fixtures/unformatted.ipynb:cell 1:1:1 ::: cell 1 1 | import numpy - maths = (numpy.arange(100)**2).sum() diff --git a/crates/ruff/tests/snapshots/format__output_format_github.snap b/crates/ruff/tests/snapshots/format__output_format_github.snap index 71a635ea1faf76..e279ad2b879103 100644 --- a/crates/ruff/tests/snapshots/format__output_format_github.snap +++ b/crates/ruff/tests/snapshots/format__output_format_github.snap @@ -14,6 +14,6 @@ info: success: false exit_code: 1 ----- stdout ----- -::error title=Ruff (unformatted),file=[TMP]/input.py,line=1,col=1,endLine=5,endColumn=35::input.py:1:1: unformatted: File would be reformatted +::error title=Ruff (unformatted),file=[TMP]/input.py,line=1,col=1,endLine=2,endColumn=1::input.py:1:1: unformatted: File would be reformatted ----- stderr ----- diff --git a/crates/ruff/tests/snapshots/format__output_format_gitlab.snap b/crates/ruff/tests/snapshots/format__output_format_gitlab.snap index 5098854caf46c8..fbf68871d3369b 100644 --- a/crates/ruff/tests/snapshots/format__output_format_gitlab.snap +++ b/crates/ruff/tests/snapshots/format__output_format_gitlab.snap @@ -28,8 +28,8 @@ exit_code: 1 "column": 1 }, "end": { - "line": 5, - "column": 35 + "line": 2, + "column": 1 } } } diff --git a/crates/ruff/tests/snapshots/format__output_format_json-lines.snap b/crates/ruff/tests/snapshots/format__output_format_json-lines.snap index ed2fdadfb6d5a9..dadde724352ce0 100644 --- a/crates/ruff/tests/snapshots/format__output_format_json-lines.snap +++ b/crates/ruff/tests/snapshots/format__output_format_json-lines.snap @@ -14,6 +14,6 @@ info: success: false exit_code: 1 ----- stdout ----- -{"cell":null,"code":"unformatted","end_location":{"column":35,"row":5},"filename":"[TMP]/input.py","fix":{"applicability":"safe","edits":[{"content":"from test import say_hy\n\nif __name__ == \"__main__\":\n say_hy(\"dear Ruff contributor\")\n","end_location":{"column":1,"row":6},"location":{"column":1,"row":1}}],"message":null},"location":{"column":1,"row":1},"message":"File would be reformatted","noqa_row":null,"url":null} +{"cell":null,"code":"unformatted","end_location":{"column":1,"row":2},"filename":"[TMP]/input.py","fix":{"applicability":"safe","edits":[{"content":"","end_location":{"column":1,"row":2},"location":{"column":1,"row":1}}],"message":null},"location":{"column":1,"row":1},"message":"File would be reformatted","noqa_row":null,"url":null} ----- stderr ----- diff --git a/crates/ruff/tests/snapshots/format__output_format_json.snap b/crates/ruff/tests/snapshots/format__output_format_json.snap index 657ca20514e0d7..1bbff05aec2b8a 100644 --- a/crates/ruff/tests/snapshots/format__output_format_json.snap +++ b/crates/ruff/tests/snapshots/format__output_format_json.snap @@ -19,18 +19,18 @@ exit_code: 1 "cell": null, "code": "unformatted", "end_location": { - "column": 35, - "row": 5 + "column": 1, + "row": 2 }, "filename": "[TMP]/input.py", "fix": { "applicability": "safe", "edits": [ { - "content": "from test import say_hy\n\nif __name__ == \"__main__\":\n say_hy(\"dear Ruff contributor\")\n", + "content": "", "end_location": { "column": 1, - "row": 6 + "row": 2 }, "location": { "column": 1, diff --git a/crates/ruff/tests/snapshots/format__output_format_rdjson.snap b/crates/ruff/tests/snapshots/format__output_format_rdjson.snap index bae7a8706b7a84..dcdb5bda8c8f91 100644 --- a/crates/ruff/tests/snapshots/format__output_format_rdjson.snap +++ b/crates/ruff/tests/snapshots/format__output_format_rdjson.snap @@ -24,8 +24,8 @@ exit_code: 1 "path": "[TMP]/input.py", "range": { "end": { - "column": 35, - "line": 5 + "column": 1, + "line": 2 }, "start": { "column": 1, @@ -39,14 +39,14 @@ exit_code: 1 "range": { "end": { "column": 1, - "line": 6 + "line": 2 }, "start": { "column": 1, "line": 1 } }, - "text": "from test import say_hy\n\nif __name__ == \"__main__\":\n say_hy(\"dear Ruff contributor\")\n" + "text": "" } ] } diff --git a/crates/ruff/tests/snapshots/format__output_format_sarif.snap b/crates/ruff/tests/snapshots/format__output_format_sarif.snap index 658d6f19ae6667..1f31ab0374fb48 100644 --- a/crates/ruff/tests/snapshots/format__output_format_sarif.snap +++ b/crates/ruff/tests/snapshots/format__output_format_sarif.snap @@ -31,12 +31,9 @@ exit_code: 1 { "deletedRegion": { "endColumn": 1, - "endLine": 6, + "endLine": 2, "startColumn": 1, "startLine": 1 - }, - "insertedContent": { - "text": "from test import say_hy\n\nif __name__ == \"__main__\":\n say_hy(\"dear Ruff contributor\")\n" } } ] @@ -55,8 +52,8 @@ exit_code: 1 "uri": "[TMP]/input.py" }, "region": { - "endColumn": 35, - "endLine": 5, + "endColumn": 1, + "endLine": 2, "startColumn": 1, "startLine": 1 } From cbf7d1f318031b917ff88d29d6c2d7c451af64e0 Mon Sep 17 00:00:00 2001 From: Brent Westbrook Date: Mon, 29 Sep 2025 13:12:38 -0400 Subject: [PATCH 30/36] account for context lines when computing the line number width --- crates/ruff/src/commands/format.rs | 12 ++++++++++++ crates/ruff/tests/format.rs | 2 +- 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/crates/ruff/src/commands/format.rs b/crates/ruff/src/commands/format.rs index 1681d2e810ec84..28a2405b803159 100644 --- a/crates/ruff/src/commands/format.rs +++ b/crates/ruff/src/commands/format.rs @@ -745,6 +745,13 @@ impl<'a> FormatResults<'a> { &self, notebook_index: &mut FxHashMap, ) -> impl Iterator { + /// The number of unmodified context lines rendered in diffs. + /// + /// Note that this should be kept in sync with the argument to `TextDiff::grouped_ops` in + /// the diff rendering in `ruff_db` (currently 3). The `similar` crate uses two times that + /// argument as a cutoff for rendering unmodified lines. + const CONTEXT_LINES: u32 = 6; + self.results.iter().filter_map(|result| { let (unformatted, formatted) = match &result.result { FormatResult::Skipped | FormatResult::Unchanged => return None, @@ -864,6 +871,11 @@ impl<'a> FormatResults<'a> { // `ruff_db::diagnostic::render::full` into `annotate-snippets`, likely as another // `DisplayLine` variant and update the `lineno_width` calculation in // `DisplayList::fmt`. That would handle this offset "automatically." + let line_count = (line_count + CONTEXT_LINES).min( + formatted + .source_code() + .count_lines(TextRange::up_to(formatted.source_code().text_len())), + ); let lines = OneIndexed::new(line_count as usize).unwrap_or_default(); diagnostic.set_header_offset(lines.digits().get()); diff --git a/crates/ruff/tests/format.rs b/crates/ruff/tests/format.rs index 53f3296cff17b2..6205fa98e8b30e 100644 --- a/crates/ruff/tests/format.rs +++ b/crates/ruff/tests/format.rs @@ -680,7 +680,7 @@ fn output_format_notebook() { exit_code: 1 ----- stdout ----- unformatted: File would be reformatted - --> resources/test/fixtures/unformatted.ipynb:cell 1:1:1 + --> resources/test/fixtures/unformatted.ipynb:cell 1:1:1 ::: cell 1 1 | import numpy - maths = (numpy.arange(100)**2).sum() From 7b8b4426936de1172004c3e3085100be5c1e82fc Mon Sep 17 00:00:00 2001 From: Brent Westbrook Date: Mon, 29 Sep 2025 13:14:54 -0400 Subject: [PATCH 31/36] mark FormatModuleError::InvalidSyntax as an internal error --- crates/ruff/src/commands/format.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/ruff/src/commands/format.rs b/crates/ruff/src/commands/format.rs index 28a2405b803159..45d53008935f9f 100644 --- a/crates/ruff/src/commands/format.rs +++ b/crates/ruff/src/commands/format.rs @@ -944,7 +944,7 @@ impl From<&FormatCommandError> for Diagnostic { } FormatCommandError::Format(_, format_module_error) => match format_module_error { FormatModuleError::ParseError(parse_error) => Diagnostic::new( - DiagnosticId::InvalidSyntax, + DiagnosticId::InternalError, Severity::Error, &parse_error.error, ), @@ -1390,7 +1390,7 @@ mod tests { io: File not found --> test.py:1:1 - invalid-syntax: Expected index or slice expression + internal-error: Expected index or slice expression --> test.py:1:1 io: Cannot write to file From 74de549c448355c14f41a6965cbd766841784042 Mon Sep 17 00:00:00 2001 From: Brent Westbrook Date: Mon, 29 Sep 2025 13:35:25 -0400 Subject: [PATCH 32/36] use render_diagnostics helper --- crates/ruff/src/commands/format.rs | 83 +++--------------------------- 1 file changed, 7 insertions(+), 76 deletions(-) diff --git a/crates/ruff/src/commands/format.rs b/crates/ruff/src/commands/format.rs index 45d53008935f9f..cd333cc7c585b0 100644 --- a/crates/ruff/src/commands/format.rs +++ b/crates/ruff/src/commands/format.rs @@ -12,12 +12,9 @@ use log::{error, warn}; use rayon::iter::Either::{Left, Right}; use rayon::iter::{IntoParallelRefIterator, ParallelIterator}; use ruff_db::diagnostic::{ - Annotation, Diagnostic, DiagnosticFormat, DiagnosticId, DisplayDiagnosticConfig, - DisplayDiagnostics, DisplayGithubDiagnostics, GithubRenderer, Severity, Span, -}; -use ruff_linter::message::{ - Emitter, EmitterContext, GroupedEmitter, SarifEmitter, create_panic_diagnostic, + Annotation, Diagnostic, DiagnosticId, DisplayDiagnosticConfig, Severity, Span, }; +use ruff_linter::message::{EmitterContext, create_panic_diagnostic, render_diagnostics}; use ruff_linter::settings::types::OutputFormat; use ruff_notebook::NotebookIndex; use ruff_python_parser::ParseError; @@ -603,78 +600,12 @@ impl<'a> FormatResults<'a> { .collect(); let context = EmitterContext::new(¬ebook_index); - let config = DisplayDiagnosticConfig::default(); - match output_format { - OutputFormat::Concise => { - let config = config - .format(DiagnosticFormat::Concise) - .hide_severity(true) - .color(!cfg!(test) && colored::control::SHOULD_COLORIZE.should_colorize()); - let value = DisplayDiagnostics::new(&context, &config, &diagnostics); - write!(f, "{value}")?; - } - OutputFormat::Full => { - let config = config - .format(DiagnosticFormat::Full) - .hide_severity(true) - .show_fix_diff(true) - .color(!cfg!(test) && colored::control::SHOULD_COLORIZE.should_colorize()); - let value = DisplayDiagnostics::new(&context, &config, &diagnostics); - write!(f, "{value}")?; - } - OutputFormat::Json => { - let config = config.format(DiagnosticFormat::Json); - let value = DisplayDiagnostics::new(&context, &config, &diagnostics); - write!(f, "{value}")?; - } - OutputFormat::JsonLines => { - let config = config.format(DiagnosticFormat::JsonLines); - let value = DisplayDiagnostics::new(&context, &config, &diagnostics); - write!(f, "{value}")?; - } - OutputFormat::Junit => { - let config = config.format(DiagnosticFormat::Junit); - let value = DisplayDiagnostics::new(&context, &config, &diagnostics); - write!(f, "{value}")?; - } - OutputFormat::Grouped => { - GroupedEmitter::default() - .emit(f, &diagnostics, &context) - .map_err(std::io::Error::other)?; - } - OutputFormat::Github => { - let renderer = GithubRenderer::new(&context, "Ruff"); - let value = DisplayGithubDiagnostics::new(&renderer, &diagnostics); - write!(f, "{value}")?; - } - OutputFormat::Gitlab => { - let config = config.format(DiagnosticFormat::Gitlab); - let value = DisplayDiagnostics::new(&context, &config, &diagnostics); - write!(f, "{value}")?; - } - OutputFormat::Pylint => { - let config = config.format(DiagnosticFormat::Pylint); - let value = DisplayDiagnostics::new(&context, &config, &diagnostics); - write!(f, "{value}")?; - } - OutputFormat::Rdjson => { - let config = config.format(DiagnosticFormat::Rdjson); - let value = DisplayDiagnostics::new(&context, &config, &diagnostics); - write!(f, "{value}")?; - } - OutputFormat::Azure => { - let config = config.format(DiagnosticFormat::Azure); - let value = DisplayDiagnostics::new(&context, &config, &diagnostics); - write!(f, "{value}")?; - } - OutputFormat::Sarif => { - SarifEmitter - .emit(f, &diagnostics, &context) - .map_err(std::io::Error::other)?; - } - } + let config = DisplayDiagnosticConfig::default() + .hide_severity(true) + .show_fix_diff(true) + .color(!cfg!(test) && colored::control::SHOULD_COLORIZE.should_colorize()); - Ok(()) + render_diagnostics(f, output_format, config, &context, &diagnostics) } /// Write a summary of the formatting results to the given writer. From 6a72314d4c0c2fd6f65aa29ff55b6b561e0a131f Mon Sep 17 00:00:00 2001 From: Brent Westbrook Date: Mon, 29 Sep 2025 14:22:59 -0400 Subject: [PATCH 33/36] fix range narrowing for strict prefixes --- crates/ruff/src/commands/format.rs | 37 +++++++++++++++++------------- 1 file changed, 21 insertions(+), 16 deletions(-) diff --git a/crates/ruff/src/commands/format.rs b/crates/ruff/src/commands/format.rs index cd333cc7c585b0..aa39e11b93fa6a 100644 --- a/crates/ruff/src/commands/format.rs +++ b/crates/ruff/src/commands/format.rs @@ -1012,6 +1012,7 @@ impl Display for FormatCommandError { } } +#[derive(Debug)] struct ModifiedRange { unformatted: TextRange, formatted: TextRange, @@ -1020,9 +1021,7 @@ struct ModifiedRange { impl ModifiedRange { /// Determine the range that differs between `unformatted` and `formatted`. /// - /// # Panics - /// - /// This function panics if the there are no differences between the two inputs. + /// If the two inputs are equal, the returned ranges will be empty. fn new(unformatted: &SourceKind, formatted: &SourceKind) -> Self { let unformatted = unformatted.source_code(); let formatted = formatted.source_code(); @@ -1030,28 +1029,31 @@ impl ModifiedRange { let start = unformatted .char_indices() .zip(formatted.chars()) - .find_map(|((offset, old), new)| (old != new).then_some(offset)) - .expect("Expected at least one difference"); - - let start_size = TextSize::try_from(start).unwrap(); - - // This finds the position of the first character where the two suffixes differ in - // `unformatted`. - let mut offset = TextSize::ZERO; - for (old, new) in unformatted[start..] + .find_map(|((offset, old), new)| { + (old != new).then_some(TextSize::try_from(offset).unwrap()) + }) + // Fall back on the shorter text length if one of the strings is a strict prefix of the + // other (i.e. the zip iterator ended before finding a difference). + .unwrap_or_else(|| unformatted.text_len().min(formatted.text_len())); + + // For the ends of the ranges, track the length of the common suffix and then subtract that + // from each total text length. Unlike for `start`, the character offsets are very unlikely + // to be equal, so they need to be treated separately. + let mut suffix_length = TextSize::ZERO; + for (old, new) in unformatted[start.to_usize()..] .chars() .rev() - .zip(formatted[start..].chars().rev()) + .zip(formatted[start.to_usize()..].chars().rev()) { if old == new { - offset += old.text_len(); + suffix_length += old.text_len(); } else { break; } } - let unformatted_range = TextRange::new(start_size, unformatted.text_len() - offset); - let formatted_range = TextRange::new(start_size, formatted.text_len() - offset); + let unformatted_range = TextRange::new(start, unformatted.text_len() - suffix_length); + let formatted_range = TextRange::new(start, formatted.text_len() - suffix_length); Self { unformatted: unformatted_range, @@ -1343,6 +1345,9 @@ mod tests { #[test_case("abcdef", "abcXYdef", 3..3, 3..5; "insertion")] #[test_case("abcXYdef", "abcdef", 3..5, 3..3; "deletion")] #[test_case("abcXdef", "abcYdef", 3..4, 3..4; "modification")] + #[test_case("abc", "abcX", 3..3, 3..4; "strict_prefix")] + #[test_case("", "", 0..0, 0..0; "empty")] + #[test_case("abc", "abc", 3..3, 3..3; "equal")] fn modified_range( unformatted: &str, formatted: &str, From 793f5261da4d5ff3175c7ae659b5b594513b3b37 Mon Sep 17 00:00:00 2001 From: Brent Westbrook Date: Mon, 29 Sep 2025 14:31:11 -0400 Subject: [PATCH 34/36] range -> modified_range, restore previous range references --- crates/ruff/src/commands/format.rs | 26 ++++++++++++++++---------- 1 file changed, 16 insertions(+), 10 deletions(-) diff --git a/crates/ruff/src/commands/format.rs b/crates/ruff/src/commands/format.rs index aa39e11b93fa6a..dd6a839cb1c525 100644 --- a/crates/ruff/src/commands/format.rs +++ b/crates/ruff/src/commands/format.rs @@ -707,7 +707,7 @@ impl<'a> FormatResults<'a> { // Locate the first and last characters that differ to use as the diagnostic // range and to narrow the `Edit` range. - let range = ModifiedRange::new(unformatted, formatted); + let modified_range = ModifiedRange::new(unformatted, formatted); let path = result.path.to_string_lossy(); // For scripts, this is a single `Edit` using the `ModifiedRange` above, but notebook @@ -740,7 +740,11 @@ impl<'a> FormatResults<'a> { // ``` // // The intersection will be `Some` for all three cells in this case. - if range.unformatted.intersect(unformatted_range).is_some() { + if modified_range + .unformatted + .intersect(unformatted_range) + .is_some() + { let formatted = &formatted.source_code()[formatted_range]; let edit = if formatted.is_empty() { Edit::range_deletion(unformatted_range) @@ -763,9 +767,9 @@ impl<'a> FormatResults<'a> { let line_count = formatted .cell_offsets() .ranges() - .filter_map(|r| { - if range.formatted.contains_range(r) { - Some(source.count_lines(r)) + .filter_map(|range| { + if modified_range.formatted.contains_range(range) { + Some(source.count_lines(range)) } else { None } @@ -774,19 +778,21 @@ impl<'a> FormatResults<'a> { .unwrap_or_default(); (fix, line_count) } else { - let formatted_code = &formatted.source_code()[range.formatted]; + let formatted_code = &formatted.source_code()[modified_range.formatted]; let edit = if formatted_code.is_empty() { - Edit::range_deletion(range.unformatted) + Edit::range_deletion(modified_range.unformatted) } else { - Edit::range_replacement(formatted_code.to_string(), range.unformatted) + Edit::range_replacement(formatted_code.to_string(), modified_range.unformatted) }; let fix = Fix::safe_edit(edit); - let line_count = formatted.source_code().count_lines(range.formatted); + let line_count = formatted + .source_code() + .count_lines(modified_range.formatted); (fix, line_count) }; let source_file = SourceFileBuilder::new(path, unformatted.source_code()).finish(); - let span = Span::from(source_file).with_range(range.unformatted); + let span = Span::from(source_file).with_range(modified_range.unformatted); let mut annotation = Annotation::primary(span); annotation.hide_snippet(true); diagnostic.annotate(annotation); From bf48b9bc4220f6626f860f6cf2260fa64b74b9a4 Mon Sep 17 00:00:00 2001 From: Brent Westbrook Date: Mon, 29 Sep 2025 14:33:26 -0400 Subject: [PATCH 35/36] only restrict the end of the script line counting Using `modified_range.formatted` was incorrect because it would fail to count unchanged lines at the start of the file, so the computed line number would be too low --- crates/ruff/src/commands/format.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/ruff/src/commands/format.rs b/crates/ruff/src/commands/format.rs index dd6a839cb1c525..0772661ba85972 100644 --- a/crates/ruff/src/commands/format.rs +++ b/crates/ruff/src/commands/format.rs @@ -787,7 +787,7 @@ impl<'a> FormatResults<'a> { let fix = Fix::safe_edit(edit); let line_count = formatted .source_code() - .count_lines(modified_range.formatted); + .count_lines(TextRange::up_to(modified_range.formatted.end())); (fix, line_count) }; From bedfc6fd063e08f4d1e76ae8a780162404079298 Mon Sep 17 00:00:00 2001 From: Brent Westbrook Date: Tue, 30 Sep 2025 10:54:35 -0400 Subject: [PATCH 36/36] use loop for prefix_length too, use guarded break for suffix --- crates/ruff/src/commands/format.rs | 30 ++++++++++++++---------------- 1 file changed, 14 insertions(+), 16 deletions(-) diff --git a/crates/ruff/src/commands/format.rs b/crates/ruff/src/commands/format.rs index 0772661ba85972..20c000a89d5dc0 100644 --- a/crates/ruff/src/commands/format.rs +++ b/crates/ruff/src/commands/format.rs @@ -1032,34 +1032,32 @@ impl ModifiedRange { let unformatted = unformatted.source_code(); let formatted = formatted.source_code(); - let start = unformatted - .char_indices() - .zip(formatted.chars()) - .find_map(|((offset, old), new)| { - (old != new).then_some(TextSize::try_from(offset).unwrap()) - }) - // Fall back on the shorter text length if one of the strings is a strict prefix of the - // other (i.e. the zip iterator ended before finding a difference). - .unwrap_or_else(|| unformatted.text_len().min(formatted.text_len())); + let mut prefix_length = TextSize::ZERO; + for (unformatted, formatted) in unformatted.chars().zip(formatted.chars()) { + if unformatted != formatted { + break; + } + prefix_length += unformatted.text_len(); + } // For the ends of the ranges, track the length of the common suffix and then subtract that // from each total text length. Unlike for `start`, the character offsets are very unlikely // to be equal, so they need to be treated separately. let mut suffix_length = TextSize::ZERO; - for (old, new) in unformatted[start.to_usize()..] + for (old, new) in unformatted[prefix_length.to_usize()..] .chars() .rev() - .zip(formatted[start.to_usize()..].chars().rev()) + .zip(formatted[prefix_length.to_usize()..].chars().rev()) { - if old == new { - suffix_length += old.text_len(); - } else { + if old != new { break; } + suffix_length += old.text_len(); } - let unformatted_range = TextRange::new(start, unformatted.text_len() - suffix_length); - let formatted_range = TextRange::new(start, formatted.text_len() - suffix_length); + let unformatted_range = + TextRange::new(prefix_length, unformatted.text_len() - suffix_length); + let formatted_range = TextRange::new(prefix_length, formatted.text_len() - suffix_length); Self { unformatted: unformatted_range,