Skip to content

Commit 0edaf9a

Browse files
committed
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 #134485.
1 parent 387b245 commit 0edaf9a

File tree

3 files changed

+608
-7
lines changed

3 files changed

+608
-7
lines changed

compiler/rustc_errors/src/emitter.rs

+51-7
Original file line numberDiff line numberDiff line change
@@ -2216,6 +2216,11 @@ impl HumanEmitter {
22162216
show_code_change
22172217
{
22182218
for part in parts {
2219+
let snippet = if let Ok(snippet) = sm.span_to_snippet(part.span) {
2220+
snippet
2221+
} else {
2222+
String::new()
2223+
};
22192224
let span_start_pos = sm.lookup_char_pos(part.span.lo()).col_display;
22202225
let span_end_pos = sm.lookup_char_pos(part.span.hi()).col_display;
22212226

@@ -2263,13 +2268,52 @@ impl HumanEmitter {
22632268
}
22642269
if let DisplaySuggestion::Diff = show_code_change {
22652270
// Colorize removal with red in diff format.
2266-
buffer.set_style_range(
2267-
row_num - 2,
2268-
(padding as isize + span_start_pos as isize) as usize,
2269-
(padding as isize + span_end_pos as isize) as usize,
2270-
Style::Removal,
2271-
true,
2272-
);
2271+
2272+
let newlines = snippet.lines().count();
2273+
if newlines > 0 && row_num > newlines {
2274+
// Account for removals where the part being removed spans multiple
2275+
// lines.
2276+
// FIXME: We check the number of rows because in some cases, like in
2277+
// `tests/ui/lint/invalid-nan-comparison-suggestion.rs`, the rendered
2278+
// suggestion will only show the first line of code being replaced. The
2279+
// proper way of doing this would be to change the suggestion rendering
2280+
// logic to show the whole prior snippet, but the current output is not
2281+
// too bad to begin with, so we side-step that issue here.
2282+
for (i, line) in snippet.lines().enumerate() {
2283+
let line = normalize_whitespace(line);
2284+
let row = row_num - 2 - (newlines - i - 1);
2285+
// On the first line, we highlight between the start of the part
2286+
// span, and the end of that line.
2287+
// On the last line, we highlight between the start of the line, and
2288+
// the column of the part span end.
2289+
// On all others, we highlight the whole line.
2290+
let start = if i == 0 {
2291+
(padding as isize + span_start_pos as isize) as usize
2292+
} else {
2293+
padding
2294+
};
2295+
let end = if i == 0 {
2296+
(padding as isize
2297+
+ span_start_pos as isize
2298+
+ line.len() as isize)
2299+
as usize
2300+
} else if i == newlines - 1 {
2301+
(padding as isize + span_end_pos as isize) as usize
2302+
} else {
2303+
(padding as isize + line.len() as isize) as usize
2304+
};
2305+
buffer.set_style_range(row, start, end, Style::Removal, true);
2306+
}
2307+
} else {
2308+
// The removed code fits all in one line.
2309+
buffer.set_style_range(
2310+
row_num - 2,
2311+
(padding as isize + span_start_pos as isize) as usize,
2312+
(padding as isize + span_end_pos as isize) as usize,
2313+
Style::Removal,
2314+
true,
2315+
);
2316+
}
22732317
}
22742318

22752319
// length of the code after substitution
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
// Make sure suggestion for removal of a span that covers multiple lines is properly highlighted.
2+
//@ compile-flags: --error-format=human --color=always
3+
//@ edition:2018
4+
//@ ignore-tidy-tab
5+
use std::collections::{HashMap, HashSet};
6+
fn foo() -> Vec<(bool, HashSet<u8>)> {
7+
let mut hm = HashMap::<bool, Vec<HashSet<u8>>>::new();
8+
hm.into_iter()
9+
.map(|(is_true, ts)| {
10+
ts.into_iter()
11+
.map(|t| {
12+
(
13+
is_true,
14+
t,
15+
)
16+
}).flatten()
17+
})
18+
.flatten()
19+
.collect()
20+
}
21+
fn bar() -> Vec<(bool, HashSet<u8>)> {
22+
let mut hm = HashMap::<bool, Vec<HashSet<u8>>>::new();
23+
hm.into_iter()
24+
.map(|(is_true, ts)| {
25+
ts.into_iter()
26+
.map(|t| (is_true, t))
27+
.flatten()
28+
})
29+
.flatten()
30+
.collect()
31+
}
32+
fn baz() -> Vec<(bool, HashSet<u8>)> {
33+
let mut hm = HashMap::<bool, Vec<HashSet<u8>>>::new();
34+
hm.into_iter()
35+
.map(|(is_true, ts)| {
36+
ts.into_iter().map(|t| {
37+
(is_true, t)
38+
}).flatten()
39+
})
40+
.flatten()
41+
.collect()
42+
}
43+
fn bay() -> Vec<(bool, HashSet<u8>)> {
44+
let mut hm = HashMap::<bool, Vec<HashSet<u8>>>::new();
45+
hm.into_iter()
46+
.map(|(is_true, ts)| {
47+
ts.into_iter()
48+
.map(|t| (is_true, t)).flatten()
49+
})
50+
.flatten()
51+
.collect()
52+
}
53+
fn main() {}

0 commit comments

Comments
 (0)