Skip to content
This repository was archived by the owner on May 28, 2025. It is now read-only.

Commit 74aa55b

Browse files
Dylan-DPCDylan-DPC
authored
Rollup merge of rust-lang#97798 - WaffleLapkin:allow_for_suggestions_that_are_quite_far_away_from_each_other, r=estebank
Hide irrelevant lines in suggestions to allow for suggestions that are far from each other to be shown This is an attempt to fix suggestions one part of which is 6 lines or more far from the first. I've noticed "the problem" (of not showing some parts of the suggestion) here: rust-lang#97759 (comment). I'm not sure about the implementation (this big closure is just bad and makes already complicated code even more so), but I want to at least discuss the result. Here is an example of how this changes the output: Before: ```text help: consider enclosing expression in a block | 3 ~ 'l: { match () { () => break 'l, 4 | 5 | 6 | 7 | 8 | ... ``` After: ```text help: consider enclosing expression in a block | 3 ~ 'l: { match () { () => break 'l, 4 | ... 31| 32~ } }; | ``` r? `@estebank` `@rustbot` label +A-diagnostics +A-suggestion-diagnostics
2 parents b516806 + e502b9a commit 74aa55b

23 files changed

+296
-132
lines changed

‎compiler/rustc_errors/src/emitter.rs

Lines changed: 169 additions & 76 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@
1010
use Destination::*;
1111

1212
use rustc_span::source_map::SourceMap;
13-
use rustc_span::{SourceFile, Span};
13+
use rustc_span::{FileLines, SourceFile, Span};
1414

