Improve message for `match_single_arms`

4 files changed, 249 insertions, 234 deletions

diff --git a/clippy_lints/src/matches/match_same_arms.rs b/clippy_lints/src/matches/match_same_arms.rs
index 39c1f0d0d01..5202df544f6 100644
--- a/clippy_lints/src/matches/match_same_arms.rs
+++ b/clippy_lints/src/matches/match_same_arms.rs
@@ -4,6 +4,7 @@ use clippy_utils::{path_to_local, search_same, SpanlessEq, SpanlessHash};
 use core::iter;
 use rustc_arena::DroplessArena;
 use rustc_ast::ast::LitKind;
+use rustc_errors::Applicability;
 use rustc_hir::def_id::DefId;
 use rustc_hir::{Arm, Expr, ExprKind, HirId, HirIdMap, HirIdSet, Pat, PatKind, RangeEnd};
 use rustc_lint::LateContext;
@@ -13,6 +14,7 @@ use std::collections::hash_map::Entry;
 
 use super::MATCH_SAME_ARMS;
 
+#[allow(clippy::too_many_lines)]
 pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, arms: &'tcx [Arm<'_>]) {
     let hash = |&(_, arm): &(usize, &Arm<'_>)| -> u64 {
         let mut h = SpanlessHash::new(cx);
@@ -96,42 +98,52 @@ pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, arms: &'tcx [Arm<'_>]) {
     };
 
     let indexed_arms: Vec<(usize, &Arm<'_>)> = arms.iter().enumerate().collect();
-    for (&(_, i), &(_, j)) in search_same(&indexed_arms, hash, eq) {
-        span_lint_and_then(
-            cx,
-            MATCH_SAME_ARMS,
-            j.body.span,
-            "this `match` has identical arm bodies",
-            |diag| {
-                diag.span_note(i.body.span, "same as this");
-
-                // Note: this does not use `span_suggestion` on purpose:
-                // there is no clean way
-                // to remove the other arm. Building a span and suggest to replace it to ""
-                // makes an even more confusing error message. Also in order not to make up a
-                // span for the whole pattern, the suggestion is only shown when there is only
-                // one pattern. The user should know about `|` if they are already using it…
+    for (&(i, arm1), &(j, arm2)) in search_same(&indexed_arms, hash, eq) {
+        if matches!(arm2.pat.kind, PatKind::Wild) {
+            span_lint_and_then(
+                cx,
+                MATCH_SAME_ARMS,
+                arm1.span,
+                "this match arm has an identical body to the `_` wildcard arm",
+                |diag| {
+                    diag.span_suggestion(
+                        arm1.span,
+                        "try removing the arm",
+                        String::new(),
+                        Applicability::MaybeIncorrect,
+                    )
+                    .help("or try changing either arm body")
+                    .span_note(arm2.span, "`_` wildcard arm here");
+                },
+            );
+        } else {
+            let back_block = backwards_blocking_idxs[j];
+            let (keep_arm, move_arm) = if back_block < i || (back_block == 0 && forwards_blocking_idxs[i] <= j) {
+                (arm1, arm2)
+            } else {
+                (arm2, arm1)
+            };
 
-                let lhs = snippet(cx, i.pat.span, "<pat1>");
-                let rhs = snippet(cx, j.pat.span, "<pat2>");
+            span_lint_and_then(
+                cx,
+                MATCH_SAME_ARMS,
+                keep_arm.span,
+                "this match arm has an identical body to another arm",
+                |diag| {
+                    let move_pat_snip = snippet(cx, move_arm.pat.span, "<pat2>");
+                    let keep_pat_snip = snippet(cx, keep_arm.pat.span, "<pat1>");
 
-                if let PatKind::Wild = j.pat.kind {
-                    // if the last arm is _, then i could be integrated into _
-                    // note that i.pat cannot be _, because that would mean that we're
-                    // hiding all the subsequent arms, and rust won't compile
-                    diag.span_note(
-                        i.body.span,
-                        &format!(
-                            "`{}` has the same arm body as the `_` wildcard, consider removing it",
-                            lhs
-                        ),
-                    );
-                } else {
-                    diag.span_help(i.pat.span, &format!("consider refactoring into `{} | {}`", lhs, rhs,))
-                        .help("...or consider changing the match arm bodies");
-                }
-            },
-        );
+                    diag.span_suggestion(
+                        keep_arm.pat.span,
+                        "try merging the arm patterns",
+                        format!("{} | {}", keep_pat_snip, move_pat_snip),
+                        Applicability::MaybeIncorrect,
+                    )
+                    .help("or try changing either arm body")
+                    .span_note(move_arm.span, "other arm here");
+                },
+            );
+        }
     }
 }
 
diff --git a/tests/ui/match_same_arms.stderr b/tests/ui/match_same_arms.stderr
index 7752a8a6ff2..b6d04263b37 100644
--- a/tests/ui/match_same_arms.stderr
+++ b/tests/ui/match_same_arms.stderr
@@ -1,128 +1,121 @@
-error: this `match` has identical arm bodies
-  --> $DIR/match_same_arms.rs:13:14
+error: this match arm has an identical body to the `_` wildcard arm
+  --> $DIR/match_same_arms.rs:11:9
    |
-LL |         _ => 0, //~ ERROR match arms have same body
-   |              ^
+LL |         Abc::A => 0,
+   |         ^^^^^^^^^^^ help: try removing the arm
    |
    = note: `-D clippy::match-same-arms` implied by `-D warnings`
-note: same as this
-  --> $DIR/match_same_arms.rs:11:19
+   = help: or try changing either arm body
+note: `_` wildcard arm here
+  --> $DIR/match_same_arms.rs:13:9
    |
-LL |         Abc::A => 0,
-   |                   ^
-note: `Abc::A` has the same arm body as the `_` wildcard, consider removing it
-  --> $DIR/match_same_arms.rs:11:19
-   |
-LL |         Abc::A => 0,
-   |                   ^
+LL |         _ => 0, //~ ERROR match arms have same body
+   |         ^^^^^^
 
-error: this `match` has identical arm bodies
-  --> $DIR/match_same_arms.rs:18:20
-   |
-LL |         (.., 3) => 42, //~ ERROR match arms have same body
-   |                    ^^
-   |
-note: same as this
-  --> $DIR/match_same_arms.rs:17:23
-   |
-LL |         (1, .., 3) => 42,
-   |                       ^^
-help: consider refactoring into `(1, .., 3) | (.., 3)`
+error: this match arm has an identical body to another arm
   --> $DIR/match_same_arms.rs:17:9
    |
 LL |         (1, .., 3) => 42,
-   |         ^^^^^^^^^^
-   = help: ...or consider changing the match arm bodies
+   |         ----------^^^^^^
+   |         |
+   |         help: try merging the arm patterns: `(1, .., 3) | (.., 3)`
+   |
+   = help: or try changing either arm body
+note: other arm here
+  --> $DIR/match_same_arms.rs:18:9
+   |
+LL |         (.., 3) => 42, //~ ERROR match arms have same body
+   |         ^^^^^^^^^^^^^
 
-error: this `match` has identical arm bodies
-  --> $DIR/match_same_arms.rs:24:15
+error: this match arm has an identical body to another arm
+  --> $DIR/match_same_arms.rs:24:9
    |
 LL |         51 => 1, //~ ERROR match arms have same body
-   |               ^
+   |         --^^^^^
+   |         |
+   |         help: try merging the arm patterns: `51 | 42`
    |
-note: same as this
-  --> $DIR/match_same_arms.rs:23:15
-   |
-LL |         42 => 1,
-   |               ^
-help: consider refactoring into `42 | 51`
+   = help: or try changing either arm body
+note: other arm here
   --> $DIR/match_same_arms.rs:23:9
    |
 LL |         42 => 1,
-   |         ^^
-   = help: ...or consider changing the match arm bodies
+   |         ^^^^^^^
 
-error: this `match` has identical arm bodies
-  --> $DIR/match_same_arms.rs:26:15
-   |
-LL |         52 => 2, //~ ERROR match arms have same body
-   |               ^
-   |
-note: same as this
-  --> $DIR/match_same_arms.rs:25:15
-   |
-LL |         41 => 2,
-   |               ^
-help: consider refactoring into `41 | 52`
+error: this match arm has an identical body to another arm
   --> $DIR/match_same_arms.rs:25:9
    |
 LL |         41 => 2,
-   |         ^^
-   = help: ...or consider changing the match arm bodies
+   |         --^^^^^
+   |         |
+   |         help: try merging the arm patterns: `41 | 52`
+   |
+   = help: or try changing either arm body
+note: other arm here
+  --> $DIR/match_same_arms.rs:26:9
+   |
+LL |         52 => 2, //~ ERROR match arms have same body
+   |         ^^^^^^^
 
-error: this `match` has identical arm bodies
-  --> $DIR/match_same_arms.rs:32:14
+error: this match arm has an identical body to another arm
+  --> $DIR/match_same_arms.rs:32:9
    |
 LL |         2 => 2, //~ ERROR 2nd matched arms have same body
-   |              ^
-   |
-note: same as this
-  --> $DIR/match_same_arms.rs:31:14
+   |         -^^^^^
+   |         |
+   |         help: try merging the arm patterns: `2 | 1`
    |
-LL |         1 => 2,
-   |              ^
-help: consider refactoring into `1 | 2`
+   = help: or try changing either arm body
+note: other arm here
   --> $DIR/match_same_arms.rs:31:9
    |
 LL |         1 => 2,
-   |         ^
-   = help: ...or consider changing the match arm bodies
+   |         ^^^^^^
 
-error: this `match` has identical arm bodies
-  --> $DIR/match_same_arms.rs:33:14
+error: this match arm has an identical body to another arm
+  --> $DIR/match_same_arms.rs:33:9
    |
 LL |         3 => 2, //~ ERROR 3rd matched arms have same body
-   |              ^
-   |
-note: same as this
-  --> $DIR/match_same_arms.rs:31:14
+   |         -^^^^^
+   |         |
+   |         help: try merging the arm patterns: `3 | 1`
    |
-LL |         1 => 2,
-   |              ^
-help: consider refactoring into `1 | 3`
+   = help: or try changing either arm body
+note: other arm here
   --> $DIR/match_same_arms.rs:31:9
    |
 LL |         1 => 2,
-   |         ^
-   = help: ...or consider changing the match arm bodies
+   |         ^^^^^^
 
-error: this `match` has identical arm bodies
-  --> $DIR/match_same_arms.rs:50:55
+error: this match arm has an identical body to another arm
+  --> $DIR/match_same_arms.rs:32:9
    |
-LL |                 CommandInfo::External { name, .. } => name.to_string(),
-   |                                                       ^^^^^^^^^^^^^^^^
+LL |         2 => 2, //~ ERROR 2nd matched arms have same body
+   |         -^^^^^
+   |         |
+   |         help: try merging the arm patterns: `2 | 3`
    |
-note: same as this
-  --> $DIR/match_same_arms.rs:49:54
+   = help: or try changing either arm body
+note: other arm here
+  --> $DIR/match_same_arms.rs:33:9
    |
-LL |                 CommandInfo::BuiltIn { name, .. } => name.to_string(),
-   |                                                      ^^^^^^^^^^^^^^^^
-help: consider refactoring into `CommandInfo::BuiltIn { name, .. } | CommandInfo::External { name, .. }`
+LL |         3 => 2, //~ ERROR 3rd matched arms have same body
+   |         ^^^^^^
+
+error: this match arm has an identical body to another arm
+  --> $DIR/match_same_arms.rs:50:17
+   |
+LL |                 CommandInfo::External { name, .. } => name.to_string(),
+   |                 ----------------------------------^^^^^^^^^^^^^^^^^^^^
+   |                 |
+   |                 help: try merging the arm patterns: `CommandInfo::External { name, .. } | CommandInfo::BuiltIn { name, .. }`
+   |
+   = help: or try changing either arm body
+note: other arm here
   --> $DIR/match_same_arms.rs:49:17
    |
 LL |                 CommandInfo::BuiltIn { name, .. } => name.to_string(),
-   |                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
-   = help: ...or consider changing the match arm bodies
+   |                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
-error: aborting due to 7 previous errors
+error: aborting due to 8 previous errors
 
diff --git a/tests/ui/match_same_arms2.rs b/tests/ui/match_same_arms2.rs
index 6dc6c4172ee..fdd88f25529 100644
--- a/tests/ui/match_same_arms2.rs
+++ b/tests/ui/match_same_arms2.rs
@@ -181,10 +181,27 @@ fn main() {
         Z(u32),
     }
 
+    // Don't lint. `Foo::X(0)` and `Foo::Z(_)` overlap with the arm in between.
     let _ = match Foo::X(0) {
         Foo::X(0) => 1,
         Foo::X(_) | Foo::Y(_) | Foo::Z(0) => 2,
         Foo::Z(_) => 1,
         _ => 0,
     };
+
+    // Suggest moving `Foo::Z(_)` up.
+    let _ = match Foo::X(0) {
+        Foo::X(0) => 1,
+        Foo::X(_) | Foo::Y(_) => 2,
+        Foo::Z(_) => 1,
+        _ => 0,
+    };
+
+    // Suggest moving `Foo::X(0)` down.
+    let _ = match Foo::X(0) {
+        Foo::X(0) => 1,
+        Foo::Y(_) | Foo::Z(0) => 2,
+        Foo::Z(_) => 1,
+        _ => 0,
+    };
 }
