From be5707ce1b608f607ca9327537076dc95869347b Mon Sep 17 00:00:00 2001 From: y21 <30553356+y21@users.noreply.github.com> Date: Sat, 13 Jan 2024 17:46:46 +0100 Subject: [PATCH] lint on `.map(|&x| x.clone())` --- clippy_lints/src/methods/useless_asref.rs | 32 +++++++---------------- tests/ui/useless_asref.fixed | 5 ++++ tests/ui/useless_asref.rs | 5 ++++ tests/ui/useless_asref.stderr | 8 +++++- 4 files changed, 27 insertions(+), 23 deletions(-) diff --git a/clippy_lints/src/methods/useless_asref.rs b/clippy_lints/src/methods/useless_asref.rs index d6c79db4d70e..514015af0455 100644 --- a/clippy_lints/src/methods/useless_asref.rs +++ b/clippy_lints/src/methods/useless_asref.rs @@ -1,7 +1,9 @@ use clippy_utils::diagnostics::span_lint_and_sugg; use clippy_utils::source::snippet_with_applicability; use clippy_utils::ty::walk_ptrs_ty_depth; -use clippy_utils::{get_parent_expr, is_diag_trait_item, match_def_path, path_to_local_id, paths, peel_blocks}; +use clippy_utils::{ + get_parent_expr, is_diag_trait_item, match_def_path, path_to_local_id, paths, peel_blocks, strip_pat_refs, +}; use rustc_errors::Applicability; use rustc_hir as hir; use rustc_lint::LateContext; @@ -108,26 +110,12 @@ fn check_qpath(cx: &LateContext<'_>, qpath: hir::QPath<'_>, hir_id: hir::HirId) fn is_calling_clone(cx: &LateContext<'_>, arg: &hir::Expr<'_>) -> bool { match arg.kind { - hir::ExprKind::Closure(&hir::Closure { body, .. }) => { + hir::ExprKind::Closure(&hir::Closure { body, .. }) // If it's a closure, we need to check what is called. - let closure_body = cx.tcx.hir().body(body); - - // |x| ... - // ^ - let [ - hir::Param { - pat: - hir::Pat { - kind: hir::PatKind::Binding(_, local_id, ..), - .. - }, - .. - }, - ] = closure_body.params - else { - return false; - }; - + if let closure_body = cx.tcx.hir().body(body) + && let [param] = closure_body.params + && let hir::PatKind::Binding(_, local_id, ..) = strip_pat_refs(param.pat).kind => + { let closure_expr = peel_blocks(closure_body.value); match closure_expr.kind { hir::ExprKind::MethodCall(method, obj, [], _) => { @@ -139,7 +127,7 @@ fn is_calling_clone(cx: &LateContext<'_>, arg: &hir::Expr<'_>) -> bool { // no autoderefs && !cx.typeck_results().expr_adjustments(obj).iter() .any(|a| matches!(a.kind, Adjust::Deref(Some(..)))) - && path_to_local_id(obj, *local_id) + && path_to_local_id(obj, local_id) { true } else { @@ -148,7 +136,7 @@ fn is_calling_clone(cx: &LateContext<'_>, arg: &hir::Expr<'_>) -> bool { }, hir::ExprKind::Call(call, [recv]) => { if let hir::ExprKind::Path(qpath) = call.kind - && path_to_local_id(recv, *local_id) + && path_to_local_id(recv, local_id) { check_qpath(cx, qpath, call.hir_id) } else { diff --git a/tests/ui/useless_asref.fixed b/tests/ui/useless_asref.fixed index d39d8a74b265..c98f2928e03c 100644 --- a/tests/ui/useless_asref.fixed +++ b/tests/ui/useless_asref.fixed @@ -171,6 +171,11 @@ mod issue12135 { x.field.clone(); //~^ ERROR: this call to `as_ref.map(...)` does nothing + // https://github.com/rust-lang/rust-clippy/pull/12136#discussion_r1451565223 + #[allow(clippy::clone_on_copy)] + Some(1).clone(); + //~^ ERROR: this call to `as_ref.map(...)` does nothing + x.field.as_ref().map(|v| v.method().clone()) } } diff --git a/tests/ui/useless_asref.rs b/tests/ui/useless_asref.rs index 76277e7b7a47..f9d603f116de 100644 --- a/tests/ui/useless_asref.rs +++ b/tests/ui/useless_asref.rs @@ -171,6 +171,11 @@ mod issue12135 { x.field.as_ref().map(|v| Clone::clone(v)); //~^ ERROR: this call to `as_ref.map(...)` does nothing + // https://github.com/rust-lang/rust-clippy/pull/12136#discussion_r1451565223 + #[allow(clippy::clone_on_copy)] + Some(1).as_ref().map(|&x| x.clone()); + //~^ ERROR: this call to `as_ref.map(...)` does nothing + x.field.as_ref().map(|v| v.method().clone()) } } diff --git a/tests/ui/useless_asref.stderr b/tests/ui/useless_asref.stderr index 583b48732d43..e158df2664d7 100644 --- a/tests/ui/useless_asref.stderr +++ b/tests/ui/useless_asref.stderr @@ -106,5 +106,11 @@ error: this call to `as_ref.map(...)` does nothing LL | x.field.as_ref().map(|v| Clone::clone(v)); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `x.field.clone()` -error: aborting due to 17 previous errors +error: this call to `as_ref.map(...)` does nothing + --> $DIR/useless_asref.rs:176:9 + | +LL | Some(1).as_ref().map(|&x| x.clone()); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `Some(1).clone()` + +error: aborting due to 18 previous errors