manual_let_else: do not suggest semantically different replacements

rust-lang · Feb 12, 2023 · 09d3097 · 09d3097
1 parent d880cae
commit 09d3097
Show file tree

Hide file tree

Showing 3 changed files with 46 additions and 5 deletions.
diff --git a/clippy_lints/src/manual_let_else.rs b/clippy_lints/src/manual_let_else.rs
@@ -115,6 +115,13 @@ impl<'tcx> LateLintPass<'tcx> for ManualLetElse {
                     .enumerate()
                     .find(|(_, arm)| expr_diverges(cx, arm.body) && pat_allowed_for_else(cx, arm.pat, check_types));
                 let Some((idx, diverging_arm)) = diverging_arm_opt else { return; };
+                // If the non-diverging arm is the first one, its pattern can be reused in a let/else statement.
+                // However, if it arrives in second position, its pattern may cover some cases already covered
+                // by the diverging one.
+                // TODO: accept the non-diverging arm as a second position if patterns are disjointed.
+                if idx == 0 {
+                    return;
+                }
                 let pat_arm = &arms[1 - idx];
                 if !expr_is_simple_identity(pat_arm.pat, pat_arm.body) {
                     return;

diff --git a/tests/ui/manual_let_else_match.rs b/tests/ui/manual_let_else_match.rs
@@ -42,13 +42,13 @@ fn fire() {
     loop {
         // More complex pattern for the identity arm and diverging arm
         let v = match h() {
-            (Some(_), Some(_)) | (None, None) => continue,
             (Some(v), None) | (None, Some(v)) => v,
+            (Some(_), Some(_)) | (None, None) => continue,
         };
         // Custom enums are supported as long as the "else" arm is a simple _
         let v = match build_enum() {
-            _ => continue,
             Variant::Bar(v) | Variant::Baz(v) => v,
+            _ => continue,
         };
     }
 
@@ -71,6 +71,12 @@ fn fire() {
         Variant::Bar(_) | Variant::Baz(_) => (),
         _ => return,
     };
+
+    let data = [1_u8, 2, 3, 4, 0, 0, 0, 0];
+    let data = match data.as_slice() {
+        [data @ .., 0, 0, 0, 0] | [data @ .., 0, 0] | [data @ .., 0] => data,
+        _ => return,
+    };
 }
 
 fn not_fire() {
@@ -125,4 +131,23 @@ fn not_fire() {
         Ok(v) | Err(Variant::Bar(v) | Variant::Baz(v)) => v,
         Err(Variant::Foo) => return,
     };
+
+    // Issue 10241
+    // The non-divergent arm arrives in second position and
+    // may cover values already matched in the first arm.
+    let v = match h() {
+        (Some(_), Some(_)) | (None, None) => return,
+        (Some(v), _) | (None, Some(v)) => v,
+    };
+
+    let v = match build_enum() {
+        _ => return,
+        Variant::Bar(v) | Variant::Baz(v) => v,
+    };
+
+    let data = [1_u8, 2, 3, 4, 0, 0, 0, 0];
+    let data = match data.as_slice() {
+        [] | [0, 0] => return,
+        [data @ .., 0, 0, 0, 0] | [data @ .., 0, 0] | [data @ ..] => data,
+    };
 }
diff --git a/tests/ui/manual_let_else_match.stderr b/tests/ui/manual_let_else_match.stderr
@@ -22,17 +22,17 @@ error: this could be rewritten as `let...else`
   --> $DIR/manual_let_else_match.rs:44:9
    |
 LL | /         let v = match h() {
-LL | |             (Some(_), Some(_)) | (None, None) => continue,
 LL | |             (Some(v), None) | (None, Some(v)) => v,
+LL | |             (Some(_), Some(_)) | (None, None) => continue,
 LL | |         };
    | |__________^ help: consider writing: `let ((Some(v), None) | (None, Some(v))) = h() else { continue };`
 
 error: this could be rewritten as `let...else`
   --> $DIR/manual_let_else_match.rs:49:9
    |
 LL | /         let v = match build_enum() {
-LL | |             _ => continue,
 LL | |             Variant::Bar(v) | Variant::Baz(v) => v,
+LL | |             _ => continue,
 LL | |         };
    | |__________^ help: consider writing: `let (Variant::Bar(v) | Variant::Baz(v)) = build_enum() else { continue };`
 
@@ -63,5 +63,14 @@ LL | |         _ => return,
 LL | |     };
    | |______^ help: consider writing: `let (Variant::Bar(_) | Variant::Baz(_)) = f else { return };`
 
-error: aborting due to 7 previous errors
+error: this could be rewritten as `let...else`
+  --> $DIR/manual_let_else_match.rs:76:5
+   |
+LL | /     let data = match data.as_slice() {
+LL | |         [data @ .., 0, 0, 0, 0] | [data @ .., 0, 0] | [data @ .., 0] => data,
+LL | |         _ => return,
+LL | |     };
+   | |______^ help: consider writing: `let ([data @ .., 0, 0, 0, 0] | [data @ .., 0, 0] | [data @ .., 0]) = data.as_slice() else { return };`
+
+error: aborting due to 8 previous errors