Reimplement the `map_clone` lint from scratch

6 files changed, 136 insertions, 3 deletions

diff --git a/CHANGELOG.md b/CHANGELOG.md
index c9bea1e8ef5..013a508cc76 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -688,8 +688,6 @@ All notable changes to this project will be documented in this file.
 [`float_arithmetic`]: https://rust-lang-nursery.github.io/rust-clippy/master/index.html#float_arithmetic
 [`float_cmp`]: https://rust-lang-nursery.github.io/rust-clippy/master/index.html#float_cmp
 [`float_cmp_const`]: https://rust-lang-nursery.github.io/rust-clippy/master/index.html#float_cmp_const
-[`fn_to_numeric_cast`]: https://rust-lang-nursery.github.io/rust-clippy/master/index.html#fn_to_numeric_cast
-[`fn_to_numeric_cast_with_truncation`]: https://rust-lang-nursery.github.io/rust-clippy/master/index.html#fn_to_numeric_cast_with_truncation
 [`for_kv_map`]: https://rust-lang-nursery.github.io/rust-clippy/master/index.html#for_kv_map
 [`for_loop_over_option`]: https://rust-lang-nursery.github.io/rust-clippy/master/index.html#for_loop_over_option
 [`for_loop_over_result`]: https://rust-lang-nursery.github.io/rust-clippy/master/index.html#for_loop_over_result
