From ad2c65bd1bd15b6a941a8d697fd0297e7557d927 Mon Sep 17 00:00:00 2001 From: Manish Goregaokar Date: Mon, 15 Apr 2019 14:39:41 -0700 Subject: [PATCH] Also suggest .copied() when .clone() is called on a Copy type --- clippy_lints/src/map_clone.rs | 7 ++++--- tests/ui/map_clone.fixed | 1 + tests/ui/map_clone.rs | 1 + tests/ui/map_clone.stderr | 10 ++++++++-- 4 files changed, 14 insertions(+), 5 deletions(-) diff --git a/clippy_lints/src/map_clone.rs b/clippy_lints/src/map_clone.rs index 27b5a2968b8e..cb504e651e47 100644 --- a/clippy_lints/src/map_clone.rs +++ b/clippy_lints/src/map_clone.rs @@ -1,6 +1,6 @@ use crate::utils::paths; use crate::utils::{ - in_macro, match_trait_method, match_type, remove_blocks, snippet_with_applicability, span_lint_and_sugg, + in_macro, match_trait_method, match_type, remove_blocks, snippet_with_applicability, span_lint_and_sugg, is_copy }; use if_chain::if_chain; use rustc::hir; @@ -88,8 +88,9 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Pass { && match_trait_method(cx, closure_expr, &paths::CLONE_TRAIT) { let obj_ty = cx.tables.expr_ty(&obj[0]); - if let ty::Ref(..) = obj_ty.sty { - lint(cx, e.span, args[0].span, false); + if let ty::Ref(_, ty, _) = obj_ty.sty { + let copy = is_copy(cx, ty); + lint(cx, e.span, args[0].span, copy); } else { lint_needless_cloning(cx, e.span, args[0].span); } diff --git a/tests/ui/map_clone.fixed b/tests/ui/map_clone.fixed index 14d456b14337..4991f010547b 100644 --- a/tests/ui/map_clone.fixed +++ b/tests/ui/map_clone.fixed @@ -11,6 +11,7 @@ fn main() { let _: Vec = vec![String::new()].iter().cloned().collect(); let _: Vec = vec![42, 43].iter().copied().collect(); let _: Option = Some(Box::new(16)).map(|b| *b); + let _: Vec = vec![1; 6].iter().copied().collect(); // Don't lint these let v = vec![5_i8; 6]; diff --git a/tests/ui/map_clone.rs b/tests/ui/map_clone.rs index a0160662ed4f..1f18fcf6398c 100644 --- a/tests/ui/map_clone.rs +++ b/tests/ui/map_clone.rs @@ -11,6 +11,7 @@ fn main() { let _: Vec = vec![String::new()].iter().map(|x| x.clone()).collect(); let _: Vec = vec![42, 43].iter().map(|&x| x).collect(); let _: Option = Some(Box::new(16)).map(|b| *b); + let _: Vec = vec![1; 6].iter().map(|x| x.clone()).collect(); // Don't lint these let v = vec![5_i8; 6]; diff --git a/tests/ui/map_clone.stderr b/tests/ui/map_clone.stderr index 8fe7f05f99f0..b1a165a42532 100644 --- a/tests/ui/map_clone.stderr +++ b/tests/ui/map_clone.stderr @@ -18,11 +18,17 @@ error: You are using an explicit closure for copying elements LL | let _: Vec = vec![42, 43].iter().map(|&x| x).collect(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: Consider calling the dedicated `copied` method: `vec![42, 43].iter().copied()` +error: You are using an explicit closure for copying elements + --> $DIR/map_clone.rs:14:22 + | +LL | let _: Vec = vec![1; 6].iter().map(|x| x.clone()).collect(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: Consider calling the dedicated `copied` method: `vec![1; 6].iter().copied()` + error: You are needlessly cloning iterator elements - --> $DIR/map_clone.rs:24:29 + --> $DIR/map_clone.rs:25:29 | LL | let _ = std::env::args().map(|v| v.clone()); | ^^^^^^^^^^^^^^^^^^^ help: Remove the map call -error: aborting due to 4 previous errors +error: aborting due to 5 previous errors