diff --git a/crates/ruff/src/commands/format.rs b/crates/ruff/src/commands/format.rs index 756e8c3584..420a670926 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 b94bf98e94..39fe3bdd89 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 08341f30f2..30eb3a0a51 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 2728cfba7b..fb88a26223 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 339cd3a850..4fca75c571 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 1648456489..1eab12d036 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 2ab885cdc5..785329af9c 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); }