From a2be0509657c4b100ba9b81b34aa0262700da83c Mon Sep 17 00:00:00 2001 From: HMPerson1 Date: Fri, 19 Oct 2018 00:03:56 -0400 Subject: [PATCH] Fix `clone_on_copy` not detecting derefs sometimes --- clippy_lints/src/methods/mod.rs | 3 +- tests/ui/unnecessary_clone.rs | 4 +++ tests/ui/unnecessary_clone.stderr | 58 +++++++++++++++++-------------- 3 files changed, 38 insertions(+), 27 deletions(-) diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs index 6d22abf2d33..a0d57e0916b 100644 --- a/clippy_lints/src/methods/mod.rs +++ b/clippy_lints/src/methods/mod.rs @@ -1250,7 +1250,8 @@ fn lint_clone_on_copy(cx: &LateContext<'_, '_>, expr: &hir::Expr, arg: &hir::Exp if is_copy(cx, ty) { let snip; if let Some(snippet) = sugg::Sugg::hir_opt(cx, arg) { - if let ty::Ref(..) = cx.tables.expr_ty(arg).sty { + // x.clone() might have dereferenced x, possibly through a Deref impl + if cx.tables.expr_ty(arg) != ty { let parent = cx.tcx.hir.get_parent_node(expr.id); match cx.tcx.hir.get(parent) { hir::Node::Expr(parent) => match parent.node { diff --git a/tests/ui/unnecessary_clone.rs b/tests/ui/unnecessary_clone.rs index df02570d692..2dd2213e138 100644 --- a/tests/ui/unnecessary_clone.rs +++ b/tests/ui/unnecessary_clone.rs @@ -13,6 +13,7 @@ #![warn(clippy::clone_on_ref_ptr)] #![allow(unused)] +use std::cell::RefCell; use std::collections::HashSet; use std::collections::VecDeque; use std::rc::{self, Rc}; @@ -30,6 +31,9 @@ fn clone_on_copy() { vec![1].clone(); // ok, not a Copy type Some(vec![1]).clone(); // ok, not a Copy type (&42).clone(); + + let rc = RefCell::new(0); + rc.borrow().clone(); } fn clone_on_ref_ptr() { diff --git a/tests/ui/unnecessary_clone.stderr b/tests/ui/unnecessary_clone.stderr index 051fc1fcdf9..63e6f3d8bd5 100644 --- a/tests/ui/unnecessary_clone.stderr +++ b/tests/ui/unnecessary_clone.stderr @@ -1,84 +1,90 @@ error: using `clone` on a `Copy` type - --> $DIR/unnecessary_clone.rs:28:5 + --> $DIR/unnecessary_clone.rs:29:5 | -28 | 42.clone(); +29 | 42.clone(); | ^^^^^^^^^^ help: try removing the `clone` call: `42` | = note: `-D clippy::clone-on-copy` implied by `-D warnings` error: using `clone` on a `Copy` type - --> $DIR/unnecessary_clone.rs:32:5 + --> $DIR/unnecessary_clone.rs:33:5 | -32 | (&42).clone(); +33 | (&42).clone(); | ^^^^^^^^^^^^^ help: try dereferencing it: `*(&42)` -error: using '.clone()' on a ref-counted pointer - --> $DIR/unnecessary_clone.rs:42:5 +error: using `clone` on a `Copy` type + --> $DIR/unnecessary_clone.rs:36:5 | -42 | rc.clone(); +36 | rc.borrow().clone(); + | ^^^^^^^^^^^^^^^^^^^ help: try dereferencing it: `*rc.borrow()` + +error: using '.clone()' on a ref-counted pointer + --> $DIR/unnecessary_clone.rs:46:5 + | +46 | rc.clone(); | ^^^^^^^^^^ help: try this: `Rc::::clone(&rc)` | = note: `-D clippy::clone-on-ref-ptr` implied by `-D warnings` error: using '.clone()' on a ref-counted pointer - --> $DIR/unnecessary_clone.rs:45:5 + --> $DIR/unnecessary_clone.rs:49:5 | -45 | arc.clone(); +49 | arc.clone(); | ^^^^^^^^^^^ help: try this: `Arc::::clone(&arc)` error: using '.clone()' on a ref-counted pointer - --> $DIR/unnecessary_clone.rs:48:5 + --> $DIR/unnecessary_clone.rs:52:5 | -48 | rcweak.clone(); +52 | rcweak.clone(); | ^^^^^^^^^^^^^^ help: try this: `Weak::::clone(&rcweak)` error: using '.clone()' on a ref-counted pointer - --> $DIR/unnecessary_clone.rs:51:5 + --> $DIR/unnecessary_clone.rs:55:5 | -51 | arc_weak.clone(); +55 | arc_weak.clone(); | ^^^^^^^^^^^^^^^^ help: try this: `Weak::::clone(&arc_weak)` error: using '.clone()' on a ref-counted pointer - --> $DIR/unnecessary_clone.rs:55:29 + --> $DIR/unnecessary_clone.rs:59:29 | -55 | let _: Arc = x.clone(); +59 | let _: Arc = x.clone(); | ^^^^^^^^^ help: try this: `Arc::::clone(&x)` error: using `clone` on a `Copy` type - --> $DIR/unnecessary_clone.rs:59:5 + --> $DIR/unnecessary_clone.rs:63:5 | -59 | t.clone(); +63 | t.clone(); | ^^^^^^^^^ help: try removing the `clone` call: `t` error: using `clone` on a `Copy` type - --> $DIR/unnecessary_clone.rs:61:5 + --> $DIR/unnecessary_clone.rs:65:5 | -61 | Some(t).clone(); +65 | Some(t).clone(); | ^^^^^^^^^^^^^^^ help: try removing the `clone` call: `Some(t)` error: using `clone` on a double-reference; this will copy the reference instead of cloning the inner type - --> $DIR/unnecessary_clone.rs:67:22 + --> $DIR/unnecessary_clone.rs:71:22 | -67 | let z: &Vec<_> = y.clone(); +71 | let z: &Vec<_> = y.clone(); | ^^^^^^^^^ | = note: #[deny(clippy::clone_double_ref)] on by default help: try dereferencing it | -67 | let z: &Vec<_> = &(*y).clone(); +71 | let z: &Vec<_> = &(*y).clone(); | ^^^^^^^^^^^^^ help: or try being explicit about what type to clone | -67 | let z: &Vec<_> = &std::vec::Vec::clone(y); +71 | let z: &Vec<_> = &std::vec::Vec::clone(y); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: called `cloned().collect()` on a slice to create a `Vec`. Calling `to_vec()` is both faster and more readable - --> $DIR/unnecessary_clone.rs:74:27 + --> $DIR/unnecessary_clone.rs:78:27 | -74 | let v2 : Vec = v.iter().cloned().collect(); +78 | let v2 : Vec = v.iter().cloned().collect(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ | = note: `-D clippy::iter-cloned-collect` implied by `-D warnings` -error: aborting due to 11 previous errors +error: aborting due to 12 previous errors