diff --git a/clippy_lints/src/loops/mod.rs b/clippy_lints/src/loops/mod.rs index 01c36b8cb12f..5b46922c5ab3 100644 --- a/clippy_lints/src/loops/mod.rs +++ b/clippy_lints/src/loops/mod.rs @@ -876,6 +876,15 @@ impl<'tcx> LateLintPass<'tcx> for Loops { missing_spin_loop::check(cx, condition, body); manual_while_let_some::check(cx, condition, body, span); } + + if let ExprKind::MethodCall(path, recv, [arg], _) = expr.kind + && matches!( + path.ident.name, + sym::all | sym::any | sym::filter_map | sym::find_map | sym::flat_map | sym::for_each | sym::map + ) + { + unused_enumerate_index::check_method(cx, expr, recv, arg); + } } } @@ -904,7 +913,7 @@ impl Loops { same_item_push::check(cx, pat, arg, body, expr, self.msrv); manual_flatten::check(cx, pat, arg, body, span, self.msrv); manual_find::check(cx, pat, arg, body, span, expr); - unused_enumerate_index::check(cx, pat, arg, body); + unused_enumerate_index::check_loop(cx, arg, pat, None, body); char_indices_as_byte_indices::check(cx, pat, arg, body); } diff --git a/clippy_lints/src/loops/unused_enumerate_index.rs b/clippy_lints/src/loops/unused_enumerate_index.rs index 51e21aa9734e..d4cbde53afb3 100644 --- a/clippy_lints/src/loops/unused_enumerate_index.rs +++ b/clippy_lints/src/loops/unused_enumerate_index.rs @@ -1,40 +1,84 @@ use super::UNUSED_ENUMERATE_INDEX; use clippy_utils::diagnostics::span_lint_and_then; -use clippy_utils::source::snippet; -use clippy_utils::{pat_is_wild, sugg}; +use clippy_utils::source::{SpanRangeExt, walk_span_to_context}; +use clippy_utils::ty::is_type_diagnostic_item; +use clippy_utils::{expr_or_init, is_lint_allowed, is_trait_method, pat_is_wild}; use rustc_errors::Applicability; -use rustc_hir::def::DefKind; -use rustc_hir::{Expr, ExprKind, Pat, PatKind}; +use rustc_hir::{Expr, ExprKind, Pat, PatKind, TyKind}; use rustc_lint::LateContext; -use rustc_middle::ty; -use rustc_span::sym; +use rustc_span::{Span, SyntaxContext, sym}; -/// Checks for the `UNUSED_ENUMERATE_INDEX` lint. -/// -/// The lint is also partially implemented in `clippy_lints/src/methods/unused_enumerate_index.rs`. -pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, pat: &Pat<'tcx>, arg: &Expr<'_>, body: &'tcx Expr<'tcx>) { - if let PatKind::Tuple([index, elem], _) = pat.kind - && let ExprKind::MethodCall(_method, self_arg, [], _) = arg.kind - && let ty = cx.typeck_results().expr_ty(arg) - && pat_is_wild(cx, &index.kind, body) - && let ty::Adt(base, _) = *ty.kind() - && cx.tcx.is_diagnostic_item(sym::Enumerate, base.did()) - && let Some((DefKind::AssocFn, call_id)) = cx.typeck_results().type_dependent_def(arg.hir_id) - && cx.tcx.is_diagnostic_item(sym::enumerate_method, call_id) +pub(super) fn check_method<'tcx>( + cx: &LateContext<'tcx>, + e: &'tcx Expr<'tcx>, + recv: &'tcx Expr<'tcx>, + arg: &'tcx Expr<'tcx>, +) { + if let ExprKind::Closure(closure) = arg.kind + && let body = cx.tcx.hir_body(closure.body) + && let [param] = body.params + && is_trait_method(cx, e, sym::Iterator) + && let [input] = closure.fn_decl.inputs + && !arg.span.from_expansion() + && !input.span.from_expansion() + && !recv.span.from_expansion() + && !param.span.from_expansion() { + let ty_spans = if let TyKind::Tup([_, inner]) = input.kind { + let Some(inner) = walk_span_to_context(inner.span, SyntaxContext::root()) else { + return; + }; + Some((input.span, inner)) + } else { + None + }; + check_loop(cx, recv, param.pat, ty_spans, body.value); + } +} + +pub(super) fn check_loop<'tcx>( + cx: &LateContext<'tcx>, + e: &'tcx Expr<'tcx>, + pat: &'tcx Pat<'tcx>, + ty_spans: Option<(Span, Span)>, + body: &'tcx Expr<'tcx>, +) { + if let PatKind::Tuple([idx_pat, inner_pat], _) = pat.kind + && is_type_diagnostic_item(cx, cx.typeck_results().expr_ty(e), sym::Enumerate) + && pat_is_wild(cx, &idx_pat.kind, body) + && let enumerate_call = expr_or_init(cx, e) + && let ExprKind::MethodCall(_, _, [], enumerate_span) = enumerate_call.kind + && let Some(enumerate_id) = cx.typeck_results().type_dependent_def_id(enumerate_call.hir_id) + && cx.tcx.is_diagnostic_item(sym::enumerate_method, enumerate_id) + && !is_lint_allowed(cx, UNUSED_ENUMERATE_INDEX, enumerate_call.hir_id) + && !enumerate_call.span.from_expansion() + && !pat.span.from_expansion() + && !idx_pat.span.from_expansion() + && !inner_pat.span.from_expansion() + && let Some(enumerate_range) = enumerate_span.map_range(cx, |_, text, range| { + text.get(..range.start)? + .ends_with('.') + .then_some(range.start - 1..range.end) + }) + { + let enumerate_span = Span::new(enumerate_range.start, enumerate_range.end, SyntaxContext::root(), None); span_lint_and_then( cx, UNUSED_ENUMERATE_INDEX, - arg.span, + enumerate_span, "you seem to use `.enumerate()` and immediately discard the index", |diag| { - let base_iter = sugg::Sugg::hir(cx, self_arg, "base iter"); + let mut spans = Vec::with_capacity(5); + spans.push((enumerate_span, String::new())); + spans.push((pat.span.with_hi(inner_pat.span.lo()), String::new())); + spans.push((pat.span.with_lo(inner_pat.span.hi()), String::new())); + if let Some((outer, inner)) = ty_spans { + spans.push((outer.with_hi(inner.lo()), String::new())); + spans.push((outer.with_lo(inner.hi()), String::new())); + } diag.multipart_suggestion( "remove the `.enumerate()` call", - vec![ - (pat.span, snippet(cx, elem.span, "..").into_owned()), - (arg.span, base_iter.to_string()), - ], + spans, Applicability::MachineApplicable, ); }, diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs index f2dabdd34387..c868753c62ec 100644 --- a/clippy_lints/src/methods/mod.rs +++ b/clippy_lints/src/methods/mod.rs @@ -134,7 +134,6 @@ mod unnecessary_min_or_max; mod unnecessary_result_map_or_else; mod unnecessary_sort_by; mod unnecessary_to_owned; -mod unused_enumerate_index; mod unwrap_expect_used; mod useless_asref; mod useless_nonzero_new_unchecked; @@ -4956,7 +4955,6 @@ impl Methods { zst_offset::check(cx, expr, recv); }, (sym::all, [arg]) => { - unused_enumerate_index::check(cx, expr, recv, arg); needless_character_iteration::check(cx, expr, recv, arg, true); match method_call(recv) { Some((sym::cloned, recv2, [], _, _)) => { @@ -4986,7 +4984,6 @@ impl Methods { } }, (sym::any, [arg]) => { - unused_enumerate_index::check(cx, expr, recv, arg); needless_character_iteration::check(cx, expr, recv, arg, false); match method_call(recv) { Some((sym::cloned, recv2, [], _, _)) => iter_overeager_cloned::check( @@ -5133,17 +5130,14 @@ impl Methods { } }, (sym::filter_map, [arg]) => { - unused_enumerate_index::check(cx, expr, recv, arg); unnecessary_filter_map::check(cx, expr, arg, name); filter_map_bool_then::check(cx, expr, arg, call_span); filter_map_identity::check(cx, expr, arg, span); }, (sym::find_map, [arg]) => { - unused_enumerate_index::check(cx, expr, recv, arg); unnecessary_filter_map::check(cx, expr, arg, name); }, (sym::flat_map, [arg]) => { - unused_enumerate_index::check(cx, expr, recv, arg); flat_map_identity::check(cx, expr, arg, span); flat_map_option::check(cx, expr, arg, span); }, @@ -5165,20 +5159,17 @@ impl Methods { manual_try_fold::check(cx, expr, init, acc, call_span, self.msrv); unnecessary_fold::check(cx, expr, init, acc, span); }, - (sym::for_each, [arg]) => { - unused_enumerate_index::check(cx, expr, recv, arg); - match method_call(recv) { - Some((sym::inspect, _, [_], span2, _)) => inspect_for_each::check(cx, expr, span2), - Some((sym::cloned, recv2, [], _, _)) => iter_overeager_cloned::check( - cx, - expr, - recv, - recv2, - iter_overeager_cloned::Op::NeedlessMove(arg), - false, - ), - _ => {}, - } + (sym::for_each, [arg]) => match method_call(recv) { + Some((sym::inspect, _, [_], span2, _)) => inspect_for_each::check(cx, expr, span2), + Some((sym::cloned, recv2, [], _, _)) => iter_overeager_cloned::check( + cx, + expr, + recv, + recv2, + iter_overeager_cloned::Op::NeedlessMove(arg), + false, + ), + _ => {}, }, (sym::get, [arg]) => { get_first::check(cx, expr, recv, arg); @@ -5239,7 +5230,6 @@ impl Methods { }, (name @ (sym::map | sym::map_err), [m_arg]) => { if name == sym::map { - unused_enumerate_index::check(cx, expr, recv, m_arg); map_clone::check(cx, expr, recv, m_arg, self.msrv); map_with_unused_argument_over_ranges::check(cx, expr, recv, m_arg, self.msrv, span); manual_is_variant_and::check_map(cx, expr); diff --git a/clippy_lints/src/methods/unused_enumerate_index.rs b/clippy_lints/src/methods/unused_enumerate_index.rs deleted file mode 100644 index af466fe091c2..000000000000 --- a/clippy_lints/src/methods/unused_enumerate_index.rs +++ /dev/null @@ -1,139 +0,0 @@ -use clippy_utils::diagnostics::span_lint_hir_and_then; -use clippy_utils::source::{SpanRangeExt, snippet}; -use clippy_utils::{expr_or_init, is_trait_method, pat_is_wild}; -use rustc_errors::Applicability; -use rustc_hir::{Expr, ExprKind, FnDecl, PatKind, TyKind}; -use rustc_lint::LateContext; -use rustc_middle::ty::AdtDef; -use rustc_span::{Span, sym}; - -use crate::loops::UNUSED_ENUMERATE_INDEX; - -/// Check for the `UNUSED_ENUMERATE_INDEX` lint outside of loops. -/// -/// The lint is declared in `clippy_lints/src/loops/mod.rs`. There, the following pattern is -/// checked: -/// ```ignore -/// for (_, x) in some_iter.enumerate() { -/// // Index is ignored -/// } -/// ``` -/// -/// This `check` function checks for chained method calls constructs where we can detect that the -/// index is unused. Currently, this checks only for the following patterns: -/// ```ignore -/// some_iter.enumerate().map_function(|(_, x)| ..) -/// let x = some_iter.enumerate(); -/// x.map_function(|(_, x)| ..) -/// ``` -/// where `map_function` is one of `all`, `any`, `filter_map`, `find_map`, `flat_map`, `for_each` or -/// `map`. -/// -/// # Preconditions -/// This function must be called not on the `enumerate` call expression itself, but on any of the -/// map functions listed above. It will ensure that `recv` is a `std::iter::Enumerate` instance and -/// that the method call is one of the `std::iter::Iterator` trait. -/// -/// * `call_expr`: The map function call expression -/// * `recv`: The receiver of the call -/// * `closure_arg`: The argument to the map function call containing the closure/function to apply -pub(super) fn check(cx: &LateContext<'_>, call_expr: &Expr<'_>, recv: &Expr<'_>, closure_arg: &Expr<'_>) { - let recv_ty = cx.typeck_results().expr_ty(recv); - if let Some(recv_ty_defid) = recv_ty.ty_adt_def().map(AdtDef::did) - // If we call a method on a `std::iter::Enumerate` instance - && cx.tcx.is_diagnostic_item(sym::Enumerate, recv_ty_defid) - // If we are calling a method of the `Iterator` trait - && is_trait_method(cx, call_expr, sym::Iterator) - // And the map argument is a closure - && let ExprKind::Closure(closure) = closure_arg.kind - && let closure_body = cx.tcx.hir_body(closure.body) - // And that closure has one argument ... - && let [closure_param] = closure_body.params - // .. which is a tuple of 2 elements - && let PatKind::Tuple([index, elem], ..) = closure_param.pat.kind - // And that the first element (the index) is either `_` or unused in the body - && pat_is_wild(cx, &index.kind, closure_body) - // Try to find the initializer for `recv`. This is needed in case `recv` is a local_binding. In the - // first example below, `expr_or_init` would return `recv`. - // ``` - // iter.enumerate().map(|(_, x)| x) - // ^^^^^^^^^^^^^^^^ `recv`, a call to `std::iter::Iterator::enumerate` - // - // let binding = iter.enumerate(); - // ^^^^^^^^^^^^^^^^ `recv_init_expr` - // binding.map(|(_, x)| x) - // ^^^^^^^ `recv`, not a call to `std::iter::Iterator::enumerate` - // ``` - && let recv_init_expr = expr_or_init(cx, recv) - // Make sure the initializer is a method call. It may be that the `Enumerate` comes from something - // that we cannot control. - // This would for instance happen with: - // ``` - // external_lib::some_function_returning_enumerate().map(|(_, x)| x) - // ``` - && let ExprKind::MethodCall(_, enumerate_recv, _, enumerate_span) = recv_init_expr.kind - && let Some(enumerate_defid) = cx.typeck_results().type_dependent_def_id(recv_init_expr.hir_id) - // Make sure the method call is `std::iter::Iterator::enumerate`. - && cx.tcx.is_diagnostic_item(sym::enumerate_method, enumerate_defid) - { - // Check if the tuple type was explicit. It may be the type system _needs_ the type of the element - // that would be explicitly in the closure. - let new_closure_param = match find_elem_explicit_type_span(closure.fn_decl) { - // We have an explicit type. Get its snippet, that of the binding name, and do `binding: ty`. - // Fallback to `..` if we fail getting either snippet. - Some(ty_span) => elem - .span - .get_source_text(cx) - .and_then(|binding_name| { - ty_span - .get_source_text(cx) - .map(|ty_name| format!("{binding_name}: {ty_name}")) - }) - .unwrap_or_else(|| "..".to_string()), - // Otherwise, we have no explicit type. We can replace with the binding name of the element. - None => snippet(cx, elem.span, "..").into_owned(), - }; - - // Suggest removing the tuple from the closure and the preceding call to `enumerate`, whose span we - // can get from the `MethodCall`. - span_lint_hir_and_then( - cx, - UNUSED_ENUMERATE_INDEX, - recv_init_expr.hir_id, - enumerate_span, - "you seem to use `.enumerate()` and immediately discard the index", - |diag| { - diag.multipart_suggestion( - "remove the `.enumerate()` call", - vec![ - (closure_param.span, new_closure_param), - ( - enumerate_span.with_lo(enumerate_recv.span.source_callsite().hi()), - String::new(), - ), - ], - Applicability::MachineApplicable, - ); - }, - ); - } -} - -/// Find the span of the explicit type of the element. -/// -/// # Returns -/// If the tuple argument: -/// * Has no explicit type, returns `None` -/// * Has an explicit tuple type with an implicit element type (`(usize, _)`), returns `None` -/// * Has an explicit tuple type with an explicit element type (`(_, i32)`), returns the span for -/// the element type. -fn find_elem_explicit_type_span(fn_decl: &FnDecl<'_>) -> Option { - if let [tuple_ty] = fn_decl.inputs - && let TyKind::Tup([_idx_ty, elem_ty]) = tuple_ty.kind - && !matches!(elem_ty.kind, TyKind::Err(..) | TyKind::Infer(())) - { - Some(elem_ty.span) - } else { - None - } -} diff --git a/tests/ui/unused_enumerate_index.stderr b/tests/ui/unused_enumerate_index.stderr index 14d1d20a66e4..c742cc8a85ba 100644 --- a/tests/ui/unused_enumerate_index.stderr +++ b/tests/ui/unused_enumerate_index.stderr @@ -1,8 +1,8 @@ error: you seem to use `.enumerate()` and immediately discard the index - --> tests/ui/unused_enumerate_index.rs:12:19 + --> tests/ui/unused_enumerate_index.rs:12:27 | LL | for (_, x) in v.iter().enumerate() { - | ^^^^^^^^^^^^^^^^^^^^ + | ^^^^^^^^^^^^ | = note: `-D clippy::unused-enumerate-index` implied by `-D warnings` = help: to override `-D warnings` add `#[allow(clippy::unused_enumerate_index)]` @@ -13,10 +13,10 @@ LL + for x in v.iter() { | error: you seem to use `.enumerate()` and immediately discard the index - --> tests/ui/unused_enumerate_index.rs:60:19 + --> tests/ui/unused_enumerate_index.rs:60:24 | LL | for (_, x) in dummy.enumerate() { - | ^^^^^^^^^^^^^^^^^ + | ^^^^^^^^^^^^ | help: remove the `.enumerate()` call | @@ -25,10 +25,10 @@ LL + for x in dummy { | error: you seem to use `.enumerate()` and immediately discard the index - --> tests/ui/unused_enumerate_index.rs:65:39 + --> tests/ui/unused_enumerate_index.rs:65:38 | LL | let _ = vec![1, 2, 3].into_iter().enumerate().map(|(_, x)| println!("{x}")); - | ^^^^^^^^^^^ + | ^^^^^^^^^^^^ | help: remove the `.enumerate()` call | @@ -37,10 +37,10 @@ LL + let _ = vec![1, 2, 3].into_iter().map(|x| println!("{x}")); | error: you seem to use `.enumerate()` and immediately discard the index - --> tests/ui/unused_enumerate_index.rs:68:39 + --> tests/ui/unused_enumerate_index.rs:68:38 | LL | let p = vec![1, 2, 3].into_iter().enumerate(); - | ^^^^^^^^^^^ + | ^^^^^^^^^^^^ | help: remove the `.enumerate()` call | @@ -50,10 +50,10 @@ LL ~ p.map(|x| println!("{x}")); | error: you seem to use `.enumerate()` and immediately discard the index - --> tests/ui/unused_enumerate_index.rs:90:17 + --> tests/ui/unused_enumerate_index.rs:90:16 | LL | _ = mac2!().enumerate().map(|(_, _v)| {}); - | ^^^^^^^^^^^ + | ^^^^^^^^^^^^ | help: remove the `.enumerate()` call | @@ -62,10 +62,10 @@ LL + _ = mac2!().map(|_v| {}); | error: you seem to use `.enumerate()` and immediately discard the index - --> tests/ui/unused_enumerate_index.rs:99:39 + --> tests/ui/unused_enumerate_index.rs:99:38 | LL | let v = [1, 2, 3].iter().copied().enumerate(); - | ^^^^^^^^^^^ + | ^^^^^^^^^^^^ | help: remove the `.enumerate()` call | @@ -75,10 +75,10 @@ LL ~ let x = v.map(|x: i32| x).sum::(); | error: you seem to use `.enumerate()` and immediately discard the index - --> tests/ui/unused_enumerate_index.rs:105:39 + --> tests/ui/unused_enumerate_index.rs:105:38 | LL | let v = [1, 2, 3].iter().copied().enumerate(); - | ^^^^^^^^^^^ + | ^^^^^^^^^^^^ | help: remove the `.enumerate()` call | @@ -88,10 +88,10 @@ LL ~ let x = v.map(|x: i32| x).sum::(); | error: you seem to use `.enumerate()` and immediately discard the index - --> tests/ui/unused_enumerate_index.rs:110:39 + --> tests/ui/unused_enumerate_index.rs:110:38 | LL | let v = [1, 2, 3].iter().copied().enumerate(); - | ^^^^^^^^^^^ + | ^^^^^^^^^^^^ | help: remove the `.enumerate()` call |