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.
This commit is contained in:
Brent Westbrook 2025-09-25 12:10:46 -04:00
parent 4ea16b7dd6
commit 39f96cc9f0
7 changed files with 76 additions and 16 deletions

View file

@ -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)

View file

@ -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 |

View file

@ -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,7 +82,8 @@ impl Display for DisplayList<'_> {
_ => max,
})
});
let lineno_width = if lineno_width == 0 {
let lineno_width = self.lineno_offset
+ if lineno_width == 0 {
lineno_width
} else if self.anonymized_line_numbers {
ANONYMIZED_LINE_NUM.len()
@ -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![];

View file

@ -23,6 +23,7 @@ pub struct Message<'a> {
pub(crate) snippets: Vec<Snippet<'a>>,
pub(crate) footer: Vec<Message<'a>>,
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,
}
}

View file

@ -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<TextSize>,
noqa_offset: Option<TextSize>,
secondary_code: Option<SecondaryCode>,
header_offset: usize,
}
struct RenderingSortKey<'a> {

View file

@ -208,6 +208,7 @@ struct ResolvedDiagnostic<'a> {
message: String,
annotations: Vec<ResolvedAnnotation<'a>>,
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);
}