Rollup merge of #137757 - estebank:trim-spans, r=davidtwco

On long spans, trim the middle of them to make them fit in the terminal width

When encountering a single line span that is wider than the terminal, we keep context at the start and end of the span but otherwise remove the code from the middle. This is somewhat independent from whether the left and right margins of the output have been trimmed as well.

```
error[E0308]: mismatched types
  --> $DIR/long-span.rs:6:15
   |
LL | ... = [0, 0, 0, 0, ..., 0, 0];
   |       ^^^^^^^^^^^^^...^^^^^^^ expected `u8`, found `[{integer}; 1681]`
```

Address part of https://github.com/rust-lang/rust/issues/137680 (missing handling of the long suggestion). Fix https://github.com/rust-lang/rust/issues/125581.

---

Change the way that underline positions are calculated by delaying using the "visual" column position until the last possible moment, instead using the "file"/byte position in the file, and then calculating visual positioning as late as possible. This should make the underlines more resilient to non-1-width unicode chars.

Unfortunately, as part of this change (which fixes some visual bugs) comes with the loss of some eager tab codepoint handling, but the output remains legible despite some minor regression on the "margin trimming" logic.

---

`-Zteach` is perma-unstable, barely used, the highlighting logic buggy and the flag being passed around is tech-debt. We should likely remove `-Zteach` in its entirely.

2 files changed, 160 insertions, 97 deletions

diff --git a/compiler/rustc_errors/src/emitter.rs b/compiler/rustc_errors/src/emitter.rs
index f7f84239308..21255fcca96 100644
--- a/compiler/rustc_errors/src/emitter.rs
+++ b/compiler/rustc_errors/src/emitter.rs
@@ -113,24 +113,11 @@ impl Margin {
         self.computed_left > 0
     }
 
