Auto merge of #5483 - alex-700:fix-redundant-pattern-matching, r=flip1995

fix redundant_pattern_matching lint

- now it handles `while let` case  (related to #5462)
- better suggestions in `if let` case

changelog: Fix suggestion in `redundant_pattern_matching` and also apply this lint to the `while let` case

4 files changed, 153 insertions, 51 deletions

diff --git a/clippy_lints/src/redundant_pattern_matching.rs b/clippy_lints/src/redundant_pattern_matching.rs
index bdc32dbba87..334ceed64c2 100644
--- a/clippy_lints/src/redundant_pattern_matching.rs
+++ b/clippy_lints/src/redundant_pattern_matching.rs
@@ -1,4 +1,5 @@
-use crate::utils::{match_qpath, paths, snippet, span_lint_and_then};
+use crate::utils::{match_qpath, match_trait_method, paths, snippet, span_lint_and_then};
+use if_chain::if_chain;
 use rustc_ast::ast::LitKind;
 use rustc_errors::Applicability;
 use rustc_hir::{Arm, Expr, ExprKind, MatchSource, PatKind, QPath};
@@ -48,9 +49,8 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for RedundantPatternMatching {
         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 { contains_else_clause } => {
-                    find_sugg_for_if_let(cx, expr, op, arms, *contains_else_clause)
-                },
+                MatchSource::IfLetDesugar { .. } => find_sugg_for_if_let(cx, expr, op, arms, "if"),
+                MatchSource::WhileLetDesugar => find_sugg_for_if_let(cx, expr, op, arms, "while"),
                 _ => return,
             }
         }
