Merge pull request #353 from Muscraft/fix-multiple-line-removals

fix: Show multiple line removals in Diff format

Author: epage

src/renderer/render.rs

@@ -1479,7 +1479,10 @@ fn emit_suggestion_default(
         row_num += 1;
     }
 
-    let file_lines = sm.span_to_lines(parts[0].span.clone());
+    let lo = parts.iter().map(|p| p.span.start).min().unwrap();
+    let hi = parts.iter().map(|p| p.span.end).max().unwrap();
+
+    let file_lines = sm.span_to_lines(lo..hi);
     let (line_start, line_end) = if suggestion.fold {
         // We use the original span to get original line_start
         sm.span_to_locations(parts[0].original_span.clone())
@@ -1613,6 +1616,7 @@ fn emit_suggestion_default(
     if let DisplaySuggestion::Diff | DisplaySuggestion::Underline | DisplaySuggestion::Add =
         show_code_change
     {
+        let mut prev_lines: Option<(usize, usize)> = None;
         for part in parts {
             let snippet = sm.span_to_snippet(part.span.clone()).unwrap_or_default();
             let (span_start, span_end) = sm.span_to_locations(part.span.clone());
@@ -1702,6 +1706,12 @@ fn emit_suggestion_default(
 
                 let newlines = snippet.lines().count();
                 if newlines > 0 && row_num > newlines {
+                    let offset = match prev_lines {
+                        Some((start, end)) => {
+                            file_lines.len().saturating_sub(end.saturating_sub(start))
+                        }
+                        None => file_lines.len(),
+                    };
                     // Account for removals where the part being removed spans multiple
                     // lines.
                     // FIXME: We check the number of rows because in some cases, like in
@@ -1715,7 +1725,7 @@ fn emit_suggestion_default(
                         // Going lower than buffer_offset (+ 1) would mean
                         // overwriting existing content in the buffer
                         let min_row = buffer_offset + usize::from(!matches_previous_suggestion);
-                        let row = (row_num - 2 - (newlines - i - 1)).max(min_row);
+                        let row = (row_num - 2 - (offset - i - 1)).max(min_row);
                         // On the first line, we highlight between the start of the part
                         // span, and the end of that line.
                         // On the last line, we highlight between the start of the line, and
@@ -1746,6 +1756,7 @@ fn emit_suggestion_default(
                         true,
                     );
                 }
+                prev_lines = Some((span_start.line, span_end.line));
             }
 
             // length of the code after substitution

tests/color/multiple_multiline_removal.ascii.term.svg

@@ -2371,6 +2371,9 @@ help: consider removing the `?Sized` bound to make the type parameter `Sized`
  9 - fuzzy
 10 - pizza
 11 - jumps
+12 - crazy
+13 - quack
+14 - zappy
  8 + campy
    |
 "#]];
@@ -2386,6 +2389,9 @@ help: consider removing the `?Sized` bound to make the type parameter `Sized`
  9 - fuzzy
 10 - pizza
 11 - jumps
+12 - crazy
+13 - quack
+14 - zappy
  8 + campy
    ╰╴
 "#]];

tests/color/multiple_multiline_removal.unicode.term.svg

@@ -5325,6 +5325,9 @@ help: you might have meant to use an associated function to build this type
 help: consider using the `Default` trait
    |
 11 -         wtf: Some(Box(U {
+12 -             wtf: None,
+13 -             x: (),
+14 -         })),
 11 +         wtf: Some(<Box as std::default::Default>::default()),
    |
 "#]];
@@ -5374,6 +5377,9 @@ help: you might have meant to use an associated function to build this type
 help: consider using the `Default` trait
    ╭╴
 11 -         wtf: Some(Box(U {
+12 -             wtf: None,
+13 -             x: (),
+14 -         })),
 11 +         wtf: Some(<Box as std::default::Default>::default()),
    ╰╴
 "#]];
@@ -5679,3 +5685,225 @@ help: Unicode character ' ' (No-Break Space) looks like ' ' (Space), but it is
     let renderer_unicode = renderer_ascii.decor_style(DecorStyle::Unicode);
     assert_data_eq!(renderer_unicode.render(report), expected_unicode);
 }
+
+#[test]
+fn issue_109854() {
+    // tests/ui/suggestions/issue-109854.rs
+    let source_0 = r##"    String::with_capacity(
+    //~^ ERROR this function takes 1 argument but 3 arguments were supplied
+    generate_setter,
+    r#"
+pub(crate) struct Person<T: Clone> {}
+"#,
+     r#""#,
+"##;
+    let source_1 = r#"    generate_setter,
+"#;
+    let title_0 = "expected type `usize`
+found fn item `fn() {generate_setter}`";
+    let source_2 = r##"    generate_setter,
+    r#"
+pub(crate) struct Person<T: Clone> {}
+"#,
+     r#""#,
+"##;
+
+    let report = &[
+        Level::ERROR
+            .primary_title("this function takes 1 argument but 3 arguments were supplied")
+            .id("E0061")
+            .element(
+                Snippet::source(source_0)
+                    .path("$DIR/issue-109854.rs")
+                    .line_start(2)
+                    .annotation(AnnotationKind::Primary.span(4..25))
+                    .annotation(
+                        AnnotationKind::Context
+                            .span(128..172)
+                            .label("unexpected argument #2 of type `&'static str`"),
+                    )
+                    .annotation(
+                        AnnotationKind::Context
+                            .span(179..184)
+                            .label("unexpected argument #3 of type `&'static str`"),
+                    ),
+            ),
+        Level::NOTE
+            .secondary_title("expected `usize`, found fn item")
+            .element(
+                Snippet::source(source_1)
+                    .path("$DIR/issue-109854.rs")
+                    .line_start(4)
+                    .annotation(AnnotationKind::Primary.span(4..19)),
+            )
+            .element(Level::NOTE.message(title_0)),
+        Level::NOTE
+            .secondary_title("associated function defined here")
+            .element(
+                Origin::path("$SRC_DIR/alloc/src/string.rs")
+                    .line(480)
+                    .char_column(11),
+            ),
+        Level::HELP
+            .secondary_title("remove the extra arguments")
+            .element(
+                Snippet::source(source_2)
+                    .path("$DIR/issue-109854.rs")
+                    .line_start(4)
+                    .patch(Patch::new(4..19, "/* usize */"))
+                    .patch(Patch::new(19..69, ""))
+                    .patch(Patch::new(69..81, "")),
+            ),
+    ];
+    let expected_ascii = str![[r##"
+error[E0061]: this function takes 1 argument but 3 arguments were supplied
+ --> $DIR/issue-109854.rs:2:5
+  |
+2 |       String::with_capacity(
+  |       ^^^^^^^^^^^^^^^^^^^^^
+...
+5 | /     r#"
+6 | | pub(crate) struct Person<T: Clone> {}
+7 | | "#,
+  | |__- unexpected argument #2 of type `&'static str`
+8 |        r#""#,
+  |        ----- unexpected argument #3 of type `&'static str`
+  |
+note: expected `usize`, found fn item
+ --> $DIR/issue-109854.rs:4:5
+  |
+4 |     generate_setter,
+  |     ^^^^^^^^^^^^^^^
+  = note: expected type `usize`
+          found fn item `fn() {generate_setter}`
+note: associated function defined here
+ --> $SRC_DIR/alloc/src/string.rs:480:11
+help: remove the extra arguments
+  |
+4 -     generate_setter,
+5 -     r#"
+6 - pub(crate) struct Person<T: Clone> {}
+7 - "#,
+8 -      r#""#,
+4 +     /* usize */,
+  |
+"##]];
+    let renderer_ascii = Renderer::plain();
+    assert_data_eq!(renderer_ascii.render(report), expected_ascii);
+
+    let expected_unicode = str![[r##"
+error[E0061]: this function takes 1 argument but 3 arguments were supplied
+  ╭▸ $DIR/issue-109854.rs:2:5
+  │
+2 │       String::with_capacity(
+  │       ━━━━━━━━━━━━━━━━━━━━━
+  ‡
+5 │ ┌     r#"
+6 │ │ pub(crate) struct Person<T: Clone> {}
+7 │ │ "#,
+  │ └──┘ unexpected argument #2 of type `&'static str`
+8 │        r#""#,
+  │        ───── unexpected argument #3 of type `&'static str`
+  ╰╴
+note: expected `usize`, found fn item
+  ╭▸ $DIR/issue-109854.rs:4:5
+  │
+4 │     generate_setter,
+  │     ━━━━━━━━━━━━━━━
+  ╰ note: expected type `usize`
+          found fn item `fn() {generate_setter}`
+note: associated function defined here
+  ─▸ $SRC_DIR/alloc/src/string.rs:480:11
+help: remove the extra arguments
+  ╭╴
+4 -     generate_setter,
+5 -     r#"
+6 - pub(crate) struct Person<T: Clone> {}
+7 - "#,
+8 -      r#""#,
+4 +     /* usize */,
+  ╰╴
+"##]];
+    let renderer_unicode = renderer_ascii.decor_style(DecorStyle::Unicode);
+    assert_data_eq!(renderer_unicode.render(report), expected_unicode);
+}
+
+#[test]
+fn match_same_arms() {
+    // src/tools/clippy/tests/ui/match_same_arms.rs
+    let source = r#"        2 => 'b',
+        3 => 'b',
+        _ => 'b',
+"#;
+
+    let report = &[
+        Level::ERROR
+            .primary_title("these match arms have identical bodies")
+            .element(
+                Snippet::source(source)
+                    .path("tests/ui/match_same_arms.rs")
+                    .line_start(20)
+                    .annotation(AnnotationKind::Primary.span(8..16))
+                    .annotation(AnnotationKind::Primary.span(26..34))
+                    .annotation(
+                        AnnotationKind::Primary
+                            .span(44..52)
+                            .label("the wildcard arm"),
+                    ),
+            )
+            .element(
+                Level::HELP
+                    .message("if this is unintentional make the arms return different values"),
+            ),
+        Level::HELP
+            .secondary_title("otherwise remove the non-wildcard arms")
+            .element(
+                Snippet::source(source)
+                    .path("tests/ui/match_same_arms.rs")
+                    .line_start(20)
+                    .patch(Patch::new(8..26, ""))
+                    .patch(Patch::new(26..44, "")),
+            ),
+    ];
+    let expected_ascii = str![[r#"
+error: these match arms have identical bodies
+  --> tests/ui/match_same_arms.rs:20:9
+   |
+20 |         2 => 'b',
+   |         ^^^^^^^^
+21 |         3 => 'b',
+   |         ^^^^^^^^
+22 |         _ => 'b',
+   |         ^^^^^^^^ the wildcard arm
+   |
+   = help: if this is unintentional make the arms return different values
+help: otherwise remove the non-wildcard arms
+   |
+20 -         2 => 'b',
+21 -         3 => 'b',
+   |
+"#]];
+    let renderer_ascii = Renderer::plain();
+    assert_data_eq!(renderer_ascii.render(report), expected_ascii);
+
+    let expected_unicode = str![[r#"
+error: these match arms have identical bodies
+   ╭▸ tests/ui/match_same_arms.rs:20:9
+   │
+20 │         2 => 'b',
+   │         ━━━━━━━━
+21 │         3 => 'b',
+   │         ━━━━━━━━
+22 │         _ => 'b',
+   │         ━━━━━━━━ the wildcard arm
+   │
+   ╰ help: if this is unintentional make the arms return different values
+help: otherwise remove the non-wildcard arms
+   ╭╴
+20 -         2 => 'b',
+21 -         3 => 'b',
+   ╰╴
+"#]];
+    let renderer_unicode = renderer_ascii.decor_style(DecorStyle::Unicode);
+    assert_data_eq!(renderer_unicode.render(report), expected_unicode);
+}