diff options
| -rw-r--r-- | clippy_lints/src/methods/filter_map_bool_then.rs | 14 | ||||
| -rw-r--r-- | tests/ui/filter_map_bool_then.fixed | 16 | ||||
| -rw-r--r-- | tests/ui/filter_map_bool_then.rs | 16 | ||||
| -rw-r--r-- | tests/ui/filter_map_bool_then.stderr | 18 |
4 files changed, 53 insertions, 11 deletions
diff --git a/clippy_lints/src/methods/filter_map_bool_then.rs b/clippy_lints/src/methods/filter_map_bool_then.rs index 4aee22a4afc..fafc9709770 100644 --- a/clippy_lints/src/methods/filter_map_bool_then.rs +++ b/clippy_lints/src/methods/filter_map_bool_then.rs @@ -1,3 +1,4 @@ +use super::FILTER_MAP_BOOL_THEN; use clippy_utils::diagnostics::span_lint_and_sugg; use clippy_utils::paths::BOOL_THEN; use clippy_utils::source::snippet_opt; @@ -7,10 +8,9 @@ use rustc_errors::Applicability; use rustc_hir::{Expr, ExprKind}; use rustc_lint::{LateContext, LintContext}; use rustc_middle::lint::in_external_macro; +use rustc_middle::ty::Binder; use rustc_span::{sym, Span}; -use super::FILTER_MAP_BOOL_THEN; - pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>, arg: &Expr<'_>, call_span: Span) { if !in_external_macro(cx.sess(), expr.span) && is_trait_method(cx, expr, sym::Iterator) @@ -21,7 +21,15 @@ pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>, arg: & // `inputs` and `params` here as we need both the type and the span && let param_ty = closure.fn_decl.inputs[0] && let param = body.params[0] - && is_copy(cx, cx.typeck_results().node_type(param_ty.hir_id).peel_refs()) + // Issue #11309 + && let param_ty = cx.tcx.liberate_late_bound_regions( + closure.def_id.to_def_id(), + Binder::bind_with_vars( + cx.typeck_results().node_type(param_ty.hir_id), + cx.tcx.late_bound_vars(cx.tcx.hir().local_def_id_to_hir_id(closure.def_id)), + ), + ) + && is_copy(cx, param_ty) && let ExprKind::MethodCall(_, recv, [then_arg], _) = value.kind && let ExprKind::Closure(then_closure) = then_arg.kind && let then_body = peel_blocks(cx.tcx.hir().body(then_closure.body).value) diff --git a/tests/ui/filter_map_bool_then.fixed b/tests/ui/filter_map_bool_then.fixed index 3e72fee4b07..e5c9f783f6b 100644 --- a/tests/ui/filter_map_bool_then.fixed +++ b/tests/ui/filter_map_bool_then.fixed @@ -4,6 +4,7 @@ clippy::clone_on_copy, clippy::map_identity, clippy::unnecessary_lazy_evaluations, + clippy::unnecessary_filter_map, unused )] #![warn(clippy::filter_map_bool_then)] @@ -29,9 +30,18 @@ fn main() { .copied() .filter(|&i| i != 1000) .filter(|&i| (i.clone() % 2 == 0)).map(|i| i + 1); + // Despite this is non-copy, `is_copy` still returns true (at least now) because it's `&NonCopy`, + // and any `&` is `Copy`. So since we can dereference it in `filter` (since it's then `&&NonCopy`), + // we can lint this and still get the same input type. + // See: <https://doc.rust-lang.org/std/primitive.reference.html#trait-implementations-1> + let v = vec![NonCopy, NonCopy]; + v.clone().iter().filter(|&i| (i == &NonCopy)).map(|i| i); // Do not lint let v = vec![NonCopy, NonCopy]; - v.clone().iter().filter_map(|i| (i == &NonCopy).then(|| i)); + v.clone().into_iter().filter_map(|i| (i == NonCopy).then(|| i)); + // `&mut` is `!Copy`. + let v = vec![NonCopy, NonCopy]; + v.clone().iter_mut().filter_map(|i| (i == &mut NonCopy).then(|| i)); external! { let v = vec![1, 2, 3, 4, 5, 6]; v.clone().into_iter().filter_map(|i| (i % 2 == 0).then(|| i + 1)); @@ -42,3 +52,7 @@ fn main() { v.clone().into_iter().filter_map(|i| (i % 2 == 0).then(|| i + 1)); } } + +fn issue11309<'a>(iter: impl Iterator<Item = (&'a str, &'a str)>) -> Vec<&'a str> { + iter.filter_map(|(_, s): (&str, _)| Some(s)).collect() +} diff --git a/tests/ui/filter_map_bool_then.rs b/tests/ui/filter_map_bool_then.rs index 38a04e57de4..7c9b99df78c 100644 --- a/tests/ui/filter_map_bool_then.rs +++ b/tests/ui/filter_map_bool_then.rs @@ -4,6 +4,7 @@ clippy::clone_on_copy, clippy::map_identity, clippy::unnecessary_lazy_evaluations, + clippy::unnecessary_filter_map, unused )] #![warn(clippy::filter_map_bool_then)] @@ -29,9 +30,18 @@ fn main() { .copied() .filter(|&i| i != 1000) .filter_map(|i| (i.clone() % 2 == 0).then(|| i + 1)); - // Do not lint + // Despite this is non-copy, `is_copy` still returns true (at least now) because it's `&NonCopy`, + // and any `&` is `Copy`. So since we can dereference it in `filter` (since it's then `&&NonCopy`), + // we can lint this and still get the same input type. + // See: <https://doc.rust-lang.org/std/primitive.reference.html#trait-implementations-1> let v = vec![NonCopy, NonCopy]; v.clone().iter().filter_map(|i| (i == &NonCopy).then(|| i)); + // Do not lint + let v = vec![NonCopy, NonCopy]; + v.clone().into_iter().filter_map(|i| (i == NonCopy).then(|| i)); + // `&mut` is `!Copy`. + let v = vec![NonCopy, NonCopy]; + v.clone().iter_mut().filter_map(|i| (i == &mut NonCopy).then(|| i)); external! { let v = vec![1, 2, 3, 4, 5, 6]; v.clone().into_iter().filter_map(|i| (i % 2 == 0).then(|| i + 1)); @@ -42,3 +52,7 @@ fn main() { v.clone().into_iter().filter_map(|i| (i % 2 == 0).then(|| i + 1)); } } + +fn issue11309<'a>(iter: impl Iterator<Item = (&'a str, &'a str)>) -> Vec<&'a str> { + iter.filter_map(|(_, s): (&str, _)| Some(s)).collect() +} diff --git a/tests/ui/filter_map_bool_then.stderr b/tests/ui/filter_map_bool_then.stderr index b411cd83dfd..fffa5252e5f 100644 --- a/tests/ui/filter_map_bool_then.stderr +++ b/tests/ui/filter_map_bool_then.stderr @@ -1,5 +1,5 @@ error: usage of `bool::then` in `filter_map` - --> $DIR/filter_map_bool_then.rs:19:22 + --> $DIR/filter_map_bool_then.rs:20:22 | LL | v.clone().iter().filter_map(|i| (i % 2 == 0).then(|| i + 1)); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `filter` then `map` instead: `filter(|&i| (i % 2 == 0)).map(|i| i + 1)` @@ -7,28 +7,34 @@ LL | v.clone().iter().filter_map(|i| (i % 2 == 0).then(|| i + 1)); = note: `-D clippy::filter-map-bool-then` implied by `-D warnings` error: usage of `bool::then` in `filter_map` - --> $DIR/filter_map_bool_then.rs:20:27 + --> $DIR/filter_map_bool_then.rs:21:27 | LL | v.clone().into_iter().filter_map(|i| (i % 2 == 0).then(|| i + 1)); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `filter` then `map` instead: `filter(|&i| (i % 2 == 0)).map(|i| i + 1)` error: usage of `bool::then` in `filter_map` - --> $DIR/filter_map_bool_then.rs:23:10 + --> $DIR/filter_map_bool_then.rs:24:10 | LL | .filter_map(|i| -> Option<_> { (i % 2 == 0).then(|| i + 1) }); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `filter` then `map` instead: `filter(|&i| (i % 2 == 0)).map(|i| i + 1)` error: usage of `bool::then` in `filter_map` - --> $DIR/filter_map_bool_then.rs:27:10 + --> $DIR/filter_map_bool_then.rs:28:10 | LL | .filter_map(|i| (i % 2 == 0).then(|| i + 1)); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `filter` then `map` instead: `filter(|&i| (i % 2 == 0)).map(|i| i + 1)` error: usage of `bool::then` in `filter_map` - --> $DIR/filter_map_bool_then.rs:31:10 + --> $DIR/filter_map_bool_then.rs:32:10 | LL | .filter_map(|i| (i.clone() % 2 == 0).then(|| i + 1)); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `filter` then `map` instead: `filter(|&i| (i.clone() % 2 == 0)).map(|i| i + 1)` -error: aborting due to 5 previous errors +error: usage of `bool::then` in `filter_map` + --> $DIR/filter_map_bool_then.rs:38:22 + | +LL | v.clone().iter().filter_map(|i| (i == &NonCopy).then(|| i)); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `filter` then `map` instead: `filter(|&i| (i == &NonCopy)).map(|i| i)` + +error: aborting due to 6 previous errors |