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.

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

Suggest .copied() instead of .cloned() in map_clone where applicable #3970

Merged
merged 3 commits into from Apr 16, 2019
Merged
Changes from 1 commit
Commits
File filter...
Filter file types
Jump to…
Jump to file or symbol
Failed to load files and symbols.

Always

Just for now

Next

Suggest .copied() instead of .cloned() in map_clone when dealing with…

… references
  • Loading branch information...
Manishearth committed Apr 15, 2019
commit d2f7ae70ae07f039321b35a1f9f48585e8ddc9bd
@@ -73,14 +73,14 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Pass {
hir::BindingAnnotation::Unannotated, .., name, None
) = inner.node {
if ident_eq(name, closure_expr) {
lint(cx, e.span, args[0].span);
lint(cx, e.span, args[0].span, true);
}
},
hir::PatKind::Binding(hir::BindingAnnotation::Unannotated, .., name, None) => {
match closure_expr.node {
hir::ExprKind::Unary(hir::UnOp::UnDeref, ref inner) => {
if ident_eq(name, inner) && !cx.tables.expr_ty(inner).is_box() {
lint(cx, e.span, args[0].span);
lint(cx, e.span, args[0].span, true);
}
},
hir::ExprKind::MethodCall(ref method, _, ref obj) => {
@@ -89,7 +89,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Pass {

let obj_ty = cx.tables.expr_ty(&obj[0]);
if let ty::Ref(..) = obj_ty.sty {
lint(cx, e.span, args[0].span);
lint(cx, e.span, args[0].span, false);
} else {
lint_needless_cloning(cx, e.span, args[0].span);
}
@@ -125,18 +125,33 @@ fn lint_needless_cloning(cx: &LateContext<'_, '_>, root: Span, receiver: Span) {
)
}

fn lint(cx: &LateContext<'_, '_>, replace: Span, root: Span) {
fn lint(cx: &LateContext<'_, '_>, replace: Span, root: Span, copied: bool) {
let mut applicability = Applicability::MachineApplicable;
span_lint_and_sugg(
cx,
MAP_CLONE,
replace,
"You are using an explicit closure for cloning elements",
"Consider calling the dedicated `cloned` method",
format!(
"{}.cloned()",
snippet_with_applicability(cx, root, "..", &mut applicability)
),
applicability,
)
if copied {
span_lint_and_sugg(
cx,
MAP_CLONE,
replace,
"You are using an explicit closure for copying elements",
"Consider calling the dedicated `copied` method",
format!(
"{}.copied()",
snippet_with_applicability(cx, root, "..", &mut applicability)
),
applicability,
)
} else {
span_lint_and_sugg(
cx,
MAP_CLONE,
replace,
"You are using an explicit closure for cloning elements",
"Consider calling the dedicated `cloned` method",
format!(
"{}.cloned()",
snippet_with_applicability(cx, root, "..", &mut applicability)
),
applicability,
)
}
}
@@ -4,11 +4,12 @@
#![allow(clippy::clone_on_copy)]
#![allow(clippy::missing_docs_in_private_items)]
#![allow(clippy::redundant_closure)]
#![feature(iter_copied)]

fn main() {
let _: Vec<i8> = vec![5_i8; 6].iter().cloned().collect();
let _: Vec<i8> = vec![5_i8; 6].iter().copied().collect();
let _: Vec<String> = vec![String::new()].iter().cloned().collect();
let _: Vec<u32> = vec![42, 43].iter().cloned().collect();
let _: Vec<u32> = vec![42, 43].iter().copied().collect();
let _: Option<u64> = Some(Box::new(16)).map(|b| *b);

// Don't lint these
@@ -4,6 +4,7 @@
#![allow(clippy::clone_on_copy)]
#![allow(clippy::missing_docs_in_private_items)]
#![allow(clippy::redundant_closure)]
#![feature(iter_copied)]

fn main() {
let _: Vec<i8> = vec![5_i8; 6].iter().map(|x| *x).collect();
@@ -1,25 +1,25 @@
error: You are using an explicit closure for cloning elements
--> $DIR/map_clone.rs:9:22
error: You are using an explicit closure for copying elements
--> $DIR/map_clone.rs:10:22
|
LL | let _: Vec<i8> = vec![5_i8; 6].iter().map(|x| *x).collect();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: Consider calling the dedicated `cloned` method: `vec![5_i8; 6].iter().cloned()`
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: Consider calling the dedicated `copied` method: `vec![5_i8; 6].iter().copied()`
|
= note: `-D clippy::map-clone` implied by `-D warnings`

error: You are using an explicit closure for cloning elements
--> $DIR/map_clone.rs:10:26
--> $DIR/map_clone.rs:11:26
|
LL | let _: Vec<String> = vec![String::new()].iter().map(|x| x.clone()).collect();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: Consider calling the dedicated `cloned` method: `vec![String::new()].iter().cloned()`

error: You are using an explicit closure for cloning elements
--> $DIR/map_clone.rs:11:23
error: You are using an explicit closure for copying elements
--> $DIR/map_clone.rs:12:23
|
LL | let _: Vec<u32> = vec![42, 43].iter().map(|&x| x).collect();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: Consider calling the dedicated `cloned` method: `vec![42, 43].iter().cloned()`
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: Consider calling the dedicated `copied` method: `vec![42, 43].iter().copied()`

error: You are needlessly cloning iterator elements
--> $DIR/map_clone.rs:23:29
--> $DIR/map_clone.rs:24:29
|
LL | let _ = std::env::args().map(|v| v.clone());
| ^^^^^^^^^^^^^^^^^^^ help: Remove the map call
ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.