Create AnnotationColumn struct to fix hard tab column numbers in errors

Author: pommicket

compiler/rustc_errors/src/annotate_snippet_emitter_writer.rs

@@ -202,7 +202,10 @@ impl AnnotateSnippetEmitterWriter {
                             annotations: annotations
                                 .iter()
                                 .map(|annotation| SourceAnnotation {
-                                    range: (annotation.start_col, annotation.end_col),
+                                    range: (
+                                        annotation.start_col.display,
+                                        annotation.end_col.display,
+                                    ),
                                     label: annotation.label.as_deref().unwrap_or_default(),
                                     annotation_type: annotation_type_for_level(*level),
                                 })

compiler/rustc_errors/src/emitter.rs

@@ -12,7 +12,9 @@ use Destination::*;
 use rustc_span::source_map::SourceMap;
 use rustc_span::{FileLines, SourceFile, Span};
 
-use crate::snippet::{Annotation, AnnotationType, Line, MultilineAnnotation, Style, StyledString};
+use crate::snippet::{
+    Annotation, AnnotationColumn, AnnotationType, Line, MultilineAnnotation, Style, StyledString,
+};
 use crate::styled_buffer::StyledBuffer;
 use crate::translation::{to_fluent_args, Translate};
 use crate::{
@@ -858,7 +860,7 @@ impl EmitterWriter {
         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).all(|c| c.is_whitespace()) {
+                if source_string.chars().take(ann.start_col.display).all(|c| c.is_whitespace()) {
                     let style = if ann.is_primary {
                         Style::UnderlinePrimary
                     } else {
@@ -1093,15 +1095,15 @@ impl EmitterWriter {
                         '_',
                         line_offset + pos,
                         width_offset + depth,
-                        (code_offset + annotation.start_col).saturating_sub(left),
+                        (code_offset + annotation.start_col.display).saturating_sub(left),
                         style,
                     );
                 }
                 _ if self.teach => {
                     buffer.set_style_range(
                         line_offset,
-                        (code_offset + annotation.start_col).saturating_sub(left),
-                        (code_offset + annotation.end_col).saturating_sub(left),
+                        (code_offset + annotation.start_col.display).saturating_sub(left),
+                        (code_offset + annotation.end_col.display).saturating_sub(left),
                         style,
                         annotation.is_primary,
                     );
@@ -1133,7 +1135,7 @@ impl EmitterWriter {
                 for p in line_offset + 1..=line_offset + pos {
                     buffer.putc(
                         p,
-                        (code_offset + annotation.start_col).saturating_sub(left),
+                        (code_offset + annotation.start_col.display).saturating_sub(left),
                         '|',
                         style,
                     );
@@ -1169,9 +1171,9 @@ impl EmitterWriter {
             let style =
                 if annotation.is_primary { Style::LabelPrimary } else { Style::LabelSecondary };
             let (pos, col) = if pos == 0 {
-                (pos + 1, (annotation.end_col + 1).saturating_sub(left))
+                (pos + 1, (annotation.end_col.display + 1).saturating_sub(left))
             } else {
-                (pos + 2, annotation.start_col.saturating_sub(left))
+                (pos + 2, annotation.start_col.display.saturating_sub(left))
             };
             if let Some(ref label) = annotation.label {
                 buffer.puts(line_offset + pos, code_offset + col, label, style);
@@ -1208,7 +1210,7 @@ impl EmitterWriter {
             } else {
                 ('-', Style::UnderlineSecondary)
             };
-            for p in annotation.start_col..annotation.end_col {
+            for p in annotation.start_col.display..annotation.end_col.display {
                 buffer.putc(
                     line_offset + 1,
                     (code_offset + p).saturating_sub(left),
@@ -1459,7 +1461,7 @@ impl EmitterWriter {
                                         &annotated_file.file.name,
                                         line.line_index
                                     ),
-                                    annotations[0].start_col + 1,
+                                    annotations[0].start_col.file + 1,
                                 ),
                                 Style::LineAndColumn,
                             );
@@ -1546,7 +1548,7 @@ impl EmitterWriter {
                 buffer.prepend(buffer_msg_line_offset + 1, "::: ", Style::LineNumber);
                 let loc = if let Some(first_line) = annotated_file.lines.first() {
                     let col = if let Some(first_annotation) = first_line.annotations.first() {
-                        format!(":{}", first_annotation.start_col + 1)
+                        format!(":{}", first_annotation.start_col.file + 1)
                     } else {
                         String::new()
                     };
@@ -1607,8 +1609,8 @@ impl EmitterWriter {
                 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);
-                        span_left_margin = min(span_left_margin, ann.end_col);
+                        span_left_margin = min(span_left_margin, ann.start_col.display);
+                        span_left_margin = min(span_left_margin, ann.end_col.display);
                     }
                 }
                 if span_left_margin == usize::MAX {
@@ -1625,11 +1627,12 @@ impl EmitterWriter {
                         annotated_file.file.get_line(line.line_index - 1).map_or(0, |s| s.len()),
                     );
                     for ann in &line.annotations {
-                        span_right_margin = max(span_right_margin, ann.start_col);
-                        span_right_margin = max(span_right_margin, ann.end_col);
+                        span_right_margin = max(span_right_margin, ann.start_col.display);
+                        span_right_margin = max(span_right_margin, ann.end_col.display);
                         // 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 + label_right);
+                        label_right_margin =
+                            max(label_right_margin, ann.end_col.display + label_right);
                     }
                 }
 
@@ -2352,17 +2355,17 @@ impl FileWithAnnotatedLines {
                         depth: 1,
                         line_start: lo.line,
                         line_end: hi.line,
-                        start_col: lo.col_display,
-                        end_col: hi.col_display,
+                        start_col: AnnotationColumn::from_loc(&lo),
+                        end_col: AnnotationColumn::from_loc(&hi),
                         is_primary,
                         label,
                         overlaps_exactly: false,
                     };
                     multiline_annotations.push((lo.file, ml));
                 } else {
                     let ann = Annotation {
-                        start_col: lo.col_display,
-                        end_col: hi.col_display,
+                        start_col: AnnotationColumn::from_loc(&lo),
+                        end_col: AnnotationColumn::from_loc(&hi),
                         is_primary,
                         label,
                         annotation_type: AnnotationType::Singleline,
@@ -2551,7 +2554,13 @@ fn num_overlap(
     (b_start..b_end + extra).contains(&a_start) || (a_start..a_end + extra).contains(&b_start)
 }
 fn overlaps(a1: &Annotation, a2: &Annotation, padding: usize) -> bool {
-    num_overlap(a1.start_col, a1.end_col + padding, a2.start_col, a2.end_col, false)
+    num_overlap(
+        a1.start_col.display,
+        a1.end_col.display + padding,
+        a2.start_col.display,
+        a2.end_col.display,
+        false,
+    )
 }
 
 fn emit_to_destination(

compiler/rustc_errors/src/snippet.rs

@@ -1,20 +1,46 @@
 // Code for annotating snippets.
 
-use crate::Level;
+use crate::{Level, Loc};
 
 #[derive(Clone, Debug, PartialOrd, Ord, PartialEq, Eq)]
 pub struct Line {
     pub line_index: usize,
     pub annotations: Vec<Annotation>,
 }
 
+#[derive(Clone, Copy, Debug, PartialOrd, Ord, PartialEq, Eq, Default)]
+pub struct AnnotationColumn {
+    /// the (0-indexed) column for *display* purposes, counted in characters, not utf-8 bytes
+    pub display: usize,
+    /// the (0-indexed) column in the file, counted in characters, not utf-8 bytes.
+    ///
+    /// this may be different from `self.display`,
+    /// e.g. if the file contains hard tabs, because we convert tabs to spaces for error messages.
+    ///
+    /// for example:
+    /// ```text
+    /// (hard tab)hello
+    ///           ^ this is display column 4, but file column 1
+    /// ```
+    ///
+    /// we want to keep around the correct file offset so that column numbers in error messages
+    /// are correct. (motivated by <https://github.com/rust-lang/rust/issues/109537>)
+    pub file: usize,
+}
+
+impl AnnotationColumn {
+    pub fn from_loc(loc: &Loc) -> AnnotationColumn {
+        AnnotationColumn { display: loc.col_display, file: loc.col.0 }
+    }
+}
+
 #[derive(Clone, Debug, PartialOrd, Ord, PartialEq, Eq)]
 pub struct MultilineAnnotation {
     pub depth: usize,
     pub line_start: usize,
     pub line_end: usize,
-    pub start_col: usize,
-    pub end_col: usize,
+    pub start_col: AnnotationColumn,
+    pub end_col: AnnotationColumn,
     pub is_primary: bool,
     pub label: Option<String>,
     pub overlaps_exactly: bool,
@@ -36,7 +62,12 @@ impl MultilineAnnotation {
     pub fn as_start(&self) -> Annotation {
         Annotation {
             start_col: self.start_col,
-            end_col: self.start_col + 1,
+            end_col: AnnotationColumn {
+                // these might not correspond to the same place anymore,
+                // but that's okay for our purposes
+                display: self.start_col.display + 1,
+                file: self.start_col.file + 1,
+            },
             is_primary: self.is_primary,
             label: None,
             annotation_type: AnnotationType::MultilineStart(self.depth),
@@ -45,7 +76,12 @@ impl MultilineAnnotation {
 
     pub fn as_end(&self) -> Annotation {
         Annotation {
-            start_col: self.end_col.saturating_sub(1),
+            start_col: AnnotationColumn {
+                // these might not correspond to the same place anymore,
+                // but that's okay for our purposes
+                display: self.end_col.display.saturating_sub(1),
+                file: self.end_col.file.saturating_sub(1),
+            },
             end_col: self.end_col,
             is_primary: self.is_primary,
             label: self.label.clone(),
@@ -55,8 +91,8 @@ impl MultilineAnnotation {
 
     pub fn as_line(&self) -> Annotation {
         Annotation {
-            start_col: 0,
-            end_col: 0,
+            start_col: Default::default(),
+            end_col: Default::default(),
             is_primary: self.is_primary,
             label: None,
             annotation_type: AnnotationType::MultilineLine(self.depth),
@@ -92,14 +128,14 @@ pub enum AnnotationType {
 
 #[derive(Clone, Debug, PartialOrd, Ord, PartialEq, Eq)]
 pub struct Annotation {
-    /// Start column, 0-based indexing -- counting *characters*, not
-    /// utf-8 bytes. Note that it is important that this field goes
+    /// Start column.
+    /// Note that it is important that this field goes
     /// first, so that when we sort, we sort orderings by start
     /// column.
-    pub start_col: usize,
+    pub start_col: AnnotationColumn,
 
     /// End column within the line (exclusive)
-    pub end_col: usize,
+    pub end_col: AnnotationColumn,
 
     /// Is this annotation derived from primary span
     pub is_primary: bool,
@@ -118,12 +154,13 @@ impl Annotation {
         matches!(self.annotation_type, AnnotationType::MultilineLine(_))
     }
 
+    /// Length of this annotation as displayed in the stderr output
     pub fn len(&self) -> usize {
         // Account for usize underflows
-        if self.end_col > self.start_col {
-            self.end_col - self.start_col
+        if self.end_col.display > self.start_col.display {
+            self.end_col.display - self.start_col.display
         } else {
-            self.start_col - self.end_col
+            self.start_col.display - self.end_col.display
         }
     }
 

tests/ui/diagnostic-width/auxiliary/tab_column_numbers.rs

@@ -0,0 +1,6 @@
+// ignore-tidy-tab
+
+pub struct S;
+impl S {
+		fn method(&self) {}
+}

tests/ui/diagnostic-width/tab-column-numbers.rs

@@ -0,0 +1,12 @@
+// Test for #109537: ensure that column numbers are correctly generated when using hard tabs.
+// aux-build:tab_column_numbers.rs
+
+// ignore-tidy-tab
+
+extern crate tab_column_numbers;
+
+fn main() {
+	let s = tab_column_numbers::S;
+	s.method();
+	//~^ ERROR method `method` is private
+}

tests/ui/diagnostic-width/tab-column-numbers.stderr

@@ -0,0 +1,14 @@
+error[E0624]: method `method` is private
+  --> $DIR/tab-column-numbers.rs:10:4
+   |
+LL |     s.method();
+   |       ^^^^^^ private method
+   |
+  ::: $DIR/auxiliary/tab_column_numbers.rs:5:3
+   |
+LL |         fn method(&self) {}
+   |         ---------------- private method defined here
+
+error: aborting due to previous error
+
+For more information about this error, try `rustc --explain E0624`.