Auto merge of rust-lang#107671 - CastilloDel:master, r=estebank

bors · bors · commit 5dd0e1b7ae7b · 2023-02-07T13:29:45.000Z
Fix suggestions rendering when the diff span is multiline Fixes rust-lang#92741 cc `@estebank` I think, I finally fixed. I still want to go back and try to clean up the code a bit. I'm open to suggestions. Some examples of the new suggestions: ``` help: consider removing the borrow | 2 - & | ``` ``` help: consider removing the borrow | 2 - & 3 - mut | ``` ``` help: consider removing the borrow | 2 - & 3 - mut if true { true } else { false } 2 + if true { true } else { false } | ``` Should we add a test to ensure this behavior doesn't disappear in the future?
diff --git a/compiler/rustc_errors/src/emitter.rs b/compiler/rustc_errors/src/emitter.rs
@@ -1882,9 +1882,8 @@ impl EmitterWriter {
                             &mut buffer,
                             &mut row_num,
                             &Vec::new(),
-                            p,
+                            p + line_start,
                             l,
-                            line_start,
                             show_code_change,
                             max_line_num_len,
                             &file_lines,
@@ -1907,9 +1906,8 @@ impl EmitterWriter {
                                 &mut buffer,
                                 &mut row_num,
                                 &Vec::new(),
-                                p,
+                                p + line_start,
                                 l,
-                                line_start,
                                 show_code_change,
                                 max_line_num_len,
                                 &file_lines,
@@ -1925,9 +1923,8 @@ impl EmitterWriter {
                                 &mut buffer,
                                 &mut row_num,
                                 &Vec::new(),
-                                p,
+                                p + line_start,
                                 l,
-                                line_start,
                                 show_code_change,
                                 max_line_num_len,
                                 &file_lines,
@@ -1941,9 +1938,8 @@ impl EmitterWriter {
                     &mut buffer,
                     &mut row_num,
                     highlight_parts,
-                    line_pos,
+                    line_pos + line_start,
                     line,
-                    line_start,
                     show_code_change,
                     max_line_num_len,
                     &file_lines,
@@ -2167,40 +2163,63 @@ impl EmitterWriter {
         buffer: &mut StyledBuffer,
         row_num: &mut usize,
         highlight_parts: &Vec<SubstitutionHighlight>,
-        line_pos: usize,
-        line: &str,
-        line_start: usize,
+        line_num: usize,
+        line_to_add: &str,
         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);
+            // We need to print more than one line if the span we need to remove is multiline.
+            // For more info: https://github.com/rust-lang/rust/issues/92741
+            let lines_to_remove = file_lines.lines.iter().take(file_lines.lines.len() - 1);
+            for (index, line_to_remove) in lines_to_remove.enumerate() {
+                buffer.puts(
+                    *row_num - 1,
+                    0,
+                    &self.maybe_anonymized(line_num + index),
+                    Style::LineNumber,
+                );
+                buffer.puts(*row_num - 1, max_line_num_len + 1, "- ", Style::Removal);
+                let line = normalize_whitespace(
+                    &file_lines.file.get_line(line_to_remove.line_index).unwrap(),
+                );
+                buffer.puts(*row_num - 1, max_line_num_len + 3, &line, Style::NoStyle);
+                *row_num += 1;
+            }
+            // If the last line is exactly equal to the line we need to add, we can skip both of them.
+            // This allows us to avoid output like the following:
+            // 2 - &
+            // 2 + if true { true } else { false }
+            // 3 - if true { true } else { false }
+            // If those lines aren't equal, we print their diff
+            let last_line_index = file_lines.lines[file_lines.lines.len() - 1].line_index;
+            let last_line = &file_lines.file.get_line(last_line_index).unwrap();
+            if last_line != line_to_add {
+                buffer.puts(
+                    *row_num - 1,
+                    0,
+                    &self.maybe_anonymized(line_num + file_lines.lines.len() - 1),
+                    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(last_line),
+                    Style::NoStyle,
+                );
+                buffer.puts(*row_num, 0, &self.maybe_anonymized(line_num), Style::LineNumber);
+                buffer.puts(*row_num, max_line_num_len + 1, "+ ", Style::Addition);
+                buffer.append(*row_num, &normalize_whitespace(line_to_add), Style::NoStyle);
+            } else {
+                *row_num -= 2;
+            }
         } else if is_multiline {
+            buffer.puts(*row_num, 0, &self.maybe_anonymized(line_num), Style::LineNumber);
             match &highlight_parts[..] {
-                [SubstitutionHighlight { start: 0, end }] if *end == line.len() => {
+                [SubstitutionHighlight { start: 0, end }] if *end == line_to_add.len() => {
                     buffer.puts(*row_num, max_line_num_len + 1, "+ ", Style::Addition);
                 }
                 [] => {
@@ -2210,17 +2229,17 @@ impl EmitterWriter {
                     buffer.puts(*row_num, max_line_num_len + 1, "~ ", Style::Addition);
                 }
             }
+            buffer.append(*row_num, &normalize_whitespace(line_to_add), Style::NoStyle);
         } else {
+            buffer.puts(*row_num, 0, &self.maybe_anonymized(line_num), Style::LineNumber);
             draw_col_separator(buffer, *row_num, max_line_num_len + 1);
+            buffer.append(*row_num, &normalize_whitespace(line_to_add), Style::NoStyle);
         }
 
-        // 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
+            let tabs: usize = line_to_add
                 .chars()
                 .take(start)
                 .map(|ch| match ch {
diff --git a/src/tools/tidy/src/ui_tests.rs b/src/tools/tidy/src/ui_tests.rs
@@ -10,7 +10,7 @@ use std::path::Path;
 const ENTRY_LIMIT: usize = 1000;
 // FIXME: The following limits should be reduced eventually.
 const ROOT_ENTRY_LIMIT: usize = 939;
-const ISSUES_ENTRY_LIMIT: usize = 1998;
+const ISSUES_ENTRY_LIMIT: usize = 2001;
 
 fn check_entries(path: &Path, bad: &mut bool) {
     for dir in Walk::new(&path.join("ui")) {
diff --git a/tests/ui/issues/issue-92741.fixed b/tests/ui/issues/issue-92741.fixed
@@ -0,0 +1,13 @@
+// run-rustfix
+fn main() {}
+fn _foo() -> bool {
+    if true { true } else { false }
+}
+
+fn _bar() -> bool {
+    if true { true } else { false }
+}
+
+fn _baz() -> bool {
+    if true { true } else { false }
+}
diff --git a/tests/ui/issues/issue-92741.rs b/tests/ui/issues/issue-92741.rs
@@ -0,0 +1,17 @@
+// run-rustfix
+fn main() {}
+fn _foo() -> bool {
+    &  //~ ERROR 4:5: 6:36: mismatched types [E0308]
+    mut
+    if true { true } else { false }
+}
+
+fn _bar() -> bool {
+    &  //~ ERROR 10:5: 11:40: mismatched types [E0308]
+    mut if true { true } else { false }
+}
+
+fn _baz() -> bool {
+    & mut //~ ERROR 15:5: 16:36: mismatched types [E0308]
+    if true { true } else { false }
+}
diff --git a/tests/ui/issues/issue-92741.stderr b/tests/ui/issues/issue-92741.stderr
@@ -0,0 +1,49 @@
+error[E0308]: mismatched types
+  --> $DIR/issue-92741.rs:4:5
+   |
+LL |   fn _foo() -> bool {
+   |                ---- expected `bool` because of return type
+LL | /     &
+LL | |     mut
+LL | |     if true { true } else { false }
+   | |___________________________________^ expected `bool`, found `&mut bool`
+   |
+help: consider removing the borrow
+   |
+LL -     &
+LL -     mut
+   |
+
+error[E0308]: mismatched types
+  --> $DIR/issue-92741.rs:10:5
+   |
+LL |   fn _bar() -> bool {
+   |                ---- expected `bool` because of return type
+LL | /     &
+LL | |     mut if true { true } else { false }
+   | |_______________________________________^ expected `bool`, found `&mut bool`
+   |
+help: consider removing the borrow
+   |
+LL -     &
+LL -     mut if true { true } else { false }
+LL +     if true { true } else { false }
+   |
+
+error[E0308]: mismatched types
+  --> $DIR/issue-92741.rs:15:5
+   |
+LL |   fn _baz() -> bool {
+   |                ---- expected `bool` because of return type
+LL | /     & mut
+LL | |     if true { true } else { false }
+   | |___________________________________^ expected `bool`, found `&mut bool`
+   |
+help: consider removing the borrow
+   |
+LL -     & mut
+   |
+
+error: aborting due to 3 previous errors
+
+For more information about this error, try `rustc --explain E0308`.
