Skip to content

Commit

Permalink
Auto merge of #4411 - mikerite:fix-4384, r=flip1995
Browse files Browse the repository at this point in the history
Fix `clone_on_copy` false positives

Closes #4384

changelog: Fix `clone_on_copy` false positives
  • Loading branch information
bors committed Aug 19, 2019
2 parents d1f1844 + 68a1af5 commit 5c71c1b
Show file tree
Hide file tree
Showing 3 changed files with 37 additions and 32 deletions.
40 changes: 20 additions & 20 deletions clippy_lints/src/methods/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1549,30 +1549,30 @@ 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) {
let parent = cx.tcx.hir().get_parent_node(expr.hir_id);
match &cx.tcx.hir().get(parent) {
hir::Node::Expr(parent) => match parent.node {
// &*x is a nop, &x.clone() is not
hir::ExprKind::AddrOf(..) |
// (*x).func() is useless, x.clone().func() can work in case func borrows mutably
hir::ExprKind::MethodCall(..) => return,
_ => {},
},
hir::Node::Stmt(stmt) => {
if let hir::StmtKind::Local(ref loc) = stmt.node {
if let hir::PatKind::Ref(..) = loc.pat.node {
// let ref y = *x borrows x, let ref y = x.clone() does not
return;
}
}
},
_ => {},
}

// x.clone() might have dereferenced x, possibly through Deref impls
if cx.tables.expr_ty(arg) == ty {
snip = Some(("try removing the `clone` call", format!("{}", snippet)));
} else {
let parent = cx.tcx.hir().get_parent_node(expr.hir_id);
match cx.tcx.hir().get(parent) {
hir::Node::Expr(parent) => match parent.node {
// &*x is a nop, &x.clone() is not
hir::ExprKind::AddrOf(..) |
// (*x).func() is useless, x.clone().func() can work in case func borrows mutably
hir::ExprKind::MethodCall(..) => return,
_ => {},
},
hir::Node::Stmt(stmt) => {
if let hir::StmtKind::Local(ref loc) = stmt.node {
if let hir::PatKind::Ref(..) = loc.pat.node {
// let ref y = *x borrows x, let ref y = x.clone() does not
return;
}
}
},
_ => {},
}

let deref_count = cx
.tables
.expr_adjustments(arg)
Expand Down
5 changes: 5 additions & 0 deletions tests/ui/unnecessary_clone.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,11 @@ fn clone_on_copy() {

let rc = RefCell::new(0);
rc.borrow().clone();

// Issue #4348
let mut x = 43;
let _ = &x.clone(); // ok, getting a ref
'a'.clone().make_ascii_uppercase(); // ok, clone and then mutate
}

fn clone_on_ref_ptr() {
Expand Down
24 changes: 12 additions & 12 deletions tests/ui/unnecessary_clone.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -19,51 +19,51 @@ LL | rc.borrow().clone();
| ^^^^^^^^^^^^^^^^^^^ help: try dereferencing it: `*rc.borrow()`

error: using '.clone()' on a ref-counted pointer
--> $DIR/unnecessary_clone.rs:34:5
--> $DIR/unnecessary_clone.rs:39:5
|
LL | rc.clone();
| ^^^^^^^^^^ help: try this: `Rc::<bool>::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:37:5
--> $DIR/unnecessary_clone.rs:42:5
|
LL | arc.clone();
| ^^^^^^^^^^^ help: try this: `Arc::<bool>::clone(&arc)`

error: using '.clone()' on a ref-counted pointer
--> $DIR/unnecessary_clone.rs:40:5
--> $DIR/unnecessary_clone.rs:45:5
|
LL | rcweak.clone();
| ^^^^^^^^^^^^^^ help: try this: `Weak::<bool>::clone(&rcweak)`

error: using '.clone()' on a ref-counted pointer
--> $DIR/unnecessary_clone.rs:43:5
--> $DIR/unnecessary_clone.rs:48:5
|
LL | arc_weak.clone();
| ^^^^^^^^^^^^^^^^ help: try this: `Weak::<bool>::clone(&arc_weak)`

error: using '.clone()' on a ref-counted pointer
--> $DIR/unnecessary_clone.rs:47:33
--> $DIR/unnecessary_clone.rs:52:33
|
LL | let _: Arc<dyn SomeTrait> = x.clone();
| ^^^^^^^^^ help: try this: `Arc::<SomeImpl>::clone(&x)`

error: using `clone` on a `Copy` type
--> $DIR/unnecessary_clone.rs:51:5
--> $DIR/unnecessary_clone.rs:56:5
|
LL | t.clone();
| ^^^^^^^^^ help: try removing the `clone` call: `t`

error: using `clone` on a `Copy` type
--> $DIR/unnecessary_clone.rs:53:5
--> $DIR/unnecessary_clone.rs:58:5
|
LL | 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:59:22
--> $DIR/unnecessary_clone.rs:64:22
|
LL | let z: &Vec<_> = y.clone();
| ^^^^^^^^^
Expand All @@ -79,21 +79,21 @@ LL | let z: &Vec<_> = &std::vec::Vec<i32>::clone(y);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error: called `iter().cloned().collect()` on a slice to create a `Vec`. Calling `to_vec()` is both faster and more readable
--> $DIR/unnecessary_clone.rs:66:27
--> $DIR/unnecessary_clone.rs:71:27
|
LL | let v2: Vec<isize> = v.iter().cloned().collect();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `.to_vec()`
|
= note: `-D clippy::iter-cloned-collect` implied by `-D warnings`

error: called `iter().cloned().collect()` on a slice to create a `Vec`. Calling `to_vec()` is both faster and more readable
--> $DIR/unnecessary_clone.rs:71:38
--> $DIR/unnecessary_clone.rs:76:38
|
LL | let _: Vec<isize> = vec![1, 2, 3].iter().cloned().collect();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `.to_vec()`

error: called `iter().cloned().collect()` on a slice to create a `Vec`. Calling `to_vec()` is both faster and more readable
--> $DIR/unnecessary_clone.rs:76:24
--> $DIR/unnecessary_clone.rs:81:24
|
LL | .to_bytes()
| ________________________^
Expand All @@ -103,7 +103,7 @@ LL | | .collect();
| |______________________^ help: try: `.to_vec()`

error: using `clone` on a `Copy` type
--> $DIR/unnecessary_clone.rs:114:20
--> $DIR/unnecessary_clone.rs:119:20
|
LL | let _: E = a.clone();
| ^^^^^^^^^ help: try dereferencing it: `*****a`
Expand Down

0 comments on commit 5c71c1b

Please sign in to comment.