Auto merge of #4352 - phansch:fix_redundant_pattern_matching, r=flip1995

Fix some suggestions for redundant_pattern_matching .. and change the Applicability to `MaybeIncorrect`. Fixes the problem displayed in #4344 (comment). We now append `{}` to the suggestion so that the conditional has the correct syntax again. (If we were to _remove_ the `if` instead, it would trigger the `unused_must_use` warning for `#[must_use]` types.) changelog: Fix some suggestions for `redundant_pattern_matching`
rust-lang · Aug 21, 2019 · 57c67a2 · 57c67a2
2 parents 45d24fd + 436d429
commit 57c67a2
Show file tree

Hide file tree

Showing 3 changed files with 104 additions and 18 deletions.
diff --git a/clippy_lints/src/redundant_pattern_matching.rs b/clippy_lints/src/redundant_pattern_matching.rs
@@ -90,8 +90,8 @@ fn find_sugg_for_if_let<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr,
                 db.span_suggestion(
                     span,
                     "try this",
-                    format!("if {}.{}", snippet(cx, op.span, "_"), good_method),
-                    Applicability::MachineApplicable, // snippet
+                    format!("{}.{}", snippet(cx, op.span, "_"), good_method),
+                    Applicability::MaybeIncorrect, // snippet
                 );
             },
         );
@@ -154,7 +154,7 @@ fn find_sugg_for_match<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr, o
                         span,
                         "try this",
                         format!("{}.{}", snippet(cx, op.span, "_"), good_method),
-                        Applicability::MachineApplicable, // snippet
+                        Applicability::MaybeIncorrect, // snippet
                     );
                 },
             );

diff --git a/tests/ui/redundant_pattern_matching.rs b/tests/ui/redundant_pattern_matching.rs
@@ -1,5 +1,6 @@
 #![warn(clippy::all)]
 #![warn(clippy::redundant_pattern_matching)]
+#![allow(clippy::unit_arg, clippy::let_unit_value)]
 
 fn main() {
     if let Ok(_) = Ok::<i32, i32>(42) {}
@@ -51,4 +52,39 @@ fn main() {
         Some(_) => false,
         None => true,
     };
+
+    let _ = match None::<()> {
+        Some(_) => false,
+        None => true,
+    };
+
+    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);
+    let y = if let Some(_) = opt {};
+    takes_unit(y);
+}
+
+fn takes_bool(x: bool) {}
+fn takes_unit(x: ()) {}
+
+fn does_something() -> bool {
+    if let Ok(_) = Ok::<i32, i32>(4) {
+        true
+    } else {
+        false
+    }
+}
+
+fn returns_unit() {
+    if let Ok(_) = Ok::<i32, i32>(4) {
+        true
+    } else {
+        false
+    };
 }
diff --git a/tests/ui/redundant_pattern_matching.stderr b/tests/ui/redundant_pattern_matching.stderr
@@ -1,31 +1,31 @@
 error: redundant pattern matching, consider using `is_ok()`
-  --> $DIR/redundant_pattern_matching.rs:5:12
+  --> $DIR/redundant_pattern_matching.rs:6:12
    |
 LL |     if let Ok(_) = Ok::<i32, i32>(42) {}
-   |     -------^^^^^------------------------ help: try this: `if Ok::<i32, i32>(42).is_ok()`
+   |     -------^^^^^------------------------ help: try this: `Ok::<i32, i32>(42).is_ok()`
    |
    = note: `-D clippy::redundant-pattern-matching` implied by `-D warnings`
 
 error: redundant pattern matching, consider using `is_err()`
-  --> $DIR/redundant_pattern_matching.rs:7:12
+  --> $DIR/redundant_pattern_matching.rs:8:12
    |
 LL |     if let Err(_) = Err::<i32, i32>(42) {}
-   |     -------^^^^^^------------------------- help: try this: `if Err::<i32, i32>(42).is_err()`
+   |     -------^^^^^^------------------------- help: try this: `Err::<i32, i32>(42).is_err()`
 
 error: redundant pattern matching, consider using `is_none()`
-  --> $DIR/redundant_pattern_matching.rs:9:12
+  --> $DIR/redundant_pattern_matching.rs:10:12
    |
 LL |     if let None = None::<()> {}
-   |     -------^^^^---------------- help: try this: `if None::<()>.is_none()`
+   |     -------^^^^---------------- help: try this: `None::<()>.is_none()`
 
 error: redundant pattern matching, consider using `is_some()`
