Skip to content

Commit 1198546

Browse files
MuscraftMuscraft
authored
Merge pull request #226 from Muscraft/color-removal-diff
fix: Color suggestion removal diff
2 parents 557126c + 01b8da7 commit 1198546

File tree

5 files changed

+266
-10
lines changed

5 files changed

+266
-10
lines changed

‎examples/custom_level.svg

Lines changed: 3 additions & 3 deletions
Loading

‎src/renderer/mod.rs

Lines changed: 81 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1874,6 +1874,7 @@ impl Renderer {
18741874
show_code_change
18751875
{
18761876
for part in parts {
1877+
let snippet = sm.span_to_snippet(part.span.clone()).unwrap_or_default();
18771878
let (span_start, span_end) = sm.span_to_locations(part.span.clone());
18781879
let span_start_pos = span_start.display;
18791880
let span_end_pos = span_end.display;
@@ -1932,13 +1933,86 @@ impl Renderer {
19321933
}
19331934
if let DisplaySuggestion::Diff = show_code_change {
19341935
// Colorize removal with red in diff format.
1935-
buffer.set_style_range(
1936-
row_num - 2,
1937-
(padding as isize + span_start_pos as isize) as usize,
1938-
(padding as isize + span_end_pos as isize) as usize,
1939-
ElementStyle::Removal,
1940-
true,
1941-
);
1936+
1937+
// Below, there's some tricky buffer indexing going on. `row_num` at this
1938+
// point corresponds to:
1939+
//
1940+
// |
1941+
// LL | CODE
1942+
// | ++++ <- `row_num`
1943+
//
1944+
// in the buffer. When we have a diff format output, we end up with
1945+
//
1946+
// |
1947+
// LL - OLDER <- row_num - 2
1948+
// LL + NEWER
1949+
// | <- row_num
1950+
//
1951+
// The `row_num - 2` is to select the buffer line that has the "old version
1952+
// of the diff" at that point. When the removal is a single line, `i` is
1953+
// `0`, `newlines` is `1` so `(newlines - i - 1)` ends up being `0`, so row
1954+
// points at `LL - OLDER`. When the removal corresponds to multiple lines,
1955+
// we end up with `newlines > 1` and `i` being `0..newlines - 1`.
1956+
//
1957+
// |
1958+
// LL - OLDER <- row_num - 2 - (newlines - last_i - 1)
1959+
// LL - CODE
1960+
// LL - BEING
1961+
// LL - REMOVED <- row_num - 2 - (newlines - first_i - 1)
1962+
// LL + NEWER
1963+
// | <- row_num
1964+
1965+
let newlines = snippet.lines().count();
1966+
if newlines > 0 && row_num > newlines {
1967+
// Account for removals where the part being removed spans multiple
1968+
// lines.
1969+
// FIXME: We check the number of rows because in some cases, like in
1970+
// `tests/ui/lint/invalid-nan-comparison-suggestion.rs`, the rendered
1971+
// suggestion will only show the first line of code being replaced. The
1972+
// proper way of doing this would be to change the suggestion rendering
1973+
// logic to show the whole prior snippet, but the current output is not
1974+
// too bad to begin with, so we side-step that issue here.
1975+
for (i, line) in snippet.lines().enumerate() {
1976+
let line = normalize_whitespace(line);
1977+
let row = row_num - 2 - (newlines - i - 1);
1978+
// On the first line, we highlight between the start of the part
1979+
// span, and the end of that line.
1980+
// On the last line, we highlight between the start of the line, and
1981+
// the column of the part span end.
1982+
// On all others, we highlight the whole line.
1983+
let start = if i == 0 {
1984+
(padding as isize + span_start_pos as isize) as usize
1985+
} else {
1986+
padding
1987+
};
1988+
let end = if i == 0 {
1989+
(padding as isize
1990+
+ span_start_pos as isize
1991+
+ line.len() as isize)
1992+
as usize
1993+
} else if i == newlines - 1 {
1994+
(padding as isize + span_end_pos as isize) as usize
1995+
} else {
1996+
(padding as isize + line.len() as isize) as usize
1997+
};
1998+
buffer.set_style_range(
1999+
row,
2000+
start,
2001+
end,
2002+
ElementStyle::Removal,
2003+
true,
2004+
);
2005+
}
2006+
} else {
2007+
// The removed code fits all in one line.
2008+
buffer.set_style_range(
2009+
row_num - 2,
2010+
(padding as isize + span_start_pos as isize) as usize,
2011+
(padding as isize + span_end_pos as isize) as usize,
2012+
ElementStyle::Removal,
2013+
true,
2014+
);
2015+
}
19422016
}
19432017

19442018
// length of the code after substitution

‎tests/color/main.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ mod fold_bad_origin_line;
99
mod fold_leading;
1010
mod fold_trailing;
1111
mod issue_9;
12+
mod multiline_removal_suggestion;
1213
mod multiple_annotations;
1314
mod simple;
1415
mod strip_line;

‎tests/color/multiline_removal_suggestion.rs

Lines changed: 113 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,113 @@
1+
use annotate_snippets::{AnnotationKind, Group, Level, Origin, Patch, Renderer, Snippet};
2+
3+
use snapbox::{assert_data_eq, file};
4+
5+
#[test]
6+
fn case() {
7+
let source = r#"// Make sure suggestion for removal of a span that covers multiple lines is properly highlighted.
8+
//@ compile-flags: --error-format=human --color=always
9+
//@ edition:2018
10+
//@ only-linux
11+
// ignore-tidy-tab
12+
// We use `\t` instead of spaces for indentation to ensure that the highlighting logic properly
13+
// accounts for replaced characters (like we do for `\t` with ` `). The naïve way of highlighting
14+
// could be counting chars of the original code, instead of operating on the code as it is being
15+
// displayed.
16+
use std::collections::{HashMap, HashSet};
17+
fn foo() -> Vec<(bool, HashSet<u8>)> {
18+
let mut hm = HashMap::<bool, Vec<HashSet<u8>>>::new();
19+
hm.into_iter()
20+
.map(|(is_true, ts)| {
21+
ts.into_iter()
22+
.map(|t| {
23+
(
24+
is_true,
25+
t,
26+
)
27+
}).flatten()
28+
})
29+
.flatten()
30+
.collect()
31+
}
32+
fn bar() -> 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()
37+
.map(|t| (is_true, t))
38+
.flatten()
39+
})
40+
.flatten()
41+
.collect()
42+
}
43+
fn baz() -> 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().map(|t| {
48+
(is_true, t)
49+
}).flatten()
50+
})
51+
.flatten()
52+
.collect()
53+
}
54+
fn bay() -> Vec<(bool, HashSet<u8>)> {
55+
let mut hm = HashMap::<bool, Vec<HashSet<u8>>>::new();
56+
hm.into_iter()
57+
.map(|(is_true, ts)| {
58+
ts.into_iter()
59+
.map(|t| (is_true, t)).flatten()
60+
})
61+
.flatten()
62+
.collect()
63+
}
64+
fn main() {}
65+
"#;
66+
67+
let input = Level::ERROR
68+
.header("`(bool, HashSet<u8>)` is not an iterator")
69+
.id("E0277")
70+
.group(
71+
Group::new()
72+
.element(
73+
Snippet::source(source)
74+
.origin("$DIR/multiline-removal-suggestion.rs")
75+
.fold(true)
76+
.annotation(
77+
AnnotationKind::Primary
78+
.span(769..776)
79+
.label("`(bool, HashSet<u8>)` is not an iterator"),
80+
),
81+
)
82+
.element(
83+
Level::HELP
84+
.title("the trait `Iterator` is not implemented for `(bool, HashSet<u8>)`"),
85+
)
86+
.element(
87+
Level::NOTE
88+
.title("required for `(bool, HashSet<u8>)` to implement `IntoIterator`"),
89+
),
90+
)
91+
.group(
92+
Group::new()
93+
.element(Level::NOTE.title("required by a bound in `flatten`"))
94+
.element(
95+
Origin::new("/rustc/FAKE_PREFIX/library/core/src/iter/traits/iterator.rs")
96+
.line(1556)
97+
.char_column(4),
98+
),
99+
)
100+
.group(
101+
Group::new()
102+
.element(Level::HELP.title("consider removing this method call, as the receiver has type `std::vec::IntoIter<HashSet<u8>>` and `std::vec::IntoIter<HashSet<u8>>: Iterator` trivially holds"))
103+
.element(
104+
Snippet::source(source)
105+
.origin("$DIR/multiline-removal-suggestion.rs")
106+
.fold(true)
107+
.patch(Patch::new(708..768, "")),
108+
),
109+
);
110+
let expected = file!["multiline_removal_suggestion.term.svg"];
111+
let renderer = Renderer::styled();
112+
assert_data_eq!(renderer.render(input), expected);
113+
}

‎tests/color/multiline_removal_suggestion.term.svg

Lines changed: 68 additions & 0 deletions
Loading

0 commit comments

Comments
 (0)