mikros/rust
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Auto merge of #11522 - y21:redundant_guards_pat_lhs, r=giraffate

Browse Source 
[`redundant_guards`]: lint if the pattern is on the left side

A tiny improvement to the `redundant_guards` lint. There's no associated issue for this, just noticed it while going through the code.
Right now it warns on `Some(x) if x == 2` and suggests `Some(2)`, but it didn't do that for `Some(x) if 2 == x` (i.e. when the local is on the right side and the pattern on the left side).

changelog: [`redundant_guards`]: also lint if the pattern is on the left side
This commit is contained in:
bors 2023-09-20 23:57:25 +00:00
commit c55669513a
4 changed files with 71 additions and 12 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

9
clippy_lints/src/matches/redundant_guards.rs
View File

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

9
tests/ui/redundant_guards.fixed
View File

@ -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 => {},
_ => {},
}
}
@ -157,6 +162,7 @@ mod issue11465 {
fn 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, .. } => {},

9
tests/ui/redundant_guards.rs
View File

@ -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 @@ impl Eq for S {}
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 @@ fn 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 @@ fn 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 @@ fn 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) => {},
_ => {},

56
tests/ui/redundant_guards.stderr
View File

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