Account for removal of multiline span in suggestion

compiler/rustc_errors/src/emitter.rs

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

tests/ui/error-emitter/multiline-removal-suggestion.rs

@@ -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() {}