-    fn was_cut_right(&self, line_len: usize) -> bool {
-        let right =
-            if self.computed_right == self.span_right || self.computed_right == self.label_right {
-                // FIXME: This comment refers to the only callsite of this method.
-                //        Rephrase it or refactor it, so it can stand on its own.
-                // Account for the "..." padding given above. Otherwise we end up with code lines
-                // that do fit but end in "..." as if they were trimmed.
-                // FIXME: Don't hard-code this offset. Is this meant to represent
-                //        `2 * str_width(self.margin())`?
-                self.computed_right - 6
-            } else {
-                self.computed_right
-            };
-        right < line_len && self.computed_left + self.column_width < line_len
-    }
-
     fn compute(&mut self, max_line_len: usize) {
         // When there's a lot of whitespace (>20), we want to trim it as it is useless.
+        // FIXME: this doesn't account for '\t', but to do so correctly we need to perform that
+        // calculation later, right before printing in order to be accurate with both unicode
+        // handling and trimming of long lines.
         self.computed_left = if self.whitespace_left > 20 {
             self.whitespace_left - 16 // We want some padding.
         } else {
@@ -616,7 +603,6 @@ pub struct HumanEmitter {
     #[setters(skip)]
     fallback_bundle: LazyFallbackBundle,
     short_message: bool,
-    teach: bool,
     ui_testing: bool,
     ignored_directories_in_source_blocks: Vec<String>,
     diagnostic_width: Option<usize>,
@@ -642,7 +628,6 @@ impl HumanEmitter {
             fluent_bundle: None,
             fallback_bundle,
             short_message: false,
-            teach: false,
             ui_testing: false,
             ignored_directories_in_source_blocks: Vec::new(),
             diagnostic_width: None,
@@ -670,43 +655,43 @@ impl HumanEmitter {
         width_offset: usize,
         code_offset: usize,
         margin: Margin,
-    ) {
-        // Tabs are assumed to have been replaced by spaces in calling code.
-        debug_assert!(!source_string.contains('\t'));
+    ) -> usize {
         let line_len = source_string.len();
         // Create the source line we will highlight.
         let left = margin.left(line_len);
         let right = margin.right(line_len);
         // FIXME: The following code looks fishy. See #132860.
         // On long lines, we strip the source line, accounting for unicode.
-        let mut taken = 0;
         let code: String = source_string
             .chars()
-            .skip(left)
-            .take_while(|ch| {
-                // Make sure that the trimming on the right will fall within the terminal width.
-                let next = char_width(*ch);
-                if taken + next > right - left {
-                    return false;
-                }
-                taken += next;
-                true
-            })
+            .enumerate()
+            .skip_while(|(i, _)| *i < left)
+            .take_while(|(i, _)| *i < right)
+            .map(|(_, c)| c)
             .collect();
+        let code = normalize_whitespace(&code);
+        let was_cut_right =
+            source_string.chars().enumerate().skip_while(|(i, _)| *i < right).next().is_some();
         buffer.puts(line_offset, code_offset, &code, Style::Quotation);
         let placeholder = self.margin();
         if margin.was_cut_left() {
             // We have stripped some code/whitespace from the beginning, make it clear.
             buffer.puts(line_offset, code_offset, placeholder, Style::LineNumber);
         }
-        if margin.was_cut_right(line_len) {
+        if was_cut_right {
             let padding = str_width(placeholder);
             // We have stripped some code after the rightmost span end, make it clear we did so.
-            buffer.puts(line_offset, code_offset + taken - padding, placeholder, Style::LineNumber);
+            buffer.puts(
+                line_offset,
+                code_offset + str_width(&code) - padding,
+                placeholder,
+                Style::LineNumber,
+            );
         }
         buffer.puts(line_offset, 0, &self.maybe_anonymized(line_index), Style::LineNumber);
 
         self.draw_col_separator_no_space(buffer, line_offset, width_offset - 2);
+        left
     }
 
     #[instrument(level = "trace", skip(self), ret)]
@@ -738,22 +723,16 @@ impl HumanEmitter {
             return Vec::new();
         }
 
-        let source_string = match file.get_line(line.line_index - 1) {
-            Some(s) => normalize_whitespace(&s),
-            None => return Vec::new(),
+        let Some(source_string) = file.get_line(line.line_index - 1) else {
+            return Vec::new();
         };
         trace!(?source_string);
 
         let line_offset = buffer.num_lines();
 
-        // Left trim
-        let left = margin.left(source_string.len());
-
+        // Left trim.
         // FIXME: This looks fishy. See #132860.
-        // Account for unicode characters of width !=0 that were removed.
-        let left = source_string.chars().take(left).map(|ch| char_width(ch)).sum();
-
-        self.draw_line(
+        let left = self.draw_line(
             buffer,
             &source_string,
             line.line_index,
@@ -784,7 +763,7 @@ impl HumanEmitter {
         let mut short_start = true;
         for ann in &line.annotations {
             if let AnnotationType::MultilineStart(depth) = ann.annotation_type {
-                if source_string.chars().take(ann.start_col.display).all(|c| c.is_whitespace()) {
+                if source_string.chars().take(ann.start_col.file).all(|c| c.is_whitespace()) {
                     let uline = self.underline(ann.is_primary);
                     let chr = uline.multiline_whole_line;
                     annotations.push((depth, uline.style));
@@ -903,11 +882,16 @@ impl HumanEmitter {
         //      |      x_span
         //      <EMPTY LINE>
         //
+        let mut overlap = vec![false; annotations.len()];
         let mut annotations_position = vec![];
         let mut line_len: usize = 0;
         let mut p = 0;
         for (i, annotation) in annotations.iter().enumerate() {
             for (j, next) in annotations.iter().enumerate() {
+                if overlaps(next, annotation, 0) && j > i {
+                    overlap[i] = true;
+                    overlap[j] = true;
+                }
                 if overlaps(next, annotation, 0)  // This label overlaps with another one and both
                     && annotation.has_label()     // take space (they have text and are not
                     && j > i                      // multiline lines).
@@ -1035,24 +1019,21 @@ impl HumanEmitter {
             let pos = pos + 1;
             match annotation.annotation_type {
                 AnnotationType::MultilineStart(depth) | AnnotationType::MultilineEnd(depth) => {
+                    let pre: usize = source_string
+                        .chars()
+                        .take(annotation.start_col.file)
+                        .skip(left)
+                        .map(|c| char_width(c))
+                        .sum();
                     self.draw_range(
                         buffer,
                         underline.multiline_horizontal,
                         line_offset + pos,
                         width_offset + depth,
-                        (code_offset + annotation.start_col.display).saturating_sub(left),
+                        code_offset + pre,
                         underline.style,
                     );
                 }
-                _ if self.teach => {
-                    buffer.set_style_range(
-                        line_offset,
-                        (code_offset + annotation.start_col.display).saturating_sub(left),
-                        (code_offset + annotation.end_col.display).saturating_sub(left),
-                        underline.style,
-                        annotation.is_primary,
-                    );
-                }
                 _ => {}
             }
         }
@@ -1072,11 +1053,18 @@ impl HumanEmitter {
             let underline = self.underline(annotation.is_primary);
             let pos = pos + 1;
 
+            let code_offset = code_offset
+                + source_string
+                    .chars()
+                    .take(annotation.start_col.file)
+                    .skip(left)
+                    .map(|c| char_width(c))
+                    .sum::<usize>();
             if pos > 1 && (annotation.has_label() || annotation.takes_space()) {
                 for p in line_offset + 1..=line_offset + pos {
                     buffer.putc(
                         p,
-                        (code_offset + annotation.start_col.display).saturating_sub(left),
+                        code_offset,
                         match annotation.annotation_type {
                             AnnotationType::MultilineLine(_) => underline.multiline_vertical,
                             _ => underline.vertical_text_line,
@@ -1087,7 +1075,7 @@ impl HumanEmitter {
                 if let AnnotationType::MultilineStart(_) = annotation.annotation_type {
                     buffer.putc(
                         line_offset + pos,
-                        (code_offset + annotation.start_col.display).saturating_sub(left),
+                        code_offset,
                         underline.bottom_right,
                         underline.style,
                     );
@@ -1097,7 +1085,7 @@ impl HumanEmitter {
                 {
                     buffer.putc(
                         line_offset + pos,
-                        (code_offset + annotation.start_col.display).saturating_sub(left),
+                        code_offset,
                         underline.multiline_bottom_right_with_text,
                         underline.style,
                     );
@@ -1155,13 +1143,30 @@ impl HumanEmitter {
             let style =
                 if annotation.is_primary { Style::LabelPrimary } else { Style::LabelSecondary };
             let (pos, col) = if pos == 0 {
-                if annotation.end_col.display == 0 {
-                    (pos + 1, (annotation.end_col.display + 2).saturating_sub(left))
+                let pre: usize = source_string
+                    .chars()
+                    .take(annotation.end_col.file)
+                    .skip(left)
+                    .map(|c| char_width(c))
+                    .sum();
+                if annotation.end_col.file == 0 {
+                    (pos + 1, (pre + 2))
                 } else {
-                    (pos + 1, (annotation.end_col.display + 1).saturating_sub(left))
+                    let pad = if annotation.end_col.file - annotation.start_col.file == 0 {
+                        2
+                    } else {
+                        1
+                    };
+                    (pos + 1, (pre + pad))
                 }
             } else {
-                (pos + 2, annotation.start_col.display.saturating_sub(left))
+                let pre: usize = source_string
+                    .chars()
+                    .take(annotation.start_col.file)
+                    .skip(left)
+                    .map(|c| char_width(c))
+                    .sum();
+                (pos + 2, pre)
             };
             if let Some(ref label) = annotation.label {
                 buffer.puts(line_offset + pos, code_offset + col, label, style);
@@ -1194,14 +1199,35 @@ impl HumanEmitter {
         //   |  _^  test
         for &(pos, annotation) in &annotations_position {
             let uline = self.underline(annotation.is_primary);
-            for p in annotation.start_col.display..annotation.end_col.display {
+            let width = annotation.end_col.file - annotation.start_col.file;
+            let previous: String =
+                source_string.chars().take(annotation.start_col.file).skip(left).collect();
+            let underlined: String =
+                source_string.chars().skip(annotation.start_col.file).take(width).collect();
+            debug!(?previous, ?underlined);
+            let code_offset = code_offset
+                + source_string
+                    .chars()
+                    .take(annotation.start_col.file)
+                    .skip(left)
+                    .map(|c| char_width(c))
+                    .sum::<usize>();
+            let ann_width: usize = source_string
+                .chars()
+                .skip(annotation.start_col.file)
+                .take(width)
+                .map(|c| char_width(c))
+                .sum();
+            let ann_width = if ann_width == 0
+                && matches!(annotation.annotation_type, AnnotationType::Singleline)
+            {
+                1
+            } else {
+                ann_width
+            };
+            for p in 0..ann_width {
                 // The default span label underline.
-                buffer.putc(
-                    line_offset + 1,
-                    (code_offset + p).saturating_sub(left),
-                    uline.underline,
-                    uline.style,
-                );
+                buffer.putc(line_offset + 1, code_offset + p, uline.underline, uline.style);
             }
 
             if pos == 0
@@ -1213,7 +1239,7 @@ impl HumanEmitter {
                 // The beginning of a multiline span with its leftward moving line on the same line.
                 buffer.putc(
                     line_offset + 1,
-                    (code_offset + annotation.start_col.display).saturating_sub(left),
+                    code_offset,
                     match annotation.annotation_type {
                         AnnotationType::MultilineStart(_) => uline.top_right_flat,
                         AnnotationType::MultilineEnd(_) => uline.multiline_end_same_line,
@@ -1231,7 +1257,7 @@ impl HumanEmitter {
                 // so we start going down first.
                 buffer.putc(
                     line_offset + 1,
-                    (code_offset + annotation.start_col.display).saturating_sub(left),
+                    code_offset,
                     match annotation.annotation_type {
                         AnnotationType::MultilineStart(_) => uline.multiline_start_down,
                         AnnotationType::MultilineEnd(_) => uline.multiline_end_up,
@@ -1241,11 +1267,37 @@ impl HumanEmitter {
                 );
             } else if pos != 0 && annotation.has_label() {
                 // The beginning of a span label with an actual label, we'll point down.
-                buffer.putc(
+                buffer.putc(line_offset + 1, code_offset, uline.label_start, uline.style);
+            }
+        }
+
+        // We look for individual *long* spans, and we trim the *middle*, so that we render
+        // LL | ...= [0, 0, 0, ..., 0, 0];
+        //    |      ^^^^^^^^^^...^^^^^^^ expected `&[u8]`, found `[{integer}; 1680]`
+        for (i, (_pos, annotation)) in annotations_position.iter().enumerate() {
+            // Skip cases where multiple spans overlap each other.
+            if overlap[i] {
+                continue;
+            };
+            let AnnotationType::Singleline = annotation.annotation_type else { continue };
+            let width = annotation.end_col.display - annotation.start_col.display;
+            if width > margin.column_width * 2 && width > 10 {
+                // If the terminal is *too* small, we keep at least a tiny bit of the span for
+                // display.
+                let pad = max(margin.column_width / 3, 5);
+                // Code line
+                buffer.replace(
+                    line_offset,
+                    annotation.start_col.file + pad,
+                    annotation.end_col.file - pad,
+                    self.margin(),
+                );
+                // Underline line
+                buffer.replace(
                     line_offset + 1,
-                    (code_offset + annotation.start_col.display).saturating_sub(left),
-                    uline.label_start,
-                    uline.style,
+                    annotation.start_col.file + pad,
+                    annotation.end_col.file - pad,
+                    self.margin(),
                 );
             }
         }
@@ -1702,17 +1754,11 @@ impl HumanEmitter {
                         // non-rustc_lexer::is_whitespace() chars are reported as an
                         // error (ex. no-break-spaces \u{a0}), and thus can't be considered
                         // for removal during error reporting.
+                        // FIXME: doesn't account for '\t' properly.
                         let leading_whitespace = source_string
                             .chars()
                             .take_while(|c| rustc_lexer::is_whitespace(*c))
-                            .map(|c| {
-                                match c {
-                                    // Tabs are displayed as 4 spaces
-                                    '\t' => 4,
-                                    _ => 1,
-                                }
-                            })
-                            .sum();
+                            .count();
                         if source_string.chars().any(|c| !rustc_lexer::is_whitespace(c)) {
                             whitespace_margin = min(whitespace_margin, leading_whitespace);
                         }
@@ -1726,8 +1772,8 @@ impl HumanEmitter {
                 let mut span_left_margin = usize::MAX;
                 for line in &annotated_file.lines {
                     for ann in &line.annotations {
-                        span_left_margin = min(span_left_margin, ann.start_col.display);
-                        span_left_margin = min(span_left_margin, ann.end_col.display);
+                        span_left_margin = min(span_left_margin, ann.start_col.file);
+                        span_left_margin = min(span_left_margin, ann.end_col.file);
                     }
                 }
                 if span_left_margin == usize::MAX {
@@ -1747,12 +1793,12 @@ impl HumanEmitter {
                             .map_or(0, |s| s.len()),
                     );
                     for ann in &line.annotations {
-                        span_right_margin = max(span_right_margin, ann.start_col.display);
-                        span_right_margin = max(span_right_margin, ann.end_col.display);
+                        span_right_margin = max(span_right_margin, ann.start_col.file);
+                        span_right_margin = max(span_right_margin, ann.end_col.file);
                         // FIXME: account for labels not in the same line
                         let label_right = ann.label.as_ref().map_or(0, |l| l.len() + 1);
                         label_right_margin =
-                            max(label_right_margin, ann.end_col.display + label_right);
+                            max(label_right_margin, ann.end_col.file + label_right);
                     }
                 }
 
@@ -1763,15 +1809,7 @@ impl HumanEmitter {
                     width_offset + annotated_file.multiline_depth + 1
                 };
 
-                let column_width = if let Some(width) = self.diagnostic_width {
-                    width.saturating_sub(code_offset)
-                } else if self.ui_testing || cfg!(miri) {
-                    DEFAULT_COLUMN_WIDTH
-                } else {
-                    termize::dimensions()
-                        .map(|(w, _)| w.saturating_sub(code_offset))
-                        .unwrap_or(DEFAULT_COLUMN_WIDTH)
-                };
+                let column_width = self.column_width(code_offset);
 
                 let margin = Margin::new(
                     whitespace_margin,
@@ -1928,6 +1966,18 @@ impl HumanEmitter {
         Ok(())
     }
 
+    fn column_width(&self, code_offset: usize) -> usize {
+        if let Some(width) = self.diagnostic_width {
+            width.saturating_sub(code_offset)
+        } else if self.ui_testing || cfg!(miri) {
+            DEFAULT_COLUMN_WIDTH
+        } else {
+            termize::dimensions()
+                .map(|(w, _)| w.saturating_sub(code_offset))
+                .unwrap_or(DEFAULT_COLUMN_WIDTH)
+        }
+    }
+
     fn emit_suggestion_default(
         &mut self,
         span: &MultiSpan,
diff --git a/compiler/rustc_errors/src/styled_buffer.rs b/compiler/rustc_errors/src/styled_buffer.rs
index 5ca9e9b18f3..790efd0286e 100644
--- a/compiler/rustc_errors/src/styled_buffer.rs
+++ b/compiler/rustc_errors/src/styled_buffer.rs
@@ -89,6 +89,19 @@ impl StyledBuffer {
         }
     }
 
+    pub(crate) fn replace(&mut self, line: usize, start: usize, end: usize, string: &str) {
+        if start == end {
+            return;
+        }
+        if start > self.lines[line].len() || end > self.lines[line].len() {
+            return;
+        }
+        let _ = self.lines[line].drain(start..(end - string.chars().count()));
+        for (i, c) in string.chars().enumerate() {
+            self.lines[line][start + i] = StyledChar::new(c, Style::LineNumber);
+        }
+    }
+
     /// For given `line` inserts `string` with `style` before old content of that line,
     /// adding lines if needed
     pub(crate) fn prepend(&mut self, line: usize, string: &str, style: Style) {