From 9575f3d0f841b93f16ef4c28bc2f1adde779cdb9 Mon Sep 17 00:00:00 2001 From: David Manca <1900179+medzernik@users.noreply.github.com> Date: Sun, 21 Jun 2026 15:47:41 +0200 Subject: [PATCH 01/27] fix `unnested_or_patterns` according to iss `2074` Signed-off-by: David Manca <1900179+medzernik@users.noreply.github.com> --- clippy_lints/src/unnested_or_patterns.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/clippy_lints/src/unnested_or_patterns.rs b/clippy_lints/src/unnested_or_patterns.rs index 184427c4dba7..bbe28df7af92 100644 --- a/clippy_lints/src/unnested_or_patterns.rs +++ b/clippy_lints/src/unnested_or_patterns.rs @@ -62,7 +62,7 @@ impl UnnestedOrPatterns { impl EarlyLintPass for UnnestedOrPatterns { fn check_arm(&mut self, cx: &EarlyContext<'_>, a: &ast::Arm) { if self.msrv.meets(msrvs::OR_PATTERNS) { - lint_unnested_or_patterns(cx, &a.pat); + lint_unnested_or_patterns(cx, *a.pat.clone()); } } @@ -70,32 +70,32 @@ impl EarlyLintPass for UnnestedOrPatterns { if self.msrv.meets(msrvs::OR_PATTERNS) && let ast::ExprKind::Let(pat, _, _, _) = &e.kind { - lint_unnested_or_patterns(cx, pat); + lint_unnested_or_patterns(cx, *pat.clone()); } } fn check_param(&mut self, cx: &EarlyContext<'_>, p: &ast::Param) { if self.msrv.meets(msrvs::OR_PATTERNS) { - lint_unnested_or_patterns(cx, &p.pat); + lint_unnested_or_patterns(cx, *p.pat.clone()); } } fn check_local(&mut self, cx: &EarlyContext<'_>, l: &ast::Local) { if self.msrv.meets(msrvs::OR_PATTERNS) { - lint_unnested_or_patterns(cx, &l.pat); + lint_unnested_or_patterns(cx, *l.pat.clone()); } } extract_msrv_attr!(); } -fn lint_unnested_or_patterns(cx: &EarlyContext<'_>, pat: &Pat) { +fn lint_unnested_or_patterns(cx: &EarlyContext<'_>, pat: Pat) { if let Ident(.., None) | Expr(_) | Wild | Path(..) | Range(..) | Rest | MacCall(_) = pat.kind { // This is a leaf pattern, so cloning is unprofitable. return; } - let mut pat = pat.clone(); + let mut pat = pat; // Nix all the paren patterns everywhere so that they aren't in our way. remove_all_parens(&mut pat); From baf03af522614019a43516e1be42ea61a23a0cde Mon Sep 17 00:00:00 2001 From: medzernik Date: Thu, 21 May 2026 15:40:31 +0200 Subject: [PATCH 02/27] add ``fn_param_ref_cloned`` lint Co-Authored by: Matej Almasi Signed-off-by: David Manca <1900179+medzernik@users.noreply.github.com> # Conflicts: # clippy_lints/src/lib.rs # Conflicts: # CHANGELOG.md --- CHANGELOG.md | 1 + clippy_lints/src/declared_lints.rs | 1 + clippy_lints/src/fn_param_ref_cloned.rs | 162 ++++++++++++++++++++++++ clippy_lints/src/lib.rs | 2 + tests/ui/fn_param_ref_cloned.rs | 131 +++++++++++++++++++ tests/ui/fn_param_ref_cloned.stderr | 69 ++++++++++ tests/ui/foo_functions_late.stderr | 20 +++ 7 files changed, 386 insertions(+) create mode 100644 clippy_lints/src/fn_param_ref_cloned.rs create mode 100644 tests/ui/fn_param_ref_cloned.rs create mode 100644 tests/ui/fn_param_ref_cloned.stderr create mode 100644 tests/ui/foo_functions_late.stderr diff --git a/CHANGELOG.md b/CHANGELOG.md index f6814f0a9c7e..c9aeb99db65a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7068,6 +7068,7 @@ Released 2018-09-13 [`float_equality_without_abs`]: https://rust-lang.github.io/rust-clippy/main/index.html#float_equality_without_abs [`fn_address_comparisons`]: https://rust-lang.github.io/rust-clippy/main/index.html#fn_address_comparisons [`fn_null_check`]: https://rust-lang.github.io/rust-clippy/main/index.html#fn_null_check +[`fn_param_ref_cloned_info`]: https://rust-lang.github.io/rust-clippy/master/index.html#fn_param_ref_cloned_info [`fn_params_excessive_bools`]: https://rust-lang.github.io/rust-clippy/main/index.html#fn_params_excessive_bools [`fn_to_numeric_cast`]: https://rust-lang.github.io/rust-clippy/main/index.html#fn_to_numeric_cast [`fn_to_numeric_cast_any`]: https://rust-lang.github.io/rust-clippy/main/index.html#fn_to_numeric_cast_any diff --git a/clippy_lints/src/declared_lints.rs b/clippy_lints/src/declared_lints.rs index da0bd90cc50c..0456d3c8945a 100644 --- a/clippy_lints/src/declared_lints.rs +++ b/clippy_lints/src/declared_lints.rs @@ -173,6 +173,7 @@ pub static LINTS: &[&::declare_clippy_lint::LintInfo] = &[ crate::float_literal::LOSSY_FLOAT_LITERAL_INFO, crate::floating_point_arithmetic::IMPRECISE_FLOPS_INFO, crate::floating_point_arithmetic::SUBOPTIMAL_FLOPS_INFO, + crate::fn_param_ref_cloned::FN_PARAM_REF_CLONED_INFO, crate::format::USELESS_FORMAT_INFO, crate::format_args::FORMAT_IN_FORMAT_ARGS_INFO, crate::format_args::POINTER_FORMAT_INFO, diff --git a/clippy_lints/src/fn_param_ref_cloned.rs b/clippy_lints/src/fn_param_ref_cloned.rs new file mode 100644 index 000000000000..ae2e8848d558 --- /dev/null +++ b/clippy_lints/src/fn_param_ref_cloned.rs @@ -0,0 +1,162 @@ +use clippy_utils::res::MaybeResPath; +use clippy_utils::ty::implements_trait; +use clippy_utils::visitors::for_each_expr; +use rustc_hir::{Body, PatKind}; +use rustc_lint::{LateContext, LateLintPass}; +use rustc_middle::ty::{Ref, Ty}; +use rustc_session::impl_lint_pass; +use rustc_span::Span; +use rustc_span::def_id::DefId; +use std::ops::ControlFlow; + +declare_clippy_lint! { + /// ### What it does + /// Checks if a function clones a parameter passed by reference. + /// + /// ### Why is this bad? + /// Caller should decide where to copy and place data. + /// The function should not hide the need of ownership of data. + /// + /// ### Example + /// ```norun + /// #[derive(Clone)] + /// struct A; + /// + /// pub fn foo(item: &A) { + /// let cloned_ref = item.clone(); + /// } + /// ``` + #[clippy::version = "1.98.0"] + pub FN_PARAM_REF_CLONED, + style, + "you should pass by value instead of cloning a passed reference" +} + +impl_lint_pass!(FnParamRefCloned => [FN_PARAM_REF_CLONED]); + +type CandidateId = rustc_hir::HirId; +type CandidateSpan = Span; +type Candidate = (CandidateId, CandidateSpan); +type CandidateRebinds = Vec; + +#[derive(Default)] +pub struct FnParamRefCloned { + candidates: Vec<(Candidate, CandidateRebinds)>, +} + +pub fn is_candidate_ty<'a>(cx: &LateContext<'a>, ty: Ty<'a>, must_impl_trait: &[DefId]) -> bool { + if let Ref(_, ty_ref, _) = ty.kind() { + must_impl_trait + .iter() + .any(|def_id| implements_trait(cx, *ty_ref, *def_id, &[])) + } else { + false + } +} + +pub fn get_param_id_span(param: &rustc_hir::Param<'_>) -> Option<(rustc_hir::HirId, Span)> { + if let PatKind::Binding(_, hir_id, ident, _) = param.pat.kind { + if !ident.span.from_expansion() && !ident.is_reserved() { + Some((hir_id, param.ty_span)) + } else { + None + } + } else { + None + } +} + +impl<'tcx> LateLintPass<'tcx> for FnParamRefCloned { + fn check_body(&mut self, cx: &LateContext<'tcx>, fn_body: &Body<'tcx>) { + // Define which traits must be implemented for the lint to work + let must_impl_trait = [ + cx.tcx.lang_items().clone_trait().unwrap(), + cx.tcx.lang_items().drop_trait().unwrap(), + ]; + + // Get the function ID + let def_id = cx.tcx.hir_body_owner_def_id(fn_body.id()); + + // Get all candidates of params that implement said traits and zip them with function signature + // params + self.candidates = cx + .tcx + .fn_sig(def_id) + .instantiate_identity() + .skip_binder() + .inputs() + .into_iter() + .zip(fn_body.params) + .filter_map(|(ty, param)| { + if let Some((id, span)) = get_param_id_span(param) + && is_candidate_ty(cx, *ty, &must_impl_trait) + { + Some(((id, span), Vec::default())) + } else { + None + } + }) + .collect(); + + // Find all rebinds of param values in the function and add them to the original candidates (tuple) + if let rustc_hir::ExprKind::Block(block, _) = fn_body.value.kind { + for statement in block.stmts.iter() { + if let rustc_hir::StmtKind::Let(let_stmt) = statement.kind + && let Some(expr) = let_stmt.init + && let rustc_hir::ExprKind::Path(qpath) = expr.kind + && let Some(hir_id) = qpath.res_local_id() + { + self.candidates.iter_mut().for_each(|(cand, relat)| { + if cand.0 == hir_id { + relat.push((let_stmt.pat.hir_id, let_stmt.span)); + } + }); + } + } + } + + // Look whether the candidates call the `.clone()` method anywhere + _ = for_each_expr::<(), ()>(cx, fn_body.value, move |x| { + match x.kind { + rustc_hir::ExprKind::MethodCall(method_name, receiver, args, span) + if method_name.ident.as_str() == "clone" + && args.is_empty() + && let rustc_hir::ExprKind::Path(qpath) = receiver.kind + && let Some(hir_id) = qpath.res_local_id() => + { + self.candidates.iter().for_each(|(original_candidate, rebinds)| { + if original_candidate.0 == hir_id { + clippy_utils::diagnostics::span_lint_and_note( + cx, + FN_PARAM_REF_CLONED, + span, + "function gets a parameter by reference, but you later unconditionally clone it", + Some(original_candidate.1), + "consider passing the reference by value instead", + ); + } + + for rebind in rebinds { + if rebind.0 == hir_id { + clippy_utils::diagnostics::span_lint_and_then( + cx, + FN_PARAM_REF_CLONED, + span, + "function gets a parameter by reference, but you later rebind and unconditionally clone it", + |diag| { + diag + .span_note(rebind.1, "you bind the parameter into a new binding here") + .span_note(original_candidate.1, "the parameter is passed by reference..."); + }, + ); + } + } + }); + }, + _ => (), + }; + + ControlFlow::Continue(()) + }) + } +} diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 6d49f44b466b..8c4af6e012c5 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -140,6 +140,7 @@ mod fallible_impl_from; mod field_scoped_visibility_modifiers; mod float_literal; mod floating_point_arithmetic; +mod fn_param_ref_cloned; mod format; mod format_args; mod format_impl; @@ -867,6 +868,7 @@ rustc_lint::late_lint_methods!( ManualAssertEq: manual_assert_eq::ManualAssertEq = manual_assert_eq::ManualAssertEq, WithCapacityZero: with_capacity_zero::WithCapacityZero = with_capacity_zero::WithCapacityZero, RefPatterns: ref_patterns::RefPatterns = ref_patterns::RefPatterns, + FnParamRefCloned: fn_param_ref_cloned::FnParamRefCloned = fn_param_ref_cloned::FnParamRefCloned::default(), RedundantElse: redundant_else::RedundantElse = redundant_else::RedundantElse, RestWhenDestructuringStruct: rest_when_destructuring_struct::RestWhenDestructuringStruct = rest_when_destructuring_struct::RestWhenDestructuringStruct, BlockScrutinee: block_scrutinee::BlockScrutinee = block_scrutinee::BlockScrutinee, diff --git a/tests/ui/fn_param_ref_cloned.rs b/tests/ui/fn_param_ref_cloned.rs new file mode 100644 index 000000000000..d478b2a82e80 --- /dev/null +++ b/tests/ui/fn_param_ref_cloned.rs @@ -0,0 +1,131 @@ +#![allow(unused)] +#![warn(clippy::fn_param_ref_cloned)] +#![feature(stmt_expr_attributes)] + +// Impl methods +#[derive(Clone, Default)] +pub struct IsClone; + +#[derive(Default)] +pub struct IsNotClone; + +#[derive(Default)] +pub struct PartialNotClone { + clone_field: IsClone, + not_clone_field: IsNotClone, +} + +#[derive(Default)] +pub struct PartialClone { + clone_field: IsClone, + not_clone_field: IsNotClone, +} + +impl IsNotClone { + fn clone(&self) {} +} + +// We know about this edgecase situation. I am not entirely sure how to filter this case +fn set_cell(cell: &std::cell::Cell) { + cell.set(5); + let a = cell.clone(); + //~^ fn_param_ref_cloned +} + +fn create_cell() { + let a = std::cell::Cell::new(0); + set_cell(&a); + println!("{}", a.get()); +} + +impl IsClone { + fn this_is_not_clone(&self) {} + + pub fn no_ref(&self) { + let is_clone = IsClone; + let cloned_no_ref = is_clone.clone(); + } + + pub fn cloning_ref(&self, is_clone: &IsClone) { + let cloned_ref_param = is_clone.clone(); + //~^ fn_param_ref_cloned + } + + pub fn using_ref(&self, is_clone: &IsClone) { + let x = ""; + let b = is_clone; + } +} + +pub fn test_gen_function(is_clone_ref: &T, is_clone_owned: T) { + // lint when we clone a param that is a reference + is_clone_ref.clone(); + //~^ fn_param_ref_cloned + + // don't lint when we call clone on an owned parameter + is_clone_owned.clone(); +} + +pub fn basic_clone_function(is_not_clone_ref: &IsNotClone, is_clone_ref: &IsClone, is_clone_owned: IsClone) { + // lint when we clone a param that is a reference + is_clone_ref.clone(); + //~^ fn_param_ref_cloned + + // don't lint when we call a different method + is_clone_ref.this_is_not_clone(); + + // don't lint when we call clone on an owned parameter + is_clone_owned.clone(); + + // don't lint when we call clone on a type that doesn't implement the trait Clone + is_not_clone_ref.clone(); + + // don't lint when we clone a local variable of IsClone type + let local_is_clone = IsClone; + local_is_clone.clone(); + + // or any other method of that local variable + local_is_clone.this_is_not_clone(); + + // lint when we clone on a re-binding of a cloneable reference + let rebound = is_clone_ref; + rebound.clone(); + //~^ fn_param_ref_cloned +} + +fn partial_clone( + partial_not_clone_owned: PartialNotClone, + partial_not_clone_ref: &PartialNotClone, + partial_clone_owned: PartialClone, + partial_clone_ref: &PartialClone, +) { + // Move owned values + let moved = partial_not_clone_owned.clone_field; + let moved = partial_not_clone_owned.not_clone_field; + + let moved = partial_clone_owned.clone_field; + let moved = partial_clone_owned.not_clone_field; + + // Clone only struct fields on non-clone struct + let cloned = partial_not_clone_ref.clone_field.clone(); + + // Clone only struct fields on clone struct + let cloned = partial_clone_ref.clone_field.clone(); +} + +fn main() { + let a = IsClone; + let b = IsClone; + let c = IsNotClone; + basic_clone_function(&c, &a, b); + + let d = PartialNotClone::default(); + let e = PartialClone::default(); + let f = PartialNotClone::default(); + let g = PartialClone::default(); + partial_clone(d, &f, e, &g); + + let h = std::cell::Cell::new(0); + set_cell(&h); + create_cell(); +} diff --git a/tests/ui/fn_param_ref_cloned.stderr b/tests/ui/fn_param_ref_cloned.stderr new file mode 100644 index 000000000000..efa16fd76583 --- /dev/null +++ b/tests/ui/fn_param_ref_cloned.stderr @@ -0,0 +1,69 @@ +error: function gets a parameter by reference, but you later unconditionally clone it + --> tests/ui/fn_param_ref_cloned.rs:31:18 + | +LL | let a = cell.clone(); + | ^^^^^^^ + | +note: consider passing the reference by value instead + --> tests/ui/fn_param_ref_cloned.rs:29:19 + | +LL | fn set_cell(cell: &std::cell::Cell) { + | ^^^^^^^^^^^^^^^^^^^^^ + = note: `-D clippy::fn-param-ref-cloned` implied by `-D warnings` + = help: to override `-D warnings` add `#[allow(clippy::fn_param_ref_cloned)]` + +error: function gets a parameter by reference, but you later unconditionally clone it + --> tests/ui/fn_param_ref_cloned.rs:50:41 + | +LL | let cloned_ref_param = is_clone.clone(); + | ^^^^^^^ + | +note: consider passing the reference by value instead + --> tests/ui/fn_param_ref_cloned.rs:49:41 + | +LL | pub fn cloning_ref(&self, is_clone: &IsClone) { + | ^^^^^^^^ + +error: function gets a parameter by reference, but you later unconditionally clone it + --> tests/ui/fn_param_ref_cloned.rs:62:18 + | +LL | is_clone_ref.clone(); + | ^^^^^^^ + | +note: consider passing the reference by value instead + --> tests/ui/fn_param_ref_cloned.rs:60:50 + | +LL | pub fn test_gen_function(is_clone_ref: &T, is_clone_owned: T) { + | ^^ + +error: function gets a parameter by reference, but you later unconditionally clone it + --> tests/ui/fn_param_ref_cloned.rs:71:18 + | +LL | is_clone_ref.clone(); + | ^^^^^^^ + | +note: consider passing the reference by value instead + --> tests/ui/fn_param_ref_cloned.rs:69:74 + | +LL | pub fn basic_clone_function(is_not_clone_ref: &IsNotClone, is_clone_ref: &IsClone, is_clone_owned: IsClone) { + | ^^^^^^^^ + +error: function gets a parameter by reference, but you later rebind and unconditionally clone it + --> tests/ui/fn_param_ref_cloned.rs:92:13 + | +LL | rebound.clone(); + | ^^^^^^^ + | +note: you bind the parameter into a new binding here + --> tests/ui/fn_param_ref_cloned.rs:91:5 + | +LL | let rebound = is_clone_ref; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ +note: the parameter is passed by reference... + --> tests/ui/fn_param_ref_cloned.rs:69:74 + | +LL | pub fn basic_clone_function(is_not_clone_ref: &IsNotClone, is_clone_ref: &IsClone, is_clone_owned: IsClone) { + | ^^^^^^^^ + +error: aborting due to 5 previous errors + diff --git a/tests/ui/foo_functions_late.stderr b/tests/ui/foo_functions_late.stderr new file mode 100644 index 000000000000..70437a6c7e9b --- /dev/null +++ b/tests/ui/foo_functions_late.stderr @@ -0,0 +1,20 @@ +error: function gets a parameter by reference, but you clone it + --> tests/ui/foo_functions_late.rs:11:22 + | +LL | let x = item.clone(); + | ^^^^^^^ + | + = help: consider passing by value instead + = note: `-D clippy::foo-functions-late` implied by `-D warnings` + = help: to override `-D warnings` add `#[allow(clippy::foo_functions_late)]` + +error: function gets a parameter by reference, but you clone it + --> tests/ui/foo_functions_late.rs:18:18 + | +LL | let y = item.clone(); + | ^^^^^^^ + | + = help: consider passing by value instead + +error: aborting due to 2 previous errors + From 62d90039bb7d695b5e48e09c49a30364d0b68467 Mon Sep 17 00:00:00 2001 From: David Manca <1900179+medzernik@users.noreply.github.com> Date: Sat, 20 Jun 2026 14:39:15 +0200 Subject: [PATCH 03/27] Update CHANGELOG.md Signed-off-by: David Manca <1900179+medzernik@users.noreply.github.com> --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c9aeb99db65a..c237114eae7f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7068,7 +7068,7 @@ Released 2018-09-13 [`float_equality_without_abs`]: https://rust-lang.github.io/rust-clippy/main/index.html#float_equality_without_abs [`fn_address_comparisons`]: https://rust-lang.github.io/rust-clippy/main/index.html#fn_address_comparisons [`fn_null_check`]: https://rust-lang.github.io/rust-clippy/main/index.html#fn_null_check -[`fn_param_ref_cloned_info`]: https://rust-lang.github.io/rust-clippy/master/index.html#fn_param_ref_cloned_info +[`fn_param_ref_cloned`]: https://rust-lang.github.io/rust-clippy/master/index.html#fn_param_ref_cloned [`fn_params_excessive_bools`]: https://rust-lang.github.io/rust-clippy/main/index.html#fn_params_excessive_bools [`fn_to_numeric_cast`]: https://rust-lang.github.io/rust-clippy/main/index.html#fn_to_numeric_cast [`fn_to_numeric_cast_any`]: https://rust-lang.github.io/rust-clippy/main/index.html#fn_to_numeric_cast_any From df73be9da962db650959a692a13afc8799720486 Mon Sep 17 00:00:00 2001 From: David Manca <1900179+medzernik@users.noreply.github.com> Date: Sat, 20 Jun 2026 14:44:25 +0200 Subject: [PATCH 04/27] Delete tutorial residual `.stderr` Signed-off-by: David Manca <1900179+medzernik@users.noreply.github.com> --- tests/ui/foo_functions_late.stderr | 20 -------------------- 1 file changed, 20 deletions(-) delete mode 100644 tests/ui/foo_functions_late.stderr diff --git a/tests/ui/foo_functions_late.stderr b/tests/ui/foo_functions_late.stderr deleted file mode 100644 index 70437a6c7e9b..000000000000 --- a/tests/ui/foo_functions_late.stderr +++ /dev/null @@ -1,20 +0,0 @@ -error: function gets a parameter by reference, but you clone it - --> tests/ui/foo_functions_late.rs:11:22 - | -LL | let x = item.clone(); - | ^^^^^^^ - | - = help: consider passing by value instead - = note: `-D clippy::foo-functions-late` implied by `-D warnings` - = help: to override `-D warnings` add `#[allow(clippy::foo_functions_late)]` - -error: function gets a parameter by reference, but you clone it - --> tests/ui/foo_functions_late.rs:18:18 - | -LL | let y = item.clone(); - | ^^^^^^^ - | - = help: consider passing by value instead - -error: aborting due to 2 previous errors - From 48e4d1248ad6577cf6236883b7e0b15ee2fadd4a Mon Sep 17 00:00:00 2001 From: David Manca <1900179+medzernik@users.noreply.github.com> Date: Sat, 20 Jun 2026 15:32:24 +0200 Subject: [PATCH 05/27] change to `pedantic` and qualify ``<>::default()`` Signed-off-by: David Manca <1900179+medzernik@users.noreply.github.com> # Conflicts: # clippy_lints/src/lib.rs --- clippy_lints/src/fn_param_ref_cloned.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clippy_lints/src/fn_param_ref_cloned.rs b/clippy_lints/src/fn_param_ref_cloned.rs index ae2e8848d558..9e26f80ef5a0 100644 --- a/clippy_lints/src/fn_param_ref_cloned.rs +++ b/clippy_lints/src/fn_param_ref_cloned.rs @@ -28,7 +28,7 @@ declare_clippy_lint! { /// ``` #[clippy::version = "1.98.0"] pub FN_PARAM_REF_CLONED, - style, + pedantic, "you should pass by value instead of cloning a passed reference" } From bf1e9011d3c1ff0ab340cbb3e5149620427fa7a2 Mon Sep 17 00:00:00 2001 From: medzernik Date: Sat, 20 Jun 2026 19:55:18 +0200 Subject: [PATCH 06/27] Use `check_fn` instead of `check_body` Signed-off-by: medzernik --- clippy_lints/src/fn_param_ref_cloned.rs | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/clippy_lints/src/fn_param_ref_cloned.rs b/clippy_lints/src/fn_param_ref_cloned.rs index 9e26f80ef5a0..ef36350912e6 100644 --- a/clippy_lints/src/fn_param_ref_cloned.rs +++ b/clippy_lints/src/fn_param_ref_cloned.rs @@ -67,16 +67,21 @@ pub fn get_param_id_span(param: &rustc_hir::Param<'_>) -> Option<(rustc_hir::Hir } impl<'tcx> LateLintPass<'tcx> for FnParamRefCloned { - fn check_body(&mut self, cx: &LateContext<'tcx>, fn_body: &Body<'tcx>) { + fn check_fn( + &mut self, + cx: &LateContext<'tcx>, + _: rustc_hir::intravisit::FnKind<'tcx>, + _: &'tcx rustc_hir::FnDecl<'tcx>, + fn_body: &'tcx rustc_hir::Body<'tcx>, + _: Span, + def_id: rustc_span::def_id::LocalDefId, + ) { // Define which traits must be implemented for the lint to work let must_impl_trait = [ cx.tcx.lang_items().clone_trait().unwrap(), cx.tcx.lang_items().drop_trait().unwrap(), ]; - // Get the function ID - let def_id = cx.tcx.hir_body_owner_def_id(fn_body.id()); - // Get all candidates of params that implement said traits and zip them with function signature // params self.candidates = cx From 75945039f245f9d9239b72288ea84b07644d2478 Mon Sep 17 00:00:00 2001 From: medzernik Date: Sat, 20 Jun 2026 21:01:34 +0200 Subject: [PATCH 07/27] only lint immutable refs Signed-off-by: medzernik --- clippy_lints/src/fn_param_ref_cloned.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/clippy_lints/src/fn_param_ref_cloned.rs b/clippy_lints/src/fn_param_ref_cloned.rs index ef36350912e6..dc08b86759dc 100644 --- a/clippy_lints/src/fn_param_ref_cloned.rs +++ b/clippy_lints/src/fn_param_ref_cloned.rs @@ -45,10 +45,11 @@ pub struct FnParamRefCloned { } pub fn is_candidate_ty<'a>(cx: &LateContext<'a>, ty: Ty<'a>, must_impl_trait: &[DefId]) -> bool { - if let Ref(_, ty_ref, _) = ty.kind() { + if let Ref(_, ty_ref, mutability) = ty.kind() { must_impl_trait .iter() .any(|def_id| implements_trait(cx, *ty_ref, *def_id, &[])) + && mutability.is_not() } else { false } From 7a44d1cd57f715fc15706eb725b5d344b58b874e Mon Sep 17 00:00:00 2001 From: medzernik Date: Sat, 20 Jun 2026 21:01:46 +0200 Subject: [PATCH 08/27] skip linting closures Signed-off-by: medzernik --- clippy_lints/src/fn_param_ref_cloned.rs | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/clippy_lints/src/fn_param_ref_cloned.rs b/clippy_lints/src/fn_param_ref_cloned.rs index dc08b86759dc..f1d19cddcd1b 100644 --- a/clippy_lints/src/fn_param_ref_cloned.rs +++ b/clippy_lints/src/fn_param_ref_cloned.rs @@ -1,7 +1,7 @@ use clippy_utils::res::MaybeResPath; use clippy_utils::ty::implements_trait; use clippy_utils::visitors::for_each_expr; -use rustc_hir::{Body, PatKind}; +use rustc_hir::PatKind; use rustc_lint::{LateContext, LateLintPass}; use rustc_middle::ty::{Ref, Ty}; use rustc_session::impl_lint_pass; @@ -71,12 +71,16 @@ impl<'tcx> LateLintPass<'tcx> for FnParamRefCloned { fn check_fn( &mut self, cx: &LateContext<'tcx>, - _: rustc_hir::intravisit::FnKind<'tcx>, + fn_kind: rustc_hir::intravisit::FnKind<'tcx>, _: &'tcx rustc_hir::FnDecl<'tcx>, fn_body: &'tcx rustc_hir::Body<'tcx>, _: Span, def_id: rustc_span::def_id::LocalDefId, ) { + if let rustc_hir::intravisit::FnKind::Closure = fn_kind { + return; + } + // Define which traits must be implemented for the lint to work let must_impl_trait = [ cx.tcx.lang_items().clone_trait().unwrap(), @@ -91,7 +95,7 @@ impl<'tcx> LateLintPass<'tcx> for FnParamRefCloned { .instantiate_identity() .skip_binder() .inputs() - .into_iter() + .iter() .zip(fn_body.params) .filter_map(|(ty, param)| { if let Some((id, span)) = get_param_id_span(param) @@ -106,7 +110,7 @@ impl<'tcx> LateLintPass<'tcx> for FnParamRefCloned { // Find all rebinds of param values in the function and add them to the original candidates (tuple) if let rustc_hir::ExprKind::Block(block, _) = fn_body.value.kind { - for statement in block.stmts.iter() { + for statement in block.stmts { if let rustc_hir::StmtKind::Let(let_stmt) = statement.kind && let Some(expr) = let_stmt.init && let rustc_hir::ExprKind::Path(qpath) = expr.kind @@ -160,9 +164,9 @@ impl<'tcx> LateLintPass<'tcx> for FnParamRefCloned { }); }, _ => (), - }; + } ControlFlow::Continue(()) - }) + }); } } From 469f9d23962b89fdc2c188311aaeacb222416181 Mon Sep 17 00:00:00 2001 From: David Manca <1900179+medzernik@users.noreply.github.com> Date: Sun, 21 Jun 2026 15:39:07 +0200 Subject: [PATCH 09/27] skip if and match blocks Signed-off-by: David Manca <1900179+medzernik@users.noreply.github.com> --- clippy_lints/src/fn_param_ref_cloned.rs | 29 +++++++++++++------------ tests/ui/fn_param_ref_cloned.rs | 11 ++++++++++ tests/ui/fn_param_ref_cloned.stderr | 14 +++++++++++- 3 files changed, 39 insertions(+), 15 deletions(-) diff --git a/clippy_lints/src/fn_param_ref_cloned.rs b/clippy_lints/src/fn_param_ref_cloned.rs index f1d19cddcd1b..01c6ac2a61ec 100644 --- a/clippy_lints/src/fn_param_ref_cloned.rs +++ b/clippy_lints/src/fn_param_ref_cloned.rs @@ -1,6 +1,6 @@ use clippy_utils::res::MaybeResPath; use clippy_utils::ty::implements_trait; -use clippy_utils::visitors::for_each_expr; +use clippy_utils::visitors::{Descend, for_each_expr}; use rustc_hir::PatKind; use rustc_lint::{LateContext, LateLintPass}; use rustc_middle::ty::{Ref, Ty}; @@ -126,15 +126,17 @@ impl<'tcx> LateLintPass<'tcx> for FnParamRefCloned { } // Look whether the candidates call the `.clone()` method anywhere - _ = for_each_expr::<(), ()>(cx, fn_body.value, move |x| { - match x.kind { - rustc_hir::ExprKind::MethodCall(method_name, receiver, args, span) - if method_name.ident.as_str() == "clone" - && args.is_empty() - && let rustc_hir::ExprKind::Path(qpath) = receiver.kind - && let Some(hir_id) = qpath.res_local_id() => - { - self.candidates.iter().for_each(|(original_candidate, rebinds)| { + _ = for_each_expr(cx, fn_body.value, move |x| match x.kind { + rustc_hir::ExprKind::If(_, _, _) | rustc_hir::ExprKind::Match(_, _, _) => { + ControlFlow::<(), Descend>::Continue(Descend::No) + }, + rustc_hir::ExprKind::MethodCall(method_name, receiver, args, span) + if method_name.ident.as_str() == "clone" + && args.is_empty() + && let rustc_hir::ExprKind::Path(qpath) = receiver.kind + && let Some(hir_id) = qpath.res_local_id() => + { + self.candidates.iter().for_each(|(original_candidate, rebinds)| { if original_candidate.0 == hir_id { clippy_utils::diagnostics::span_lint_and_note( cx, @@ -162,11 +164,10 @@ impl<'tcx> LateLintPass<'tcx> for FnParamRefCloned { } } }); - }, - _ => (), - } + ControlFlow::<(), Descend>::Continue(Descend::Yes) + }, - ControlFlow::Continue(()) + _ => ControlFlow::<(), Descend>::Continue(Descend::Yes), }); } } diff --git a/tests/ui/fn_param_ref_cloned.rs b/tests/ui/fn_param_ref_cloned.rs index d478b2a82e80..ea11dd560ea0 100644 --- a/tests/ui/fn_param_ref_cloned.rs +++ b/tests/ui/fn_param_ref_cloned.rs @@ -113,6 +113,16 @@ fn partial_clone( let cloned = partial_clone_ref.clone_field.clone(); } +fn dont_check_if_stmts(clone_ref: &IsClone, if_arg: usize) { + // #[clippy::dump] + if if_arg == 0usize { + let should_allow_clone = clone_ref.clone(); + } + + let should_not_allow_clone = clone_ref.clone(); + //~^ fn_param_ref_cloned +} + fn main() { let a = IsClone; let b = IsClone; @@ -128,4 +138,5 @@ fn main() { let h = std::cell::Cell::new(0); set_cell(&h); create_cell(); + dont_check_if_stmts(&a, 0usize); } diff --git a/tests/ui/fn_param_ref_cloned.stderr b/tests/ui/fn_param_ref_cloned.stderr index efa16fd76583..54d5bef53107 100644 --- a/tests/ui/fn_param_ref_cloned.stderr +++ b/tests/ui/fn_param_ref_cloned.stderr @@ -65,5 +65,17 @@ note: the parameter is passed by reference... LL | pub fn basic_clone_function(is_not_clone_ref: &IsNotClone, is_clone_ref: &IsClone, is_clone_owned: IsClone) { | ^^^^^^^^ -error: aborting due to 5 previous errors +error: function gets a parameter by reference, but you later unconditionally clone it + --> tests/ui/fn_param_ref_cloned.rs:122:44 + | +LL | let should_not_allow_clone = clone_ref.clone(); + | ^^^^^^^ + | +note: consider passing the reference by value instead + --> tests/ui/fn_param_ref_cloned.rs:116:35 + | +LL | fn dont_check_if_stmts(clone_ref: &IsClone, if_arg: usize) { + | ^^^^^^^^ + +error: aborting due to 6 previous errors From 3d70a0aa3d4d42b5a8f132aeae74a4d5562269c3 Mon Sep 17 00:00:00 2001 From: David Manca <1900179+medzernik@users.noreply.github.com> Date: Sun, 28 Jun 2026 11:26:36 +0200 Subject: [PATCH 10/27] remove forgotten `clippy::dump` --- tests/ui/fn_param_ref_cloned.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/ui/fn_param_ref_cloned.rs b/tests/ui/fn_param_ref_cloned.rs index ea11dd560ea0..01e9277a3928 100644 --- a/tests/ui/fn_param_ref_cloned.rs +++ b/tests/ui/fn_param_ref_cloned.rs @@ -114,7 +114,6 @@ fn partial_clone( } fn dont_check_if_stmts(clone_ref: &IsClone, if_arg: usize) { - // #[clippy::dump] if if_arg == 0usize { let should_allow_clone = clone_ref.clone(); } From 04c21449c4443244e0a7cd3e3ca4cfbb3037b89e Mon Sep 17 00:00:00 2001 From: David Manca <1900179+medzernik@users.noreply.github.com> Date: Fri, 7 Aug 2026 17:59:35 +0200 Subject: [PATCH 11/27] revert bad rebase resolution.. Signed-off-by: David Manca <1900179+medzernik@users.noreply.github.com> --- clippy_lints/src/lib.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 8c4af6e012c5..fd3cdb42584b 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -868,11 +868,12 @@ rustc_lint::late_lint_methods!( ManualAssertEq: manual_assert_eq::ManualAssertEq = manual_assert_eq::ManualAssertEq, WithCapacityZero: with_capacity_zero::WithCapacityZero = with_capacity_zero::WithCapacityZero, RefPatterns: ref_patterns::RefPatterns = ref_patterns::RefPatterns, - FnParamRefCloned: fn_param_ref_cloned::FnParamRefCloned = fn_param_ref_cloned::FnParamRefCloned::default(), RedundantElse: redundant_else::RedundantElse = redundant_else::RedundantElse, RestWhenDestructuringStruct: rest_when_destructuring_struct::RestWhenDestructuringStruct = rest_when_destructuring_struct::RestWhenDestructuringStruct, BlockScrutinee: block_scrutinee::BlockScrutinee = block_scrutinee::BlockScrutinee, NonnullUncheckedOnBoxPtr: nonnull_unchecked_on_box_ptr::NonnullUncheckedOnBoxPtr = nonnull_unchecked_on_box_ptr::NonnullUncheckedOnBoxPtr::new(conf), + FnParamRefCloned: fn_param_ref_cloned::FnParamRefCloned = fn_param_ref_cloned::FnParamRefCloned::default(), + UnnecessaryNonzeroGet: unnecessary_nonzero_get::UnnecessaryNonzeroGet = unnecessary_nonzero_get::UnnecessaryNonzeroGet::new(conf), NeedlessNonzeroGet: needless_nonzero_get::NeedlessNonzeroGet = needless_nonzero_get::NeedlessNonzeroGet::new(conf), // add late passes here, used by `cargo dev new_lint` ]] From 37d0a7c5993cabdbfddd16569d5fa8b9c84b6957 Mon Sep 17 00:00:00 2001 From: David Manca <1900179+medzernik@users.noreply.github.com> Date: Fri, 7 Aug 2026 18:04:26 +0200 Subject: [PATCH 12/27] update lint to new ``for_each_expr`` Signed-off-by: David Manca <1900179+medzernik@users.noreply.github.com> --- clippy_lints/src/fn_param_ref_cloned.rs | 2 +- tests/ui/fn_param_ref_cloned.stderr | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/clippy_lints/src/fn_param_ref_cloned.rs b/clippy_lints/src/fn_param_ref_cloned.rs index 01c6ac2a61ec..e39b4ac9c472 100644 --- a/clippy_lints/src/fn_param_ref_cloned.rs +++ b/clippy_lints/src/fn_param_ref_cloned.rs @@ -126,7 +126,7 @@ impl<'tcx> LateLintPass<'tcx> for FnParamRefCloned { } // Look whether the candidates call the `.clone()` method anywhere - _ = for_each_expr(cx, fn_body.value, move |x| match x.kind { + _ = for_each_expr(cx.tcx, fn_body.value, move |x| match x.kind { rustc_hir::ExprKind::If(_, _, _) | rustc_hir::ExprKind::Match(_, _, _) => { ControlFlow::<(), Descend>::Continue(Descend::No) }, diff --git a/tests/ui/fn_param_ref_cloned.stderr b/tests/ui/fn_param_ref_cloned.stderr index 54d5bef53107..dea758acd8c8 100644 --- a/tests/ui/fn_param_ref_cloned.stderr +++ b/tests/ui/fn_param_ref_cloned.stderr @@ -66,7 +66,7 @@ LL | pub fn basic_clone_function(is_not_clone_ref: &IsNotClone, is_clone_ref: &I | ^^^^^^^^ error: function gets a parameter by reference, but you later unconditionally clone it - --> tests/ui/fn_param_ref_cloned.rs:122:44 + --> tests/ui/fn_param_ref_cloned.rs:121:44 | LL | let should_not_allow_clone = clone_ref.clone(); | ^^^^^^^ From f6399ce6f97bff8eb354b51fb08ae283fedf5393 Mon Sep 17 00:00:00 2001 From: David Manca <1900179+medzernik@users.noreply.github.com> Date: Fri, 7 Aug 2026 18:19:02 +0200 Subject: [PATCH 13/27] anonymously use import of trait for sideeffects Signed-off-by: David Manca <1900179+medzernik@users.noreply.github.com> --- clippy_lints/src/fn_param_ref_cloned.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clippy_lints/src/fn_param_ref_cloned.rs b/clippy_lints/src/fn_param_ref_cloned.rs index e39b4ac9c472..cfa758593559 100644 --- a/clippy_lints/src/fn_param_ref_cloned.rs +++ b/clippy_lints/src/fn_param_ref_cloned.rs @@ -1,4 +1,4 @@ -use clippy_utils::res::MaybeResPath; +use clippy_utils::res::MaybeResPath as _; use clippy_utils::ty::implements_trait; use clippy_utils::visitors::{Descend, for_each_expr}; use rustc_hir::PatKind; From e89fc70629bec6f7e64ae896527f27e338a07519 Mon Sep 17 00:00:00 2001 From: David Manca <1900179+medzernik@users.noreply.github.com> Date: Thu, 13 Aug 2026 17:56:20 +0200 Subject: [PATCH 14/27] Doc comment apply from @blyxyas MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Alejandra González --- clippy_lints/src/fn_param_ref_cloned.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/clippy_lints/src/fn_param_ref_cloned.rs b/clippy_lints/src/fn_param_ref_cloned.rs index cfa758593559..b9f2bc63c51d 100644 --- a/clippy_lints/src/fn_param_ref_cloned.rs +++ b/clippy_lints/src/fn_param_ref_cloned.rs @@ -44,6 +44,7 @@ pub struct FnParamRefCloned { candidates: Vec<(Candidate, CandidateRebinds)>, } +/// Returns true if `ty` is `&T` where `T` implements any trait in `must_impl_trait` pub fn is_candidate_ty<'a>(cx: &LateContext<'a>, ty: Ty<'a>, must_impl_trait: &[DefId]) -> bool { if let Ref(_, ty_ref, mutability) = ty.kind() { must_impl_trait From e9fd465e1594263d8d673af8502c33de3ee98b3c Mon Sep 17 00:00:00 2001 From: David Manca <1900179+medzernik@users.noreply.github.com> Date: Thu, 13 Aug 2026 17:56:55 +0200 Subject: [PATCH 15/27] Simplify logic by @blyxyas MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Alejandra González --- clippy_lints/src/fn_param_ref_cloned.rs | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/clippy_lints/src/fn_param_ref_cloned.rs b/clippy_lints/src/fn_param_ref_cloned.rs index b9f2bc63c51d..937c5341cecf 100644 --- a/clippy_lints/src/fn_param_ref_cloned.rs +++ b/clippy_lints/src/fn_param_ref_cloned.rs @@ -57,12 +57,11 @@ pub fn is_candidate_ty<'a>(cx: &LateContext<'a>, ty: Ty<'a>, must_impl_trait: &[ } pub fn get_param_id_span(param: &rustc_hir::Param<'_>) -> Option<(rustc_hir::HirId, Span)> { - if let PatKind::Binding(_, hir_id, ident, _) = param.pat.kind { - if !ident.span.from_expansion() && !ident.is_reserved() { - Some((hir_id, param.ty_span)) - } else { - None - } + if let PatKind::Binding(_, hir_id, ident, _) = param.pat.kind + && !ident.span.from_expansion() + && !ident.is_reserved() + { + Some((hir_id, param.ty_span)) } else { None } From da54958d9d94214704346a5743af6b71fd565ad9 Mon Sep 17 00:00:00 2001 From: David Manca <1900179+medzernik@users.noreply.github.com> Date: Thu, 13 Aug 2026 17:58:36 +0200 Subject: [PATCH 16/27] eliminate single-letter var names in lambda by @blyxyas MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Alejandra González --- clippy_lints/src/fn_param_ref_cloned.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clippy_lints/src/fn_param_ref_cloned.rs b/clippy_lints/src/fn_param_ref_cloned.rs index 937c5341cecf..b27e0c41271c 100644 --- a/clippy_lints/src/fn_param_ref_cloned.rs +++ b/clippy_lints/src/fn_param_ref_cloned.rs @@ -126,7 +126,7 @@ impl<'tcx> LateLintPass<'tcx> for FnParamRefCloned { } // Look whether the candidates call the `.clone()` method anywhere - _ = for_each_expr(cx.tcx, fn_body.value, move |x| match x.kind { + _ = for_each_expr(cx.tcx, fn_body.value, move |expr| match expr.kind { rustc_hir::ExprKind::If(_, _, _) | rustc_hir::ExprKind::Match(_, _, _) => { ControlFlow::<(), Descend>::Continue(Descend::No) }, From 45e325e23a2c57ccb44381d188701352f11fabdd Mon Sep 17 00:00:00 2001 From: David Manca <1900179+medzernik@users.noreply.github.com> Date: Thu, 13 Aug 2026 18:06:16 +0200 Subject: [PATCH 17/27] improve name for lambda param by @blyxyas MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Alejandra González --- clippy_lints/src/fn_param_ref_cloned.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/clippy_lints/src/fn_param_ref_cloned.rs b/clippy_lints/src/fn_param_ref_cloned.rs index b27e0c41271c..fecb5347a196 100644 --- a/clippy_lints/src/fn_param_ref_cloned.rs +++ b/clippy_lints/src/fn_param_ref_cloned.rs @@ -116,9 +116,9 @@ impl<'tcx> LateLintPass<'tcx> for FnParamRefCloned { && let rustc_hir::ExprKind::Path(qpath) = expr.kind && let Some(hir_id) = qpath.res_local_id() { - self.candidates.iter_mut().for_each(|(cand, relat)| { + self.candidates.iter_mut().for_each(|(cand, rebinds)| { if cand.0 == hir_id { - relat.push((let_stmt.pat.hir_id, let_stmt.span)); + rebinds.push((let_stmt.pat.hir_id, let_stmt.span)); } }); } From e195cecf6f0d2758e31b6549bdf2f00c9daadbcf Mon Sep 17 00:00:00 2001 From: David Manca <1900179+medzernik@users.noreply.github.com> Date: Fri, 14 Aug 2026 17:07:36 +0200 Subject: [PATCH 18/27] Dissolve the type aliases Signed-off-by: David Manca <1900179+medzernik@users.noreply.github.com> --- clippy_lints/src/fn_param_ref_cloned.rs | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/clippy_lints/src/fn_param_ref_cloned.rs b/clippy_lints/src/fn_param_ref_cloned.rs index fecb5347a196..ef754e00aa4e 100644 --- a/clippy_lints/src/fn_param_ref_cloned.rs +++ b/clippy_lints/src/fn_param_ref_cloned.rs @@ -34,14 +34,9 @@ declare_clippy_lint! { impl_lint_pass!(FnParamRefCloned => [FN_PARAM_REF_CLONED]); -type CandidateId = rustc_hir::HirId; -type CandidateSpan = Span; -type Candidate = (CandidateId, CandidateSpan); -type CandidateRebinds = Vec; - #[derive(Default)] pub struct FnParamRefCloned { - candidates: Vec<(Candidate, CandidateRebinds)>, + candidates: Vec<((rustc_hir::HirId, Span), Vec<(rustc_hir::HirId, Span)>)>, } /// Returns true if `ty` is `&T` where `T` implements any trait in `must_impl_trait` From 68ac28cc9a620144a825463d676a1664732dd087 Mon Sep 17 00:00:00 2001 From: David Manca <1900179+medzernik@users.noreply.github.com> Date: Fri, 14 Aug 2026 19:10:22 +0200 Subject: [PATCH 19/27] Apply a `let else` chain per review Signed-off-by: David Manca <1900179+medzernik@users.noreply.github.com> --- clippy_lints/src/fn_param_ref_cloned.rs | 28 +++++++++++++------------ 1 file changed, 15 insertions(+), 13 deletions(-) diff --git a/clippy_lints/src/fn_param_ref_cloned.rs b/clippy_lints/src/fn_param_ref_cloned.rs index ef754e00aa4e..f4543dc9a8c9 100644 --- a/clippy_lints/src/fn_param_ref_cloned.rs +++ b/clippy_lints/src/fn_param_ref_cloned.rs @@ -104,19 +104,21 @@ impl<'tcx> LateLintPass<'tcx> for FnParamRefCloned { .collect(); // Find all rebinds of param values in the function and add them to the original candidates (tuple) - if let rustc_hir::ExprKind::Block(block, _) = fn_body.value.kind { - for statement in block.stmts { - if let rustc_hir::StmtKind::Let(let_stmt) = statement.kind - && let Some(expr) = let_stmt.init - && let rustc_hir::ExprKind::Path(qpath) = expr.kind - && let Some(hir_id) = qpath.res_local_id() - { - self.candidates.iter_mut().for_each(|(cand, rebinds)| { - if cand.0 == hir_id { - rebinds.push((let_stmt.pat.hir_id, let_stmt.span)); - } - }); - } + let rustc_hir::ExprKind::Block(block, _) = fn_body.value.kind else { + return; + }; + + for statement in block.stmts { + if let rustc_hir::StmtKind::Let(let_stmt) = statement.kind + && let Some(expr) = let_stmt.init + && let rustc_hir::ExprKind::Path(qpath) = expr.kind + && let Some(hir_id) = qpath.res_local_id() + { + self.candidates.iter_mut().for_each(|(cand, rebinds)| { + if cand.0 == hir_id { + rebinds.push((let_stmt.pat.hir_id, let_stmt.span)); + } + }); } } From 4478482a7d5fe1eefb48efa8ecc764d7e09b7b29 Mon Sep 17 00:00:00 2001 From: David Manca <1900179+medzernik@users.noreply.github.com> Date: Fri, 14 Aug 2026 19:32:01 +0200 Subject: [PATCH 20/27] extract lint emits into `fn emit_lint()` Signed-off-by: David Manca <1900179+medzernik@users.noreply.github.com> --- clippy_lints/src/fn_param_ref_cloned.rs | 63 ++++++++++++++----------- 1 file changed, 36 insertions(+), 27 deletions(-) diff --git a/clippy_lints/src/fn_param_ref_cloned.rs b/clippy_lints/src/fn_param_ref_cloned.rs index f4543dc9a8c9..4bb9597c1c11 100644 --- a/clippy_lints/src/fn_param_ref_cloned.rs +++ b/clippy_lints/src/fn_param_ref_cloned.rs @@ -134,33 +134,8 @@ impl<'tcx> LateLintPass<'tcx> for FnParamRefCloned { && let Some(hir_id) = qpath.res_local_id() => { self.candidates.iter().for_each(|(original_candidate, rebinds)| { - if original_candidate.0 == hir_id { - clippy_utils::diagnostics::span_lint_and_note( - cx, - FN_PARAM_REF_CLONED, - span, - "function gets a parameter by reference, but you later unconditionally clone it", - Some(original_candidate.1), - "consider passing the reference by value instead", - ); - } - - for rebind in rebinds { - if rebind.0 == hir_id { - clippy_utils::diagnostics::span_lint_and_then( - cx, - FN_PARAM_REF_CLONED, - span, - "function gets a parameter by reference, but you later rebind and unconditionally clone it", - |diag| { - diag - .span_note(rebind.1, "you bind the parameter into a new binding here") - .span_note(original_candidate.1, "the parameter is passed by reference..."); - }, - ); - } - } - }); + emit_lint(cx, span, original_candidate, rebinds, hir_id); + }); ControlFlow::<(), Descend>::Continue(Descend::Yes) }, @@ -168,3 +143,37 @@ impl<'tcx> LateLintPass<'tcx> for FnParamRefCloned { }); } } + +fn emit_lint( + cx: &LateContext<'_>, + span: Span, + original_candidate: &(rustc_hir::HirId, Span), + rebinds: &SmallVec<[(rustc_hir::HirId, Span); 32]>, + hir_id: rustc_hir::HirId, +) { + if original_candidate.0 == hir_id { + clippy_utils::diagnostics::span_lint_and_note( + cx, + FN_PARAM_REF_CLONED, + span, + "function gets a parameter by reference, but you later unconditionally clone it", + Some(original_candidate.1), + "consider passing the reference by value instead", + ); + } + + for rebind in rebinds { + if rebind.0 == hir_id { + clippy_utils::diagnostics::span_lint_and_then( + cx, + FN_PARAM_REF_CLONED, + span, + "function gets a parameter by reference, but you later rebind and unconditionally clone it", + |diag| { + diag.span_note(rebind.1, "you bind the parameter into a new binding here") + .span_note(original_candidate.1, "the parameter is passed by reference..."); + }, + ); + } + } +} From c3882ee5a20c58775175d2214c6ab87f8dbcfc06 Mon Sep 17 00:00:00 2001 From: David Manca <1900179+medzernik@users.noreply.github.com> Date: Fri, 14 Aug 2026 19:45:49 +0200 Subject: [PATCH 21/27] simplify candidates logic Signed-off-by: David Manca <1900179+medzernik@users.noreply.github.com> --- clippy_lints/src/fn_param_ref_cloned.rs | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/clippy_lints/src/fn_param_ref_cloned.rs b/clippy_lints/src/fn_param_ref_cloned.rs index 4bb9597c1c11..35bed20da2b4 100644 --- a/clippy_lints/src/fn_param_ref_cloned.rs +++ b/clippy_lints/src/fn_param_ref_cloned.rs @@ -1,5 +1,5 @@ use clippy_utils::res::MaybeResPath as _; -use clippy_utils::ty::implements_trait; +use clippy_utils::ty::{implements_trait, ty_from_hir_ty}; use clippy_utils::visitors::{Descend, for_each_expr}; use rustc_hir::PatKind; use rustc_lint::{LateContext, LateLintPass}; @@ -67,10 +67,10 @@ impl<'tcx> LateLintPass<'tcx> for FnParamRefCloned { &mut self, cx: &LateContext<'tcx>, fn_kind: rustc_hir::intravisit::FnKind<'tcx>, - _: &'tcx rustc_hir::FnDecl<'tcx>, + fn_decl: &'tcx rustc_hir::FnDecl<'tcx>, fn_body: &'tcx rustc_hir::Body<'tcx>, _: Span, - def_id: rustc_span::def_id::LocalDefId, + _: rustc_span::def_id::LocalDefId, ) { if let rustc_hir::intravisit::FnKind::Closure = fn_kind { return; @@ -84,17 +84,13 @@ impl<'tcx> LateLintPass<'tcx> for FnParamRefCloned { // Get all candidates of params that implement said traits and zip them with function signature // params - self.candidates = cx - .tcx - .fn_sig(def_id) - .instantiate_identity() - .skip_binder() - .inputs() + self.candidates = fn_decl + .inputs .iter() .zip(fn_body.params) .filter_map(|(ty, param)| { if let Some((id, span)) = get_param_id_span(param) - && is_candidate_ty(cx, *ty, &must_impl_trait) + && is_candidate_ty(cx, ty_from_hir_ty(cx, ty), &must_impl_trait) { Some(((id, span), Vec::default())) } else { From f35bacfa876d9b68430af90eecaf42c307822527 Mon Sep 17 00:00:00 2001 From: David Manca <1900179+medzernik@users.noreply.github.com> Date: Sat, 15 Aug 2026 12:19:56 +0200 Subject: [PATCH 22/27] Remove struct since we only use one lintcheck Signed-off-by: David Manca <1900179+medzernik@users.noreply.github.com> --- clippy_lints/src/fn_param_ref_cloned.rs | 17 ++++++++--------- clippy_lints/src/lib.rs | 2 +- 2 files changed, 9 insertions(+), 10 deletions(-) diff --git a/clippy_lints/src/fn_param_ref_cloned.rs b/clippy_lints/src/fn_param_ref_cloned.rs index 35bed20da2b4..c326f540b120 100644 --- a/clippy_lints/src/fn_param_ref_cloned.rs +++ b/clippy_lints/src/fn_param_ref_cloned.rs @@ -4,7 +4,7 @@ use clippy_utils::visitors::{Descend, for_each_expr}; use rustc_hir::PatKind; use rustc_lint::{LateContext, LateLintPass}; use rustc_middle::ty::{Ref, Ty}; -use rustc_session::impl_lint_pass; +use rustc_session::declare_lint_pass; use rustc_span::Span; use rustc_span::def_id::DefId; use std::ops::ControlFlow; @@ -32,12 +32,9 @@ declare_clippy_lint! { "you should pass by value instead of cloning a passed reference" } -impl_lint_pass!(FnParamRefCloned => [FN_PARAM_REF_CLONED]); +declare_lint_pass!(FnParamRefCloned => [FN_PARAM_REF_CLONED]); -#[derive(Default)] -pub struct FnParamRefCloned { - candidates: Vec<((rustc_hir::HirId, Span), Vec<(rustc_hir::HirId, Span)>)>, -} + type Candidates = Vec<((rustc_hir::HirId, Span), Vec<(rustc_hir::HirId, Span)>)>; /// Returns true if `ty` is `&T` where `T` implements any trait in `must_impl_trait` pub fn is_candidate_ty<'a>(cx: &LateContext<'a>, ty: Ty<'a>, must_impl_trait: &[DefId]) -> bool { @@ -72,6 +69,8 @@ impl<'tcx> LateLintPass<'tcx> for FnParamRefCloned { _: Span, _: rustc_span::def_id::LocalDefId, ) { + let mut candidates: Candidates; + if let rustc_hir::intravisit::FnKind::Closure = fn_kind { return; } @@ -84,7 +83,7 @@ impl<'tcx> LateLintPass<'tcx> for FnParamRefCloned { // Get all candidates of params that implement said traits and zip them with function signature // params - self.candidates = fn_decl + candidates = fn_decl .inputs .iter() .zip(fn_body.params) @@ -110,7 +109,7 @@ impl<'tcx> LateLintPass<'tcx> for FnParamRefCloned { && let rustc_hir::ExprKind::Path(qpath) = expr.kind && let Some(hir_id) = qpath.res_local_id() { - self.candidates.iter_mut().for_each(|(cand, rebinds)| { + candidates.iter_mut().for_each(|(cand, rebinds)| { if cand.0 == hir_id { rebinds.push((let_stmt.pat.hir_id, let_stmt.span)); } @@ -129,7 +128,7 @@ impl<'tcx> LateLintPass<'tcx> for FnParamRefCloned { && let rustc_hir::ExprKind::Path(qpath) = receiver.kind && let Some(hir_id) = qpath.res_local_id() => { - self.candidates.iter().for_each(|(original_candidate, rebinds)| { + candidates.iter().for_each(|(original_candidate, rebinds)| { emit_lint(cx, span, original_candidate, rebinds, hir_id); }); ControlFlow::<(), Descend>::Continue(Descend::Yes) diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index fd3cdb42584b..7e8c7b2afb4e 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -872,7 +872,7 @@ rustc_lint::late_lint_methods!( RestWhenDestructuringStruct: rest_when_destructuring_struct::RestWhenDestructuringStruct = rest_when_destructuring_struct::RestWhenDestructuringStruct, BlockScrutinee: block_scrutinee::BlockScrutinee = block_scrutinee::BlockScrutinee, NonnullUncheckedOnBoxPtr: nonnull_unchecked_on_box_ptr::NonnullUncheckedOnBoxPtr = nonnull_unchecked_on_box_ptr::NonnullUncheckedOnBoxPtr::new(conf), - FnParamRefCloned: fn_param_ref_cloned::FnParamRefCloned = fn_param_ref_cloned::FnParamRefCloned::default(), + FnParamRefCloned: fn_param_ref_cloned::FnParamRefCloned = fn_param_ref_cloned::FnParamRefCloned, UnnecessaryNonzeroGet: unnecessary_nonzero_get::UnnecessaryNonzeroGet = unnecessary_nonzero_get::UnnecessaryNonzeroGet::new(conf), NeedlessNonzeroGet: needless_nonzero_get::NeedlessNonzeroGet = needless_nonzero_get::NeedlessNonzeroGet::new(conf), // add late passes here, used by `cargo dev new_lint` From 742a5774d82b40d623615e58fa4f18774732b854 Mon Sep 17 00:00:00 2001 From: David Manca <1900179+medzernik@users.noreply.github.com> Date: Sun, 6 Sep 2026 11:02:30 +0200 Subject: [PATCH 23/27] revert the smallVec idea Signed-off-by: David Manca <1900179+medzernik@users.noreply.github.com> --- clippy_lints/src/fn_param_ref_cloned.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/clippy_lints/src/fn_param_ref_cloned.rs b/clippy_lints/src/fn_param_ref_cloned.rs index c326f540b120..455bb68615de 100644 --- a/clippy_lints/src/fn_param_ref_cloned.rs +++ b/clippy_lints/src/fn_param_ref_cloned.rs @@ -34,7 +34,7 @@ declare_clippy_lint! { declare_lint_pass!(FnParamRefCloned => [FN_PARAM_REF_CLONED]); - type Candidates = Vec<((rustc_hir::HirId, Span), Vec<(rustc_hir::HirId, Span)>)>; +type Candidates = Vec<((rustc_hir::HirId, Span), Vec<(rustc_hir::HirId, Span)>)>; /// Returns true if `ty` is `&T` where `T` implements any trait in `must_impl_trait` pub fn is_candidate_ty<'a>(cx: &LateContext<'a>, ty: Ty<'a>, must_impl_trait: &[DefId]) -> bool { @@ -143,7 +143,7 @@ fn emit_lint( cx: &LateContext<'_>, span: Span, original_candidate: &(rustc_hir::HirId, Span), - rebinds: &SmallVec<[(rustc_hir::HirId, Span); 32]>, + rebinds: &Vec<(rustc_hir::HirId, Span)>, hir_id: rustc_hir::HirId, ) { if original_candidate.0 == hir_id { From db49bcee5540512c54a60aff1273315be84a2677 Mon Sep 17 00:00:00 2001 From: medzernik Date: Thu, 24 Sep 2026 22:06:10 +0200 Subject: [PATCH 24/27] rebase fixes (will squash later) Signed-off-by: medzernik --- CHANGELOG.md | 2 +- clippy_lints/src/fn_param_ref_cloned.rs | 3 +-- clippy_lints/src/lib.rs | 1 - 3 files changed, 2 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c237114eae7f..08fda0d156a7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7068,7 +7068,7 @@ Released 2018-09-13 [`float_equality_without_abs`]: https://rust-lang.github.io/rust-clippy/main/index.html#float_equality_without_abs [`fn_address_comparisons`]: https://rust-lang.github.io/rust-clippy/main/index.html#fn_address_comparisons [`fn_null_check`]: https://rust-lang.github.io/rust-clippy/main/index.html#fn_null_check -[`fn_param_ref_cloned`]: https://rust-lang.github.io/rust-clippy/master/index.html#fn_param_ref_cloned +[`fn_param_ref_cloned`]: https://rust-lang.github.io/rust-clippy/main/index.html#fn_param_ref_cloned [`fn_params_excessive_bools`]: https://rust-lang.github.io/rust-clippy/main/index.html#fn_params_excessive_bools [`fn_to_numeric_cast`]: https://rust-lang.github.io/rust-clippy/main/index.html#fn_to_numeric_cast [`fn_to_numeric_cast_any`]: https://rust-lang.github.io/rust-clippy/main/index.html#fn_to_numeric_cast_any diff --git a/clippy_lints/src/fn_param_ref_cloned.rs b/clippy_lints/src/fn_param_ref_cloned.rs index 455bb68615de..1b50ebb1c15f 100644 --- a/clippy_lints/src/fn_param_ref_cloned.rs +++ b/clippy_lints/src/fn_param_ref_cloned.rs @@ -2,9 +2,8 @@ use clippy_utils::res::MaybeResPath as _; use clippy_utils::ty::{implements_trait, ty_from_hir_ty}; use clippy_utils::visitors::{Descend, for_each_expr}; use rustc_hir::PatKind; -use rustc_lint::{LateContext, LateLintPass}; +use rustc_lint::{LateContext, LateLintPass, declare_lint_pass}; use rustc_middle::ty::{Ref, Ty}; -use rustc_session::declare_lint_pass; use rustc_span::Span; use rustc_span::def_id::DefId; use std::ops::ControlFlow; diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 7e8c7b2afb4e..f6d62e7f4fdb 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -873,7 +873,6 @@ rustc_lint::late_lint_methods!( BlockScrutinee: block_scrutinee::BlockScrutinee = block_scrutinee::BlockScrutinee, NonnullUncheckedOnBoxPtr: nonnull_unchecked_on_box_ptr::NonnullUncheckedOnBoxPtr = nonnull_unchecked_on_box_ptr::NonnullUncheckedOnBoxPtr::new(conf), FnParamRefCloned: fn_param_ref_cloned::FnParamRefCloned = fn_param_ref_cloned::FnParamRefCloned, - UnnecessaryNonzeroGet: unnecessary_nonzero_get::UnnecessaryNonzeroGet = unnecessary_nonzero_get::UnnecessaryNonzeroGet::new(conf), NeedlessNonzeroGet: needless_nonzero_get::NeedlessNonzeroGet = needless_nonzero_get::NeedlessNonzeroGet::new(conf), // add late passes here, used by `cargo dev new_lint` ]] From b4c5cc6a7b78ef78686d2ea9b0b8d1658950b49c Mon Sep 17 00:00:00 2001 From: medzernik Date: Thu, 24 Sep 2026 22:22:20 +0200 Subject: [PATCH 25/27] my code was linted by newer lints Signed-off-by: medzernik --- clippy_lints/src/fn_param_ref_cloned.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/clippy_lints/src/fn_param_ref_cloned.rs b/clippy_lints/src/fn_param_ref_cloned.rs index 1b50ebb1c15f..a56ea5c77970 100644 --- a/clippy_lints/src/fn_param_ref_cloned.rs +++ b/clippy_lints/src/fn_param_ref_cloned.rs @@ -108,11 +108,11 @@ impl<'tcx> LateLintPass<'tcx> for FnParamRefCloned { && let rustc_hir::ExprKind::Path(qpath) = expr.kind && let Some(hir_id) = qpath.res_local_id() { - candidates.iter_mut().for_each(|(cand, rebinds)| { + for (cand, rebinds) in &mut candidates { if cand.0 == hir_id { rebinds.push((let_stmt.pat.hir_id, let_stmt.span)); } - }); + } } } @@ -127,9 +127,9 @@ impl<'tcx> LateLintPass<'tcx> for FnParamRefCloned { && let rustc_hir::ExprKind::Path(qpath) = receiver.kind && let Some(hir_id) = qpath.res_local_id() => { - candidates.iter().for_each(|(original_candidate, rebinds)| { + for (original_candidate, rebinds) in &candidates { emit_lint(cx, span, original_candidate, rebinds, hir_id); - }); + } ControlFlow::<(), Descend>::Continue(Descend::Yes) }, From efd272883f7f4ac8ccdec632e89c48dbd63921e9 Mon Sep 17 00:00:00 2001 From: medzernik Date: Sat, 26 Sep 2026 15:11:38 +0200 Subject: [PATCH 26/27] add better lint example text Signed-off-by: medzernik --- clippy_lints/src/fn_param_ref_cloned.rs | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/clippy_lints/src/fn_param_ref_cloned.rs b/clippy_lints/src/fn_param_ref_cloned.rs index a56ea5c77970..d7bf876ff389 100644 --- a/clippy_lints/src/fn_param_ref_cloned.rs +++ b/clippy_lints/src/fn_param_ref_cloned.rs @@ -9,13 +9,6 @@ use rustc_span::def_id::DefId; use std::ops::ControlFlow; declare_clippy_lint! { - /// ### What it does - /// Checks if a function clones a parameter passed by reference. - /// - /// ### Why is this bad? - /// Caller should decide where to copy and place data. - /// The function should not hide the need of ownership of data. - /// /// ### Example /// ```norun /// #[derive(Clone)] @@ -25,6 +18,20 @@ declare_clippy_lint! { /// let cloned_ref = item.clone(); /// } /// ``` + /// Instead, take it by value, to demand the ownership of the data right away. + /// Or try reworking the function to use another type. + /// ```norun + /// #[derive(Clone)] + /// struct A; + /// + /// pub fn foo_by_value(item: A) { + /// let cloned_ref = item.clone(); + /// } + /// + /// pub fn foo_with_arc(item: &A) { + /// // Turns out we don't actually need ownership! + /// } + /// ``` #[clippy::version = "1.98.0"] pub FN_PARAM_REF_CLONED, pedantic, From 9193dc81a4729166bb52a7f4f40a7331ae2d9ba3 Mon Sep 17 00:00:00 2001 From: medzernik Date: Sat, 26 Sep 2026 16:24:45 +0200 Subject: [PATCH 27/27] added a test; but its not being caught; Should it? Signed-off-by: medzernik --- tests/ui/fn_param_ref_cloned.rs | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/tests/ui/fn_param_ref_cloned.rs b/tests/ui/fn_param_ref_cloned.rs index 01e9277a3928..f7dfd4b07903 100644 --- a/tests/ui/fn_param_ref_cloned.rs +++ b/tests/ui/fn_param_ref_cloned.rs @@ -122,6 +122,20 @@ fn dont_check_if_stmts(clone_ref: &IsClone, if_arg: usize) { //~^ fn_param_ref_cloned } +#[derive(Clone)] +pub struct IsClone2<'a>(&'a IsClone); + +impl<'a> From<&'a IsClone> for IsClone2<'a> { + fn from(value: &'a IsClone) -> Self { + IsClone2(value) + } +} + +fn rebind(y: &IsClone) { + let x: IsClone2 = y.into(); + let z = x.clone(); +} + fn main() { let a = IsClone; let b = IsClone; @@ -138,4 +152,5 @@ fn main() { set_cell(&h); create_cell(); dont_check_if_stmts(&a, 0usize); + rebind(&a); }