bjoernager/rust
bjoernager/rust
1
Fork 0
Code Issues Pull requests Activity

Account for removal of multiline span in suggestion

Browse source 
When highlighting the removed parts of a suggestion, properly account for spans that cover more than one line.

Fix #134485.
This commit is contained in:
Esteban Küber 2024-12-22 20:20:02 +00:00
parent 387b245664
commit 12d66d9506
3 changed files with 641 additions and 7 deletions
Download patch file Download diff file Expand all files Collapse all files

86
compiler/rustc_errors/src/emitter.rs
View file

@ -2216,6 +2216,11 @@ impl HumanEmitter {
show_code_change
{
for part in parts {
let snippet = if let Ok(snippet) = sm.span_to_snippet(part.span) {
snippet
} else {
String::new()
};
let span_start_pos = sm.lookup_char_pos(part.span.lo()).col_display;
let span_end_pos = sm.lookup_char_pos(part.span.hi()).col_display;
@ -2263,13 +2268,80 @@ impl HumanEmitter {
}
if let DisplaySuggestion::Diff = show_code_change {
// Colorize removal with red in diff format.
buffer.set_style_range(
row_num - 2,
(padding as isize + span_start_pos as isize) as usize,
(padding as isize + span_end_pos as isize) as usize,
Style::Removal,
true,
);
// Below, there's some tricky buffer indexing going on. `row_num` at this
// point corresponds to:
//
// |
// LL | CODE
// | ++++ <- `row_num`
//
// in the buffer. When we have a diff format output, we end up with
//
// |
// LL - OLDER <- row_num - 2
// LL + NEWER
// | <- row_num
//
// The `row_num - 2` is to select the buffer line that has the "old version
// of the diff" at that point. When the removal is a single line, `i` is
// `0`, `newlines` is `1` so `(newlines - i - 1)` ends up being `0`, so row
// points at `LL - OLDER`. When the removal corresponds to multiple lines,
// we end up with `newlines > 1` and `i` being `0..newlines - 1`.
//
// |
// LL - OLDER <- row_num - 2 - (newlines - last_i - 1)
// LL - CODE
// LL - BEING
// LL - REMOVED <- row_num - 2 - (newlines - first_i - 1)
// LL + NEWER
// | <- row_num
let newlines = snippet.lines().count();
if newlines > 0 && row_num > newlines {
// Account for removals where the part being removed spans multiple
// lines.
// FIXME: We check the number of rows because in some cases, like in
// `tests/ui/lint/invalid-nan-comparison-suggestion.rs`, the rendered
// suggestion will only show the first line of code being replaced. The
// proper way of doing this would be to change the suggestion rendering
// logic to show the whole prior snippet, but the current output is not
// too bad to begin with, so we side-step that issue here.
for (i, line) in snippet.lines().enumerate() {
let line = normalize_whitespace(line);
let row = row_num - 2 - (newlines - i - 1);
// On the first line, we highlight between the start of the part
// span, and the end of that line.
// On the last line, we highlight between the start of the line, and
// the column of the part span end.
// On all others, we highlight the whole line.
let start = if i == 0 {
(padding as isize + span_start_pos as isize) as usize
} else {
padding
};
let end = if i == 0 {
(padding as isize
+ span_start_pos as isize
+ line.len() as isize)
as usize
} else if i == newlines - 1 {
(padding as isize + span_end_pos as isize) as usize
} else {
(padding as isize + line.len() as isize) as usize
};
buffer.set_style_range(row, start, end, Style::Removal, true);
}
} else {
// The removed code fits all in one line.
buffer.set_style_range(
row_num - 2,
(padding as isize + span_start_pos as isize) as usize,
(padding as isize + span_end_pos as isize) as usize,
Style::Removal,
true,
);
}
}
// length of the code after substitution