Fix: if let _ = .. is not the same is if ..

Jarcho · Apr 3, 2021 · ebc6469 · ebc6469
1 parent 86fb0e8
commit ebc6469
Show file tree

Hide file tree

Showing 10 changed files with 68 additions and 29 deletions.
diff --git a/clippy_lints/src/matches.rs b/clippy_lints/src/matches.rs
@@ -1714,8 +1714,8 @@ mod redundant_pattern_match {
         if let ExprKind::Match(op, arms, ref match_source) = &expr.kind {
             match match_source {
                 MatchSource::Normal => find_sugg_for_match(cx, expr, op, arms),
-                MatchSource::IfLetDesugar { .. } => find_sugg_for_if_let(cx, expr, op, arms, "if"),
-                MatchSource::WhileLetDesugar => find_sugg_for_if_let(cx, expr, op, arms, "while"),
+                MatchSource::IfLetDesugar { .. } => find_sugg_for_if_let(cx, expr, op, &arms[0], "if"),
+                MatchSource::WhileLetDesugar => find_sugg_for_if_let(cx, expr, op, &arms[0], "while"),
                 _ => {},
             }
         }
@@ -1725,7 +1725,7 @@ mod redundant_pattern_match {
         cx: &LateContext<'tcx>,
         expr: &'tcx Expr<'_>,
         op: &Expr<'_>,
-        arms: &[Arm<'_>],
+        arm: &Arm<'_>,
         keyword: &'static str,
     ) {
         // also look inside refs
@@ -1737,7 +1737,10 @@ mod redundant_pattern_match {
         let good_method = match kind {
             PatKind::TupleStruct(ref path, ref patterns, _) if patterns.len() == 1 => {
                 if let PatKind::Wild = patterns[0].kind {
-                    if match_qpath(path, &paths::RESULT_OK) {
+                    if cx.typeck_results().expr_ty(op).needs_drop(cx.tcx, cx.param_env) {
+                        // if let doesn't drop the value until after the block ends
+                        return;
+                    } else if match_qpath(path, &paths::RESULT_OK) {
                         "is_ok()"
                     } else if match_qpath(path, &paths::RESULT_ERR) {
                         "is_err()"
@@ -1786,7 +1789,7 @@ mod redundant_pattern_match {
         span_lint_and_then(
             cx,
             REDUNDANT_PATTERN_MATCHING,
-            arms[0].pat.span,
+            arm.pat.span,
             &format!("redundant pattern matching, consider using `{}`", good_method),
             |diag| {
                 // while let ... = ... { ... }

diff --git a/tests/ui/redundant_pattern_matching_option.fixed b/tests/ui/redundant_pattern_matching_option.fixed
@@ -48,6 +48,10 @@ fn main() {
     } else {
         3
     };
+
+    // Issue #5746
+    if let Some(_) = Some(vec![0]) {}
+    if Some(vec![0]).is_none() {}
 }
 
 fn gen_opt() -> Option<()> {

diff --git a/tests/ui/redundant_pattern_matching_option.rs b/tests/ui/redundant_pattern_matching_option.rs
@@ -57,6 +57,10 @@ fn main() {
     } else {
         3
     };
+
+    // Issue #5746
+    if let Some(_) = Some(vec![0]) {}
+    if let None = Some(vec![0]) {}
 }
 
 fn gen_opt() -> Option<()> {

diff --git a/tests/ui/redundant_pattern_matching_option.stderr b/tests/ui/redundant_pattern_matching_option.stderr
@@ -88,32 +88,38 @@ error: redundant pattern matching, consider using `is_none()`
 LL |     } else if let None = gen_opt() {
    |            -------^^^^------------ help: try this: `if gen_opt().is_none()`
 
+error: redundant pattern matching, consider using `is_none()`
+  --> $DIR/redundant_pattern_matching_option.rs:63:12
+   |
+LL |     if let None = Some(vec![0]) {}
+   |     -------^^^^---------------- help: try this: `if Some(vec![0]).is_none()`
+
 error: redundant pattern matching, consider using `is_some()`
-  --> $DIR/redundant_pattern_matching_option.rs:74:12
+  --> $DIR/redundant_pattern_matching_option.rs:78:12
    |
 LL |     if let Some(_) = Some(42) {}
    |     -------^^^^^^^----------- help: try this: `if Some(42).is_some()`
 
 error: redundant pattern matching, consider using `is_none()`
-  --> $DIR/redundant_pattern_matching_option.rs:76:12
+  --> $DIR/redundant_pattern_matching_option.rs:80:12
    |
 LL |     if let None = None::<()> {}
    |     -------^^^^------------- help: try this: `if None::<()>.is_none()`
 
 error: redundant pattern matching, consider using `is_some()`
-  --> $DIR/redundant_pattern_matching_option.rs:78:15
+  --> $DIR/redundant_pattern_matching_option.rs:82:15
    |
 LL |     while let Some(_) = Some(42) {}
    |     ----------^^^^^^^----------- help: try this: `while Some(42).is_some()`
 
 error: redundant pattern matching, consider using `is_none()`
-  --> $DIR/redundant_pattern_matching_option.rs:80:15
+  --> $DIR/redundant_pattern_matching_option.rs:84:15
    |
 LL |     while let None = None::<()> {}
    |     ----------^^^^------------- help: try this: `while None::<()>.is_none()`
 
 error: redundant pattern matching, consider using `is_some()`
-  --> $DIR/redundant_pattern_matching_option.rs:82:5
+  --> $DIR/redundant_pattern_matching_option.rs:86:5
    |
 LL | /     match Some(42) {
 LL | |         Some(_) => true,
@@ -122,13 +128,13 @@ LL | |     };
    | |_____^ help: try this: `Some(42).is_some()`
 
 error: redundant pattern matching, consider using `is_none()`
-  --> $DIR/redundant_pattern_matching_option.rs:87:5
+  --> $DIR/redundant_pattern_matching_option.rs:91:5
    |
 LL | /     match None::<()> {
 LL | |         Some(_) => false,
 LL | |         None => true,
 LL | |     };
    | |_____^ help: try this: `None::<()>.is_none()`
 
-error: aborting due to 19 previous errors
+error: aborting due to 20 previous errors
 
diff --git a/tests/ui/redundant_pattern_matching_poll.fixed b/tests/ui/redundant_pattern_matching_poll.fixed
@@ -45,6 +45,10 @@ fn main() {
     } else {
         3
     };
+
+    // Issue #5746
+    if let Ready(_) = Ready(vec![0]) {}
+    if Ready(vec![0]).is_pending() {}
 }
 
 fn gen_poll() -> Poll<()> {

diff --git a/tests/ui/redundant_pattern_matching_poll.rs b/tests/ui/redundant_pattern_matching_poll.rs
@@ -54,6 +54,10 @@ fn main() {
     } else {
         3
     };
+
+    // Issue #5746
+    if let Ready(_) = Ready(vec![0]) {}
+    if let Pending = Ready(vec![0]) {}
 }
 
 fn gen_poll() -> Poll<()> {

diff --git a/tests/ui/redundant_pattern_matching_poll.stderr b/tests/ui/redundant_pattern_matching_poll.stderr
@@ -82,32 +82,38 @@ error: redundant pattern matching, consider using `is_pending()`
 LL |     } else if let Pending = gen_poll() {
    |            -------^^^^^^^------------- help: try this: `if gen_poll().is_pending()`
 
+error: redundant pattern matching, consider using `is_pending()`
+  --> $DIR/redundant_pattern_matching_poll.rs:60:12
+   |
+LL |     if let Pending = Ready(vec![0]) {}
+   |     -------^^^^^^^----------------- help: try this: `if Ready(vec![0]).is_pending()`
+
 error: redundant pattern matching, consider using `is_ready()`
-  --> $DIR/redundant_pattern_matching_poll.rs:68:12
+  --> $DIR/redundant_pattern_matching_poll.rs:72:12
    |
 LL |     if let Ready(_) = Ready(42) {}
    |     -------^^^^^^^^------------ help: try this: `if Ready(42).is_ready()`
 
 error: redundant pattern matching, consider using `is_pending()`
-  --> $DIR/redundant_pattern_matching_poll.rs:70:12
+  --> $DIR/redundant_pattern_matching_poll.rs:74:12
    |
 LL |     if let Pending = Pending::<()> {}
    |     -------^^^^^^^---------------- help: try this: `if Pending::<()>.is_pending()`
 
 error: redundant pattern matching, consider using `is_ready()`
-  --> $DIR/redundant_pattern_matching_poll.rs:72:15
+  --> $DIR/redundant_pattern_matching_poll.rs:76:15
    |
 LL |     while let Ready(_) = Ready(42) {}
    |     ----------^^^^^^^^------------ help: try this: `while Ready(42).is_ready()`
 
 error: redundant pattern matching, consider using `is_pending()`
-  --> $DIR/redundant_pattern_matching_poll.rs:74:15
+  --> $DIR/redundant_pattern_matching_poll.rs:78:15
    |
 LL |     while let Pending = Pending::<()> {}
    |     ----------^^^^^^^---------------- help: try this: `while Pending::<()>.is_pending()`
 
 error: redundant pattern matching, consider using `is_ready()`
-  --> $DIR/redundant_pattern_matching_poll.rs:76:5
+  --> $DIR/redundant_pattern_matching_poll.rs:80:5
    |
 LL | /     match Ready(42) {
 LL | |         Ready(_) => true,
@@ -116,13 +122,13 @@ LL | |     };
    | |_____^ help: try this: `Ready(42).is_ready()`
 
 error: redundant pattern matching, consider using `is_pending()`
-  --> $DIR/redundant_pattern_matching_poll.rs:81:5
+  --> $DIR/redundant_pattern_matching_poll.rs:85:5
    |
 LL | /     match Pending::<()> {
 LL | |         Ready(_) => false,
 LL | |         Pending => true,
 LL | |     };
    | |_____^ help: try this: `Pending::<()>.is_pending()`
 
-error: aborting due to 18 previous errors
+error: aborting due to 19 previous errors
 
diff --git a/tests/ui/redundant_pattern_matching_result.fixed b/tests/ui/redundant_pattern_matching_result.fixed
@@ -51,6 +51,10 @@ fn main() {
     } else {
         3
     };
+
+    // Issue #5746
+    if let Ok(_) = Ok::<_, ()>(vec![0]) {}
+    if let Err(_) = Err::<(), _>(vec![0]) {}
 }
 
 fn gen_res() -> Result<(), ()> {

diff --git a/tests/ui/redundant_pattern_matching_result.rs b/tests/ui/redundant_pattern_matching_result.rs
@@ -63,6 +63,10 @@ fn main() {
     } else {
         3
     };
+
+    // Issue #5746
+    if let Ok(_) = Ok::<_, ()>(vec![0]) {}
+    if let Err(_) = Err::<(), _>(vec![0]) {}
 }
 
 fn gen_res() -> Result<(), ()> {

diff --git a/tests/ui/redundant_pattern_matching_result.stderr b/tests/ui/redundant_pattern_matching_result.stderr
@@ -85,55 +85,55 @@ LL |     } else if let Err(_) = gen_res() {
    |            -------^^^^^^------------ help: try this: `if gen_res().is_err()`
 
 error: redundant pattern matching, consider using `is_some()`
-  --> $DIR/redundant_pattern_matching_result.rs:84:19
+  --> $DIR/redundant_pattern_matching_result.rs:88:19
    |
 LL |         while let Some(_) = r#try!(result_opt()) {}
    |         ----------^^^^^^^----------------------- help: try this: `while r#try!(result_opt()).is_some()`
 
 error: redundant pattern matching, consider using `is_some()`
-  --> $DIR/redundant_pattern_matching_result.rs:85:16
+  --> $DIR/redundant_pattern_matching_result.rs:89:16
    |
 LL |         if let Some(_) = r#try!(result_opt()) {}
    |         -------^^^^^^^----------------------- help: try this: `if r#try!(result_opt()).is_some()`
 
 error: redundant pattern matching, consider using `is_some()`
-  --> $DIR/redundant_pattern_matching_result.rs:91:12
+  --> $DIR/redundant_pattern_matching_result.rs:95:12
    |
 LL |     if let Some(_) = m!() {}
    |     -------^^^^^^^------- help: try this: `if m!().is_some()`
 
 error: redundant pattern matching, consider using `is_some()`
-  --> $DIR/redundant_pattern_matching_result.rs:92:15
+  --> $DIR/redundant_pattern_matching_result.rs:96:15
    |
 LL |     while let Some(_) = m!() {}
    |     ----------^^^^^^^------- help: try this: `while m!().is_some()`
 
 error: redundant pattern matching, consider using `is_ok()`
-  --> $DIR/redundant_pattern_matching_result.rs:110:12
+  --> $DIR/redundant_pattern_matching_result.rs:114:12
    |
 LL |     if let Ok(_) = Ok::<i32, i32>(42) {}
    |     -------^^^^^--------------------- help: try this: `if Ok::<i32, i32>(42).is_ok()`
 
 error: redundant pattern matching, consider using `is_err()`
-  --> $DIR/redundant_pattern_matching_result.rs:112:12
+  --> $DIR/redundant_pattern_matching_result.rs:116:12
    |
 LL |     if let Err(_) = Err::<i32, i32>(42) {}
    |     -------^^^^^^---------------------- help: try this: `if Err::<i32, i32>(42).is_err()`
 
 error: redundant pattern matching, consider using `is_ok()`
-  --> $DIR/redundant_pattern_matching_result.rs:114:15
+  --> $DIR/redundant_pattern_matching_result.rs:118:15
    |
 LL |     while let Ok(_) = Ok::<i32, i32>(10) {}
    |     ----------^^^^^--------------------- help: try this: `while Ok::<i32, i32>(10).is_ok()`
 
 error: redundant pattern matching, consider using `is_err()`
-  --> $DIR/redundant_pattern_matching_result.rs:116:15
+  --> $DIR/redundant_pattern_matching_result.rs:120:15
    |
 LL |     while let Err(_) = Ok::<i32, i32>(10) {}
    |     ----------^^^^^^--------------------- help: try this: `while Ok::<i32, i32>(10).is_err()`
 
 error: redundant pattern matching, consider using `is_ok()`
-  --> $DIR/redundant_pattern_matching_result.rs:118:5
+  --> $DIR/redundant_pattern_matching_result.rs:122:5
    |
 LL | /     match Ok::<i32, i32>(42) {
 LL | |         Ok(_) => true,
@@ -142,7 +142,7 @@ LL | |     };
    | |_____^ help: try this: `Ok::<i32, i32>(42).is_ok()`
 
 error: redundant pattern matching, consider using `is_err()`
-  --> $DIR/redundant_pattern_matching_result.rs:123:5
+  --> $DIR/redundant_pattern_matching_result.rs:127:5
    |
 LL | /     match Err::<i32, i32>(42) {
 LL | |         Ok(_) => false,