diff --git a/clippy_lints/src/methods/implicit_clone.rs b/clippy_lints/src/methods/implicit_clone.rs index 90492ffda3cc..865e7702b715 100644 --- a/clippy_lints/src/methods/implicit_clone.rs +++ b/clippy_lints/src/methods/implicit_clone.rs @@ -1,31 +1,40 @@ use clippy_utils::diagnostics::span_lint_and_sugg; +use clippy_utils::source::snippet_with_context; +use clippy_utils::ty::peel_mid_ty_refs; use clippy_utils::{is_diag_item_method, is_diag_trait_item}; use if_chain::if_chain; use rustc_errors::Applicability; use rustc_hir as hir; use rustc_lint::LateContext; use rustc_middle::ty::TyS; -use rustc_span::{sym, Span}; +use rustc_span::sym; use super::IMPLICIT_CLONE; -pub fn check(cx: &LateContext<'_>, method_name: &str, expr: &hir::Expr<'_>, recv: &hir::Expr<'_>, span: Span) { +pub fn check(cx: &LateContext<'_>, method_name: &str, expr: &hir::Expr<'_>, recv: &hir::Expr<'_>) { if_chain! { if let Some(method_def_id) = cx.typeck_results().type_dependent_def_id(expr.hir_id); if is_clone_like(cx, method_name, method_def_id); let return_type = cx.typeck_results().expr_ty(expr); - let input_type = cx.typeck_results().expr_ty(recv).peel_refs(); + let input_type = cx.typeck_results().expr_ty(recv); + let (input_type, ref_count) = peel_mid_ty_refs(input_type); if let Some(ty_name) = input_type.ty_adt_def().map(|adt_def| cx.tcx.item_name(adt_def.did)); if TyS::same_type(return_type, input_type); then { + let mut app = Applicability::MachineApplicable; + let recv_snip = snippet_with_context(cx, recv.span, expr.span.ctxt(), "..", &mut app).0; span_lint_and_sugg( cx, IMPLICIT_CLONE, - span, + expr.span, &format!("implicitly cloning a `{}` by calling `{}` on its dereferenced type", ty_name, method_name), "consider using", - "clone".to_string(), - Applicability::MachineApplicable + if ref_count > 1 { + format!("({}{}).clone()", "*".repeat(ref_count - 1), recv_snip) + } else { + format!("{}.clone()", recv_snip) + }, + app, ); } } diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs index 8a2a468c8523..ae5bfadbc5f6 100644 --- a/clippy_lints/src/methods/mod.rs +++ b/clippy_lints/src/methods/mod.rs @@ -2342,7 +2342,7 @@ fn check_methods<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, msrv: Optio }, ("step_by", [arg]) => iterator_step_by_zero::check(cx, expr, arg), ("to_os_string" | "to_owned" | "to_path_buf" | "to_vec", []) => { - implicit_clone::check(cx, name, expr, recv, span); + implicit_clone::check(cx, name, expr, recv); }, ("unwrap", []) => match method_call!(recv) { Some(("get", [recv, get_arg], _)) => get_unwrap::check(cx, expr, recv, get_arg, false), diff --git a/tests/ui/implicit_clone.rs b/tests/ui/implicit_clone.rs index cef71cf79d79..639fecb8927b 100644 --- a/tests/ui/implicit_clone.rs +++ b/tests/ui/implicit_clone.rs @@ -105,4 +105,13 @@ fn main() { let os_str = OsStr::new("foo"); let _ = os_str.to_owned(); let _ = os_str.to_os_string(); + + // issue #8227 + let pathbuf_ref = &pathbuf; + let pathbuf_ref = &pathbuf_ref; + let _ = pathbuf_ref.to_owned(); // Don't lint. Returns `&PathBuf` + let _ = pathbuf_ref.to_path_buf(); + let pathbuf_ref = &pathbuf_ref; + let _ = pathbuf_ref.to_owned(); // Don't lint. Returns `&&PathBuf` + let _ = pathbuf_ref.to_path_buf(); } diff --git a/tests/ui/implicit_clone.stderr b/tests/ui/implicit_clone.stderr index e6f7527b6721..0f4124241907 100644 --- a/tests/ui/implicit_clone.stderr +++ b/tests/ui/implicit_clone.stderr @@ -1,64 +1,76 @@ error: implicitly cloning a `Vec` by calling `to_owned` on its dereferenced type - --> $DIR/implicit_clone.rs:65:17 + --> $DIR/implicit_clone.rs:65:13 | LL | let _ = vec.to_owned(); - | ^^^^^^^^ help: consider using: `clone` + | ^^^^^^^^^^^^^^ help: consider using: `vec.clone()` | = note: `-D clippy::implicit-clone` implied by `-D warnings` error: implicitly cloning a `Vec` by calling `to_vec` on its dereferenced type - --> $DIR/implicit_clone.rs:66:17 + --> $DIR/implicit_clone.rs:66:13 | LL | let _ = vec.to_vec(); - | ^^^^^^ help: consider using: `clone` + | ^^^^^^^^^^^^ help: consider using: `vec.clone()` error: implicitly cloning a `Vec` by calling `to_owned` on its dereferenced type - --> $DIR/implicit_clone.rs:70:21 + --> $DIR/implicit_clone.rs:70:13 | LL | let _ = vec_ref.to_owned(); - | ^^^^^^^^ help: consider using: `clone` + | ^^^^^^^^^^^^^^^^^^ help: consider using: `vec_ref.clone()` error: implicitly cloning a `Vec` by calling `to_vec` on its dereferenced type - --> $DIR/implicit_clone.rs:71:21 + --> $DIR/implicit_clone.rs:71:13 | LL | let _ = vec_ref.to_vec(); - | ^^^^^^ help: consider using: `clone` + | ^^^^^^^^^^^^^^^^ help: consider using: `vec_ref.clone()` error: implicitly cloning a `String` by calling `to_owned` on its dereferenced type - --> $DIR/implicit_clone.rs:83:17 + --> $DIR/implicit_clone.rs:83:13 | LL | let _ = str.to_owned(); - | ^^^^^^^^ help: consider using: `clone` + | ^^^^^^^^^^^^^^ help: consider using: `str.clone()` error: implicitly cloning a `Kitten` by calling `to_owned` on its dereferenced type - --> $DIR/implicit_clone.rs:87:20 + --> $DIR/implicit_clone.rs:87:13 | LL | let _ = kitten.to_owned(); - | ^^^^^^^^ help: consider using: `clone` + | ^^^^^^^^^^^^^^^^^ help: consider using: `kitten.clone()` error: implicitly cloning a `PathBuf` by calling `to_owned` on its dereferenced type - --> $DIR/implicit_clone.rs:97:21 + --> $DIR/implicit_clone.rs:97:13 | LL | let _ = pathbuf.to_owned(); - | ^^^^^^^^ help: consider using: `clone` + | ^^^^^^^^^^^^^^^^^^ help: consider using: `pathbuf.clone()` error: implicitly cloning a `PathBuf` by calling `to_path_buf` on its dereferenced type - --> $DIR/implicit_clone.rs:98:21 + --> $DIR/implicit_clone.rs:98:13 | LL | let _ = pathbuf.to_path_buf(); - | ^^^^^^^^^^^ help: consider using: `clone` + | ^^^^^^^^^^^^^^^^^^^^^ help: consider using: `pathbuf.clone()` error: implicitly cloning a `OsString` by calling `to_owned` on its dereferenced type - --> $DIR/implicit_clone.rs:101:23 + --> $DIR/implicit_clone.rs:101:13 | LL | let _ = os_string.to_owned(); - | ^^^^^^^^ help: consider using: `clone` + | ^^^^^^^^^^^^^^^^^^^^ help: consider using: `os_string.clone()` error: implicitly cloning a `OsString` by calling `to_os_string` on its dereferenced type - --> $DIR/implicit_clone.rs:102:23 + --> $DIR/implicit_clone.rs:102:13 | LL | let _ = os_string.to_os_string(); - | ^^^^^^^^^^^^ help: consider using: `clone` + | ^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using: `os_string.clone()` -error: aborting due to 10 previous errors +error: implicitly cloning a `PathBuf` by calling `to_path_buf` on its dereferenced type + --> $DIR/implicit_clone.rs:113:13 + | +LL | let _ = pathbuf_ref.to_path_buf(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using: `(*pathbuf_ref).clone()` + +error: implicitly cloning a `PathBuf` by calling `to_path_buf` on its dereferenced type + --> $DIR/implicit_clone.rs:116:13 + | +LL | let _ = pathbuf_ref.to_path_buf(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using: `(**pathbuf_ref).clone()` + +error: aborting due to 12 previous errors