From 0edaf9ae4196699c36d10d61b6287dd8a98ae4d2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Sun, 22 Dec 2024 20:20:02 +0000 Subject: [PATCH] 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. --- compiler/rustc_errors/src/emitter.rs | 58 +- .../multiline-removal-suggestion.rs | 53 ++ .../multiline-removal-suggestion.svg | 504 ++++++++++++++++++ 3 files changed, 608 insertions(+), 7 deletions(-) create mode 100644 tests/ui/error-emitter/multiline-removal-suggestion.rs create mode 100644 tests/ui/error-emitter/multiline-removal-suggestion.svg diff --git a/compiler/rustc_errors/src/emitter.rs b/compiler/rustc_errors/src/emitter.rs index 977721a5b8a2d..1df191957d779 100644 --- a/compiler/rustc_errors/src/emitter.rs +++ b/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,52 @@ 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, - ); + + 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 diff --git a/tests/ui/error-emitter/multiline-removal-suggestion.rs b/tests/ui/error-emitter/multiline-removal-suggestion.rs new file mode 100644 index 0000000000000..0ac891b843b24 --- /dev/null +++ b/tests/ui/error-emitter/multiline-removal-suggestion.rs @@ -0,0 +1,53 @@ +// Make sure suggestion for removal of a span that covers multiple lines is properly highlighted. +//@ compile-flags: --error-format=human --color=always +//@ edition:2018 +//@ ignore-tidy-tab +use std::collections::{HashMap, HashSet}; +fn foo() -> Vec<(bool, HashSet)> { + let mut hm = HashMap::>>::new(); + hm.into_iter() + .map(|(is_true, ts)| { + ts.into_iter() + .map(|t| { + ( + is_true, + t, + ) + }).flatten() + }) + .flatten() + .collect() +} +fn bar() -> Vec<(bool, HashSet)> { + let mut hm = HashMap::>>::new(); + hm.into_iter() + .map(|(is_true, ts)| { + ts.into_iter() + .map(|t| (is_true, t)) + .flatten() + }) + .flatten() + .collect() +} +fn baz() -> Vec<(bool, HashSet)> { + let mut hm = HashMap::>>::new(); + hm.into_iter() + .map(|(is_true, ts)| { + ts.into_iter().map(|t| { + (is_true, t) + }).flatten() + }) + .flatten() + .collect() +} +fn bay() -> Vec<(bool, HashSet)> { + let mut hm = HashMap::>>::new(); + hm.into_iter() + .map(|(is_true, ts)| { + ts.into_iter() + .map(|t| (is_true, t)).flatten() + }) + .flatten() + .collect() +} +fn main() {} diff --git a/tests/ui/error-emitter/multiline-removal-suggestion.svg b/tests/ui/error-emitter/multiline-removal-suggestion.svg new file mode 100644 index 0000000000000..0d485eb74166b --- /dev/null +++ b/tests/ui/error-emitter/multiline-removal-suggestion.svg @@ -0,0 +1,504 @@ + + + + + + + error[E0277]: `(bool, HashSet<u8>)` is not an iterator + + --> $DIR/multiline-removal-suggestion.rs:16:8 + + | + + LL | }).flatten() + + | ^^^^^^^ `(bool, HashSet<u8>)` is not an iterator + + | + + = help: the trait `Iterator` is not implemented for `(bool, HashSet<u8>)` + + = note: required for `(bool, HashSet<u8>)` to implement `IntoIterator` + + note: required by a bound in `flatten` + + --> $SRC_DIR/core/src/iter/traits/iterator.rs:LL:COL + + help: consider removing this method call, as the receiver has type `std::vec::IntoIter<HashSet<u8>>` and `std::vec::IntoIter<HashSet<u8>>: Iterator` trivially holds + + | + + LL - ts.into_iter() + + LL - .map(|t| { + + LL - ( + + LL - is_true, + + LL - t, + + LL - ) + + LL - }).flatten() + + LL + ts.into_iter().flatten() + + | + + + + error[E0277]: `(bool, HashSet<u8>)` is not an iterator + + --> $DIR/multiline-removal-suggestion.rs:8:2 + + | + + LL | / hm.into_iter() + + LL | | .map(|(is_true, ts)| { + + LL | | ts.into_iter() + + LL | | .map(|t| { + + ... | + + LL | | }).flatten() + + LL | | }) + + | |__________^ `(bool, HashSet<u8>)` is not an iterator + + | + + = help: the trait `Iterator` is not implemented for `(bool, HashSet<u8>)` + + = note: required for `(bool, HashSet<u8>)` to implement `IntoIterator` + + note: required by a bound in `Flatten` + + --> $SRC_DIR/core/src/iter/adapters/flatten.rs:LL:COL + + + + error[E0599]: the method `collect` exists for struct `Flatten<Map<IntoIter<bool, Vec<HashSet<u8>>>, {closure@multiline-removal-suggestion.rs:9:8}>>`, but its trait bounds were not satisfied + + --> $DIR/multiline-removal-suggestion.rs:19:4 + + | + + LL | / hm.into_iter() + + LL | | .map(|(is_true, ts)| { + + LL | | ts.into_iter() + + LL | | .map(|t| { + + ... | + + LL | | .flatten() + + LL | | .collect() + + | | -^^^^^^^ method cannot be called due to unsatisfied trait bounds + + | |_________| + + | + + | + + = note: the following trait bounds were not satisfied: + + `<Flatten<Map<std::vec::IntoIter<HashSet<u8>>, {closure@$DIR/multiline-removal-suggestion.rs:11:10: 11:13}>> as IntoIterator>::IntoIter = _` + + which is required by `Flatten<Map<std::collections::hash_map::IntoIter<bool, Vec<HashSet<u8>>>, {closure@$DIR/multiline-removal-suggestion.rs:9:8: 9:23}>>: Iterator` + + `<Flatten<Map<std::vec::IntoIter<HashSet<u8>>, {closure@$DIR/multiline-removal-suggestion.rs:11:10: 11:13}>> as IntoIterator>::Item = _` + + which is required by `Flatten<Map<std::collections::hash_map::IntoIter<bool, Vec<HashSet<u8>>>, {closure@$DIR/multiline-removal-suggestion.rs:9:8: 9:23}>>: Iterator` + + `Flatten<Map<std::vec::IntoIter<HashSet<u8>>, {closure@$DIR/multiline-removal-suggestion.rs:11:10: 11:13}>>: IntoIterator` + + which is required by `Flatten<Map<std::collections::hash_map::IntoIter<bool, Vec<HashSet<u8>>>, {closure@$DIR/multiline-removal-suggestion.rs:9:8: 9:23}>>: Iterator` + + `Flatten<Map<std::collections::hash_map::IntoIter<bool, Vec<HashSet<u8>>>, {closure@$DIR/multiline-removal-suggestion.rs:9:8: 9:23}>>: Iterator` + + which is required by `&mut Flatten<Map<std::collections::hash_map::IntoIter<bool, Vec<HashSet<u8>>>, {closure@$DIR/multiline-removal-suggestion.rs:9:8: 9:23}>>: Iterator` + + + + error[E0277]: `(bool, HashSet<u8>)` is not an iterator + + --> $DIR/multiline-removal-suggestion.rs:27:6 + + | + + LL | .flatten() + + | ^^^^^^^ `(bool, HashSet<u8>)` is not an iterator + + | + + = help: the trait `Iterator` is not implemented for `(bool, HashSet<u8>)` + + = note: required for `(bool, HashSet<u8>)` to implement `IntoIterator` + + note: required by a bound in `flatten` + + --> $SRC_DIR/core/src/iter/traits/iterator.rs:LL:COL + + help: consider removing this method call, as the receiver has type `std::vec::IntoIter<HashSet<u8>>` and `std::vec::IntoIter<HashSet<u8>>: Iterator` trivially holds + + | + + LL - ts.into_iter() + + LL - .map(|t| (is_true, t)) + + LL + ts.into_iter() + + | + + + + error[E0277]: `(bool, HashSet<u8>)` is not an iterator + + --> $DIR/multiline-removal-suggestion.rs:23:2 + + | + + LL | / hm.into_iter() + + LL | | .map(|(is_true, ts)| { + + LL | | ts.into_iter() + + LL | | .map(|t| (is_true, t)) + + LL | | .flatten() + + LL | | }) + + | |__________^ `(bool, HashSet<u8>)` is not an iterator + + | + + = help: the trait `Iterator` is not implemented for `(bool, HashSet<u8>)` + + = note: required for `(bool, HashSet<u8>)` to implement `IntoIterator` + + note: required by a bound in `Flatten` + + --> $SRC_DIR/core/src/iter/adapters/flatten.rs:LL:COL + + + + error[E0599]: the method `collect` exists for struct `Flatten<Map<IntoIter<bool, Vec<HashSet<u8>>>, {closure@multiline-removal-suggestion.rs:24:8}>>`, but its trait bounds were not satisfied + + --> $DIR/multiline-removal-suggestion.rs:30:4 + + | + + LL | / hm.into_iter() + + LL | | .map(|(is_true, ts)| { + + LL | | ts.into_iter() + + LL | | .map(|t| (is_true, t)) + + ... | + + LL | | .flatten() + + LL | | .collect() + + | | -^^^^^^^ method cannot be called due to unsatisfied trait bounds + + | |_________| + + | + + | + + = note: the following trait bounds were not satisfied: + + `<Flatten<Map<std::vec::IntoIter<HashSet<u8>>, {closure@$DIR/multiline-removal-suggestion.rs:26:10: 26:13}>> as IntoIterator>::IntoIter = _` + + which is required by `Flatten<Map<std::collections::hash_map::IntoIter<bool, Vec<HashSet<u8>>>, {closure@$DIR/multiline-removal-suggestion.rs:24:8: 24:23}>>: Iterator` + + `<Flatten<Map<std::vec::IntoIter<HashSet<u8>>, {closure@$DIR/multiline-removal-suggestion.rs:26:10: 26:13}>> as IntoIterator>::Item = _` + + which is required by `Flatten<Map<std::collections::hash_map::IntoIter<bool, Vec<HashSet<u8>>>, {closure@$DIR/multiline-removal-suggestion.rs:24:8: 24:23}>>: Iterator` + + `Flatten<Map<std::vec::IntoIter<HashSet<u8>>, {closure@$DIR/multiline-removal-suggestion.rs:26:10: 26:13}>>: IntoIterator` + + which is required by `Flatten<Map<std::collections::hash_map::IntoIter<bool, Vec<HashSet<u8>>>, {closure@$DIR/multiline-removal-suggestion.rs:24:8: 24:23}>>: Iterator` + + `Flatten<Map<std::collections::hash_map::IntoIter<bool, Vec<HashSet<u8>>>, {closure@$DIR/multiline-removal-suggestion.rs:24:8: 24:23}>>: Iterator` + + which is required by `&mut Flatten<Map<std::collections::hash_map::IntoIter<bool, Vec<HashSet<u8>>>, {closure@$DIR/multiline-removal-suggestion.rs:24:8: 24:23}>>: Iterator` + + + + error[E0277]: `(bool, HashSet<u8>)` is not an iterator + + --> $DIR/multiline-removal-suggestion.rs:38:7 + + | + + LL | }).flatten() + + | ^^^^^^^ `(bool, HashSet<u8>)` is not an iterator + + | + + = help: the trait `Iterator` is not implemented for `(bool, HashSet<u8>)` + + = note: required for `(bool, HashSet<u8>)` to implement `IntoIterator` + + note: required by a bound in `flatten` + + --> $SRC_DIR/core/src/iter/traits/iterator.rs:LL:COL + + help: consider removing this method call, as the receiver has type `std::vec::IntoIter<HashSet<u8>>` and `std::vec::IntoIter<HashSet<u8>>: Iterator` trivially holds + + | + + LL - ts.into_iter().map(|t| { + + LL - (is_true, t) + + LL - }).flatten() + + LL + ts.into_iter().flatten() + + | + + + + error[E0277]: `(bool, HashSet<u8>)` is not an iterator + + --> $DIR/multiline-removal-suggestion.rs:34:2 + + | + + LL | / hm.into_iter() + + LL | | .map(|(is_true, ts)| { + + LL | | ts.into_iter().map(|t| { + + LL | | (is_true, t) + + LL | | }).flatten() + + LL | | }) + + | |__________^ `(bool, HashSet<u8>)` is not an iterator + + | + + = help: the trait `Iterator` is not implemented for `(bool, HashSet<u8>)` + + = note: required for `(bool, HashSet<u8>)` to implement `IntoIterator` + + note: required by a bound in `Flatten` + + --> $SRC_DIR/core/src/iter/adapters/flatten.rs:LL:COL + + + + error[E0599]: the method `collect` exists for struct `Flatten<Map<IntoIter<bool, Vec<HashSet<u8>>>, {closure@multiline-removal-suggestion.rs:35:8}>>`, but its trait bounds were not satisfied + + --> $DIR/multiline-removal-suggestion.rs:41:4 + + | + + LL | / hm.into_iter() + + LL | | .map(|(is_true, ts)| { + + LL | | ts.into_iter().map(|t| { + + LL | | (is_true, t) + + ... | + + LL | | .flatten() + + LL | | .collect() + + | | -^^^^^^^ method cannot be called due to unsatisfied trait bounds + + | |_________| + + | + + | + + = note: the following trait bounds were not satisfied: + + `<Flatten<Map<std::vec::IntoIter<HashSet<u8>>, {closure@$DIR/multiline-removal-suggestion.rs:36:23: 36:26}>> as IntoIterator>::IntoIter = _` + + which is required by `Flatten<Map<std::collections::hash_map::IntoIter<bool, Vec<HashSet<u8>>>, {closure@$DIR/multiline-removal-suggestion.rs:35:8: 35:23}>>: Iterator` + + `<Flatten<Map<std::vec::IntoIter<HashSet<u8>>, {closure@$DIR/multiline-removal-suggestion.rs:36:23: 36:26}>> as IntoIterator>::Item = _` + + which is required by `Flatten<Map<std::collections::hash_map::IntoIter<bool, Vec<HashSet<u8>>>, {closure@$DIR/multiline-removal-suggestion.rs:35:8: 35:23}>>: Iterator` + + `Flatten<Map<std::vec::IntoIter<HashSet<u8>>, {closure@$DIR/multiline-removal-suggestion.rs:36:23: 36:26}>>: IntoIterator` + + which is required by `Flatten<Map<std::collections::hash_map::IntoIter<bool, Vec<HashSet<u8>>>, {closure@$DIR/multiline-removal-suggestion.rs:35:8: 35:23}>>: Iterator` + + `Flatten<Map<std::collections::hash_map::IntoIter<bool, Vec<HashSet<u8>>>, {closure@$DIR/multiline-removal-suggestion.rs:35:8: 35:23}>>: Iterator` + + which is required by `&mut Flatten<Map<std::collections::hash_map::IntoIter<bool, Vec<HashSet<u8>>>, {closure@$DIR/multiline-removal-suggestion.rs:35:8: 35:23}>>: Iterator` + + + + error[E0277]: `(bool, HashSet<u8>)` is not an iterator + + --> $DIR/multiline-removal-suggestion.rs:48:28 + + | + + LL | .map(|t| (is_true, t)).flatten() + + | ^^^^^^^ `(bool, HashSet<u8>)` is not an iterator + + | + + = help: the trait `Iterator` is not implemented for `(bool, HashSet<u8>)` + + = note: required for `(bool, HashSet<u8>)` to implement `IntoIterator` + + note: required by a bound in `flatten` + + --> $SRC_DIR/core/src/iter/traits/iterator.rs:LL:COL + + help: consider removing this method call, as the receiver has type `std::vec::IntoIter<HashSet<u8>>` and `std::vec::IntoIter<HashSet<u8>>: Iterator` trivially holds + + | + + LL - ts.into_iter() + + LL - .map(|t| (is_true, t)).flatten() + + LL + ts.into_iter().flatten() + + | + + + + error[E0277]: `(bool, HashSet<u8>)` is not an iterator + + --> $DIR/multiline-removal-suggestion.rs:45:2 + + | + + LL | / hm.into_iter() + + LL | | .map(|(is_true, ts)| { + + LL | | ts.into_iter() + + LL | | .map(|t| (is_true, t)).flatten() + + LL | | }) + + | |__________^ `(bool, HashSet<u8>)` is not an iterator + + | + + = help: the trait `Iterator` is not implemented for `(bool, HashSet<u8>)` + + = note: required for `(bool, HashSet<u8>)` to implement `IntoIterator` + + note: required by a bound in `Flatten` + + --> $SRC_DIR/core/src/iter/adapters/flatten.rs:LL:COL + + + + error[E0599]: the method `collect` exists for struct `Flatten<Map<IntoIter<bool, Vec<HashSet<u8>>>, {closure@multiline-removal-suggestion.rs:46:8}>>`, but its trait bounds were not satisfied + + --> $DIR/multiline-removal-suggestion.rs:51:4 + + | + + LL | / hm.into_iter() + + LL | | .map(|(is_true, ts)| { + + LL | | ts.into_iter() + + LL | | .map(|t| (is_true, t)).flatten() + + LL | | }) + + LL | | .flatten() + + LL | | .collect() + + | | -^^^^^^^ method cannot be called due to unsatisfied trait bounds + + | |_________| + + | + + | + + = note: the following trait bounds were not satisfied: + + `<Flatten<Map<std::vec::IntoIter<HashSet<u8>>, {closure@$DIR/multiline-removal-suggestion.rs:48:10: 48:13}>> as IntoIterator>::IntoIter = _` + + which is required by `Flatten<Map<std::collections::hash_map::IntoIter<bool, Vec<HashSet<u8>>>, {closure@$DIR/multiline-removal-suggestion.rs:46:8: 46:23}>>: Iterator` + + `<Flatten<Map<std::vec::IntoIter<HashSet<u8>>, {closure@$DIR/multiline-removal-suggestion.rs:48:10: 48:13}>> as IntoIterator>::Item = _` + + which is required by `Flatten<Map<std::collections::hash_map::IntoIter<bool, Vec<HashSet<u8>>>, {closure@$DIR/multiline-removal-suggestion.rs:46:8: 46:23}>>: Iterator` + + `Flatten<Map<std::vec::IntoIter<HashSet<u8>>, {closure@$DIR/multiline-removal-suggestion.rs:48:10: 48:13}>>: IntoIterator` + + which is required by `Flatten<Map<std::collections::hash_map::IntoIter<bool, Vec<HashSet<u8>>>, {closure@$DIR/multiline-removal-suggestion.rs:46:8: 46:23}>>: Iterator` + + `Flatten<Map<std::collections::hash_map::IntoIter<bool, Vec<HashSet<u8>>>, {closure@$DIR/multiline-removal-suggestion.rs:46:8: 46:23}>>: Iterator` + + which is required by `&mut Flatten<Map<std::collections::hash_map::IntoIter<bool, Vec<HashSet<u8>>>, {closure@$DIR/multiline-removal-suggestion.rs:46:8: 46:23}>>: Iterator` + + + + error: aborting due to 12 previous errors + + + + Some errors have detailed explanations: E0277, E0599. + + For more information about an error, try `rustc --explain E0277`. + + + + + +