about summary refs log tree commit diff
diff options
context:
space:
mode:
authorbors <bors@rust-lang.org>2022-01-14 22:17:29 +0000
committerbors <bors@rust-lang.org>2022-01-14 22:17:29 +0000
commit7a4acf9fa6ad31309a855132c4b9ff80f39bed65 (patch)
tree6559c4f0021f69edf4afe18e8f8bb27e057d08c3
parent5cada57f306ee47ee4e9a78008798d734ff7b9d7 (diff)
parentad95279c34272949d6babc31d54209434a97f46e (diff)
downloadrust-7a4acf9fa6ad31309a855132c4b9ff80f39bed65.tar.gz
rust-7a4acf9fa6ad31309a855132c4b9ff80f39bed65.zip
Auto merge of #8231 - Jarcho:implicit_clone_8227, r=camsteffen
Fix `implicit_clone` for `&&T`

fixes #8227

changelog: Don't lint `implicit_clone` on `&&T`
-rw-r--r--clippy_lints/src/methods/implicit_clone.rs21
-rw-r--r--clippy_lints/src/methods/mod.rs2
-rw-r--r--tests/ui/implicit_clone.rs9
-rw-r--r--tests/ui/implicit_clone.stderr54
4 files changed, 58 insertions, 28 deletions
diff --git a/clippy_lints/src/methods/implicit_clone.rs b/clippy_lints/src/methods/implicit_clone.rs
index 90492ffda3c..865e7702b71 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 1041f644e32..a3cfdd3d4ba 100644
--- a/clippy_lints/src/methods/mod.rs
+++ b/clippy_lints/src/methods/mod.rs
@@ -2338,7 +2338,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 cef71cf79d7..639fecb8927 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 e6f7527b672..0f412424190 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