make match_like_matches_macro only apply to matches with a wildcard

rust-lang · Jul 8, 2020 · cc32305 · cc32305
1 parent 1740dda
commit cc32305
Show file tree

Hide file tree

Showing 5 changed files with 29 additions and 16 deletions.
diff --git a/clippy_lints/src/assign_ops.rs b/clippy_lints/src/assign_ops.rs
@@ -237,7 +237,6 @@ fn is_commutative(op: hir::BinOpKind) -> bool {
     use rustc_hir::BinOpKind::{
         Add, And, BitAnd, BitOr, BitXor, Div, Eq, Ge, Gt, Le, Lt, Mul, Ne, Or, Rem, Shl, Shr, Sub,
     };
-    #[allow(clippy::match_like_matches_macro)]
     match op {
         Add | Mul | And | Or | BitXor | BitAnd | BitOr | Eq | Ne => true,
         Sub | Div | Rem | Shl | Shr | Lt | Le | Ge | Gt => false,

diff --git a/clippy_lints/src/matches.rs b/clippy_lints/src/matches.rs
@@ -446,11 +446,12 @@ declare_clippy_lint! {
 }
 
 declare_clippy_lint! {
-    /// **What it does:** Checks for `match` expressions producing a `bool` that could be written using `matches!`
+    /// **What it does:** Checks for `match`  or `if let` expressions producing a
+    /// `bool` that could be written using `matches!`
     ///
     /// **Why is this bad?** Readability and needless complexity.
     ///
-    /// **Known problems:** This can turn an intentionally exhaustive match into a non-exhaustive one.
+    /// **Known problems:** None
     ///
     /// **Example:**
     /// ```rust
@@ -462,8 +463,14 @@ declare_clippy_lint! {
     ///     _ => false,
     /// };
     ///
+    /// let a = if let Some(0) = x {
+    ///     true
+    /// } else {
+    ///     false
+    /// }
+    ///
     /// // Good
-    /// let a = matches!(x, Some(5));
+    /// let a = matches!(x, Some(0));
     /// ```
     pub MATCH_LIKE_MATCHES_MACRO,
     style,
@@ -1068,6 +1075,7 @@ fn find_matches_sugg(cx: &LateContext<'_>, ex: &Expr<'_>, arms: &[Arm<'_>], expr
     if_chain! {
         if arms.len() == 2;
         if cx.tables().expr_ty(expr).is_bool();
+        if is_wild(&arms[1].pat);
         if let Some(first) = find_bool_lit(&arms[0].body.kind, desugared);
         if let Some(second) = find_bool_lit(&arms[1].body.kind, desugared);
         if first != second;

diff --git a/tests/ui/question_mark.fixed b/tests/ui/question_mark.fixed
@@ -23,7 +23,10 @@ pub enum SeemsOption<T> {
 
 impl<T> SeemsOption<T> {
     pub fn is_none(&self) -> bool {
-        matches!(*self, SeemsOption::None)
+        match *self {
+            SeemsOption::None => true,
+            SeemsOption::Some(_) => false,
+        }
     }
 }
 

diff --git a/tests/ui/question_mark.rs b/tests/ui/question_mark.rs
@@ -25,7 +25,10 @@ pub enum SeemsOption<T> {
 
 impl<T> SeemsOption<T> {
     pub fn is_none(&self) -> bool {
-        matches!(*self, SeemsOption::None)
+        match *self {
+            SeemsOption::None => true,
+            SeemsOption::Some(_) => false,
+        }
     }
 }
 

diff --git a/tests/ui/question_mark.stderr b/tests/ui/question_mark.stderr
@@ -9,23 +9,23 @@ LL | |     }
    = note: `-D clippy::question-mark` implied by `-D warnings`
 
 error: this block may be rewritten with the `?` operator
-  --> $DIR/question_mark.rs:47:9
+  --> $DIR/question_mark.rs:50:9
    |
 LL | /         if (self.opt).is_none() {
 LL | |             return None;
 LL | |         }
    | |_________^ help: replace it with: `(self.opt)?;`
 
 error: this block may be rewritten with the `?` operator
-  --> $DIR/question_mark.rs:51:9
+  --> $DIR/question_mark.rs:54:9
    |
 LL | /         if self.opt.is_none() {
 LL | |             return None
 LL | |         }
    | |_________^ help: replace it with: `self.opt?;`
 
 error: this block may be rewritten with the `?` operator
-  --> $DIR/question_mark.rs:55:17
+  --> $DIR/question_mark.rs:58:17
    |
 LL |           let _ = if self.opt.is_none() {
    |  _________________^
@@ -36,7 +36,7 @@ LL | |         };
    | |_________^ help: replace it with: `Some(self.opt?)`
 
 error: this if-let-else may be rewritten with the `?` operator
-  --> $DIR/question_mark.rs:61:17
+  --> $DIR/question_mark.rs:64:17
    |
 LL |           let _ = if let Some(x) = self.opt {
    |  _________________^
@@ -47,31 +47,31 @@ LL | |         };
    | |_________^ help: replace it with: `self.opt?`
 
 error: this block may be rewritten with the `?` operator
-  --> $DIR/question_mark.rs:78:9
+  --> $DIR/question_mark.rs:81:9
    |
 LL | /         if self.opt.is_none() {
 LL | |             return None;
 LL | |         }
    | |_________^ help: replace it with: `self.opt.as_ref()?;`
 
 error: this block may be rewritten with the `?` operator
-  --> $DIR/question_mark.rs:86:9
+  --> $DIR/question_mark.rs:89:9
    |
 LL | /         if self.opt.is_none() {
 LL | |             return None;
 LL | |         }
    | |_________^ help: replace it with: `self.opt.as_ref()?;`
 
 error: this block may be rewritten with the `?` operator
-  --> $DIR/question_mark.rs:94:9
+  --> $DIR/question_mark.rs:97:9
    |
 LL | /         if self.opt.is_none() {
 LL | |             return None;
 LL | |         }
    | |_________^ help: replace it with: `self.opt.as_ref()?;`
 
 error: this if-let-else may be rewritten with the `?` operator
-  --> $DIR/question_mark.rs:101:26
+  --> $DIR/question_mark.rs:104:26
    |
 LL |           let v: &Vec<_> = if let Some(ref v) = self.opt {
    |  __________________________^
@@ -82,7 +82,7 @@ LL | |         };
    | |_________^ help: replace it with: `self.opt.as_ref()?`
 
 error: this if-let-else may be rewritten with the `?` operator
-  --> $DIR/question_mark.rs:111:17
+  --> $DIR/question_mark.rs:114:17
    |
 LL |           let v = if let Some(v) = self.opt {
    |  _________________^
@@ -93,7 +93,7 @@ LL | |         };
    | |_________^ help: replace it with: `self.opt?`
 
 error: this block may be rewritten with the `?` operator
-  --> $DIR/question_mark.rs:126:5
+  --> $DIR/question_mark.rs:129:5
    |
 LL | /     if f().is_none() {
 LL | |         return None;