diff --git a/tests/ui/match_same_arms2.stderr b/tests/ui/match_same_arms2.stderr
index e1ed12f9370..596cc8432b3 100644
--- a/tests/ui/match_same_arms2.stderr
+++ b/tests/ui/match_same_arms2.stderr
@@ -1,175 +1,138 @@
-error: this `match` has identical arm bodies
-  --> $DIR/match_same_arms2.rs:20:14
+error: this match arm has an identical body to the `_` wildcard arm
+  --> $DIR/match_same_arms2.rs:11:9
    |
-LL |           _ => {
-   |  ______________^
-LL | |             //~ ERROR match arms have same body
+LL | /         42 => {
 LL | |             foo();
 LL | |             let mut a = 42 + [23].len() as i32;
+LL | |             if true {
 ...  |
 LL | |             a
 LL | |         },
-   | |_________^
+   | |_________^ help: try removing the arm
    |
    = note: `-D clippy::match-same-arms` implied by `-D warnings`
-note: same as this
-  --> $DIR/match_same_arms2.rs:11:15
-   |
-LL |           42 => {
-   |  _______________^
-LL | |             foo();
-LL | |             let mut a = 42 + [23].len() as i32;
-LL | |             if true {
-...  |
-LL | |             a
-LL | |         },
-   | |_________^
-note: `42` has the same arm body as the `_` wildcard, consider removing it
-  --> $DIR/match_same_arms2.rs:11:15
+   = help: or try changing either arm body
+note: `_` wildcard arm here
+  --> $DIR/match_same_arms2.rs:20:9
    |
-LL |           42 => {
-   |  _______________^
+LL | /         _ => {
+LL | |             //~ ERROR match arms have same body
 LL | |             foo();
 LL | |             let mut a = 42 + [23].len() as i32;
-LL | |             if true {
 ...  |
 LL | |             a
 LL | |         },
    | |_________^
 
-error: this `match` has identical arm bodies
-  --> $DIR/match_same_arms2.rs:34:15
+error: this match arm has an identical body to another arm
+  --> $DIR/match_same_arms2.rs:34:9
    |
 LL |         51 => foo(), //~ ERROR match arms have same body
-   |               ^^^^^
-   |
-note: same as this
-  --> $DIR/match_same_arms2.rs:33:15
+   |         --^^^^^^^^^
+   |         |
+   |         help: try merging the arm patterns: `51 | 42`
    |
-LL |         42 => foo(),
-   |               ^^^^^
-help: consider refactoring into `42 | 51`
+   = help: or try changing either arm body
+note: other arm here
   --> $DIR/match_same_arms2.rs:33:9
    |
 LL |         42 => foo(),
-   |         ^^
-   = help: ...or consider changing the match arm bodies
+   |         ^^^^^^^^^^^
 
-error: this `match` has identical arm bodies
-  --> $DIR/match_same_arms2.rs:40:17
-   |
-LL |         None => 24, //~ ERROR match arms have same body
-   |                 ^^
-   |
-note: same as this
-  --> $DIR/match_same_arms2.rs:39:20
-   |
-LL |         Some(_) => 24,
-   |                    ^^
-help: consider refactoring into `Some(_) | None`
+error: this match arm has an identical body to another arm
   --> $DIR/match_same_arms2.rs:39:9
    |
 LL |         Some(_) => 24,
-   |         ^^^^^^^
-   = help: ...or consider changing the match arm bodies
-
-error: this `match` has identical arm bodies
-  --> $DIR/match_same_arms2.rs:62:28
+   |         -------^^^^^^
+   |         |
+   |         help: try merging the arm patterns: `Some(_) | None`
    |
-LL |         (None, Some(a)) => bar(a), //~ ERROR match arms have same body
-   |                            ^^^^^^
-   |
-note: same as this
-  --> $DIR/match_same_arms2.rs:61:28
+   = help: or try changing either arm body
+note: other arm here
+  --> $DIR/match_same_arms2.rs:40:9
    |
-LL |         (Some(a), None) => bar(a),
-   |                            ^^^^^^
-help: consider refactoring into `(Some(a), None) | (None, Some(a))`
+LL |         None => 24, //~ ERROR match arms have same body
+   |         ^^^^^^^^^^
+
+error: this match arm has an identical body to another arm
   --> $DIR/match_same_arms2.rs:61:9
    |
 LL |         (Some(a), None) => bar(a),
-   |         ^^^^^^^^^^^^^^^
-   = help: ...or consider changing the match arm bodies
-
-error: this `match` has identical arm bodies
-  --> $DIR/match_same_arms2.rs:68:26
+   |         ---------------^^^^^^^^^^
+   |         |
+   |         help: try merging the arm patterns: `(Some(a), None) | (None, Some(a))`
    |
-LL |         (.., Some(a)) => bar(a), //~ ERROR match arms have same body
-   |                          ^^^^^^
-   |
-note: same as this
-  --> $DIR/match_same_arms2.rs:67:26
+   = help: or try changing either arm body
+note: other arm here
+  --> $DIR/match_same_arms2.rs:62:9
    |
-LL |         (Some(a), ..) => bar(a),
-   |                          ^^^^^^
-help: consider refactoring into `(Some(a), ..) | (.., Some(a))`
+LL |         (None, Some(a)) => bar(a), //~ ERROR match arms have same body
+   |         ^^^^^^^^^^^^^^^^^^^^^^^^^
+
+error: this match arm has an identical body to another arm
   --> $DIR/match_same_arms2.rs:67:9
    |
 LL |         (Some(a), ..) => bar(a),
-   |         ^^^^^^^^^^^^^
-   = help: ...or consider changing the match arm bodies
-
-error: this `match` has identical arm bodies
-  --> $DIR/match_same_arms2.rs:102:29
-   |
-LL |         (Ok(_), Some(x)) => println!("ok {}", x),
-   |                             ^^^^^^^^^^^^^^^^^^^^
+   |         -------------^^^^^^^^^^
+   |         |
+   |         help: try merging the arm patterns: `(Some(a), ..) | (.., Some(a))`
    |
-note: same as this
-  --> $DIR/match_same_arms2.rs:101:29
+   = help: or try changing either arm body
+note: other arm here
+  --> $DIR/match_same_arms2.rs:68:9
    |
-LL |         (Ok(x), Some(_)) => println!("ok {}", x),
-   |                             ^^^^^^^^^^^^^^^^^^^^
-help: consider refactoring into `(Ok(x), Some(_)) | (Ok(_), Some(x))`
+LL |         (.., Some(a)) => bar(a), //~ ERROR match arms have same body
+   |         ^^^^^^^^^^^^^^^^^^^^^^^
+
+error: this match arm has an identical body to another arm
   --> $DIR/match_same_arms2.rs:101:9
    |
 LL |         (Ok(x), Some(_)) => println!("ok {}", x),
-   |         ^^^^^^^^^^^^^^^^
-   = help: ...or consider changing the match arm bodies
-   = note: this error originates in the macro `println` (in Nightly builds, run with -Z macro-backtrace for more info)
+   |         ----------------^^^^^^^^^^^^^^^^^^^^^^^^
+   |         |
+   |         help: try merging the arm patterns: `(Ok(x), Some(_)) | (Ok(_), Some(x))`
+   |
+   = help: or try changing either arm body
+note: other arm here
+  --> $DIR/match_same_arms2.rs:102:9
+   |
+LL |         (Ok(_), Some(x)) => println!("ok {}", x),
+   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
-error: this `match` has identical arm bodies
-  --> $DIR/match_same_arms2.rs:117:18
+error: this match arm has an identical body to another arm
+  --> $DIR/match_same_arms2.rs:117:9
    |
 LL |         Ok(_) => println!("ok"),
-   |                  ^^^^^^^^^^^^^^
+   |         -----^^^^^^^^^^^^^^^^^^
+   |         |
+   |         help: try merging the arm patterns: `Ok(_) | Ok(3)`
    |
-note: same as this
-  --> $DIR/match_same_arms2.rs:116:18
-   |
-LL |         Ok(3) => println!("ok"),
-   |                  ^^^^^^^^^^^^^^
-help: consider refactoring into `Ok(3) | Ok(_)`
+   = help: or try changing either arm body
+note: other arm here
   --> $DIR/match_same_arms2.rs:116:9
    |
 LL |         Ok(3) => println!("ok"),
-   |         ^^^^^
-   = help: ...or consider changing the match arm bodies
-   = note: this error originates in the macro `println` (in Nightly builds, run with -Z macro-backtrace for more info)
+   |         ^^^^^^^^^^^^^^^^^^^^^^^
 
-error: this `match` has identical arm bodies
-  --> $DIR/match_same_arms2.rs:144:14
+error: this match arm has an identical body to another arm
+  --> $DIR/match_same_arms2.rs:144:9
    |
 LL |           1 => {
-   |  ______________^
+   |           ^ help: try merging the arm patterns: `1 | 0`
+   |  _________|
+   | |
 LL | |             empty!(0);
 LL | |         },
    | |_________^
    |
-note: same as this
-  --> $DIR/match_same_arms2.rs:141:14
+   = help: or try changing either arm body
+note: other arm here
+  --> $DIR/match_same_arms2.rs:141:9
    |
-LL |           0 => {
-   |  ______________^
+LL | /         0 => {
 LL | |             empty!(0);
 LL | |         },
    | |_________^
-help: consider refactoring into `0 | 1`
-  --> $DIR/match_same_arms2.rs:141:9
-   |
-LL |         0 => {
-   |         ^
-   = help: ...or consider changing the match arm bodies
 
 error: match expression looks like `matches!` macro
   --> $DIR/match_same_arms2.rs:162:16
@@ -184,5 +147,35 @@ LL | |     };
    |
    = note: `-D clippy::match-like-matches-macro` implied by `-D warnings`
 
-error: aborting due to 9 previous errors
+error: this match arm has an identical body to another arm
+  --> $DIR/match_same_arms2.rs:194:9
+   |
+LL |         Foo::X(0) => 1,
+   |         ---------^^^^^
+   |         |
+   |         help: try merging the arm patterns: `Foo::X(0) | Foo::Z(_)`
+   |
+   = help: or try changing either arm body
+note: other arm here
+  --> $DIR/match_same_arms2.rs:196:9
+   |
+LL |         Foo::Z(_) => 1,
+   |         ^^^^^^^^^^^^^^
+
+error: this match arm has an identical body to another arm
+  --> $DIR/match_same_arms2.rs:204:9
+   |
+LL |         Foo::Z(_) => 1,
+   |         ---------^^^^^
+   |         |
+   |         help: try merging the arm patterns: `Foo::Z(_) | Foo::X(0)`
+   |
+   = help: or try changing either arm body
+note: other arm here
+  --> $DIR/match_same_arms2.rs:202:9
+   |
+LL |         Foo::X(0) => 1,
+   |         ^^^^^^^^^^^^^^
+
+error: aborting due to 11 previous errors