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

Suggest .copied() for map_clone on iterators too #4043

Merged
merged 1 commit into from Apr 28, 2019
Merged

Suggest .copied() for map_clone on iterators too #4043

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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
11 changes: 4 additions & 7 deletions clippy_lints/src/map_clone.rs
View file
Open in desktop
Expand Up @@ -52,8 +52,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for MapClone {
if args.len() == 2;
if method.ident.as_str() == "map";
let ty = cx.tables.expr_ty(&args[0]);
let is_option = match_type(cx, ty, &paths::OPTION);
if is_option || match_trait_method(cx, e, &paths::ITERATOR);
if match_type(cx, ty, &paths::OPTION) || match_trait_method(cx, e, &paths::ITERATOR);
if let hir::ExprKind::Closure(_, _, body_id, _, _) = args[1].node;
let closure_body = cx.tcx.hir().body(body_id);
let closure_expr = remove_blocks(&closure_body.value);
Expand All @@ -63,16 +62,14 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for MapClone {
hir::BindingAnnotation::Unannotated, .., name, None
) = inner.node {
if ident_eq(name, closure_expr) {
// FIXME When Iterator::copied() stabilizes we can remove is_option
// from here and the other lint() calls
lint(cx, e.span, args[0].span, is_option);
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, is_option);
lint(cx, e.span, args[0].span, true);
}
},
hir::ExprKind::MethodCall(ref method, _, ref obj) => {
Expand All @@ -82,7 +79,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for MapClone {
let obj_ty = cx.tables.expr_ty(&obj[0]);
if let ty::Ref(_, ty, _) = obj_ty.sty {
let copy = is_copy(cx, ty);
lint(cx, e.span, args[0].span, is_option && copy);
lint(cx, e.span, args[0].span, copy);
} else {
lint_needless_cloning(cx, e.span, args[0].span);
}
Expand Down
4 changes: 2 additions & 2 deletions tests/ui/map_clone.fixed
View file
Open in desktop
Expand Up @@ -6,9 +6,9 @@
#![allow(clippy::redundant_closure)]

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);
let _: Option<u64> = Some(&16).copied();
let _: Option<u8> = Some(&1).copied();
Expand Down
8 changes: 4 additions & 4 deletions tests/ui/map_clone.stderr
View file
Open in desktop
@@ -1,8 +1,8 @@
error: You are using an explicit closure for cloning elements
error: You are using an explicit closure for copying elements
--> $DIR/map_clone.rs:9: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`

Expand All @@ -12,11 +12,11 @@ error: You are using an explicit closure for cloning elements
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
error: You are using an explicit closure for copying elements
--> $DIR/map_clone.rs:11: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 using an explicit closure for copying elements
--> $DIR/map_clone.rs:13:26
Expand Down