Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix clone_on_copy false positives #4411

Merged
merged 1 commit into from Aug 19, 2019
Merged

Fix clone_on_copy false positives #4411

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 20 additions & 20 deletions clippy_lints/src/methods/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1521,30 +1521,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
View file
Open in desktop
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
View file
Open in desktop
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