diff options
Diffstat (limited to 'clippy_lints/src/methods')
| -rw-r--r-- | clippy_lints/src/methods/implicit_clone.rs | 1 | ||||
| -rw-r--r-- | clippy_lints/src/methods/iter_overeager_cloned.rs | 126 | ||||
| -rw-r--r-- | clippy_lints/src/methods/mod.rs | 42 | ||||
| -rw-r--r-- | clippy_lints/src/methods/or_fun_call.rs | 19 | ||||
| -rw-r--r-- | clippy_lints/src/methods/unnecessary_sort_by.rs | 3 |
5 files changed, 162 insertions, 29 deletions
diff --git a/clippy_lints/src/methods/implicit_clone.rs b/clippy_lints/src/methods/implicit_clone.rs index 043425300d8..e91ce64d8c8 100644 --- a/clippy_lints/src/methods/implicit_clone.rs +++ b/clippy_lints/src/methods/implicit_clone.rs @@ -17,6 +17,7 @@ pub fn check(cx: &LateContext<'_>, method_name: &str, expr: &hir::Expr<'_>, recv let return_type = cx.typeck_results().expr_ty(expr); let input_type = cx.typeck_results().expr_ty(recv); let (input_type, ref_count) = peel_mid_ty_refs(input_type); + if !(ref_count > 0 && is_diag_trait_item(cx, method_def_id, sym::ToOwned)); if let Some(ty_name) = input_type.ty_adt_def().map(|adt_def| cx.tcx.item_name(adt_def.did())); if return_type == input_type; if let Some(clone_trait) = cx.tcx.lang_items().clone_trait(); diff --git a/clippy_lints/src/methods/iter_overeager_cloned.rs b/clippy_lints/src/methods/iter_overeager_cloned.rs index 9f7ec19aa59..ee405a3e30c 100644 --- a/clippy_lints/src/methods/iter_overeager_cloned.rs +++ b/clippy_lints/src/methods/iter_overeager_cloned.rs @@ -1,21 +1,45 @@ use clippy_utils::diagnostics::span_lint_and_then; use clippy_utils::source::snippet_opt; use clippy_utils::ty::{implements_trait, is_copy}; +use rustc_ast::BindingAnnotation; use rustc_errors::Applicability; -use rustc_hir::Expr; +use rustc_hir::{Body, Expr, ExprKind, HirId, HirIdSet, PatKind}; +use rustc_hir_typeck::expr_use_visitor::{Delegate, ExprUseVisitor, PlaceBase, PlaceWithHirId}; use rustc_lint::LateContext; -use rustc_middle::ty; +use rustc_middle::mir::{FakeReadCause, Mutability}; +use rustc_middle::ty::{self, BorrowKind}; use rustc_span::sym; use super::ITER_OVEREAGER_CLONED; use crate::redundant_clone::REDUNDANT_CLONE; +use crate::rustc_trait_selection::infer::TyCtxtInferExt; + +#[derive(Clone, Copy)] +pub(super) enum Op<'a> { + // rm `.cloned()` + // e.g. `count` + RmCloned, + + // rm `.cloned()` + // e.g. `map` `for_each` + NeedlessMove(&'a str, &'a Expr<'a>), + + // later `.cloned()` + // and add `&` to the parameter of closure parameter + // e.g. `find` `filter` + FixClosure(&'a str, &'a Expr<'a>), + + // later `.cloned()` + // e.g. `skip` `take` + LaterCloned, +} pub(super) fn check<'tcx>( cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, cloned_call: &'tcx Expr<'_>, cloned_recv: &'tcx Expr<'_>, - is_count: bool, + op: Op<'tcx>, needs_into_iter: bool, ) { let typeck = cx.typeck_results(); @@ -35,10 +59,47 @@ pub(super) fn check<'tcx>( return; } - let (lint, msg, trailing_clone) = if is_count { - (REDUNDANT_CLONE, "unneeded cloning of iterator items", "") - } else { - (ITER_OVEREAGER_CLONED, "unnecessarily eager cloning of iterator items", ".cloned()") + if let Op::NeedlessMove(_, expr) = op { + let rustc_hir::ExprKind::Closure(closure) = expr.kind else { return } ; + let body @ Body { params: [p], .. } = cx.tcx.hir().body(closure.body) else { return }; + let mut delegate = MoveDelegate {used_move : HirIdSet::default()}; + let infcx = cx.tcx.infer_ctxt().build(); + + ExprUseVisitor::new( + &mut delegate, + &infcx, + closure.body.hir_id.owner.def_id, + cx.param_env, + cx.typeck_results(), + ) + .consume_body(body); + + let mut to_be_discarded = false; + + p.pat.walk(|it| { + if delegate.used_move.contains(&it.hir_id){ + to_be_discarded = true; + return false; + } + + match it.kind { + PatKind::Binding(BindingAnnotation(_, Mutability::Mut), _, _, _) + | PatKind::Ref(_, Mutability::Mut) => { + to_be_discarded = true; + false + } + _ => { true } + } + }); + + if to_be_discarded { + return; + } + } + + let (lint, msg, trailing_clone) = match op { + Op::RmCloned | Op::NeedlessMove(_, _) => (REDUNDANT_CLONE, "unneeded cloning of iterator items", ""), + Op::LaterCloned | Op::FixClosure(_, _) => (ITER_OVEREAGER_CLONED, "unnecessarily eager cloning of iterator items", ".cloned()"), }; span_lint_and_then( @@ -47,13 +108,54 @@ pub(super) fn check<'tcx>( expr.span, msg, |diag| { - let method_span = expr.span.with_lo(cloned_call.span.hi()); - if let Some(mut snip) = snippet_opt(cx, method_span) { - snip.push_str(trailing_clone); - let replace_span = expr.span.with_lo(cloned_recv.span.hi()); - diag.span_suggestion(replace_span, "try", snip, Applicability::MachineApplicable); + match op { + Op::RmCloned | Op::LaterCloned => { + let method_span = expr.span.with_lo(cloned_call.span.hi()); + if let Some(mut snip) = snippet_opt(cx, method_span) { + snip.push_str(trailing_clone); + let replace_span = expr.span.with_lo(cloned_recv.span.hi()); + diag.span_suggestion(replace_span, "try", snip, Applicability::MachineApplicable); + } + } + Op::FixClosure(name, predicate_expr) => { + if let Some(predicate) = snippet_opt(cx, predicate_expr.span) { + let new_closure = if let ExprKind::Closure(_) = predicate_expr.kind { + predicate.replacen('|', "|&", 1) + } else { + format!("|&x| {predicate}(x)") + }; + let snip = format!(".{name}({new_closure}).cloned()" ); + let replace_span = expr.span.with_lo(cloned_recv.span.hi()); + diag.span_suggestion(replace_span, "try", snip, Applicability::MachineApplicable); + } + } + Op::NeedlessMove(_, _) => { + let method_span = expr.span.with_lo(cloned_call.span.hi()); + if let Some(snip) = snippet_opt(cx, method_span) { + let replace_span = expr.span.with_lo(cloned_recv.span.hi()); + diag.span_suggestion(replace_span, "try", snip, Applicability::MaybeIncorrect); + } + } } } ); } } + +struct MoveDelegate { + used_move: HirIdSet, +} + +impl<'tcx> Delegate<'tcx> for MoveDelegate { + fn consume(&mut self, place_with_id: &PlaceWithHirId<'tcx>, _: HirId) { + if let PlaceBase::Local(l) = place_with_id.place.base { + self.used_move.insert(l); + } + } + + fn borrow(&mut self, _: &PlaceWithHirId<'tcx>, _: HirId, _: BorrowKind) {} + + fn mutate(&mut self, _: &PlaceWithHirId<'tcx>, _: HirId) {} + + fn fake_read(&mut self, _: &PlaceWithHirId<'tcx>, _: FakeReadCause, _: HirId) {} +} diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs index 42756b27d01..5075137caa0 100644 --- a/clippy_lints/src/methods/mod.rs +++ b/clippy_lints/src/methods/mod.rs @@ -301,7 +301,7 @@ declare_clippy_lint! { /// let val2 = 1; /// let val3 = 1; /// ``` - #[clippy::version = "1.69.0"] + #[clippy::version = "1.72.0"] pub UNNECESSARY_LITERAL_UNWRAP, complexity, "using `unwrap()` related calls on `Result` and `Option` constructors" @@ -3328,7 +3328,7 @@ declare_clippy_lint! { /// mem::take(v) /// } /// ``` - #[clippy::version = "1.71.0"] + #[clippy::version = "1.72.0"] pub DRAIN_COLLECT, perf, "calling `.drain(..).collect()` to move all elements into a new collection" @@ -3919,7 +3919,7 @@ impl Methods { } }, ("count", []) if is_trait_method(cx, expr, sym::Iterator) => match method_call(recv) { - Some(("cloned", recv2, [], _, _)) => iter_overeager_cloned::check(cx, expr, recv, recv2, true, false), + Some(("cloned", recv2, [], _, _)) => iter_overeager_cloned::check(cx, expr, recv, recv2, iter_overeager_cloned::Op::RmCloned , false), Some((name2 @ ("into_iter" | "iter" | "iter_mut"), recv2, [], _, _)) => { iter_count::check(cx, expr, recv2, name2); }, @@ -3973,6 +3973,13 @@ impl Methods { string_extend_chars::check(cx, expr, recv, arg); extend_with_drain::check(cx, expr, recv, arg); }, + (name @ ( "filter" | "find" ) , [arg]) => { + if let Some(("cloned", recv2, [], _span2, _)) = method_call(recv) { + // if `arg` has side-effect, the semantic will change + iter_overeager_cloned::check(cx, expr, recv, recv2, + iter_overeager_cloned::Op::FixClosure(name, arg), false); + } + } ("filter_map", [arg]) => { unnecessary_filter_map::check(cx, expr, arg, name); filter_map_bool_then::check(cx, expr, arg, call_span); @@ -3987,16 +3994,18 @@ impl Methods { }, ("flatten", []) => match method_call(recv) { Some(("map", recv, [map_arg], map_span, _)) => map_flatten::check(cx, expr, recv, map_arg, map_span), - Some(("cloned", recv2, [], _, _)) => iter_overeager_cloned::check(cx, expr, recv, recv2, false, true), + Some(("cloned", recv2, [], _, _)) => iter_overeager_cloned::check(cx, expr, recv, recv2, iter_overeager_cloned::Op::LaterCloned , true), _ => {}, }, ("fold", [init, acc]) => { manual_try_fold::check(cx, expr, init, acc, call_span, &self.msrv); unnecessary_fold::check(cx, expr, init, acc, span); }, - ("for_each", [_]) => { - if let Some(("inspect", _, [_], span2, _)) = method_call(recv) { - inspect_for_each::check(cx, expr, span2); + ("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(name, arg), false), + _ => {} } }, ("get", [arg]) => { @@ -4021,7 +4030,8 @@ impl Methods { }, ("last", []) => { if let Some(("cloned", recv2, [], _span2, _)) = method_call(recv) { - iter_overeager_cloned::check(cx, expr, recv, recv2, false, false); + iter_overeager_cloned::check(cx, expr, recv, recv2, + iter_overeager_cloned::Op::LaterCloned , false); } }, ("lock", []) => { @@ -4030,8 +4040,10 @@ impl Methods { (name @ ("map" | "map_err"), [m_arg]) => { if name == "map" { map_clone::check(cx, expr, recv, m_arg, &self.msrv); - if let Some((map_name @ ("iter" | "into_iter"), recv2, _, _, _)) = method_call(recv) { - iter_kv_map::check(cx, map_name, expr, recv2, m_arg); + match method_call(recv) { + Some((map_name @ ("iter" | "into_iter"), recv2, _, _, _)) => iter_kv_map::check(cx, map_name, expr, recv2, m_arg), + Some(("cloned", recv2, [], _, _)) => iter_overeager_cloned::check(cx, expr, recv, recv2, iter_overeager_cloned::Op::NeedlessMove(name, m_arg), false), + _ => {} } } else { map_err_ignore::check(cx, expr, m_arg); @@ -4058,7 +4070,7 @@ impl Methods { ("next", []) => { if let Some((name2, recv2, args2, _, _)) = method_call(recv) { match (name2, args2) { - ("cloned", []) => iter_overeager_cloned::check(cx, expr, recv, recv2, false, false), + ("cloned", []) => iter_overeager_cloned::check(cx, expr, recv, recv2, iter_overeager_cloned::Op::LaterCloned, false), ("filter", [arg]) => filter_next::check(cx, expr, recv2, arg), ("filter_map", [arg]) => filter_map_next::check(cx, expr, recv2, arg, &self.msrv), ("iter", []) => iter_next_slice::check(cx, expr, recv2), @@ -4071,7 +4083,7 @@ impl Methods { }, ("nth", [n_arg]) => match method_call(recv) { Some(("bytes", recv2, [], _, _)) => bytes_nth::check(cx, expr, recv2, n_arg), - Some(("cloned", recv2, [], _, _)) => iter_overeager_cloned::check(cx, expr, recv, recv2, false, false), + Some(("cloned", recv2, [], _, _)) => iter_overeager_cloned::check(cx, expr, recv, recv2, 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), _ => iter_nth_zero::check(cx, expr, recv, n_arg), @@ -4126,7 +4138,8 @@ impl Methods { iter_skip_zero::check(cx, expr, arg); if let Some(("cloned", recv2, [], _span2, _)) = method_call(recv) { - iter_overeager_cloned::check(cx, expr, recv, recv2, false, false); + iter_overeager_cloned::check(cx, expr, recv, recv2, + iter_overeager_cloned::Op::LaterCloned , false); } } ("sort", []) => { @@ -4152,7 +4165,8 @@ impl Methods { ("step_by", [arg]) => iterator_step_by_zero::check(cx, expr, arg), ("take", [_arg]) => { if let Some(("cloned", recv2, [], _span2, _)) = method_call(recv) { - iter_overeager_cloned::check(cx, expr, recv, recv2, false, false); + iter_overeager_cloned::check(cx, expr, recv, recv2, + iter_overeager_cloned::Op::LaterCloned, false); } }, ("take", []) => needless_option_take::check(cx, expr, recv), diff --git a/clippy_lints/src/methods/or_fun_call.rs b/clippy_lints/src/methods/or_fun_call.rs index 8b2f57160af..942f3bd79a6 100644 --- a/clippy_lints/src/methods/or_fun_call.rs +++ b/clippy_lints/src/methods/or_fun_call.rs @@ -65,11 +65,26 @@ pub(super) fn check<'tcx>( }; let sugg = match (name, call_expr.is_some()) { - ("unwrap_or", true) | ("unwrap_or_else", false) => "unwrap_or_default", - ("or_insert", true) | ("or_insert_with", false) => "or_default", + ("unwrap_or", true) | ("unwrap_or_else", false) => sym!(unwrap_or_default), + ("or_insert", true) | ("or_insert_with", false) => sym!(or_default), _ => return false, }; + let receiver_ty = cx.typeck_results().expr_ty_adjusted(receiver).peel_refs(); + let has_suggested_method = receiver_ty.ty_adt_def().is_some_and(|adt_def| { + cx.tcx + .inherent_impls(adt_def.did()) + .iter() + .flat_map(|impl_id| cx.tcx.associated_items(impl_id).filter_by_name_unhygienic(sugg)) + .any(|assoc| { + assoc.fn_has_self_parameter + && cx.tcx.fn_sig(assoc.def_id).skip_binder().inputs().skip_binder().len() == 1 + }) + }); + if !has_suggested_method { + return false; + } + // needs to target Default::default in particular or be *::new and have a Default impl // available if (is_new(fun) && output_type_implements_default(fun)) diff --git a/clippy_lints/src/methods/unnecessary_sort_by.rs b/clippy_lints/src/methods/unnecessary_sort_by.rs index e62a65a2712..79bff39379c 100644 --- a/clippy_lints/src/methods/unnecessary_sort_by.rs +++ b/clippy_lints/src/methods/unnecessary_sort_by.rs @@ -6,7 +6,8 @@ use if_chain::if_chain; 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; |