diff --git a/README.md b/README.md
index 40ece34c6fa..a2b61703a82 100644
--- a/README.md
+++ b/README.md
@@ -9,7 +9,7 @@ We are currently in the process of discussing Clippy 1.0 via the RFC process in
 
 A collection of lints to catch common mistakes and improve your [Rust](https://github.com/rust-lang/rust) code.
 
-[There are 279 lints included in this crate!](https://rust-lang-nursery.github.io/rust-clippy/master/index.html)
+[There are 277 lints included in this crate!](https://rust-lang-nursery.github.io/rust-clippy/master/index.html)
 
 We have a bunch of lint categories to allow you to choose how much Clippy is supposed to ~~annoy~~ help you:
 
diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs
index 689dbfa7da9..8b1b626be44 100644
--- a/clippy_lints/src/lib.rs
+++ b/clippy_lints/src/lib.rs
@@ -129,6 +129,7 @@ pub mod let_if_seq;
 pub mod lifetimes;
 pub mod literal_representation;
 pub mod loops;
+pub mod map_clone;
 pub mod map_unit_fn;
 pub mod matches;
 pub mod mem_forget;
@@ -327,6 +328,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry<'_>, conf: &Conf) {
     reg.register_late_lint_pass(box strings::StringAdd);
     reg.register_early_lint_pass(box returns::ReturnPass);
     reg.register_late_lint_pass(box methods::Pass);
+    reg.register_late_lint_pass(box map_clone::Pass);
     reg.register_late_lint_pass(box shadow::Pass);
     reg.register_late_lint_pass(box types::LetPass);
     reg.register_late_lint_pass(box types::UnitCmp);
@@ -583,6 +585,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry<'_>, conf: &Conf) {
         loops::WHILE_IMMUTABLE_CONDITION,
         loops::WHILE_LET_LOOP,
         loops::WHILE_LET_ON_ITERATOR,
+        map_clone::MAP_CLONE,
         map_unit_fn::OPTION_MAP_UNIT_FN,
         map_unit_fn::RESULT_MAP_UNIT_FN,
         matches::MATCH_AS_REF,
@@ -742,6 +745,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry<'_>, conf: &Conf) {
         loops::FOR_KV_MAP,
         loops::NEEDLESS_RANGE_LOOP,
         loops::WHILE_LET_ON_ITERATOR,
+        map_clone::MAP_CLONE,
         matches::MATCH_BOOL,
         matches::MATCH_OVERLAPPING_ARM,
         matches::MATCH_REF_PATS,
diff --git a/clippy_lints/src/map_clone.rs b/clippy_lints/src/map_clone.rs
new file mode 100644
index 00000000000..ddbb55a26ff
--- /dev/null
+++ b/clippy_lints/src/map_clone.rs
@@ -0,0 +1,100 @@
+use crate::rustc::hir;
+use crate::rustc::lint::{LateContext, LateLintPass, LintArray, LintPass};
+use crate::rustc::{declare_tool_lint, lint_array};
+use crate::syntax::source_map::Span;
+use crate::utils::paths;
+use crate::utils::{
+    in_macro, match_trait_method, match_type,
+    remove_blocks, snippet,
+    span_lint_and_sugg,
+};
+use if_chain::if_chain;
+use crate::syntax::ast::Ident;
+
+#[derive(Clone)]
+pub struct Pass;
+
+/// **What it does:** Checks for usage of `iterator.map(|x| x.clone())` and suggests
+/// `iterator.cloned()` instead
+///
+/// **Why is this bad?** Readability, this can be written more concisely
+///
+/// **Known problems:** None.
+///
+/// **Example:**
+///
+/// ```rust
+/// let x = vec![42, 43];
+/// let y = x.iter();
+/// let z = y.map(|i| *i);
+/// ```
+///
+/// The correct use would be:
+///
+/// ```rust
+/// let x = vec![42, 43];
+/// let y = x.iter();
+/// let z = y.cloned();
+/// ```
+declare_clippy_lint! {
+    pub MAP_CLONE,
+    style,
+    "using `iterator.map(|x| x.clone())`, or dereferencing closures for `Copy` types"
+}
+
+impl LintPass for Pass {
+    fn get_lints(&self) -> LintArray {
+        lint_array!(MAP_CLONE)
+    }
+}
+
+impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Pass {
+    fn check_expr(&mut self, cx: &LateContext<'_, '_>, e: &hir::Expr) {
+        if in_macro(e.span) {
+            return;
+        }
+
+        if_chain! {
+            if let hir::ExprKind::MethodCall(ref method, _, ref args) = e.node;
+            if args.len() == 2;
+            if method.ident.as_str() == "map";
+            let ty = cx.tables.expr_ty(&args[0]);
+            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);
+            then {
+                match closure_body.arguments[0].pat.node {
+                    hir::PatKind::Ref(ref inner, _) => if let hir::PatKind::Binding(hir::BindingAnnotation::Unannotated, _, name, None) = inner.node {
+                        lint(cx, e.span, args[0].span, name, closure_expr);
+                    },
+                    hir::PatKind::Binding(hir::BindingAnnotation::Unannotated, _, name, None) => match closure_expr.node {
+                        hir::ExprKind::Unary(hir::UnOp::UnDeref, ref inner) => lint(cx, e.span, args[0].span, name, inner),
+                        hir::ExprKind::MethodCall(ref method, _, ref obj) => if method.ident.as_str() == "clone" {
+                            if match_trait_method(cx, closure_expr, &paths::CLONE_TRAIT) {
+                                lint(cx, e.span, args[0].span, name, &obj[0]);
+                            }
+                        }
+                        _ => {},
+                    },
+                    _ => {},
+                }
+            }
+        }
+    }
+}
+
+fn lint(cx: &LateContext<'_, '_>, replace: Span, root: Span, name: Ident, path: &hir::Expr) {
+    if let hir::ExprKind::Path(hir::QPath::Resolved(None, ref path)) = path.node {
+        if path.segments.len() == 1 && path.segments[0].ident == name {
+            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(cx, root, "..")),
+            )
+        }
+    }
+}
\ No newline at end of file
diff --git a/tests/ui/map_clone.rs b/tests/ui/map_clone.rs
new file mode 100644
index 00000000000..11a5316a367
--- /dev/null
+++ b/tests/ui/map_clone.rs
@@ -0,0 +1,9 @@
+#![feature(tool_lints)]
+#![warn(clippy::all, clippy::pedantic)]
+#![allow(clippy::missing_docs_in_private_items)]
+
+fn main() {
+    let _: Vec<i8> = vec![5_i8; 6].iter().map(|x| *x).collect();
+    let _: Vec<String> = vec![String::new()].iter().map(|x| x.clone()).collect();
+    let _: Vec<u32> = vec![42, 43].iter().map(|&x| x).collect();
+}
diff --git a/tests/ui/map_clone.stderr b/tests/ui/map_clone.stderr
new file mode 100644
index 00000000000..e80983cdbf7
--- /dev/null
+++ b/tests/ui/map_clone.stderr
@@ -0,0 +1,22 @@
+error: You are using an explicit closure for cloning elements
+ --> $DIR/map_clone.rs:6:22
+  |
+6 |     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()`
+  |
+  = note: `-D clippy::map-clone` implied by `-D warnings`
+
+error: You are using an explicit closure for cloning elements
+ --> $DIR/map_clone.rs:7:26
+  |
+7 |     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:8:23
+  |
+8 |     let _: Vec<u32> = vec![42, 43].iter().map(|&x| x).collect();
+  |                       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: Consider calling the dedicated `cloned` method: `vec![42, 43].iter().cloned()`
+
+error: aborting due to 3 previous errors
+