diff options
| -rw-r--r-- | clippy_lints/src/matches/redundant_guards.rs | 9 | ||||
| -rw-r--r-- | tests/ui/redundant_guards.fixed | 9 | ||||
| -rw-r--r-- | tests/ui/redundant_guards.rs | 9 | ||||
| -rw-r--r-- | tests/ui/redundant_guards.stderr | 56 |
4 files changed, 71 insertions, 12 deletions
diff --git a/clippy_lints/src/matches/redundant_guards.rs b/clippy_lints/src/matches/redundant_guards.rs index d470c05f351..0efeeacc9d9 100644 --- a/clippy_lints/src/matches/redundant_guards.rs +++ b/clippy_lints/src/matches/redundant_guards.rs @@ -70,10 +70,10 @@ pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, arms: &'tcx [Arm<'tcx>]) { ); } // `Some(x) if x == Some(2)` + // `Some(x) if Some(2) == x` else if let Guard::If(if_expr) = guard && let ExprKind::Binary(bin_op, local, pat) = if_expr.kind && matches!(bin_op.node, BinOpKind::Eq) - && expr_can_be_pat(cx, pat) // Ensure they have the same type. If they don't, we'd need deref coercion which isn't // possible (currently) in a pattern. In some cases, you can use something like // `as_deref` or similar but in general, we shouldn't lint this as it'd create an @@ -81,7 +81,12 @@ pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, arms: &'tcx [Arm<'tcx>]) { // // This isn't necessary in the other two checks, as they must be a pattern already. && cx.typeck_results().expr_ty(local) == cx.typeck_results().expr_ty(pat) - && let Some(binding) = get_pat_binding(cx, local, outer_arm) + // Since we want to lint on both `x == Some(2)` and `Some(2) == x`, we might have to "swap" + // `local` and `pat`, depending on which side they are. + && let Some((binding, pat)) = get_pat_binding(cx, local, outer_arm) + .map(|binding| (binding, pat)) + .or_else(|| get_pat_binding(cx, pat, outer_arm).map(|binding| (binding, local))) + && expr_can_be_pat(cx, pat) { let pat_span = match (pat.kind, binding.byref_ident) { (ExprKind::AddrOf(BorrowKind::Ref, _, expr), Some(_)) => expr.span, diff --git a/tests/ui/redundant_guards.fixed b/tests/ui/redundant_guards.fixed index 20fcc1254a6..f23116a7e1c 100644 --- a/tests/ui/redundant_guards.fixed +++ b/tests/ui/redundant_guards.fixed @@ -43,6 +43,7 @@ fn main() { }, Some(Some(1)) => .., Some(Some(2)) => .., + Some(Some(2)) => .., // Don't lint, since x is used in the body Some(x) if let Some(1) = x => { x; @@ -56,11 +57,13 @@ fn main() { Some(x) if matches!(y, 1 if true) => .., Some(x) if let 1 = y => .., Some(x) if y == 2 => .., + Some(x) if 2 == y => .., _ => todo!(), }; let a = A(1); match a { _ if a.0 == 1 => {}, + _ if 1 == a.0 => {}, _ => todo!(), } let b = B { e: Some(A(0)) }; @@ -119,6 +122,7 @@ fn h(v: Option<u32>) { fn f(s: Option<std::ffi::OsString>) { match s { Some(x) if x == "a" => {}, + Some(x) if "a" == x => {}, _ => {}, } } @@ -140,6 +144,7 @@ static CONST_S: S = S { a: 1 }; fn g(opt_s: Option<S>) { match opt_s { Some(x) if x == CONST_S => {}, + Some(x) if CONST_S == x => {}, _ => {}, } } @@ -158,6 +163,7 @@ mod issue11465 { let c = Some(1); match c { Some(1) => {}, + Some(1) => {}, Some(2) => {}, Some(3) => {}, _ => {}, @@ -166,6 +172,7 @@ mod issue11465 { let enum_a = A::Foo([98, 97, 114]); match enum_a { A::Foo(ref arr) if arr == b"foo" => {}, + A::Foo(ref arr) if b"foo" == arr => {}, A::Foo(ref arr) if let b"bar" = arr => {}, A::Foo(ref arr) if matches!(arr, b"baz") => {}, _ => {}, @@ -177,6 +184,8 @@ mod issue11465 { }; match struct_b { B { ref b, .. } if b == "bar" => {}, + B { ref b, .. } if "bar" == b => {}, + B { c: 1, .. } => {}, B { c: 1, .. } => {}, B { c: 1, .. } => {}, B { c: 1, .. } => {}, diff --git a/tests/ui/redundant_guards.rs b/tests/ui/redundant_guards.rs index e63cd29e8af..c0206b4cec7 100644 --- a/tests/ui/redundant_guards.rs +++ b/tests/ui/redundant_guards.rs @@ -43,6 +43,7 @@ fn main() { }, Some(x) if let Some(1) = x => .., Some(x) if x == Some(2) => .., + Some(x) if Some(2) == x => .., // Don't lint, since x is used in the body Some(x) if let Some(1) = x => { x; @@ -56,11 +57,13 @@ fn main() { Some(x) if matches!(y, 1 if true) => .., Some(x) if let 1 = y => .., Some(x) if y == 2 => .., + Some(x) if 2 == y => .., _ => todo!(), }; let a = A(1); match a { _ if a.0 == 1 => {}, + _ if 1 == a.0 => {}, _ => todo!(), } let b = B { e: Some(A(0)) }; @@ -119,6 +122,7 @@ fn h(v: Option<u32>) { fn f(s: Option<std::ffi::OsString>) { match s { Some(x) if x == "a" => {}, + Some(x) if "a" == x => {}, _ => {}, } } @@ -140,6 +144,7 @@ static CONST_S: S = S { a: 1 }; fn g(opt_s: Option<S>) { match opt_s { Some(x) if x == CONST_S => {}, + Some(x) if CONST_S == x => {}, _ => {}, } } @@ -158,6 +163,7 @@ mod issue11465 { let c = Some(1); match c { Some(ref x) if x == &1 => {}, + Some(ref x) if &1 == x => {}, Some(ref x) if let &2 = x => {}, Some(ref x) if matches!(x, &3) => {}, _ => {}, @@ -166,6 +172,7 @@ mod issue11465 { let enum_a = A::Foo([98, 97, 114]); match enum_a { A::Foo(ref arr) if arr == b"foo" => {}, + A::Foo(ref arr) if b"foo" == arr => {}, A::Foo(ref arr) if let b"bar" = arr => {}, A::Foo(ref arr) if matches!(arr, b"baz") => {}, _ => {}, @@ -177,7 +184,9 @@ mod issue11465 { }; match struct_b { B { ref b, .. } if b == "bar" => {}, + B { ref b, .. } if "bar" == b => {}, B { ref c, .. } if c == &1 => {}, + B { ref c, .. } if &1 == c => {}, B { ref c, .. } if let &1 = c => {}, B { ref c, .. } if matches!(c, &1) => {}, _ => {}, diff --git a/tests/ui/redundant_guards.stderr b/tests/ui/redundant_guards.stderr index f208e556f2e..b8d7834e358 100644 --- a/tests/ui/redundant_guards.stderr +++ b/tests/ui/redundant_guards.stderr @@ -60,7 +60,19 @@ LL + Some(Some(2)) => .., | error: redundant guard - --> $DIR/redundant_guards.rs:68:20 + --> $DIR/redundant_guards.rs:46:20 + | +LL | Some(x) if Some(2) == x => .., + | ^^^^^^^^^^^^ + | +help: try + | +LL - Some(x) if Some(2) == x => .., +LL + Some(Some(2)) => .., + | + +error: redundant guard + --> $DIR/redundant_guards.rs:71:20 | LL | B { e } if matches!(e, Some(A(2))) => .., | ^^^^^^^^^^^^^^^^^^^^^^^ @@ -72,7 +84,7 @@ LL + B { e: Some(A(2)) } => .., | error: redundant guard - --> $DIR/redundant_guards.rs:105:20 + --> $DIR/redundant_guards.rs:108:20 | LL | E::A(y) if y == "not from an or pattern" => {}, | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -84,7 +96,7 @@ LL + E::A("not from an or pattern") => {}, | error: redundant guard - --> $DIR/redundant_guards.rs:112:14 + --> $DIR/redundant_guards.rs:115:14 | LL | x if matches!(x, Some(0)) => .., | ^^^^^^^^^^^^^^^^^^^^ @@ -96,7 +108,7 @@ LL + Some(0) => .., | error: redundant guard - --> $DIR/redundant_guards.rs:160:28 + --> $DIR/redundant_guards.rs:165:28 | LL | Some(ref x) if x == &1 => {}, | ^^^^^^^ @@ -108,7 +120,19 @@ LL + Some(1) => {}, | error: redundant guard - --> $DIR/redundant_guards.rs:161:28 + --> $DIR/redundant_guards.rs:166:28 + | +LL | Some(ref x) if &1 == x => {}, + | ^^^^^^^ + | +help: try + | +LL - Some(ref x) if &1 == x => {}, +LL + Some(1) => {}, + | + +error: redundant guard + --> $DIR/redundant_guards.rs:167:28 | LL | Some(ref x) if let &2 = x => {}, | ^^^^^^^^^^ @@ -120,7 +144,7 @@ LL + Some(2) => {}, | error: redundant guard - --> $DIR/redundant_guards.rs:162:28 + --> $DIR/redundant_guards.rs:168:28 | LL | Some(ref x) if matches!(x, &3) => {}, | ^^^^^^^^^^^^^^^ @@ -132,7 +156,7 @@ LL + Some(3) => {}, | error: redundant guard - --> $DIR/redundant_guards.rs:180:32 + --> $DIR/redundant_guards.rs:188:32 | LL | B { ref c, .. } if c == &1 => {}, | ^^^^^^^ @@ -144,7 +168,19 @@ LL + B { c: 1, .. } => {}, | error: redundant guard - --> $DIR/redundant_guards.rs:181:32 + --> $DIR/redundant_guards.rs:189:32 + | +LL | B { ref c, .. } if &1 == c => {}, + | ^^^^^^^ + | +help: try + | +LL - B { ref c, .. } if &1 == c => {}, +LL + B { c: 1, .. } => {}, + | + +error: redundant guard + --> $DIR/redundant_guards.rs:190:32 | LL | B { ref c, .. } if let &1 = c => {}, | ^^^^^^^^^^ @@ -156,7 +192,7 @@ LL + B { c: 1, .. } => {}, | error: redundant guard - --> $DIR/redundant_guards.rs:182:32 + --> $DIR/redundant_guards.rs:191:32 | LL | B { ref c, .. } if matches!(c, &1) => {}, | ^^^^^^^^^^^^^^^ @@ -167,5 +203,5 @@ LL - B { ref c, .. } if matches!(c, &1) => {}, LL + B { c: 1, .. } => {}, | -error: aborting due to 14 previous errors +error: aborting due to 17 previous errors |