diff options
| -rw-r--r-- | clippy_lints/src/methods/map_clone.rs | 31 | ||||
| -rw-r--r-- | tests/ui/map_clone.fixed | 5 | ||||
| -rw-r--r-- | tests/ui/map_clone.rs | 5 | ||||
| -rw-r--r-- | tests/ui/map_clone.stderr | 20 |
4 files changed, 45 insertions, 16 deletions
diff --git a/clippy_lints/src/methods/map_clone.rs b/clippy_lints/src/methods/map_clone.rs index c8d5a358098..652d2b80081 100644 --- a/clippy_lints/src/methods/map_clone.rs +++ b/clippy_lints/src/methods/map_clone.rs @@ -61,26 +61,39 @@ pub(super) fn check(cx: &LateContext<'_>, e: &hir::Expr<'_>, recv: &hir::Expr<'_ } } }, + hir::ExprKind::Call(call, [_]) => { + if let hir::ExprKind::Path(qpath) = call.kind { + handle_path(cx, call, &qpath, e, recv); + } + }, _ => {}, } }, _ => {}, } }, - hir::ExprKind::Path(qpath) => { - if let Some(path_def_id) = cx.qpath_res(&qpath, arg.hir_id).opt_def_id() - && match_def_path(cx, path_def_id, &paths::CLONE_TRAIT_METHOD) - { - // FIXME: It would be better to infer the type to check if it's copyable or not - // to suggest to use `.copied()` instead of `.cloned()` where applicable. - lint_path(cx, e.span, recv.span); - } - }, + hir::ExprKind::Path(qpath) => handle_path(cx, arg, &qpath, e, recv), _ => {}, } } } +fn handle_path( + cx: &LateContext<'_>, + arg: &hir::Expr<'_>, + qpath: &hir::QPath<'_>, + e: &hir::Expr<'_>, + recv: &hir::Expr<'_>, +) { + if let Some(path_def_id) = cx.qpath_res(qpath, arg.hir_id).opt_def_id() + && match_def_path(cx, path_def_id, &paths::CLONE_TRAIT_METHOD) + { + // FIXME: It would be better to infer the type to check if it's copyable or not + // to suggest to use `.copied()` instead of `.cloned()` where applicable. + lint_path(cx, e.span, recv.span); + } +} + fn ident_eq(name: Ident, path: &hir::Expr<'_>) -> bool { if let hir::ExprKind::Path(hir::QPath::Resolved(None, path)) = path.kind { path.segments.len() == 1 && path.segments[0].ident == name diff --git a/tests/ui/map_clone.fixed b/tests/ui/map_clone.fixed index dd979013d3c..b7cf12c1a84 100644 --- a/tests/ui/map_clone.fixed +++ b/tests/ui/map_clone.fixed @@ -4,6 +4,7 @@ clippy::iter_cloned_collect, clippy::many_single_char_names, clippy::redundant_clone, + clippy::redundant_closure, clippy::useless_vec )] @@ -60,4 +61,8 @@ fn main() { let _ = Some(RefCell::new(String::new()).borrow()).map(|s| s.clone()); } + + let x = Some(String::new()); + let y = x.as_ref().cloned(); + //~^ ERROR: you are explicitly cloning with `.map()` } diff --git a/tests/ui/map_clone.rs b/tests/ui/map_clone.rs index 96cba71965f..693914ec30a 100644 --- a/tests/ui/map_clone.rs +++ b/tests/ui/map_clone.rs @@ -4,6 +4,7 @@ clippy::iter_cloned_collect, clippy::many_single_char_names, clippy::redundant_clone, + clippy::redundant_closure, clippy::useless_vec )] @@ -60,4 +61,8 @@ fn main() { let _ = Some(RefCell::new(String::new()).borrow()).map(|s| s.clone()); } + + let x = Some(String::new()); + let y = x.as_ref().map(|x| String::clone(x)); + //~^ ERROR: you are explicitly cloning with `.map()` } diff --git a/tests/ui/map_clone.stderr b/tests/ui/map_clone.stderr index eb11f084887..e6192a487ab 100644 --- a/tests/ui/map_clone.stderr +++ b/tests/ui/map_clone.stderr @@ -1,5 +1,5 @@ error: you are using an explicit closure for copying elements - --> $DIR/map_clone.rs:11:22 + --> $DIR/map_clone.rs:12:22 | LL | let _: Vec<i8> = vec![5_i8; 6].iter().map(|x| *x).collect(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider calling the dedicated `copied` method: `vec![5_i8; 6].iter().copied()` @@ -8,34 +8,40 @@ LL | let _: Vec<i8> = vec![5_i8; 6].iter().map(|x| *x).collect(); = help: to override `-D warnings` add `#[allow(clippy::map_clone)]` error: you are using an explicit closure for cloning elements - --> $DIR/map_clone.rs:12:26 + --> $DIR/map_clone.rs:13: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 copying elements - --> $DIR/map_clone.rs:13:23 + --> $DIR/map_clone.rs:14:23 | LL | let _: Vec<u32> = 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:15:26 + --> $DIR/map_clone.rs:16:26 | LL | let _: Option<u64> = Some(&16).map(|b| *b); | ^^^^^^^^^^^^^^^^^^^^^ help: consider calling the dedicated `copied` method: `Some(&16).copied()` error: you are using an explicit closure for copying elements - --> $DIR/map_clone.rs:16:25 + --> $DIR/map_clone.rs:17:25 | LL | let _: Option<u8> = Some(&1).map(|x| x.clone()); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider calling the dedicated `copied` method: `Some(&1).copied()` error: you are needlessly cloning iterator elements - --> $DIR/map_clone.rs:27:29 + --> $DIR/map_clone.rs:28:29 | LL | let _ = std::env::args().map(|v| v.clone()); | ^^^^^^^^^^^^^^^^^^^ help: remove the `map` call -error: aborting due to 6 previous errors +error: you are explicitly cloning with `.map()` + --> $DIR/map_clone.rs:66:13 + | +LL | let y = x.as_ref().map(|x| String::clone(x)); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider calling the dedicated `cloned` method: `x.as_ref().cloned()` + +error: aborting due to 7 previous errors |