diff options
Diffstat (limited to 'clippy_lints/src/methods')
| -rw-r--r-- | clippy_lints/src/methods/expect_fun_call.rs | 2 | ||||
| -rw-r--r-- | clippy_lints/src/methods/is_empty.rs | 49 | ||||
| -rw-r--r-- | clippy_lints/src/methods/iter_nth.rs | 49 | ||||
| -rw-r--r-- | clippy_lints/src/methods/map_clone.rs | 11 | ||||
| -rw-r--r-- | clippy_lints/src/methods/mod.rs | 143 | ||||
| -rw-r--r-- | clippy_lints/src/methods/no_effect_replace.rs | 1 | ||||
| -rw-r--r-- | clippy_lints/src/methods/unnecessary_sort_by.rs | 3 | ||||
| -rw-r--r-- | clippy_lints/src/methods/unused_enumerate_index.rs | 135 |
8 files changed, 318 insertions, 75 deletions
diff --git a/clippy_lints/src/methods/expect_fun_call.rs b/clippy_lints/src/methods/expect_fun_call.rs index e2c2997594a..4d8fb217f7f 100644 --- a/clippy_lints/src/methods/expect_fun_call.rs +++ b/clippy_lints/src/methods/expect_fun_call.rs @@ -91,7 +91,7 @@ pub(super) fn check<'tcx>( }, hir::ExprKind::Path(ref p) => matches!( cx.qpath_res(p, arg.hir_id), - hir::def::Res::Def(hir::def::DefKind::Const | hir::def::DefKind::Static{..}, _) + hir::def::Res::Def(hir::def::DefKind::Const | hir::def::DefKind::Static { .. }, _) ), _ => false, } diff --git a/clippy_lints/src/methods/is_empty.rs b/clippy_lints/src/methods/is_empty.rs new file mode 100644 index 00000000000..7fe66062251 --- /dev/null +++ b/clippy_lints/src/methods/is_empty.rs @@ -0,0 +1,49 @@ +use clippy_utils::consts::constant_is_empty; +use clippy_utils::diagnostics::span_lint; +use clippy_utils::{find_binding_init, path_to_local}; +use rustc_hir::{Expr, HirId}; +use rustc_lint::{LateContext, LintContext}; +use rustc_middle::lint::in_external_macro; +use rustc_span::sym; + +use super::CONST_IS_EMPTY; + +/// Expression whose initialization depend on a constant conditioned by a `#[cfg(…)]` directive will +/// not trigger the lint. +pub(super) fn check(cx: &LateContext<'_>, expr: &'_ Expr<'_>, receiver: &Expr<'_>) { + if in_external_macro(cx.sess(), expr.span) || !receiver.span.eq_ctxt(expr.span) { + return; + } + let init_expr = expr_or_init(cx, receiver); + if !receiver.span.eq_ctxt(init_expr.span) { + return; + } + if let Some(init_is_empty) = constant_is_empty(cx, init_expr) { + span_lint( + cx, + CONST_IS_EMPTY, + expr.span, + &format!("this expression always evaluates to {init_is_empty:?}"), + ); + } +} + +fn is_under_cfg(cx: &LateContext<'_>, id: HirId) -> bool { + cx.tcx + .hir() + .parent_id_iter(id) + .any(|id| cx.tcx.hir().attrs(id).iter().any(|attr| attr.has_name(sym::cfg))) +} + +/// Similar to [`clippy_utils::expr_or_init`], but does not go up the chain if the initialization +/// value depends on a `#[cfg(…)]` directive. +fn expr_or_init<'a, 'b, 'tcx: 'b>(cx: &LateContext<'tcx>, mut expr: &'a Expr<'b>) -> &'a Expr<'b> { + while let Some(init) = path_to_local(expr) + .and_then(|id| find_binding_init(cx, id)) + .filter(|init| cx.typeck_results().expr_adjustments(init).is_empty()) + .filter(|init| !is_under_cfg(cx, init.hir_id)) + { + expr = init; + } + expr +} diff --git a/clippy_lints/src/methods/iter_nth.rs b/clippy_lints/src/methods/iter_nth.rs index 12104310405..5b0b70b4b96 100644 --- a/clippy_lints/src/methods/iter_nth.rs +++ b/clippy_lints/src/methods/iter_nth.rs @@ -1,10 +1,10 @@ -use super::utils::derefs_to_slice; -use crate::methods::iter_nth_zero; -use clippy_utils::diagnostics::span_lint_and_help; -use clippy_utils::ty::is_type_diagnostic_item; +use clippy_utils::diagnostics::span_lint_and_then; +use clippy_utils::ty::get_type_diagnostic_name; +use rustc_errors::Applicability; use rustc_hir as hir; use rustc_lint::LateContext; use rustc_span::symbol::sym; +use rustc_span::Span; use super::ITER_NTH; @@ -12,28 +12,33 @@ pub(super) fn check<'tcx>( cx: &LateContext<'tcx>, expr: &hir::Expr<'_>, iter_recv: &'tcx hir::Expr<'tcx>, - nth_recv: &hir::Expr<'_>, - nth_arg: &hir::Expr<'_>, - is_mut: bool, -) { - let mut_str = if is_mut { "_mut" } else { "" }; - let caller_type = if derefs_to_slice(cx, iter_recv, cx.typeck_results().expr_ty(iter_recv)).is_some() { - "slice" - } else if is_type_diagnostic_item(cx, cx.typeck_results().expr_ty(iter_recv), sym::Vec) { - "`Vec`" - } else if is_type_diagnostic_item(cx, cx.typeck_results().expr_ty(iter_recv), sym::VecDeque) { - "`VecDeque`" - } else { - iter_nth_zero::check(cx, expr, nth_recv, nth_arg); - return; // caller is not a type that we want to lint + iter_method: &str, + iter_span: Span, + nth_span: Span, +) -> bool { + let caller_type = match get_type_diagnostic_name(cx, cx.typeck_results().expr_ty(iter_recv).peel_refs()) { + Some(sym::Vec) => "`Vec`", + Some(sym::VecDeque) => "`VecDeque`", + _ if cx.typeck_results().expr_ty_adjusted(iter_recv).peel_refs().is_slice() => "slice", + // caller is not a type that we want to lint + _ => return false, }; - span_lint_and_help( + span_lint_and_then( cx, ITER_NTH, expr.span, - &format!("called `.iter{mut_str}().nth()` on a {caller_type}"), - None, - &format!("calling `.get{mut_str}()` is both faster and more readable"), + &format!("called `.{iter_method}().nth()` on a {caller_type}"), + |diag| { + let get_method = if iter_method == "iter_mut" { "get_mut" } else { "get" }; + diag.span_suggestion_verbose( + iter_span.to(nth_span), + format!("`{get_method}` is equivalent but more concise"), + get_method, + Applicability::MachineApplicable, + ); + }, ); + + true } diff --git a/clippy_lints/src/methods/map_clone.rs b/clippy_lints/src/methods/map_clone.rs index 27e17b43b01..c3c7a3a0033 100644 --- a/clippy_lints/src/methods/map_clone.rs +++ b/clippy_lints/src/methods/map_clone.rs @@ -86,8 +86,11 @@ pub(super) fn check(cx: &LateContext<'_>, e: &hir::Expr<'_>, recv: &hir::Expr<'_ } } }, - hir::ExprKind::Call(call, [_]) => { - if let hir::ExprKind::Path(qpath) = call.kind { + hir::ExprKind::Call(call, args) => { + if let hir::ExprKind::Path(qpath) = call.kind + && let [arg] = args + && ident_eq(name, arg) + { handle_path(cx, call, &qpath, e, recv); } }, @@ -118,7 +121,9 @@ fn handle_path( if let ty::Adt(_, args) = cx.typeck_results().expr_ty(recv).kind() && let args = args.as_slice() && let Some(ty) = args.iter().find_map(|generic_arg| generic_arg.as_type()) - && ty.is_ref() + && let ty::Ref(_, ty, Mutability::Not) = ty.kind() + && let ty::FnDef(_, lst) = cx.typeck_results().expr_ty(arg).kind() + && lst.iter().all(|l| l.as_type() == Some(*ty)) { lint_path(cx, e.span, recv.span, is_copy(cx, ty.peel_refs())); } diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs index 8a24ccea3a1..b6c474212cd 100644 --- a/clippy_lints/src/methods/mod.rs +++ b/clippy_lints/src/methods/mod.rs @@ -36,6 +36,7 @@ mod inefficient_to_string; mod inspect_for_each; mod into_iter_on_ref; mod is_digit_ascii_radix; +mod is_empty; mod iter_cloned_collect; mod iter_count; mod iter_filter; @@ -118,6 +119,7 @@ mod unnecessary_literal_unwrap; 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 utils; @@ -1235,12 +1237,11 @@ declare_clippy_lint! { declare_clippy_lint! { /// ### What it does - /// Checks for usage of `.iter().nth()` (and the related - /// `.iter_mut().nth()`) on standard library types with *O*(1) element access. + /// Checks for usage of `.iter().nth()`/`.iter_mut().nth()` on standard library types that have + /// equivalent `.get()`/`.get_mut()` methods. /// /// ### Why is this bad? - /// `.get()` and `.get_mut()` are more efficient and more - /// readable. + /// `.get()` and `.get_mut()` are equivalent but more concise. /// /// ### Example /// ```no_run @@ -1256,7 +1257,7 @@ declare_clippy_lint! { /// ``` #[clippy::version = "pre 1.29.0"] pub ITER_NTH, - perf, + style, "using `.iter().nth()` on a standard library type with O(1) element access" } @@ -2848,7 +2849,7 @@ declare_clippy_lint! { /// the file is created from scratch, or ensure `truncate` is /// called so that the truncation behaviour is explicit. `truncate(true)` /// will ensure the file is entirely overwritten with new data, whereas - /// `truncate(false)` will explicitely keep the default behavior. + /// `truncate(false)` will explicitly keep the default behavior. /// /// ### Example /// ```rust,no_run @@ -2862,7 +2863,7 @@ declare_clippy_lint! { /// /// OpenOptions::new().create(true).truncate(true); /// ``` - #[clippy::version = "1.75.0"] + #[clippy::version = "1.77.0"] pub SUSPICIOUS_OPEN_OPTIONS, suspicious, "suspicious combination of options for opening a file" @@ -3182,8 +3183,8 @@ declare_clippy_lint! { declare_clippy_lint! { /// ### What it does /// - /// Checks an argument of `seek` method of `Seek` trait - /// and if it start seek from `SeekFrom::Current(0)`, suggests `stream_position` instead. + /// Checks if the `seek` method of the `Seek` trait is called with `SeekFrom::Current(0)`, + /// and if it is, suggests using `stream_position` instead. /// /// ### Why is this bad? /// @@ -3590,7 +3591,7 @@ declare_clippy_lint! { /// ``` #[clippy::version = "1.73.0"] pub READONLY_WRITE_LOCK, - nursery, + perf, "acquiring a write lock when a read lock would work" } @@ -3816,7 +3817,7 @@ declare_clippy_lint! { /// ```no_run /// let _ = std::iter::empty::<Result<i32, ()>>().flatten(); /// ``` - #[clippy::version = "1.76.0"] + #[clippy::version = "1.77.0"] pub RESULT_FILTER_MAP, complexity, "filtering `Result` for `Ok` then force-unwrapping, which can be one type-safe operation" @@ -3825,7 +3826,7 @@ declare_clippy_lint! { declare_clippy_lint! { /// ### What it does /// Checks for usage of `.filter(Option::is_some)` that may be replaced with a `.flatten()` call. - /// This lint will require additional changes to the follow-up calls as it appects the type. + /// This lint will require additional changes to the follow-up calls as it affects the type. /// /// ### Why is this bad? /// This pattern is often followed by manual unwrapping of the `Option`. The simplification @@ -3842,7 +3843,7 @@ declare_clippy_lint! { /// // example code which does not raise clippy warning /// vec![Some(1)].into_iter().flatten(); /// ``` - #[clippy::version = "1.76.0"] + #[clippy::version = "1.77.0"] pub ITER_FILTER_IS_SOME, pedantic, "filtering an iterator over `Option`s for `Some` can be achieved with `flatten`" @@ -3851,7 +3852,7 @@ declare_clippy_lint! { declare_clippy_lint! { /// ### What it does /// Checks for usage of `.filter(Result::is_ok)` that may be replaced with a `.flatten()` call. - /// This lint will require additional changes to the follow-up calls as it appects the type. + /// This lint will require additional changes to the follow-up calls as it affects the type. /// /// ### Why is this bad? /// This pattern is often followed by manual unwrapping of `Result`. The simplification @@ -3868,7 +3869,7 @@ declare_clippy_lint! { /// // example code which does not raise clippy warning /// vec![Ok::<i32, String>(1)].into_iter().flatten(); /// ``` - #[clippy::version = "1.76.0"] + #[clippy::version = "1.77.0"] pub ITER_FILTER_IS_OK, pedantic, "filtering an iterator over `Result`s for `Ok` can be achieved with `flatten`" @@ -3895,7 +3896,7 @@ declare_clippy_lint! { /// option.is_some_and(|a| a > 10); /// result.is_ok_and(|a| a > 10); /// ``` - #[clippy::version = "1.76.0"] + #[clippy::version = "1.77.0"] pub MANUAL_IS_VARIANT_AND, pedantic, "using `.map(f).unwrap_or_default()`, which is more succinctly expressed as `is_some_and(f)` or `is_ok_and(f)`" @@ -3925,7 +3926,7 @@ declare_clippy_lint! { /// `"\r\n"`), for example during the parsing of a specific file format in which precisely one newline type is /// valid. /// ``` - #[clippy::version = "1.76.0"] + #[clippy::version = "1.77.0"] pub STR_SPLIT_AT_NEWLINE, pedantic, "splitting a trimmed string at hard-coded newlines" @@ -4044,6 +4045,31 @@ declare_clippy_lint! { "calling `.get().is_some()` or `.get().is_none()` instead of `.contains()` or `.contains_key()`" } +declare_clippy_lint! { + /// ### What it does + /// It identifies calls to `.is_empty()` on constant values. + /// + /// ### Why is this bad? + /// String literals and constant values are known at compile time. Checking if they + /// are empty will always return the same value. This might not be the intention of + /// the expression. + /// + /// ### Example + /// ```no_run + /// let value = ""; + /// if value.is_empty() { + /// println!("the string is empty"); + /// } + /// ``` + /// Use instead: + /// ```no_run + /// println!("the string is empty"); + /// ``` + #[clippy::version = "1.78.0"] + pub CONST_IS_EMPTY, + suspicious, + "is_empty() called on strings known at compile time" +} pub struct Methods { avoid_breaking_exported_api: bool, msrv: Msrv, @@ -4092,6 +4118,7 @@ impl_lint_pass!(Methods => [ CLONE_ON_COPY, CLONE_ON_REF_PTR, COLLAPSIBLE_STR_REPLACE, + CONST_IS_EMPTY, ITER_OVEREAGER_CLONED, CLONED_INSTEAD_OF_COPIED, FLAT_MAP_OPTION, @@ -4403,6 +4430,7 @@ impl Methods { zst_offset::check(cx, expr, recv); }, ("all", [arg]) => { + unused_enumerate_index::check(cx, expr, recv, arg); if let Some(("cloned", recv2, [], _, _)) = method_call(recv) { iter_overeager_cloned::check( cx, @@ -4421,23 +4449,26 @@ impl Methods { unnecessary_lazy_eval::check(cx, expr, recv, arg, "and"); } }, - ("any", [arg]) => match method_call(recv) { - Some(("cloned", recv2, [], _, _)) => iter_overeager_cloned::check( - cx, - expr, - recv, - recv2, - iter_overeager_cloned::Op::NeedlessMove(arg), - false, - ), - Some(("chars", recv, _, _, _)) - if let ExprKind::Closure(arg) = arg.kind - && let body = cx.tcx.hir().body(arg.body) - && let [param] = body.params => - { - string_lit_chars_any::check(cx, expr, recv, param, peel_blocks(body.value), &self.msrv); - }, - _ => {}, + ("any", [arg]) => { + unused_enumerate_index::check(cx, expr, recv, arg); + match method_call(recv) { + Some(("cloned", recv2, [], _, _)) => iter_overeager_cloned::check( + cx, + expr, + recv, + recv2, + iter_overeager_cloned::Op::NeedlessMove(arg), + false, + ), + Some(("chars", recv, _, _, _)) + if let ExprKind::Closure(arg) = arg.kind + && let body = cx.tcx.hir().body(arg.body) + && let [param] = body.params => + { + string_lit_chars_any::check(cx, expr, recv, param, peel_blocks(body.value), &self.msrv); + }, + _ => {}, + } }, ("arg", [arg]) => { suspicious_command_arg_space::check(cx, recv, arg, span); @@ -4445,7 +4476,7 @@ impl Methods { ("as_deref" | "as_deref_mut", []) => { needless_option_as_deref::check(cx, expr, recv, name); }, - ("as_bytes" | "is_empty", []) => { + ("as_bytes", []) => { if let Some(("as_str", recv, [], as_str_span, _)) = method_call(recv) { redundant_as_str::check(cx, expr, recv, as_str_span, span); } @@ -4570,14 +4601,17 @@ impl Methods { } }, ("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); }, ("find_map", [arg]) => { + unused_enumerate_index::check(cx, expr, recv, arg); unnecessary_filter_map::check(cx, expr, arg, name); }, ("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); }, @@ -4599,17 +4633,20 @@ impl Methods { manual_try_fold::check(cx, expr, init, acc, call_span, &self.msrv); unnecessary_fold::check(cx, expr, init, acc, span); }, - ("for_each", [arg]) => match method_call(recv) { - Some(("inspect", _, [_], span2, _)) => inspect_for_each::check(cx, expr, span2), - Some(("cloned", recv2, [], _, _)) => iter_overeager_cloned::check( - cx, - expr, - recv, - recv2, - iter_overeager_cloned::Op::NeedlessMove(arg), - false, - ), - _ => {}, + ("for_each", [arg]) => { + unused_enumerate_index::check(cx, expr, recv, arg); + match method_call(recv) { + Some(("inspect", _, [_], span2, _)) => inspect_for_each::check(cx, expr, span2), + Some(("cloned", recv2, [], _, _)) => iter_overeager_cloned::check( + cx, + expr, + recv, + recv2, + iter_overeager_cloned::Op::NeedlessMove(arg), + false, + ), + _ => {}, + } }, ("get", [arg]) => { get_first::check(cx, expr, recv, arg); @@ -4619,6 +4656,12 @@ impl Methods { ("hash", [arg]) => { unit_hash::check(cx, expr, recv, arg); }, + ("is_empty", []) => { + if let Some(("as_str", recv, [], as_str_span, _)) = method_call(recv) { + redundant_as_str::check(cx, expr, recv, as_str_span, span); + } + is_empty::check(cx, expr, recv); + }, ("is_file", []) => filetype_is_file::check(cx, expr, recv), ("is_digit", [radix]) => is_digit_ascii_radix::check(cx, expr, recv, radix, &self.msrv), ("is_none", []) => check_is_some_is_none(cx, expr, recv, call_span, false), @@ -4650,6 +4693,7 @@ impl Methods { }, (name @ ("map" | "map_err"), [m_arg]) => { if name == "map" { + unused_enumerate_index::check(cx, expr, recv, m_arg); map_clone::check(cx, expr, recv, m_arg, &self.msrv); match method_call(recv) { Some((map_name @ ("iter" | "into_iter"), recv2, _, _, _)) => { @@ -4723,8 +4767,11 @@ impl Methods { iter_overeager_cloned::Op::LaterCloned, false, ), - Some(("iter", recv2, [], _, _)) => iter_nth::check(cx, expr, recv2, recv, n_arg, false), - Some(("iter_mut", recv2, [], _, _)) => iter_nth::check(cx, expr, recv2, recv, n_arg, true), + Some((iter_method @ ("iter" | "iter_mut"), iter_recv, [], iter_span, _)) => { + if !iter_nth::check(cx, expr, iter_recv, iter_method, iter_span, span) { + iter_nth_zero::check(cx, expr, recv, n_arg); + } + }, _ => iter_nth_zero::check(cx, expr, recv, n_arg), }, ("ok_or_else", [arg]) => unnecessary_lazy_eval::check(cx, expr, recv, arg, "ok_or"), diff --git a/clippy_lints/src/methods/no_effect_replace.rs b/clippy_lints/src/methods/no_effect_replace.rs index 81df32bdee2..a301a5f7d65 100644 --- a/clippy_lints/src/methods/no_effect_replace.rs +++ b/clippy_lints/src/methods/no_effect_replace.rs @@ -25,6 +25,7 @@ pub(super) fn check<'tcx>( && param1 == param2.as_str() { span_lint(cx, NO_EFFECT_REPLACE, expr.span, "replacing text with itself"); + return; } if SpanlessEq::new(cx).eq_expr(arg1, arg2) { diff --git a/clippy_lints/src/methods/unnecessary_sort_by.rs b/clippy_lints/src/methods/unnecessary_sort_by.rs index 6911da69b94..a92fe9e55a4 100644 --- a/clippy_lints/src/methods/unnecessary_sort_by.rs +++ b/clippy_lints/src/methods/unnecessary_sort_by.rs @@ -5,7 +5,8 @@ use clippy_utils::ty::implements_trait; use rustc_errors::Applicability; use rustc_hir::{Closure, Expr, ExprKind, Mutability, Param, Pat, PatKind, Path, PathSegment, QPath}; use rustc_lint::LateContext; -use rustc_middle::ty::{self, GenericArgKind}; +use rustc_middle::ty; +use rustc_middle::ty::GenericArgKind; use rustc_span::sym; use rustc_span::symbol::Ident; use std::iter; diff --git a/clippy_lints/src/methods/unused_enumerate_index.rs b/clippy_lints/src/methods/unused_enumerate_index.rs new file mode 100644 index 00000000000..e5cc898612e --- /dev/null +++ b/clippy_lints/src/methods/unused_enumerate_index.rs @@ -0,0 +1,135 @@ +use clippy_utils::diagnostics::{multispan_sugg_with_applicability, span_lint_hir_and_then}; +use clippy_utils::paths::{CORE_ITER_ENUMERATE_METHOD, CORE_ITER_ENUMERATE_STRUCT}; +use clippy_utils::source::{snippet, snippet_opt}; +use clippy_utils::{expr_or_init, is_trait_method, match_def_path, 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::{sym, Span}; + +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 + && match_def_path(cx, recv_ty_defid, &CORE_ITER_ENUMERATE_STRUCT) + // 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`. + && match_def_path(cx, enumerate_defid, &CORE_ITER_ENUMERATE_METHOD) + { + // Check if the tuple type was explicit. It may be the type system _needs_ the type of the element + // that would be explicited 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) => snippet_opt(cx, elem.span) + .and_then(|binding_name| snippet_opt(cx, ty_span).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| { + multispan_sugg_with_applicability( + diag, + "remove the `.enumerate()` call", + Applicability::MachineApplicable, + vec![ + (closure_param.span, new_closure_param), + ( + enumerate_span.with_lo(enumerate_recv.span.source_callsite().hi()), + String::new(), + ), + ], + ); + }, + ); + } +} + +/// 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<Span> { + 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 + } +} |