@@ -62,7 +62,7 @@ fn find_sugg_for_if_let<'a, 'tcx>(
     expr: &'tcx Expr<'_>,
     op: &Expr<'_>,
     arms: &[Arm<'_>],
-    has_else: bool,
+    keyword: &'static str,
 ) {
     let good_method = match arms[0].pat.kind {
         PatKind::TupleStruct(ref path, ref patterns, _) if patterns.len() == 1 => {
@@ -86,7 +86,16 @@ fn find_sugg_for_if_let<'a, 'tcx>(
         _ => return,
     };
 
-    let maybe_semi = if has_else { "" } else { ";" };
+    // check that `while_let_on_iterator` lint does not trigger
+    if_chain! {
+        if keyword == "while";
+        if let ExprKind::MethodCall(method_path, _, _) = op.kind;
+        if method_path.ident.name == sym!(next);
+        if match_trait_method(cx, op, &paths::ITERATOR);
+        then {
+            return;
+        }
+    }
 
     span_lint_and_then(
         cx,
@@ -94,12 +103,15 @@ fn find_sugg_for_if_let<'a, 'tcx>(
         arms[0].pat.span,
         &format!("redundant pattern matching, consider using `{}`", good_method),
         |diag| {
-            let span = expr.span.to(op.span);
+            // in the case of WhileLetDesugar expr.span == op.span incorrectly.
+            // this is a workaround to restore true value of expr.span
+            let expr_span = expr.span.to(arms[1].span);
+            let span = expr_span.until(op.span.shrink_to_hi());
             diag.span_suggestion(
                 span,
                 "try this",
-                format!("{}.{}{}", snippet(cx, op.span, "_"), good_method, maybe_semi),
-                Applicability::MaybeIncorrect, // snippet
+                format!("{} {}.{}", keyword, snippet(cx, op.span, "_"), good_method),
+                Applicability::MachineApplicable, // snippet
             );
         },
     );
diff --git a/tests/ui/redundant_pattern_matching.fixed b/tests/ui/redundant_pattern_matching.fixed
index 538fa1ed9cb..c58db5493ad 100644
--- a/tests/ui/redundant_pattern_matching.fixed
+++ b/tests/ui/redundant_pattern_matching.fixed
@@ -2,16 +2,37 @@
 
 #![warn(clippy::all)]
 #![warn(clippy::redundant_pattern_matching)]
-#![allow(clippy::unit_arg, unused_must_use)]
+#![allow(clippy::unit_arg, unused_must_use, clippy::needless_bool)]
 
 fn main() {
-    Ok::<i32, i32>(42).is_ok();
+    if Ok::<i32, i32>(42).is_ok() {}
 
-    Err::<i32, i32>(42).is_err();
+    if Err::<i32, i32>(42).is_err() {}
 
-    None::<()>.is_none();
+    if None::<()>.is_none() {}
 
-    Some(42).is_some();
+    if Some(42).is_some() {}
+
+    if Some(42).is_some() {
+        foo();
+    } else {
+        bar();
+    }
+
+    while Some(42).is_some() {}
+
+    while Some(42).is_none() {}
+
+    while None::<()>.is_none() {}
+
+    while Ok::<i32, i32>(10).is_ok() {}
+
+    while Ok::<i32, i32>(10).is_err() {}
+
+    let mut v = vec![1, 2, 3];
+    while v.pop().is_some() {
+        foo();
+    }
 
     if Ok::<i32, i32>(42).is_ok() {}
 
@@ -39,22 +60,34 @@ fn main() {
 
     let _ = None::<()>.is_none();
 
-    let _ = Ok::<usize, ()>(4).is_ok();
+    let _ = if Ok::<usize, ()>(4).is_ok() { true } else { false };
 
     let _ = does_something();
     let _ = returns_unit();
 
     let opt = Some(false);
-    let x = opt.is_some();
+    let x = if opt.is_some() { true } else { false };
     takes_bool(x);
 }
 
 fn takes_bool(_: bool) {}
 
+fn foo() {}
+
+fn bar() {}
+
 fn does_something() -> bool {
-    Ok::<i32, i32>(4).is_ok()
+    if Ok::<i32, i32>(4).is_ok() {
+        true
+    } else {
+        false
+    }
 }
 
 fn returns_unit() {
-    Ok::<i32, i32>(4).is_ok();
+    if Ok::<i32, i32>(4).is_ok() {
+        true
+    } else {
+        false
+    };
 }
diff --git a/tests/ui/redundant_pattern_matching.rs b/tests/ui/redundant_pattern_matching.rs
index 34d2cd62e54..9a9b3fb5ca0 100644
--- a/tests/ui/redundant_pattern_matching.rs
+++ b/tests/ui/redundant_pattern_matching.rs
@@ -2,7 +2,7 @@
 
 #![warn(clippy::all)]
 #![warn(clippy::redundant_pattern_matching)]
-#![allow(clippy::unit_arg, unused_must_use)]
+#![allow(clippy::unit_arg, unused_must_use, clippy::needless_bool)]
 
 fn main() {
     if let Ok(_) = Ok::<i32, i32>(42) {}
@@ -13,6 +13,27 @@ fn main() {
 
     if let Some(_) = Some(42) {}
 
+    if let Some(_) = Some(42) {
+        foo();
+    } else {
+        bar();
+    }
+
+    while let Some(_) = Some(42) {}
+
+    while let None = Some(42) {}
+
+    while let None = None::<()> {}
+
+    while let Ok(_) = Ok::<i32, i32>(10) {}
+
+    while let Err(_) = Ok::<i32, i32>(10) {}
+
+    let mut v = vec![1, 2, 3];
+    while let Some(_) = v.pop() {
+        foo();
+    }
+
     if Ok::<i32, i32>(42).is_ok() {}
 
     if Err::<i32, i32>(42).is_err() {}
@@ -72,6 +93,10 @@ fn main() {
 
 fn takes_bool(_: bool) {}
 
+fn foo() {}
+
+fn bar() {}
+
 fn does_something() -> bool {
     if let Ok(_) = Ok::<i32, i32>(4) {
         true
diff --git a/tests/ui/redundant_pattern_matching.stderr b/tests/ui/redundant_pattern_matching.stderr
index 5a4a69b1220..6285a7f5fcd 100644
--- a/tests/ui/redundant_pattern_matching.stderr
+++ b/tests/ui/redundant_pattern_matching.stderr
@@ -2,7 +2,7 @@ error: redundant pattern matching, consider using `is_ok()`
   --> $DIR/redundant_pattern_matching.rs:8:12
    |
 LL |     if let Ok(_) = Ok::<i32, i32>(42) {}
-   |     -------^^^^^------------------------ help: try this: `Ok::<i32, i32>(42).is_ok();`
+   |     -------^^^^^--------------------- help: try this: `if Ok::<i32, i32>(42).is_ok()`
    |
    = note: `-D clippy::redundant-pattern-matching` implied by `-D warnings`
 
@@ -10,22 +10,64 @@ error: redundant pattern matching, consider using `is_err()`
   --> $DIR/redundant_pattern_matching.rs:10:12
    |
 LL |     if let Err(_) = Err::<i32, i32>(42) {}
-   |     -------^^^^^^------------------------- help: try this: `Err::<i32, i32>(42).is_err();`
+   |     -------^^^^^^---------------------- help: try this: `if Err::<i32, i32>(42).is_err()`
 
 error: redundant pattern matching, consider using `is_none()`
   --> $DIR/redundant_pattern_matching.rs:12:12
    |
 LL |     if let None = None::<()> {}
-   |     -------^^^^---------------- help: try this: `None::<()>.is_none();`
+   |     -------^^^^------------- help: try this: `if None::<()>.is_none()`
 
 error: redundant pattern matching, consider using `is_some()`
   --> $DIR/redundant_pattern_matching.rs:14:12
    |
 LL |     if let Some(_) = Some(42) {}
-   |     -------^^^^^^^-------------- help: try this: `Some(42).is_some();`
+   |     -------^^^^^^^----------- help: try this: `if Some(42).is_some()`
+
+error: redundant pattern matching, consider using `is_some()`
+  --> $DIR/redundant_pattern_matching.rs:16:12
+   |
+LL |     if let Some(_) = Some(42) {
+   |     -------^^^^^^^----------- help: try this: `if Some(42).is_some()`
+
+error: redundant pattern matching, consider using `is_some()`
+  --> $DIR/redundant_pattern_matching.rs:22: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.rs:24:15
+   |
+LL |     while let None = Some(42) {}
+   |     ----------^^^^----------- help: try this: `while Some(42).is_none()`
+
+error: redundant pattern matching, consider using `is_none()`
+  --> $DIR/redundant_pattern_matching.rs:26:15
+   |
+LL |     while let None = None::<()> {}
+   |     ----------^^^^------------- help: try this: `while None::<()>.is_none()`
 
 error: redundant pattern matching, consider using `is_ok()`
-  --> $DIR/redundant_pattern_matching.rs:28:5
+  --> $DIR/redundant_pattern_matching.rs:28: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.rs:30: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_some()`
+  --> $DIR/redundant_pattern_matching.rs:33:15
+   |
+LL |     while let Some(_) = v.pop() {
+   |     ----------^^^^^^^---------- help: try this: `while v.pop().is_some()`
+
+error: redundant pattern matching, consider using `is_ok()`
+  --> $DIR/redundant_pattern_matching.rs:49:5
    |
 LL | /     match Ok::<i32, i32>(42) {
 LL | |         Ok(_) => true,
@@ -34,7 +76,7 @@ LL | |     };
    | |_____^ help: try this: `Ok::<i32, i32>(42).is_ok()`
 
 error: redundant pattern matching, consider using `is_err()`
-  --> $DIR/redundant_pattern_matching.rs:33:5
+  --> $DIR/redundant_pattern_matching.rs:54:5
    |
 LL | /     match Ok::<i32, i32>(42) {
 LL | |         Ok(_) => false,
@@ -43,7 +85,7 @@ LL | |     };
    | |_____^ help: try this: `Ok::<i32, i32>(42).is_err()`
 
 error: redundant pattern matching, consider using `is_err()`
-  --> $DIR/redundant_pattern_matching.rs:38:5
+  --> $DIR/redundant_pattern_matching.rs:59:5
    |
 LL | /     match Err::<i32, i32>(42) {
 LL | |         Ok(_) => false,
@@ -52,7 +94,7 @@ LL | |     };
    | |_____^ help: try this: `Err::<i32, i32>(42).is_err()`
 
 error: redundant pattern matching, consider using `is_ok()`
-  --> $DIR/redundant_pattern_matching.rs:43:5
+  --> $DIR/redundant_pattern_matching.rs:64:5
    |
 LL | /     match Err::<i32, i32>(42) {
 LL | |         Ok(_) => true,
@@ -61,7 +103,7 @@ LL | |     };
    | |_____^ help: try this: `Err::<i32, i32>(42).is_ok()`
 
 error: redundant pattern matching, consider using `is_some()`
-  --> $DIR/redundant_pattern_matching.rs:48:5
+  --> $DIR/redundant_pattern_matching.rs:69:5
    |
 LL | /     match Some(42) {
 LL | |         Some(_) => true,
@@ -70,7 +112,7 @@ LL | |     };
    | |_____^ help: try this: `Some(42).is_some()`
 
 error: redundant pattern matching, consider using `is_none()`
-  --> $DIR/redundant_pattern_matching.rs:53:5
+  --> $DIR/redundant_pattern_matching.rs:74:5
    |
 LL | /     match None::<()> {
 LL | |         Some(_) => false,
@@ -79,7 +121,7 @@ LL | |     };
    | |_____^ help: try this: `None::<()>.is_none()`
 
 error: redundant pattern matching, consider using `is_none()`
-  --> $DIR/redundant_pattern_matching.rs:58:13
+  --> $DIR/redundant_pattern_matching.rs:79:13
    |
 LL |       let _ = match None::<()> {
    |  _____________^
@@ -89,38 +131,28 @@ LL | |     };
    | |_____^ help: try this: `None::<()>.is_none()`
 
 error: redundant pattern matching, consider using `is_ok()`
-  --> $DIR/redundant_pattern_matching.rs:63:20
+  --> $DIR/redundant_pattern_matching.rs:84:20
    |
 LL |     let _ = if let Ok(_) = Ok::<usize, ()>(4) { true } else { false };
-   |             -------^^^^^--------------------------------------------- help: try this: `Ok::<usize, ()>(4).is_ok()`
+   |             -------^^^^^--------------------- help: try this: `if Ok::<usize, ()>(4).is_ok()`
 
 error: redundant pattern matching, consider using `is_some()`
-  --> $DIR/redundant_pattern_matching.rs:69:20
+  --> $DIR/redundant_pattern_matching.rs:90:20
    |
 LL |     let x = if let Some(_) = opt { true } else { false };
-   |             -------^^^^^^^------------------------------ help: try this: `opt.is_some()`
+   |             -------^^^^^^^------ help: try this: `if opt.is_some()`
 
 error: redundant pattern matching, consider using `is_ok()`
-  --> $DIR/redundant_pattern_matching.rs:76:12
+  --> $DIR/redundant_pattern_matching.rs:101:12
    |
-LL |       if let Ok(_) = Ok::<i32, i32>(4) {
-   |  _____-      ^^^^^
-LL | |         true
-LL | |     } else {
-LL | |         false
-LL | |     }
-   | |_____- help: try this: `Ok::<i32, i32>(4).is_ok()`
+LL |     if let Ok(_) = Ok::<i32, i32>(4) {
+   |     -------^^^^^-------------------- help: try this: `if Ok::<i32, i32>(4).is_ok()`
 
 error: redundant pattern matching, consider using `is_ok()`
-  --> $DIR/redundant_pattern_matching.rs:84:12
+  --> $DIR/redundant_pattern_matching.rs:109:12
    |
-LL |       if let Ok(_) = Ok::<i32, i32>(4) {
-   |  _____-      ^^^^^
-LL | |         true
-LL | |     } else {
-LL | |         false
-LL | |     };
-   | |_____- help: try this: `Ok::<i32, i32>(4).is_ok()`
+LL |     if let Ok(_) = Ok::<i32, i32>(4) {
+   |     -------^^^^^-------------------- help: try this: `if Ok::<i32, i32>(4).is_ok()`
 
-error: aborting due to 15 previous errors
+error: aborting due to 22 previous errors