From e284907061eef3c38a2403bb702c85060080857f Mon Sep 17 00:00:00 2001 From: Scott Schafer Date: Tue, 10 Jun 2025 05:12:11 -0600 Subject: [PATCH 1/2] test: Add rustc multiline-removal-suggestion --- tests/color/main.rs | 1 + tests/color/multiline_removal_suggestion.rs | 113 ++++++++++++++++++ .../multiline_removal_suggestion.term.svg | 68 +++++++++++ 3 files changed, 182 insertions(+) create mode 100644 tests/color/multiline_removal_suggestion.rs create mode 100644 tests/color/multiline_removal_suggestion.term.svg diff --git a/tests/color/main.rs b/tests/color/main.rs index f954bb7a..a9885a0b 100644 --- a/tests/color/main.rs +++ b/tests/color/main.rs @@ -9,6 +9,7 @@ mod fold_bad_origin_line; mod fold_leading; mod fold_trailing; mod issue_9; +mod multiline_removal_suggestion; mod multiple_annotations; mod simple; mod strip_line; diff --git a/tests/color/multiline_removal_suggestion.rs b/tests/color/multiline_removal_suggestion.rs new file mode 100644 index 00000000..fbc47540 --- /dev/null +++ b/tests/color/multiline_removal_suggestion.rs @@ -0,0 +1,113 @@ +use annotate_snippets::{AnnotationKind, Group, Level, Origin, Patch, Renderer, Snippet}; + +use snapbox::{assert_data_eq, file}; + +#[test] +fn case() { + let source = r#"// 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)> { + 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() {} +"#; + + let input = Level::ERROR + .header("`(bool, HashSet)` is not an iterator") + .id("E0277") + .group( + Group::new() + .element( + Snippet::source(source) + .origin("$DIR/multiline-removal-suggestion.rs") + .fold(true) + .annotation( + AnnotationKind::Primary + .span(769..776) + .label("`(bool, HashSet)` is not an iterator"), + ), + ) + .element( + Level::HELP + .title("the trait `Iterator` is not implemented for `(bool, HashSet)`"), + ) + .element( + Level::NOTE + .title("required for `(bool, HashSet)` to implement `IntoIterator`"), + ), + ) + .group( + Group::new() + .element(Level::NOTE.title("required by a bound in `flatten`")) + .element( + Origin::new("/rustc/FAKE_PREFIX/library/core/src/iter/traits/iterator.rs") + .line(1556) + .char_column(4), + ), + ) + .group( + Group::new() + .element(Level::HELP.title("consider removing this method call, as the receiver has type `std::vec::IntoIter>` and `std::vec::IntoIter>: Iterator` trivially holds")) + .element( + Snippet::source(source) + .origin("$DIR/multiline-removal-suggestion.rs") + .fold(true) + .patch(Patch::new(708..768, "")), + ), + ); + let expected = file!["multiline_removal_suggestion.term.svg"]; + let renderer = Renderer::styled(); + assert_data_eq!(renderer.render(input), expected); +} diff --git a/tests/color/multiline_removal_suggestion.term.svg b/tests/color/multiline_removal_suggestion.term.svg new file mode 100644 index 00000000..bef4b25a --- /dev/null +++ b/tests/color/multiline_removal_suggestion.term.svg @@ -0,0 +1,68 @@ + + + + + + + error[E0277]: `(bool, HashSet<u8>)` is not an iterator + + --> $DIR/multiline-removal-suggestion.rs:21:8 + + | + + 21 | }).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` + + ::: /rustc/FAKE_PREFIX/library/core/src/iter/traits/iterator.rs:1556:4 + + 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 + + | + + 15 - ts.into_iter() + + 16 - .map(|t| { + + 17 - ( + + 18 - is_true, + + 19 - t, + + 20 - ) + + 21 - }).flatten() + + 15 + ts.into_iter().flatten() + + | + + + + From 01b8da75479b6b4f64bad39a06c8cffe0c8c17ad Mon Sep 17 00:00:00 2001 From: Scott Schafer Date: Tue, 3 Jun 2025 09:45:06 -0600 Subject: [PATCH 2/2] fix: Color suggestion removal diff --- examples/custom_level.svg | 6 +- src/renderer/mod.rs | 88 +++++++++++++++++-- .../multiline_removal_suggestion.term.svg | 12 +-- 3 files changed, 90 insertions(+), 16 deletions(-) diff --git a/examples/custom_level.svg b/examples/custom_level.svg index 62dded57..46d3165c 100644 --- a/examples/custom_level.svg +++ b/examples/custom_level.svg @@ -45,11 +45,11 @@ ╭╴ - 22 - break (|| { //~ ERROR `break` with value from a `while` loop + 22 - break (|| { //~ ERROR `break` with value from a `while` loop - 23 - let local = 9; + 23 - let local = 9; - 24 - }); + 24 - }); 22 + break; diff --git a/src/renderer/mod.rs b/src/renderer/mod.rs index f6af1e3b..fadf7805 100644 --- a/src/renderer/mod.rs +++ b/src/renderer/mod.rs @@ -1874,6 +1874,7 @@ impl Renderer { show_code_change { for part in parts { + let snippet = sm.span_to_snippet(part.span.clone()).unwrap_or_default(); let (span_start, span_end) = sm.span_to_locations(part.span.clone()); let span_start_pos = span_start.display; let span_end_pos = span_end.display; @@ -1932,13 +1933,86 @@ impl Renderer { } 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, - ElementStyle::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, + ElementStyle::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, + ElementStyle::Removal, + true, + ); + } } // length of the code after substitution diff --git a/tests/color/multiline_removal_suggestion.term.svg b/tests/color/multiline_removal_suggestion.term.svg index bef4b25a..ed203237 100644 --- a/tests/color/multiline_removal_suggestion.term.svg +++ b/tests/color/multiline_removal_suggestion.term.svg @@ -47,17 +47,17 @@ 15 - ts.into_iter() - 16 - .map(|t| { + 16 - .map(|t| { - 17 - ( + 17 - ( - 18 - is_true, + 18 - is_true, - 19 - t, + 19 - t, - 20 - ) + 20 - ) - 21 - }).flatten() + 21 - }).flatten() 15 + ts.into_iter().flatten()