From 6c28f0765ea6e7c819292e1ed800895aac9f00ae Mon Sep 17 00:00:00 2001 From: y21 <30553356+y21@users.noreply.github.com> Date: Thu, 28 Dec 2023 05:27:56 +0100 Subject: [PATCH] recognize `contains` calls on ranges --- clippy_lints/src/transmute/eager_transmute.rs | 16 ++++ tests/ui/eager_transmute.fixed | 13 ++++ tests/ui/eager_transmute.rs | 13 ++++ tests/ui/eager_transmute.stderr | 76 +++++++++++++++++-- 4 files changed, 113 insertions(+), 5 deletions(-) diff --git a/clippy_lints/src/transmute/eager_transmute.rs b/clippy_lints/src/transmute/eager_transmute.rs index 7539b6af385..c44f5150dd1 100644 --- a/clippy_lints/src/transmute/eager_transmute.rs +++ b/clippy_lints/src/transmute/eager_transmute.rs @@ -36,11 +36,27 @@ fn range_fully_contained(from: WrappingRange, to: WrappingRange) -> bool { /// * `x < 4 && x > 1` /// * `x.field < 4 && x.field > 1` (given `x.field`) /// * `x.field < 4 && unrelated()` +/// * `(1..=3).contains(&x)` fn binops_with_local(cx: &LateContext<'_>, local_expr: &Expr<'_>, expr: &Expr<'_>) -> bool { match expr.kind { ExprKind::Binary(_, lhs, rhs) => { binops_with_local(cx, local_expr, lhs) || binops_with_local(cx, local_expr, rhs) }, + ExprKind::MethodCall(path, receiver, [arg], _) + if path.ident.name == sym!(contains) + // ... `contains` called on some kind of range + && let Some(receiver_adt) = cx.typeck_results().expr_ty(receiver).peel_refs().ty_adt_def() + && let lang_items = cx.tcx.lang_items() + && [ + lang_items.range_from_struct(), + lang_items.range_inclusive_struct(), + lang_items.range_struct(), + lang_items.range_to_inclusive_struct(), + lang_items.range_to_struct() + ].into_iter().any(|did| did == Some(receiver_adt.did())) => + { + eq_expr_value(cx, local_expr, arg.peel_borrows()) + }, _ => eq_expr_value(cx, local_expr, expr), } } diff --git a/tests/ui/eager_transmute.fixed b/tests/ui/eager_transmute.fixed index 58c6ee914c6..bece09bba1a 100644 --- a/tests/ui/eager_transmute.fixed +++ b/tests/ui/eager_transmute.fixed @@ -42,6 +42,19 @@ fn f(op: u8, op2: Data, unrelated: u8) { // don't lint: wrong variable let _: Option = (op2.foo[0] > 0 && op2.bar[1] < 10).then_some(unsafe { std::mem::transmute(op) }); + + // range contains checks + let _: Option = (1..=3).contains(&op).then(|| unsafe { std::mem::transmute(op) }); + let _: Option = ((1..=3).contains(&op) || op == 4).then(|| unsafe { std::mem::transmute(op) }); + let _: Option = (1..3).contains(&op).then(|| unsafe { std::mem::transmute(op) }); + let _: Option = (1..).contains(&op).then(|| unsafe { std::mem::transmute(op) }); + let _: Option = (..3).contains(&op).then(|| unsafe { std::mem::transmute(op) }); + let _: Option = (..=3).contains(&op).then(|| unsafe { std::mem::transmute(op) }); + + // unrelated binding in contains + let _: Option = (1..=3) + .contains(&unrelated) + .then_some(unsafe { std::mem::transmute(op) }); } unsafe fn f2(op: u8) { diff --git a/tests/ui/eager_transmute.rs b/tests/ui/eager_transmute.rs index 2af311d6c51..a82bd578f76 100644 --- a/tests/ui/eager_transmute.rs +++ b/tests/ui/eager_transmute.rs @@ -42,6 +42,19 @@ fn f(op: u8, op2: Data, unrelated: u8) { // don't lint: wrong variable let _: Option = (op2.foo[0] > 0 && op2.bar[1] < 10).then_some(unsafe { std::mem::transmute(op) }); + + // range contains checks + let _: Option = (1..=3).contains(&op).then_some(unsafe { std::mem::transmute(op) }); + let _: Option = ((1..=3).contains(&op) || op == 4).then_some(unsafe { std::mem::transmute(op) }); + let _: Option = (1..3).contains(&op).then_some(unsafe { std::mem::transmute(op) }); + let _: Option = (1..).contains(&op).then_some(unsafe { std::mem::transmute(op) }); + let _: Option = (..3).contains(&op).then_some(unsafe { std::mem::transmute(op) }); + let _: Option = (..=3).contains(&op).then_some(unsafe { std::mem::transmute(op) }); + + // unrelated binding in contains + let _: Option = (1..=3) + .contains(&unrelated) + .then_some(unsafe { std::mem::transmute(op) }); } unsafe fn f2(op: u8) { diff --git a/tests/ui/eager_transmute.stderr b/tests/ui/eager_transmute.stderr index 65dd6815c3a..5f42eec544f 100644 --- a/tests/ui/eager_transmute.stderr +++ b/tests/ui/eager_transmute.stderr @@ -78,7 +78,73 @@ LL | let _: Option = (op2.foo[0] > 0 && op2.foo[0] < 10).then(|| uns | ~~~~ ++ error: this transmute is always evaluated eagerly, even if the condition is false - --> $DIR/eager_transmute.rs:48:24 + --> $DIR/eager_transmute.rs:47:70 + | +LL | let _: Option = (1..=3).contains(&op).then_some(unsafe { std::mem::transmute(op) }); + | ^^^^^^^^^^^^^^^^^^^^^^^ + | +help: consider using `bool::then` to only transmute if the condition holds + | +LL | let _: Option = (1..=3).contains(&op).then(|| unsafe { std::mem::transmute(op) }); + | ~~~~ ++ + +error: this transmute is always evaluated eagerly, even if the condition is false + --> $DIR/eager_transmute.rs:48:83 + | +LL | let _: Option = ((1..=3).contains(&op) || op == 4).then_some(unsafe { std::mem::transmute(op) }); + | ^^^^^^^^^^^^^^^^^^^^^^^ + | +help: consider using `bool::then` to only transmute if the condition holds + | +LL | let _: Option = ((1..=3).contains(&op) || op == 4).then(|| unsafe { std::mem::transmute(op) }); + | ~~~~ ++ + +error: this transmute is always evaluated eagerly, even if the condition is false + --> $DIR/eager_transmute.rs:49:69 + | +LL | let _: Option = (1..3).contains(&op).then_some(unsafe { std::mem::transmute(op) }); + | ^^^^^^^^^^^^^^^^^^^^^^^ + | +help: consider using `bool::then` to only transmute if the condition holds + | +LL | let _: Option = (1..3).contains(&op).then(|| unsafe { std::mem::transmute(op) }); + | ~~~~ ++ + +error: this transmute is always evaluated eagerly, even if the condition is false + --> $DIR/eager_transmute.rs:50:68 + | +LL | let _: Option = (1..).contains(&op).then_some(unsafe { std::mem::transmute(op) }); + | ^^^^^^^^^^^^^^^^^^^^^^^ + | +help: consider using `bool::then` to only transmute if the condition holds + | +LL | let _: Option = (1..).contains(&op).then(|| unsafe { std::mem::transmute(op) }); + | ~~~~ ++ + +error: this transmute is always evaluated eagerly, even if the condition is false + --> $DIR/eager_transmute.rs:51:68 + | +LL | let _: Option = (..3).contains(&op).then_some(unsafe { std::mem::transmute(op) }); + | ^^^^^^^^^^^^^^^^^^^^^^^ + | +help: consider using `bool::then` to only transmute if the condition holds + | +LL | let _: Option = (..3).contains(&op).then(|| unsafe { std::mem::transmute(op) }); + | ~~~~ ++ + +error: this transmute is always evaluated eagerly, even if the condition is false + --> $DIR/eager_transmute.rs:52:69 + | +LL | let _: Option = (..=3).contains(&op).then_some(unsafe { std::mem::transmute(op) }); + | ^^^^^^^^^^^^^^^^^^^^^^^ + | +help: consider using `bool::then` to only transmute if the condition holds + | +LL | let _: Option = (..=3).contains(&op).then(|| unsafe { std::mem::transmute(op) }); + | ~~~~ ++ + +error: this transmute is always evaluated eagerly, even if the condition is false + --> $DIR/eager_transmute.rs:61:24 | LL | (op < 4).then_some(std::mem::transmute::<_, Opcode>(op)); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -89,7 +155,7 @@ LL | (op < 4).then(|| std::mem::transmute::<_, Opcode>(op)); | ~~~~ ++ error: this transmute is always evaluated eagerly, even if the condition is false - --> $DIR/eager_transmute.rs:77:60 + --> $DIR/eager_transmute.rs:90:60 | LL | let _: Option = (v1 > 0).then_some(unsafe { std::mem::transmute(v1) }); | ^^^^^^^^^^^^^^^^^^^^^^^ @@ -100,7 +166,7 @@ LL | let _: Option = (v1 > 0).then(|| unsafe { std::mem::transmut | ~~~~ ++ error: this transmute is always evaluated eagerly, even if the condition is false - --> $DIR/eager_transmute.rs:83:86 + --> $DIR/eager_transmute.rs:96:86 | LL | let _: Option = (v2 < NonZeroU8::new(255).unwrap()).then_some(unsafe { std::mem::transmute(v2) }); | ^^^^^^^^^^^^^^^^^^^^^^^ @@ -111,7 +177,7 @@ LL | let _: Option = (v2 < NonZeroU8::new(255).unwrap()).then(|| u | ~~~~ ++ error: this transmute is always evaluated eagerly, even if the condition is false - --> $DIR/eager_transmute.rs:89:93 + --> $DIR/eager_transmute.rs:102:93 | LL | let _: Option = (v2 < NonZeroU8::new(255).unwrap()).then_some(unsafe { std::mem::transmute(v2) }); | ^^^^^^^^^^^^^^^^^^^^^^^ @@ -121,5 +187,5 @@ help: consider using `bool::then` to only transmute if the condition holds LL | let _: Option = (v2 < NonZeroU8::new(255).unwrap()).then(|| unsafe { std::mem::transmute(v2) }); | ~~~~ ++ -error: aborting due to 11 previous errors +error: aborting due to 17 previous errors