-  --> $DIR/redundant_pattern_matching.rs:11:12
+  --> $DIR/redundant_pattern_matching.rs:12:12
    |
 LL |     if let Some(_) = Some(42) {}
-   |     -------^^^^^^^-------------- help: try this: `if Some(42).is_some()`
+   |     -------^^^^^^^-------------- help: try this: `Some(42).is_some()`
 
 error: redundant pattern matching, consider using `is_ok()`
-  --> $DIR/redundant_pattern_matching.rs:25:5
+  --> $DIR/redundant_pattern_matching.rs:26:5
    |
 LL | /     match Ok::<i32, i32>(42) {
 LL | |         Ok(_) => true,
@@ -34,7 +34,7 @@ LL | |     };
    | |_____^ help: try this: `Ok::<i32, i32>(42).is_ok()`
 
 error: redundant pattern matching, consider using `is_err()`
-  --> $DIR/redundant_pattern_matching.rs:30:5
+  --> $DIR/redundant_pattern_matching.rs:31:5
    |
 LL | /     match Ok::<i32, i32>(42) {
 LL | |         Ok(_) => false,
@@ -43,7 +43,7 @@ LL | |     };
    | |_____^ help: try this: `Ok::<i32, i32>(42).is_err()`
 
 error: redundant pattern matching, consider using `is_err()`
-  --> $DIR/redundant_pattern_matching.rs:35:5
+  --> $DIR/redundant_pattern_matching.rs:36:5
    |
 LL | /     match Err::<i32, i32>(42) {
 LL | |         Ok(_) => false,
@@ -52,7 +52,7 @@ LL | |     };
    | |_____^ help: try this: `Err::<i32, i32>(42).is_err()`
 
 error: redundant pattern matching, consider using `is_ok()`
-  --> $DIR/redundant_pattern_matching.rs:40:5
+  --> $DIR/redundant_pattern_matching.rs:41:5
    |
 LL | /     match Err::<i32, i32>(42) {
 LL | |         Ok(_) => true,
@@ -61,7 +61,7 @@ LL | |     };
    | |_____^ help: try this: `Err::<i32, i32>(42).is_ok()`
 
 error: redundant pattern matching, consider using `is_some()`
-  --> $DIR/redundant_pattern_matching.rs:45:5
+  --> $DIR/redundant_pattern_matching.rs:46:5
    |
 LL | /     match Some(42) {
 LL | |         Some(_) => true,
@@ -70,13 +70,63 @@ LL | |     };
    | |_____^ help: try this: `Some(42).is_some()`
 
 error: redundant pattern matching, consider using `is_none()`
-  --> $DIR/redundant_pattern_matching.rs:50:5
+  --> $DIR/redundant_pattern_matching.rs:51:5
    |
 LL | /     match None::<()> {
 LL | |         Some(_) => false,
 LL | |         None => true,
 LL | |     };
    | |_____^ help: try this: `None::<()>.is_none()`
 
-error: aborting due to 10 previous errors
+error: redundant pattern matching, consider using `is_none()`
+  --> $DIR/redundant_pattern_matching.rs:56:13
+   |
+LL |       let _ = match None::<()> {
+   |  _____________^
+LL | |         Some(_) => false,
+LL | |         None => true,
+LL | |     };
+   | |_____^ help: try this: `None::<()>.is_none()`
+
+error: redundant pattern matching, consider using `is_ok()`
+  --> $DIR/redundant_pattern_matching.rs:61:20
+   |
+LL |     let _ = if let Ok(_) = Ok::<usize, ()>(4) { true } else { false };
+   |             -------^^^^^--------------------------------------------- help: try this: `Ok::<usize, ()>(4).is_ok()`
+
+error: redundant pattern matching, consider using `is_some()`
+  --> $DIR/redundant_pattern_matching.rs:67:20
+   |
+LL |     let x = if let Some(_) = opt { true } else { false };
+   |             -------^^^^^^^------------------------------ help: try this: `opt.is_some()`
+
+error: redundant pattern matching, consider using `is_some()`
+  --> $DIR/redundant_pattern_matching.rs:69:20
+   |
+LL |     let y = if let Some(_) = opt {};
+   |             -------^^^^^^^--------- help: try this: `opt.is_some()`
+
+error: redundant pattern matching, consider using `is_ok()`
+  --> $DIR/redundant_pattern_matching.rs:77: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()`
+
+error: redundant pattern matching, consider using `is_ok()`
+  --> $DIR/redundant_pattern_matching.rs:85: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()`
+
+error: aborting due to 16 previous errors