make match_like_matches_macro only apply to matches with a wildcard

This commit is contained in:
robojumper 2020-07-08 18:04:51 +02:00
parent 1740dda763
commit 37d75da266
8 changed files with 77 additions and 54 deletions

View file

@ -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,

View file

@ -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,
@ -499,9 +506,8 @@ impl<'tcx> LateLintPass<'tcx> for Matches {
return;
}
if !redundant_pattern_match::check(cx, expr) {
check_match_like_matches(cx, expr);
}
redundant_pattern_match::check(cx, expr);
check_match_like_matches(cx, expr);
if let ExprKind::Match(ref ex, ref arms, MatchSource::Normal) = expr.kind {
check_single_match(cx, ex, arms, expr);
@ -1068,6 +1074,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;
@ -1437,16 +1444,14 @@ mod redundant_pattern_match {
use rustc_mir::const_eval::is_const_fn;
use rustc_span::source_map::Symbol;
pub fn check<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) -> bool {
pub fn check<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
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 { .. } => find_sugg_for_if_let(cx, expr, op, arms, "if"),
MatchSource::WhileLetDesugar => find_sugg_for_if_let(cx, expr, op, arms, "while"),
_ => false,
_ => {},
}
} else {
false
}
}
@ -1456,7 +1461,7 @@ mod redundant_pattern_match {
op: &Expr<'_>,
arms: &[Arm<'_>],
keyword: &'static str,
) -> bool {
) {
fn find_suggestion(cx: &LateContext<'_>, hir_id: HirId, path: &QPath<'_>) -> Option<&'static str> {
if match_qpath(path, &paths::RESULT_OK) && can_suggest(cx, hir_id, sym!(result_type), "is_ok") {
return Some("is_ok()");
@ -1487,7 +1492,7 @@ mod redundant_pattern_match {
};
let good_method = match good_method {
Some(method) => method,
None => return false,
None => return,
};
// check that `while_let_on_iterator` lint does not trigger
@ -1497,7 +1502,7 @@ mod redundant_pattern_match {
if method_path.ident.name == sym!(next);
if match_trait_method(cx, op, &paths::ITERATOR);
then {
return false;
return;
}
}
@ -1526,15 +1531,9 @@ mod redundant_pattern_match {
);
},
);
true
}
fn find_sugg_for_match<'tcx>(
cx: &LateContext<'tcx>,
expr: &'tcx Expr<'_>,
op: &Expr<'_>,
arms: &[Arm<'_>],
) -> bool {
fn find_sugg_for_match<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, op: &Expr<'_>, arms: &[Arm<'_>]) {
if arms.len() == 2 {
let node_pair = (&arms[0].pat.kind, &arms[1].pat.kind);
@ -1599,10 +1598,8 @@ mod redundant_pattern_match {
);
},
);
return true;
}
}
false
}
#[allow(clippy::too_many_arguments)]

View file

@ -1,6 +1,7 @@
// run-rustfix
#![warn(clippy::match_like_matches_macro)]
#![allow(unreachable_patterns)]
fn main() {
let x = Some(5);
@ -8,25 +9,28 @@ fn main() {
// Lint
let _y = matches!(x, Some(0));
// Lint
let _w = matches!(x, Some(_));
// Turn into is_none
let _z = x.is_none();
// Lint
let _z = !matches!(x, Some(r) if r == 0);
let _zz = !matches!(x, Some(r) if r == 0);
// Lint
let _zz = matches!(x, Some(5));
let _zzz = matches!(x, Some(5));
// No lint
let _a = match x {
Some(_) => false,
None => false,
_ => false,
};
// No lint
let _a = match x {
let _ab = match x {
Some(0) => false,
Some(_) => true,
_ => true,
None => false,
};
}

View file

@ -1,6 +1,7 @@
// run-rustfix
#![warn(clippy::match_like_matches_macro)]
#![allow(unreachable_patterns)]
fn main() {
let x = Some(5);
@ -11,6 +12,12 @@ fn main() {
_ => false,
};
// Lint
let _w = match x {
Some(_) => true,
_ => false,
};
// Turn into is_none
let _z = match x {
Some(_) => false,
@ -18,24 +25,24 @@ fn main() {
};
// Lint
let _z = match x {
let _zz = match x {
Some(r) if r == 0 => false,
_ => true,
};
// Lint
let _zz = if let Some(5) = x { true } else { false };
let _zzz = if let Some(5) = x { true } else { false };
// No lint
let _a = match x {
Some(_) => false,
None => false,
_ => false,
};
// No lint
let _a = match x {
let _ab = match x {
Some(0) => false,
Some(_) => true,
_ => true,
None => false,
};
}

View file

@ -1,5 +1,5 @@
error: match expression looks like `matches!` macro
--> $DIR/match_expr_like_matches_macro.rs:9:14
--> $DIR/match_expr_like_matches_macro.rs:10:14
|
LL | let _y = match x {
| ______________^
@ -10,8 +10,18 @@ LL | | };
|
= note: `-D clippy::match-like-matches-macro` implied by `-D warnings`
error: match expression looks like `matches!` macro
--> $DIR/match_expr_like_matches_macro.rs:16:14
|
LL | let _w = match x {
| ______________^
LL | | Some(_) => true,
LL | | _ => false,
LL | | };
| |_____^ help: try this: `matches!(x, Some(_))`
error: redundant pattern matching, consider using `is_none()`
--> $DIR/match_expr_like_matches_macro.rs:15:14
--> $DIR/match_expr_like_matches_macro.rs:22:14
|
LL | let _z = match x {
| ______________^
@ -23,20 +33,20 @@ LL | | };
= note: `-D clippy::redundant-pattern-matching` implied by `-D warnings`
error: match expression looks like `matches!` macro
--> $DIR/match_expr_like_matches_macro.rs:21:14
--> $DIR/match_expr_like_matches_macro.rs:28:15
|
LL | let _z = match x {
| ______________^
LL | let _zz = match x {
| _______________^
LL | | Some(r) if r == 0 => false,
LL | | _ => true,
LL | | };
| |_____^ help: try this: `!matches!(x, Some(r) if r == 0)`
error: if let .. else expression looks like `matches!` macro
--> $DIR/match_expr_like_matches_macro.rs:27:15
--> $DIR/match_expr_like_matches_macro.rs:34:16
|
LL | let _zz = if let Some(5) = x { true } else { false };
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `matches!(x, Some(5))`
LL | let _zzz = if let Some(5) = x { true } else { false };
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `matches!(x, Some(5))`
error: aborting due to 4 previous errors
error: aborting due to 5 previous errors

View file

@ -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,
}
}
}

View file

@ -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,
}
}
}

View file

@ -9,7 +9,7 @@ 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;
@ -17,7 +17,7 @@ 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
@ -25,7 +25,7 @@ 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,7 +47,7 @@ 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;
@ -55,7 +55,7 @@ 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;
@ -63,7 +63,7 @@ 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;
@ -71,7 +71,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: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;