Skip to content

Commit

Permalink
Unrolled build for rust-lang#134664
Browse files Browse the repository at this point in the history
Rollup merge of rust-lang#134664 - estebank:sugg-highlighting, r=jieyouxu

Account for removal of multiline span in suggestion

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

Fix rust-lang#134485.

![Screenshot of the highlighted output](https://github.com/user-attachments/assets/18bcd9bc-3bec-4b79-a9d7-e4ea4e6289ad)
  • Loading branch information
rust-timer authored Dec 26, 2024
2 parents 19e75f4 + 12d66d9 commit 223d53c
Show file tree
Hide file tree
Showing 3 changed files with 641 additions and 7 deletions.
86 changes: 79 additions & 7 deletions compiler/rustc_errors/src/emitter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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
Expand Down
58 changes: 58 additions & 0 deletions tests/ui/error-emitter/multiline-removal-suggestion.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
// Make sure suggestion for removal of a span that covers multiple lines is properly highlighted.
//@ compile-flags: --error-format=human --color=always
//@ edition:2018
//@ only-linux
// ignore-tidy-tab
// We use `\t` instead of spaces for indentation to ensure that the highlighting logic properly
// accounts for replaced characters (like we do for `\t` with ` `). The naïve way of highlighting
// could be counting chars of the original code, instead of operating on the code as it is being
// displayed.
use std::collections::{HashMap, HashSet};
fn foo() -> Vec<(bool, HashSet<u8>)> {
let mut hm = HashMap::<bool, Vec<HashSet<u8>>>::new();
hm.into_iter()
.map(|(is_true, ts)| {
ts.into_iter()
.map(|t| {
(
is_true,
t,
)
}).flatten()
})
.flatten()
.collect()
}
fn bar() -> Vec<(bool, HashSet<u8>)> {
let mut hm = HashMap::<bool, Vec<HashSet<u8>>>::new();
hm.into_iter()
.map(|(is_true, ts)| {
ts.into_iter()
.map(|t| (is_true, t))
.flatten()
})
.flatten()
.collect()
}
fn baz() -> Vec<(bool, HashSet<u8>)> {
let mut hm = HashMap::<bool, Vec<HashSet<u8>>>::new();
hm.into_iter()
.map(|(is_true, ts)| {
ts.into_iter().map(|t| {
(is_true, t)
}).flatten()
})
.flatten()
.collect()
}
fn bay() -> Vec<(bool, HashSet<u8>)> {
let mut hm = HashMap::<bool, Vec<HashSet<u8>>>::new();
hm.into_iter()
.map(|(is_true, ts)| {
ts.into_iter()
.map(|t| (is_true, t)).flatten()
})
.flatten()
.collect()
}
fn main() {}
Loading

0 comments on commit 223d53c

Please sign in to comment.