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

Author: Dylan-DPC

compiler/rustc_errors/src/emitter.rs

@@ -10,7 +10,7 @@
 use Destination::*;
 
 use rustc_span::source_map::SourceMap;
-use rustc_span::{SourceFile, Span};
+use rustc_span::{FileLines, SourceFile, Span};
 
 use crate::snippet::{Annotation, AnnotationType, Line, MultilineAnnotation, Style, StyledString};
 use crate::styled_buffer::StyledBuffer;
@@ -1761,12 +1761,6 @@ impl EmitterWriter {
             let has_deletion = parts.iter().any(|p| p.is_deletion());
             let is_multiline = complete.lines().count() > 1;
 
-            enum DisplaySuggestion {
-                Underline,
-                Diff,
-                None,
-            }
-
             if let Some(span) = span.primary_span() {
                 // Compare the primary span of the diagnostic with the span of the suggestion
                 // being emitted.  If they belong to the same file, we don't *need* to show the
@@ -1839,79 +1833,94 @@ impl EmitterWriter {
                 }
                 row_num += line_end - line_start;
             }
-            for (line_pos, (line, highlight_parts)) in
-                lines.by_ref().zip(highlights).take(MAX_SUGGESTION_HIGHLIGHT_LINES).enumerate()
-            {
-                // Print the span column to avoid confusion
-                buffer.puts(
-                    row_num,
-                    0,
-                    &self.maybe_anonymized(line_start + line_pos),
-                    Style::LineNumber,
-                );
-                if let DisplaySuggestion::Diff = show_code_change {
-                    // Add the line number for both addition and removal to drive the point home.
-                    //
-                    // N - fn foo<A: T>(bar: A) {
-                    // N + fn foo(bar: impl T) {
-                    buffer.puts(
-                        row_num - 1,
-                        0,
-                        &self.maybe_anonymized(line_start + line_pos),
-                        Style::LineNumber,
-                    );
-                    buffer.puts(row_num - 1, max_line_num_len + 1, "- ", Style::Removal);
-                    buffer.puts(
-                        row_num - 1,
-                        max_line_num_len + 3,
-                        &normalize_whitespace(
-                            &*file_lines
-                                .file
-                                .get_line(file_lines.lines[line_pos].line_index)
-                                .unwrap(),
-                        ),
-                        Style::NoStyle,
-                    );
-                    buffer.puts(row_num, max_line_num_len + 1, "+ ", Style::Addition);
-                } else if is_multiline {
-                    match &highlight_parts[..] {
-                        [SubstitutionHighlight { start: 0, end }] if *end == line.len() => {
-                            buffer.puts(row_num, max_line_num_len + 1, "+ ", Style::Addition);
-                        }
-                        [] => {
-                            draw_col_separator(&mut buffer, row_num, max_line_num_len + 1);
-                        }
-                        _ => {
-                            buffer.puts(row_num, max_line_num_len + 1, "~ ", Style::Addition);
-                        }
-                    }
-                } else {
-                    draw_col_separator(&mut buffer, row_num, max_line_num_len + 1);
+            let mut unhighlighted_lines = Vec::new();
+            for (line_pos, (line, highlight_parts)) in lines.by_ref().zip(highlights).enumerate() {
+                debug!(%line_pos, %line, ?highlight_parts);
+
+                // Remember lines that are not highlighted to hide them if needed
+                if highlight_parts.is_empty() {
+                    unhighlighted_lines.push((line_pos, line));
+                    continue;
                 }
 
-                // print the suggestion
-                buffer.append(row_num, &normalize_whitespace(line), Style::NoStyle);
+                match unhighlighted_lines.len() {
+                    0 => (),
+                    // Since we show first line, "..." line and last line,
+                    // There is no reason to hide if there are 3 or less lines
+                    // (because then we just replace a line with ... which is
+                    // not helpful)
+                    n if n <= 3 => unhighlighted_lines.drain(..).for_each(|(p, l)| {
+                        self.draw_code_line(
+                            &mut buffer,
+                            &mut row_num,
+                            &Vec::new(),
+                            p,
+                            l,
+                            line_start,
+                            show_code_change,
+                            max_line_num_len,
+                            &file_lines,
+                            is_multiline,
+                        )
+                    }),
+                    // Print first unhighlighted line, "..." and last unhighlighted line, like so:
+                    //
+                    // LL | this line was highlighted
+                    // LL | this line is just for context
+                    //   ...
+                    // LL | this line is just for context
+                    // LL | this line was highlighted
+                    _ => {
+                        let last_line = unhighlighted_lines.pop();
+                        let first_line = unhighlighted_lines.drain(..).next();
+
+                        first_line.map(|(p, l)| {
+                            self.draw_code_line(
+                                &mut buffer,
+                                &mut row_num,
+                                &Vec::new(),
+                                p,
+                                l,
+                                line_start,
+                                show_code_change,
+                                max_line_num_len,
+                                &file_lines,
+                                is_multiline,
+                            )
+                        });
 
-                // Colorize addition/replacements with green.
-                for &SubstitutionHighlight { start, end } in highlight_parts {
-                    // Account for tabs when highlighting (#87972).
-                    let tabs: usize = line
-                        .chars()
-                        .take(start)
-                        .map(|ch| match ch {
-                            '\t' => 3,
-                            _ => 0,
-                        })
-                        .sum();
-                    buffer.set_style_range(
-                        row_num,
-                        max_line_num_len + 3 + start + tabs,
-                        max_line_num_len + 3 + end + tabs,
-                        Style::Addition,
-                        true,
-                    );
+                        buffer.puts(row_num, max_line_num_len - 1, "...", Style::LineNumber);
+                        row_num += 1;
+
+                        last_line.map(|(p, l)| {
+                            self.draw_code_line(
+                                &mut buffer,
+                                &mut row_num,
+                                &Vec::new(),
+                                p,
+                                l,
+                                line_start,
+                                show_code_change,
+                                max_line_num_len,
+                                &file_lines,
+                                is_multiline,
+                            )
+                        });
+                    }
                 }
-                row_num += 1;
+
+                self.draw_code_line(
+                    &mut buffer,
+                    &mut row_num,
+                    highlight_parts,
+                    line_pos,
+                    line,
+                    line_start,
+                    show_code_change,
+                    max_line_num_len,
+                    &file_lines,
+                    is_multiline,
+                )
             }
 
             // This offset and the ones below need to be signed to account for replacement code
@@ -2096,6 +2105,90 @@ impl EmitterWriter {
             }
         }
     }
+
+    fn draw_code_line(
+        &self,
+        buffer: &mut StyledBuffer,
+        row_num: &mut usize,
+        highlight_parts: &Vec<SubstitutionHighlight>,
+        line_pos: usize,
+        line: &str,
+        line_start: usize,
+        show_code_change: DisplaySuggestion,
+        max_line_num_len: usize,
+        file_lines: &FileLines,
+        is_multiline: bool,
+    ) {
+        // Print the span column to avoid confusion
+        buffer.puts(*row_num, 0, &self.maybe_anonymized(line_start + line_pos), Style::LineNumber);
+        if let DisplaySuggestion::Diff = show_code_change {
+            // Add the line number for both addition and removal to drive the point home.
+            //
+            // N - fn foo<A: T>(bar: A) {
+            // N + fn foo(bar: impl T) {
+            buffer.puts(
+                *row_num - 1,
+                0,
+                &self.maybe_anonymized(line_start + line_pos),
+                Style::LineNumber,
+            );
+            buffer.puts(*row_num - 1, max_line_num_len + 1, "- ", Style::Removal);
+            buffer.puts(
+                *row_num - 1,
+                max_line_num_len + 3,
+                &normalize_whitespace(
+                    &*file_lines.file.get_line(file_lines.lines[line_pos].line_index).unwrap(),
+                ),
+                Style::NoStyle,
+            );
+            buffer.puts(*row_num, max_line_num_len + 1, "+ ", Style::Addition);
+        } else if is_multiline {
+            match &highlight_parts[..] {
+                [SubstitutionHighlight { start: 0, end }] if *end == line.len() => {
+                    buffer.puts(*row_num, max_line_num_len + 1, "+ ", Style::Addition);
+                }
+                [] => {
+                    draw_col_separator(buffer, *row_num, max_line_num_len + 1);
+                }
+                _ => {
+                    buffer.puts(*row_num, max_line_num_len + 1, "~ ", Style::Addition);
+                }
+            }
+        } else {
+            draw_col_separator(buffer, *row_num, max_line_num_len + 1);
+        }
+
+        // print the suggestion
+        buffer.append(*row_num, &normalize_whitespace(line), Style::NoStyle);
+
+        // Colorize addition/replacements with green.
+        for &SubstitutionHighlight { start, end } in highlight_parts {
+            // Account for tabs when highlighting (#87972).
+            let tabs: usize = line
+                .chars()
+                .take(start)
+                .map(|ch| match ch {
+                    '\t' => 3,
+                    _ => 0,
+                })
+                .sum();
+            buffer.set_style_range(
+                *row_num,
+                max_line_num_len + 3 + start + tabs,
+                max_line_num_len + 3 + end + tabs,
+                Style::Addition,
+                true,
+            );
+        }
+        *row_num += 1;
+    }
+}
+
+#[derive(Clone, Copy)]
+enum DisplaySuggestion {
+    Underline,
+    Diff,
+    None,
 }
 
 impl FileWithAnnotatedLines {

compiler/rustc_errors/src/lib.rs

@@ -1145,7 +1145,7 @@ impl HandlerInner {
             !this.emitted_diagnostics.insert(diagnostic_hash)
         };
 
-        // Only emit the diagnostic if we've been asked to deduplicate and
+        // Only emit the diagnostic if we've been asked to deduplicate or
         // haven't already emitted an equivalent diagnostic.
         if !(self.flags.deduplicate_diagnostics && already_emitted(self)) {
             debug!(?diagnostic);

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

@@ -19,11 +19,10 @@ help: you might have meant to write a `struct` literal
    |
 LL ~ fn e() { SomeStruct {
 LL |     p:a<p:p<e=6>>
-LL |
-LL |
-LL |
-LL |
  ...
+LL |
+LL ~ }}
+   |
 help: maybe you meant to write a path separator here
    |
 LL |     p::a<p:p<e=6>>

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

@@ -17,11 +17,10 @@ help: add a dummy let to cause `fptr` to be fully captured
    |
 LL ~     thread::spawn(move || { let _ = &fptr; unsafe {
 LL |
-LL |
-LL |
-LL |
-LL |         *fptr.0 = 20;
  ...
+LL |
+LL ~     } });
+   |
 
 error: changes to closure capture in Rust 2021 will affect which traits the closure implements
   --> $DIR/auto_traits.rs:42:19
@@ -39,12 +38,11 @@ LL |         *fptr.0.0 = 20;
 help: add a dummy let to cause `fptr` to be fully captured
    |
 LL ~     thread::spawn(move || { let _ = &fptr; unsafe {
-LL |
-LL |
-LL |
-LL |
 LL |
  ...
+LL |
+LL ~     } });
+   |
 
 error: changes to closure capture in Rust 2021 will affect drop order and which traits the closure implements
   --> $DIR/auto_traits.rs:67:13

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

@@ -108,12 +108,11 @@ LL |         *fptr2.0 = 20;
 help: add a dummy let to cause `fptr1`, `fptr2` to be fully captured
    |
 LL ~     thread::spawn(move || { let _ = (&fptr1, &fptr2); unsafe {
-LL |
-LL |
-LL |
-LL |
 LL |
  ...
+LL |
+LL ~     } });
+   |
 
 error: aborting due to 5 previous errors
 

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

@@ -209,8 +209,7 @@ help: a macro with this name exists at the root of the crate
    |
 LL ~         issue_59764::{makro as foobar, 
 LL | 
-LL |             foobaz,
-LL | 
+ ...
 LL | 
 LL ~             foo::{baz}
    |

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

@@ -89,12 +89,11 @@ LL |                    5);
 help: try comparing the cast value
    |
 LL ~     println!("{}", (a
-LL | 
-LL | 
-LL |                    as
-LL | 
 LL | 
  ...
+LL | 
+LL ~                    usize)
+   |
 
 error: `<` is interpreted as a start of generic arguments for `usize`, not a shift
   --> $DIR/issue-22644.rs:32:31

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

@@ -8,8 +8,7 @@ help: try placing this code inside a block
    |
 LL ~     let Some(_) = Some(()) else { if true {
 LL |
-LL |         return;
-LL |     } else {
+ ...
 LL |         return;
 LL ~     } };
    |

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

@@ -54,11 +54,10 @@ help: consider enclosing expression in a block
    |
 LL ~     let _i = 'label: { match x {
 LL |         0 => 42,
-LL |         1 if false => break 'label 17,
-LL |         1 => {
-LL |             if true {
-LL |                 break 'label 13
  ...
+LL |         _ => 1,
+LL ~     } };
+   |
 
 error: expected `while`, `for`, `loop` or `{` after a label
   --> $DIR/recover-labeled-non-block-expr.rs:26:24