fix redundant_pattern_matching lint

- now it gives correct suggestion in case of macros
- better tests
- remove a couple of non-relevant tests

clippy_lints/src/redundant_pattern_matching.rs

@@ -59,7 +59,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for RedundantPatternMatching {
 
 fn find_sugg_for_if_let<'a, 'tcx>(
     cx: &LateContext<'a, 'tcx>,
-    expr: &'tcx Expr<'_>,
+    _expr: &'tcx Expr<'_>,
     op: &Expr<'_>,
     arms: &[Arm<'_>],
     keyword: &'static str,
@@ -103,14 +103,21 @@ fn find_sugg_for_if_let<'a, 'tcx>(
         arms[0].pat.span,
         &format!("redundant pattern matching, consider using `{}`", good_method),
         |diag| {
-            // 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());
+            // while let ... = ... { ... }
+            // ^^^^^^^^^^^^^^^^^^^^^^^^^^^
+            let expr_span = arms[1].pat.span;
+
+            // while let ... = ... { ... }
+            //                 ^^^
+            let op_span = op.span.source_callsite();
+
+            // while let ... = ... { ... }
+            // ^^^^^^^^^^^^^^^^^^^
+            let span = expr_span.until(op_span.shrink_to_hi());
             diag.span_suggestion(
                 span,
                 "try this",
-                format!("{} {}.{}", keyword, snippet(cx, op.span, "_"), good_method),
+                format!("{} {}.{}", keyword, snippet(cx, op_span, "_"), good_method),
                 Applicability::MachineApplicable, // snippet
             );
         },

tests/ui/redundant_pattern_matching.fixed

@@ -2,7 +2,7 @@
 
 #![warn(clippy::all)]
 #![warn(clippy::redundant_pattern_matching)]
-#![allow(clippy::unit_arg, unused_must_use, clippy::needless_bool)]
+#![allow(clippy::unit_arg, unused_must_use, clippy::needless_bool, deprecated)]
 
 fn main() {
     if Ok::<i32, i32>(42).is_ok() {}
@@ -62,12 +62,31 @@ fn main() {
 
     let _ = if Ok::<usize, ()>(4).is_ok() { true } else { false };
 
-    let _ = does_something();
-    let _ = returns_unit();
-
     let opt = Some(false);
     let x = if opt.is_some() { true } else { false };
     takes_bool(x);
+
+    issue5504();
+
+    let _ = if gen_opt().is_some() {
+        1
+    } else if gen_opt().is_none() {
+        2
+    } else if gen_res().is_ok() {
+        3
+    } else if gen_res().is_err() {
+        4
+    } else {
+        5
+    };
+}
+
+fn gen_opt() -> Option<()> {
+    None
+}
+
+fn gen_res() -> Result<(), ()> {
+    Ok(())
 }
 
 fn takes_bool(_: bool) {}
@@ -76,18 +95,25 @@ fn foo() {}
 
 fn bar() {}
 
-fn does_something() -> bool {
-    if Ok::<i32, i32>(4).is_ok() {
-        true
-    } else {
-        false
-    }
+macro_rules! m {
+    () => {
+        Some(42u32)
+    };
 }
 
-fn returns_unit() {
-    if Ok::<i32, i32>(4).is_ok() {
-        true
-    } else {
-        false
-    };
+fn issue5504() {
+    fn result_opt() -> Result<Option<i32>, i32> {
+        Err(42)
+    }
+
+    fn try_result_opt() -> Result<i32, i32> {
+        while r#try!(result_opt()).is_some() {}
+        if r#try!(result_opt()).is_some() {}
+        Ok(42)
+    }
+
+    try_result_opt();
+
+    if m!().is_some() {}
+    while m!().is_some() {}
 }

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, clippy::needless_bool)]
+#![allow(clippy::unit_arg, unused_must_use, clippy::needless_bool, deprecated)]
 
 fn main() {
     if let Ok(_) = Ok::<i32, i32>(42) {}
@@ -83,12 +83,31 @@ fn main() {
 
     let _ = if let Ok(_) = Ok::<usize, ()>(4) { true } else { false };
 
-    let _ = does_something();
-    let _ = returns_unit();
-
     let opt = Some(false);
     let x = if let Some(_) = opt { true } else { false };
     takes_bool(x);
+
+    issue5504();
+
+    let _ = if let Some(_) = gen_opt() {
+        1
+    } else if let None = gen_opt() {
+        2
+    } else if let Ok(_) = gen_res() {
+        3
+    } else if let Err(_) = gen_res() {
+        4
+    } else {
+        5
+    };
+}
+
+fn gen_opt() -> Option<()> {
+    None
+}
+
+fn gen_res() -> Result<(), ()> {
+    Ok(())
 }
 
 fn takes_bool(_: bool) {}
@@ -97,18 +116,25 @@ fn foo() {}
 
 fn bar() {}
 
-fn does_something() -> bool {
-    if let Ok(_) = Ok::<i32, i32>(4) {
-        true
-    } else {
-        false
-    }
+macro_rules! m {
+    () => {
+        Some(42u32)
+    };
 }
 
-fn returns_unit() {
-    if let Ok(_) = Ok::<i32, i32>(4) {
-        true
-    } else {
-        false
-    };
+fn issue5504() {
+    fn result_opt() -> Result<Option<i32>, i32> {
+        Err(42)
+    }
+
+    fn try_result_opt() -> Result<i32, i32> {
+        while let Some(_) = r#try!(result_opt()) {}
+        if let Some(_) = r#try!(result_opt()) {}
+        Ok(42)
+    }
+
+    try_result_opt();
+
+    if let Some(_) = m!() {}
+    while let Some(_) = m!() {}
 }

tests/ui/redundant_pattern_matching.stderr

@@ -137,22 +137,58 @@ LL |     let _ = if let Ok(_) = Ok::<usize, ()>(4) { true } else { false };
    |             -------^^^^^--------------------- help: try this: `if Ok::<usize, ()>(4).is_ok()`
 
 error: redundant pattern matching, consider using `is_some()`
-  --> $DIR/redundant_pattern_matching.rs:90:20
+  --> $DIR/redundant_pattern_matching.rs:87:20
    |
 LL |     let x = if let Some(_) = opt { true } else { false };
    |             -------^^^^^^^------ help: try this: `if opt.is_some()`
 
-error: redundant pattern matching, consider using `is_ok()`
-  --> $DIR/redundant_pattern_matching.rs:101:12
+error: redundant pattern matching, consider using `is_some()`
+  --> $DIR/redundant_pattern_matching.rs:92:20
+   |
+LL |     let _ = if let Some(_) = gen_opt() {
+   |             -------^^^^^^^------------ help: try this: `if gen_opt().is_some()`
+
+error: redundant pattern matching, consider using `is_none()`
+  --> $DIR/redundant_pattern_matching.rs:94:19
    |
-LL |     if let Ok(_) = Ok::<i32, i32>(4) {
-   |     -------^^^^^-------------------- help: try this: `if Ok::<i32, i32>(4).is_ok()`
+LL |     } else if let None = gen_opt() {
+   |            -------^^^^------------ help: try this: `if gen_opt().is_none()`
 
 error: redundant pattern matching, consider using `is_ok()`
-  --> $DIR/redundant_pattern_matching.rs:109:12
+  --> $DIR/redundant_pattern_matching.rs:96:19
+   |
+LL |     } else if let Ok(_) = gen_res() {
+   |            -------^^^^^------------ help: try this: `if gen_res().is_ok()`
+
+error: redundant pattern matching, consider using `is_err()`
+  --> $DIR/redundant_pattern_matching.rs:98:19
+   |
+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.rs:131: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.rs:132: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.rs:138: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.rs:139:15
    |
-LL |     if let Ok(_) = Ok::<i32, i32>(4) {
-   |     -------^^^^^-------------------- help: try this: `if Ok::<i32, i32>(4).is_ok()`
+LL |     while let Some(_) = m!() {}
+   |     ----------^^^^^^^------- help: try this: `while m!().is_some()`
 
-error: aborting due to 22 previous errors
+error: aborting due to 28 previous errors