1515
use crate::snippet::{Annotation, AnnotationType, Line, MultilineAnnotation, Style, StyledString};
1616
use crate::styled_buffer::StyledBuffer;
@@ -1761,12 +1761,6 @@ impl EmitterWriter {
17611761
let has_deletion = parts.iter().any(|p| p.is_deletion());
17621762
let is_multiline = complete.lines().count() > 1;
17631763

1764-
enum DisplaySuggestion {
1765-
Underline,
1766-
Diff,
1767-
None,
1768-
}
1769-
17701764
if let Some(span) = span.primary_span() {
17711765
// Compare the primary span of the diagnostic with the span of the suggestion
17721766
// being emitted. If they belong to the same file, we don't *need* to show the
@@ -1839,79 +1833,94 @@ impl EmitterWriter {
18391833
}
18401834
row_num += line_end - line_start;
18411835
}
1842-
for (line_pos, (line, highlight_parts)) in
1843-
lines.by_ref().zip(highlights).take(MAX_SUGGESTION_HIGHLIGHT_LINES).enumerate()
1844-
{
1845-
// Print the span column to avoid confusion
1846-
buffer.puts(
1847-
row_num,
1848-
0,
1849-
&self.maybe_anonymized(line_start + line_pos),
1850-
Style::LineNumber,
1851-
);
1852-
if let DisplaySuggestion::Diff = show_code_change {
1853-
// Add the line number for both addition and removal to drive the point home.
1854-
//
1855-
// N - fn foo<A: T>(bar: A) {
1856-
// N + fn foo(bar: impl T) {
1857-
buffer.puts(
1858-
row_num - 1,
1859-
0,
1860-
&self.maybe_anonymized(line_start + line_pos),
1861-
Style::LineNumber,
1862-
);
1863-
buffer.puts(row_num - 1, max_line_num_len + 1, "- ", Style::Removal);
1864-
buffer.puts(
1865-
row_num - 1,
1866-
max_line_num_len + 3,
1867-
&normalize_whitespace(
1868-
&*file_lines
1869-
.file
1870-
.get_line(file_lines.lines[line_pos].line_index)
1871-
.unwrap(),
1872-
),
1873-
Style::NoStyle,
1874-
);
1875-
buffer.puts(row_num, max_line_num_len + 1, "+ ", Style::Addition);
1876-
} else if is_multiline {
1877-
match &highlight_parts[..] {
1878-
[SubstitutionHighlight { start: 0, end }] if *end == line.len() => {
1879-
buffer.puts(row_num, max_line_num_len + 1, "+ ", Style::Addition);
1880-
}
1881-
[] => {
1882-
draw_col_separator(&mut buffer, row_num, max_line_num_len + 1);
1883-
}
1884-
_ => {
1885-
buffer.puts(row_num, max_line_num_len + 1, "~ ", Style::Addition);
1886-
}
1887-
}
1888-
} else {
1889-
draw_col_separator(&mut buffer, row_num, max_line_num_len + 1);
1836+
let mut unhighlighted_lines = Vec::new();
1837+
for (line_pos, (line, highlight_parts)) in lines.by_ref().zip(highlights).enumerate() {
1838+
debug!(%line_pos, %line, ?highlight_parts);
1839+
1840+
// Remember lines that are not highlighted to hide them if needed
1841+
if highlight_parts.is_empty() {
1842+
unhighlighted_lines.push((line_pos, line));
1843+
continue;
18901844
}
18911845

1892-
// print the suggestion
1893-
buffer.append(row_num, &normalize_whitespace(line), Style::NoStyle);
1846+
match unhighlighted_lines.len() {
1847+
0 => (),
1848+
// Since we show first line, "..." line and last line,
1849+
// There is no reason to hide if there are 3 or less lines
1850+
// (because then we just replace a line with ... which is
1851+
// not helpful)
1852+
n if n <= 3 => unhighlighted_lines.drain(..).for_each(|(p, l)| {
1853+
self.draw_code_line(
1854+
&mut buffer,
1855+
&mut row_num,
1856+
&Vec::new(),
1857+
p,
1858+
l,
1859+
line_start,
1860+
show_code_change,
1861+
max_line_num_len,
1862+
&file_lines,
1863+
is_multiline,
1864+
)
1865+
}),
1866+
// Print first unhighlighted line, "..." and last unhighlighted line, like so:
1867+
//
1868+
// LL | this line was highlighted
1869+
// LL | this line is just for context
1870+
// ...
1871+
// LL | this line is just for context
1872+
// LL | this line was highlighted
1873+
_ => {
1874+
let last_line = unhighlighted_lines.pop();
1875+
let first_line = unhighlighted_lines.drain(..).next();
1876+
1877+
first_line.map(|(p, l)| {
1878+
self.draw_code_line(
1879+
&mut buffer,
1880+
&mut row_num,
1881+
&Vec::new(),
1882+
p,
1883+
l,
1884+
line_start,
1885+
show_code_change,
1886+
max_line_num_len,
1887+
&file_lines,
1888+
is_multiline,
1889+
)
1890+
});
18941891

1895-
// Colorize addition/replacements with green.
1896-
for &SubstitutionHighlight { start, end } in highlight_parts {
1897-
// Account for tabs when highlighting (#87972).
1898-
let tabs: usize = line
1899-
.chars()
1900-
.take(start)
1901-
.map(|ch| match ch {
1902-
'\t' => 3,
1903-
_ => 0,
1904-
})
1905-
.sum();
1906-
buffer.set_style_range(
1907-
row_num,
1908-
max_line_num_len + 3 + start + tabs,
1909-
max_line_num_len + 3 + end + tabs,
1910-
Style::Addition,
1911-
true,
1912-
);
1892+
buffer.puts(row_num, max_line_num_len - 1, "...", Style::LineNumber);
1893+
row_num += 1;
1894+
1895+
last_line.map(|(p, l)| {
1896+
self.draw_code_line(
1897+
&mut buffer,
1898+
&mut row_num,
1899+
&Vec::new(),
1900+
p,
1901+
l,
1902+
line_start,
1903+
show_code_change,
1904+
max_line_num_len,
1905+
&file_lines,
1906+
is_multiline,
1907+
)
1908+
});
1909+
}
19131910
}
1914-
row_num += 1;
1911+
1912+
self.draw_code_line(
1913+
&mut buffer,
1914+
&mut row_num,
1915+
highlight_parts,
1916+
line_pos,
1917+
line,
1918+
line_start,
1919+
show_code_change,
1920+
max_line_num_len,
1921+
&file_lines,
1922+
is_multiline,
1923+
)
19151924
}
19161925

19171926
// This offset and the ones below need to be signed to account for replacement code
@@ -2096,6 +2105,90 @@ impl EmitterWriter {
20962105
}
20972106
}
20982107
}
2108+
2109+
fn draw_code_line(
2110+
&self,
2111+
buffer: &mut StyledBuffer,
2112+
row_num: &mut usize,
2113+
highlight_parts: &Vec<SubstitutionHighlight>,
2114+
line_pos: usize,
2115+
line: &str,
2116+
line_start: usize,
2117+
show_code_change: DisplaySuggestion,
2118+
max_line_num_len: usize,
2119+
file_lines: &FileLines,
2120+
is_multiline: bool,
2121+
) {
2122+
// Print the span column to avoid confusion
2123+
buffer.puts(*row_num, 0, &self.maybe_anonymized(line_start + line_pos), Style::LineNumber);
2124+
if let DisplaySuggestion::Diff = show_code_change {
2125+
// Add the line number for both addition and removal to drive the point home.
2126+
//
2127+
// N - fn foo<A: T>(bar: A) {
2128+
// N + fn foo(bar: impl T) {
2129+
buffer.puts(
2130+
*row_num - 1,
2131+
0,
2132+
&self.maybe_anonymized(line_start + line_pos),
2133+
Style::LineNumber,
2134+
);
2135+
buffer.puts(*row_num - 1, max_line_num_len + 1, "- ", Style::Removal);
2136+
buffer.puts(
2137+
*row_num - 1,
2138+
max_line_num_len + 3,
2139+
&normalize_whitespace(
2140+
&*file_lines.file.get_line(file_lines.lines[line_pos].line_index).unwrap(),
2141+
),
2142+
Style::NoStyle,
2143+
);
2144+
buffer.puts(*row_num, max_line_num_len + 1, "+ ", Style::Addition);
2145+
} else if is_multiline {
2146+
match &highlight_parts[..] {
2147+
[SubstitutionHighlight { start: 0, end }] if *end == line.len() => {
2148+
buffer.puts(*row_num, max_line_num_len + 1, "+ ", Style::Addition);
2149+
}
2150+
[] => {
2151+
draw_col_separator(buffer, *row_num, max_line_num_len + 1);
2152+
}
2153+
_ => {
2154+
buffer.puts(*row_num, max_line_num_len + 1, "~ ", Style::Addition);
2155+
}
2156+
}
2157+
} else {
2158+
draw_col_separator(buffer, *row_num, max_line_num_len + 1);
2159+
}
2160+
2161+
// print the suggestion
2162+
buffer.append(*row_num, &normalize_whitespace(line), Style::NoStyle);
2163+
2164+
// Colorize addition/replacements with green.
2165+
for &SubstitutionHighlight { start, end } in highlight_parts {
2166+
// Account for tabs when highlighting (#87972).
2167+
let tabs: usize = line
2168+
.chars()
2169+
.take(start)
2170+
.map(|ch| match ch {
2171+
'\t' => 3,
2172+
_ => 0,
2173+
})
2174+
.sum();
2175+
buffer.set_style_range(
2176+
*row_num,
2177+
max_line_num_len + 3 + start + tabs,
2178+
max_line_num_len + 3 + end + tabs,
2179+
Style::Addition,
2180+
true,
2181+
);
2182+
}
2183+
*row_num += 1;
2184+
}
2185+
}
2186+
2187+
#[derive(Clone, Copy)]
2188+
enum DisplaySuggestion {
2189+
Underline,
2190+
Diff,
2191+
None,
20992192
}
21002193

21012194
impl FileWithAnnotatedLines {

‎compiler/rustc_errors/src/lib.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1174,7 +1174,7 @@ impl HandlerInner {
11741174
!this.emitted_diagnostics.insert(diagnostic_hash)
11751175
};
11761176

1177-
// Only emit the diagnostic if we've been asked to deduplicate and
1177+
// Only emit the diagnostic if we've been asked to deduplicate or
11781178
// haven't already emitted an equivalent diagnostic.
11791179
if !(self.flags.deduplicate_diagnostics && already_emitted(self)) {
11801180
debug!(?diagnostic);

‎src/test/ui/associated-consts/issue-93835.stderr

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -19,11 +19,10 @@ help: you might have meant to write a `struct` literal
1919
|
2020
LL ~ fn e() { SomeStruct {
2121
LL | p:a<p:p<e=6>>
22-
LL |
23-
LL |
24-
LL |
25-
LL |
2622
...
23+
LL |
24+
LL ~ }}
25+
|
2726
help: maybe you meant to write a path separator here
2827
|
2928
LL | p::a<p:p<e=6>>

‎src/test/ui/closures/2229_closure_analysis/migrations/auto_traits.stderr

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -17,11 +17,10 @@ help: add a dummy let to cause `fptr` to be fully captured
1717
|
1818
LL ~ thread::spawn(move || { let _ = &fptr; unsafe {
1919
LL |
20-
LL |
21-
LL |
22-
LL |
23-
LL | *fptr.0 = 20;
2420
...
21+
LL |
22+
LL ~ } });
23+
|
2524

2625
error: changes to closure capture in Rust 2021 will affect which traits the closure implements
2726
--> $DIR/auto_traits.rs:42:19
@@ -39,12 +38,11 @@ LL | *fptr.0.0 = 20;
3938
help: add a dummy let to cause `fptr` to be fully captured
4039
|
4140
LL ~ thread::spawn(move || { let _ = &fptr; unsafe {
42-
LL |
43-
LL |
44-
LL |
45-
LL |
4641
LL |
4742
...
43+
LL |
44+
LL ~ } });
45+
|
4846

4947
error: changes to closure capture in Rust 2021 will affect drop order and which traits the closure implements
5048
--> $DIR/auto_traits.rs:67:13

‎src/test/ui/closures/2229_closure_analysis/migrations/multi_diagnostics.stderr

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -108,12 +108,11 @@ LL | *fptr2.0 = 20;
108108
help: add a dummy let to cause `fptr1`, `fptr2` to be fully captured
109109
|
110110
LL ~ thread::spawn(move || { let _ = (&fptr1, &fptr2); unsafe {
111-
LL |
112-
LL |
113-
LL |
114-
LL |
115111
LL |
116112
...
113+
LL |
114+
LL ~ } });
115+
|
117116

118117
error: aborting due to 5 previous errors
119118

‎src/test/ui/imports/issue-59764.stderr

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -209,8 +209,7 @@ help: a macro with this name exists at the root of the crate
209209
|
210210
LL ~ issue_59764::{makro as foobar,
211211
LL |
212-
LL | foobaz,
213-
LL |
212+
...
214213
LL |
215214
LL ~ foo::{baz}
216215
|

‎src/test/ui/issues/issue-22644.stderr

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -89,12 +89,11 @@ LL | 5);
8989
help: try comparing the cast value
9090
|
9191
LL ~ println!("{}", (a
92-
LL |
93-
LL |
94-
LL | as
95-
LL |
9692
LL |
9793
...
94+
LL |
95+
LL ~ usize)
96+
|
9897

9998
error: `<` is interpreted as a start of generic arguments for `usize`, not a shift
10099
--> $DIR/issue-22644.rs:32:31

‎src/test/ui/let-else/let-else-if.stderr

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,7 @@ help: try placing this code inside a block
88
|
99
LL ~ let Some(_) = Some(()) else { if true {
1010
LL |
11-
LL | return;
12-
LL | } else {
11+
...
1312
LL | return;
1413
LL ~ } };
1514
|

‎src/test/ui/parser/recover-labeled-non-block-expr.stderr

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -54,11 +54,10 @@ help: consider enclosing expression in a block
5454
|
5555
LL ~ let _i = 'label: { match x {
5656
LL | 0 => 42,
57-
LL | 1 if false => break 'label 17,
58-
LL | 1 => {
59-
LL | if true {
60-
LL | break 'label 13
6157
...
58+
LL | _ => 1,
59+
LL ~ } };
60+
|
6261

6362
error: expected `while`, `for`, `loop` or `{` after a label
6463
--> $DIR/recover-labeled-non-block-expr.rs:26:24

0 commit comments

Comments
 (0)