Add fn_param_ref_cloned lint

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`]: 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

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,

clippy_lints/src/fn_param_ref_cloned.rs

@@ -0,0 +1,180 @@
+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, declare_lint_pass};
+use rustc_middle::ty::{Ref, Ty};
+use rustc_span::Span;
+use rustc_span::def_id::DefId;
+use std::ops::ControlFlow;
+
+declare_clippy_lint! {
+    /// ### Example
+    /// ```norun
+    /// #[derive(Clone)]
+    /// struct A;
+    ///
+    /// pub fn foo(item: &A) {
+    ///     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,
+    "you should pass by value instead of cloning a passed reference"
+}
+
+declare_lint_pass!(FnParamRefCloned => [FN_PARAM_REF_CLONED]);
+
+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 {
+    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
+    }
+}
+
+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
+        && !ident.span.from_expansion()
+        && !ident.is_reserved()
+    {
+        Some((hir_id, param.ty_span))
+    } else {
+        None
+    }
+}
+
+impl<'tcx> LateLintPass<'tcx> for FnParamRefCloned {
+    fn check_fn(
+        &mut self,
+        cx: &LateContext<'tcx>,
+        fn_kind: rustc_hir::intravisit::FnKind<'tcx>,
+        fn_decl: &'tcx rustc_hir::FnDecl<'tcx>,
+        fn_body: &'tcx rustc_hir::Body<'tcx>,
+        _: Span,
+        _: rustc_span::def_id::LocalDefId,
+    ) {
+        let mut candidates: Candidates;
+
+        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(),
+            cx.tcx.lang_items().drop_trait().unwrap(),
+        ];
+
+        // Get all candidates of params that implement said traits and zip them with function signature
+        // params
+        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_from_hir_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)
+        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()
+            {
+                for (cand, rebinds) in &mut candidates {
+                    if cand.0 == hir_id {
+                        rebinds.push((let_stmt.pat.hir_id, let_stmt.span));
+                    }
+                }
+            }
+        }
+
+        // Look whether the candidates call the `.clone()` method anywhere
+        _ = 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)
+            },
+            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() =>
+            {
+                for (original_candidate, rebinds) in &candidates {
+                    emit_lint(cx, span, original_candidate, rebinds, hir_id);
+                }
+                ControlFlow::<(), Descend>::Continue(Descend::Yes)
+            },
+
+            _ => ControlFlow::<(), Descend>::Continue(Descend::Yes),
+        });
+    }
+}
+
+fn emit_lint(
+    cx: &LateContext<'_>,
+    span: Span,
+    original_candidate: &(rustc_hir::HirId, Span),
+    rebinds: &Vec<(rustc_hir::HirId, Span)>,
+    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...");
+                },
+            );
+        }
+    }
+}

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;
@@ -871,6 +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,
         NeedlessNonzeroGet: needless_nonzero_get::NeedlessNonzeroGet = needless_nonzero_get::NeedlessNonzeroGet::new(conf),
         // add late passes here, used by `cargo dev new_lint`
     ]]

clippy_lints/src/unnested_or_patterns.rs

@@ -62,40 +62,40 @@ 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());
         }
     }
 
     fn check_expr(&mut self, cx: &EarlyContext<'_>, e: &ast::Expr) {
         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);