fix redundant_pattern_matching lint

- now it gives correct suggestion in case of macros
rust-lang · Apr 23, 2020 · ef9c38c · ef9c38c
1 parent d01a498
commit ef9c38c
Show file tree

Hide file tree

Showing 4 changed files with 92 additions and 11 deletions.
diff --git a/clippy_lints/src/redundant_pattern_matching.rs b/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
             );
         },

diff --git a/tests/ui/redundant_pattern_matching.fixed b/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() {}
@@ -68,6 +68,8 @@ fn main() {
     let opt = Some(false);
     let x = if opt.is_some() { true } else { false };
     takes_bool(x);
+
+    issue5504();
 }
 
 fn takes_bool(_: bool) {}
@@ -91,3 +93,26 @@ fn returns_unit() {
         false
     };
 }
+
+macro_rules! m {
+    () => {
+        Some(42u32)
+    };
+}
+
+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() {}
+}
diff --git 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, clippy::needless_bool)]
+#![allow(clippy::unit_arg, unused_must_use, clippy::needless_bool, deprecated)]
 
 fn main() {
     if let Ok(_) = Ok::<i32, i32>(42) {}
@@ -89,6 +89,8 @@ fn main() {
     let opt = Some(false);
     let x = if let Some(_) = opt { true } else { false };
     takes_bool(x);
+
+    issue5504();
 }
 
 fn takes_bool(_: bool) {}
@@ -112,3 +114,26 @@ fn returns_unit() {
         false
     };
 }
+
+macro_rules! m {
+    () => {
+        Some(42u32)
+    };
+}
+
+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!() {}
+}
diff --git a/tests/ui/redundant_pattern_matching.stderr b/tests/ui/redundant_pattern_matching.stderr
@@ -143,16 +143,40 @@ 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
+  --> $DIR/redundant_pattern_matching.rs:103:12
    |
 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:109:12
+  --> $DIR/redundant_pattern_matching.rs:111:12
    |
 LL |     if let Ok(_) = Ok::<i32, i32>(4) {
    |     -------^^^^^-------------------- help: try this: `if Ok::<i32, i32>(4).is_ok()`
 
-error: aborting due to 22 previous errors
+error: redundant pattern matching, consider using `is_some()`
+  --> $DIR/redundant_pattern_matching.rs:130: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:131: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:137: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:138:15
+   |
+LL |     while let Some(_) = m!() {}
+   |     ----------^^^^^^^------- help: try this: `while m!().is_some()`
+
+error: aborting due to 26 previous errors