Move `MapClone` into `Methods` lint pass

7 files changed, 159 insertions, 173 deletions

diff --git a/clippy_lints/src/lib.register_all.rs b/clippy_lints/src/lib.register_all.rs
index 8ce0a26915f..c496fd289bb 100644
--- a/clippy_lints/src/lib.register_all.rs
+++ b/clippy_lints/src/lib.register_all.rs
@@ -127,7 +127,6 @@ store.register_group(true, "clippy::all", Some("clippy_all"), vec![
     LintId::of(manual_rem_euclid::MANUAL_REM_EUCLID),
     LintId::of(manual_retain::MANUAL_RETAIN),
     LintId::of(manual_strip::MANUAL_STRIP),
-    LintId::of(map_clone::MAP_CLONE),
     LintId::of(map_unit_fn::OPTION_MAP_UNIT_FN),
     LintId::of(map_unit_fn::RESULT_MAP_UNIT_FN),
     LintId::of(match_result_ok::MATCH_RESULT_OK),
@@ -179,6 +178,7 @@ store.register_group(true, "clippy::all", Some("clippy_all"), vec![
     LintId::of(methods::MANUAL_SATURATING_ARITHMETIC),
     LintId::of(methods::MANUAL_SPLIT_ONCE),
     LintId::of(methods::MANUAL_STR_REPEAT),
+    LintId::of(methods::MAP_CLONE),
     LintId::of(methods::MAP_COLLECT_RESULT_UNIT),
     LintId::of(methods::MAP_FLATTEN),
     LintId::of(methods::MAP_IDENTITY),
diff --git a/clippy_lints/src/lib.register_lints.rs b/clippy_lints/src/lib.register_lints.rs
index ac30c8b6eac..c7ea7f703f2 100644
--- a/clippy_lints/src/lib.register_lints.rs
+++ b/clippy_lints/src/lib.register_lints.rs
@@ -246,7 +246,6 @@ store.register_lints(&[
     manual_rem_euclid::MANUAL_REM_EUCLID,
     manual_retain::MANUAL_RETAIN,
     manual_strip::MANUAL_STRIP,
-    map_clone::MAP_CLONE,
     map_err_ignore::MAP_ERR_IGNORE,
     map_unit_fn::OPTION_MAP_UNIT_FN,
     map_unit_fn::RESULT_MAP_UNIT_FN,
@@ -325,6 +324,7 @@ store.register_lints(&[
     methods::MANUAL_SATURATING_ARITHMETIC,
     methods::MANUAL_SPLIT_ONCE,
     methods::MANUAL_STR_REPEAT,
+    methods::MAP_CLONE,
     methods::MAP_COLLECT_RESULT_UNIT,
     methods::MAP_FLATTEN,
     methods::MAP_IDENTITY,
diff --git a/clippy_lints/src/lib.register_style.rs b/clippy_lints/src/lib.register_style.rs
index 5ddaba2396e..52bf019c82e 100644
--- a/clippy_lints/src/lib.register_style.rs
+++ b/clippy_lints/src/lib.register_style.rs
@@ -45,7 +45,6 @@ store.register_group(true, "clippy::style", Some("clippy_style"), vec![
     LintId::of(manual_bits::MANUAL_BITS),
     LintId::of(manual_empty_string_creations::MANUAL_EMPTY_STRING_CREATIONS),
     LintId::of(manual_non_exhaustive::MANUAL_NON_EXHAUSTIVE),
-    LintId::of(map_clone::MAP_CLONE),
     LintId::of(match_result_ok::MATCH_RESULT_OK),
     LintId::of(matches::COLLAPSIBLE_MATCH),
     LintId::of(matches::INFALLIBLE_DESTRUCTURING_MATCH),
@@ -69,6 +68,7 @@ store.register_group(true, "clippy::style", Some("clippy_style"), vec![
     LintId::of(methods::ITER_NTH_ZERO),
     LintId::of(methods::ITER_SKIP_NEXT),
     LintId::of(methods::MANUAL_SATURATING_ARITHMETIC),
+    LintId::of(methods::MAP_CLONE),
     LintId::of(methods::MAP_COLLECT_RESULT_UNIT),
     LintId::of(methods::NEW_RET_NO_SELF),
     LintId::of(methods::OBFUSCATED_IF_ELSE),
diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs
index 774ae01cc4f..8f31e4b938c 100644
--- a/clippy_lints/src/lib.rs
+++ b/clippy_lints/src/lib.rs
@@ -273,7 +273,6 @@ mod manual_non_exhaustive;
 mod manual_rem_euclid;
 mod manual_retain;
 mod manual_strip;
-mod map_clone;
 mod map_err_ignore;
 mod map_unit_fn;
 mod match_result_ok;
@@ -628,8 +627,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
     store.register_late_pass(move || Box::new(needless_question_mark::NeedlessQuestionMark));
     store.register_late_pass(move || Box::new(casts::Casts::new(msrv)));
     store.register_early_pass(move || Box::new(unnested_or_patterns::UnnestedOrPatterns::new(msrv)));
-    store.register_late_pass(move || Box::new(map_clone::MapClone::new(msrv)));
-
     store.register_late_pass(|| Box::new(size_of_in_element_count::SizeOfInElementCount));
     store.register_late_pass(|| Box::new(same_name_method::SameNameMethod));
     let max_suggested_slice_pattern_length = conf.max_suggested_slice_pattern_length;
diff --git a/clippy_lints/src/map_clone.rs b/clippy_lints/src/map_clone.rs
deleted file mode 100644
index 95c312f1fe2..00000000000
--- a/clippy_lints/src/map_clone.rs
+++ /dev/null
@@ -1,167 +0,0 @@
-use clippy_utils::diagnostics::span_lint_and_sugg;
-use clippy_utils::source::snippet_with_applicability;
-use clippy_utils::ty::{is_copy, is_type_diagnostic_item};
-use clippy_utils::{is_trait_method, meets_msrv, msrvs, peel_blocks};
-use if_chain::if_chain;
-use rustc_errors::Applicability;
-use rustc_hir as hir;
-use rustc_lint::{LateContext, LateLintPass};
-use rustc_middle::mir::Mutability;
-use rustc_middle::ty;
-use rustc_middle::ty::adjustment::Adjust;
-use rustc_semver::RustcVersion;
-use rustc_session::{declare_tool_lint, impl_lint_pass};
-use rustc_span::symbol::Ident;
-use rustc_span::{sym, Span};
-
-declare_clippy_lint! {
-    /// ### What it does
-    /// Checks for usage of `map(|x| x.clone())` or
-    /// dereferencing closures for `Copy` types, on `Iterator` or `Option`,
-    /// and suggests `cloned()` or `copied()` instead
-    ///
-    /// ### Why is this bad?
-    /// Readability, this can be written more concisely
-    ///
-    /// ### 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();
-    /// ```
-    #[clippy::version = "pre 1.29.0"]
-    pub MAP_CLONE,
-    style,
-    "using `iterator.map(|x| x.clone())`, or dereferencing closures for `Copy` types"
-}
-
-pub struct MapClone {
-    msrv: Option<RustcVersion>,
-}
-
-impl_lint_pass!(MapClone => [MAP_CLONE]);
-
-impl MapClone {
-    pub fn new(msrv: Option<RustcVersion>) -> Self {
-        Self { msrv }
-    }
-}
-
-impl<'tcx> LateLintPass<'tcx> for MapClone {
-    fn check_expr(&mut self, cx: &LateContext<'_>, e: &hir::Expr<'_>) {
-        if e.span.from_expansion() {
-            return;
-        }
-
-        if_chain! {
-            if let hir::ExprKind::MethodCall(method, args, _) = e.kind;
-            if args.len() == 2;
-            if method.ident.name == sym::map;
-            let ty = cx.typeck_results().expr_ty(&args[0]);
-            if is_type_diagnostic_item(cx, ty, sym::Option) || is_trait_method(cx, e, sym::Iterator);
-            if let hir::ExprKind::Closure(&hir::Closure { body, .. }) = args[1].kind;
-            then {
-                let closure_body = cx.tcx.hir().body(body);
-                let closure_expr = peel_blocks(&closure_body.value);
-                match closure_body.params[0].pat.kind {
-                    hir::PatKind::Ref(inner, hir::Mutability::Not) => if let hir::PatKind::Binding(
-                        hir::BindingAnnotation::Unannotated, .., name, None
-                    ) = inner.kind {
-                        if ident_eq(name, closure_expr) {
-                            self.lint_explicit_closure(cx, e.span, args[0].span, true);
-                        }
-                    },
-                    hir::PatKind::Binding(hir::BindingAnnotation::Unannotated, .., name, None) => {
-                        match closure_expr.kind {
-                            hir::ExprKind::Unary(hir::UnOp::Deref, inner) => {
-                                if ident_eq(name, inner) {
-                                    if let ty::Ref(.., Mutability::Not) = cx.typeck_results().expr_ty(inner).kind() {
-                                        self.lint_explicit_closure(cx, e.span, args[0].span, true);
-                                    }
-                                }
-                            },
-                            hir::ExprKind::MethodCall(method, [obj], _) => if_chain! {
-                                if ident_eq(name, obj) && method.ident.name == sym::clone;
-                                if let Some(fn_id) = cx.typeck_results().type_dependent_def_id(closure_expr.hir_id);
-                                if let Some(trait_id) = cx.tcx.trait_of_item(fn_id);
-                                if cx.tcx.lang_items().clone_trait().map_or(false, |id| id == trait_id);
-                                // no autoderefs
-                                if !cx.typeck_results().expr_adjustments(obj).iter()
-                                    .any(|a| matches!(a.kind, Adjust::Deref(Some(..))));
-                                then {
-                                    let obj_ty = cx.typeck_results().expr_ty(obj);
-                                    if let ty::Ref(_, ty, mutability) = obj_ty.kind() {
-                                        if matches!(mutability, Mutability::Not) {
-                                            let copy = is_copy(cx, *ty);
-                                            self.lint_explicit_closure(cx, e.span, args[0].span, copy);
-                                        }
-                                    } else {
-                                        lint_needless_cloning(cx, e.span, args[0].span);
-                                    }
-                                }
-                            },
-                            _ => {},
-                        }
-                    },
-                    _ => {},
-                }
-            }
-        }
-    }
-
-    extract_msrv_attr!(LateContext);
-}
-
-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
-    } else {
-        false
-    }
-}
-
-fn lint_needless_cloning(cx: &LateContext<'_>, root: Span, receiver: Span) {
-    span_lint_and_sugg(
-        cx,
-        MAP_CLONE,
-        root.trim_start(receiver).unwrap(),
-        "you are needlessly cloning iterator elements",
-        "remove the `map` call",
-        String::new(),
-        Applicability::MachineApplicable,
-    );
-}
-
-impl MapClone {
-    fn lint_explicit_closure(&self, cx: &LateContext<'_>, replace: Span, root: Span, is_copy: bool) {
-        let mut applicability = Applicability::MachineApplicable;
-
-        let (message, sugg_method) = if is_copy && meets_msrv(self.msrv, msrvs::ITERATOR_COPIED) {
-            ("you are using an explicit closure for copying elements", "copied")
-        } else {
-            ("you are using an explicit closure for cloning elements", "cloned")
-        };
-
-        span_lint_and_sugg(
-            cx,
-            MAP_CLONE,
-            replace,
-            message,
-            &format!("consider calling the dedicated `{}` method", sugg_method),
-            format!(
-                "{}.{}()",
-                snippet_with_applicability(cx, root, "..", &mut applicability),
-                sugg_method,
-            ),
-            applicability,
-        );
-    }
-}
diff --git a/clippy_lints/src/methods/map_clone.rs b/clippy_lints/src/methods/map_clone.rs
new file mode 100644
index 00000000000..ffedda95ff8
--- /dev/null
+++ b/clippy_lints/src/methods/map_clone.rs
@@ -0,0 +1,122 @@
+use clippy_utils::diagnostics::span_lint_and_sugg;
+use clippy_utils::source::snippet_with_applicability;
+use clippy_utils::ty::{is_copy, is_type_diagnostic_item};
+use clippy_utils::{is_diag_trait_item, meets_msrv, msrvs, peel_blocks};
+use if_chain::if_chain;
+use rustc_errors::Applicability;
+use rustc_hir as hir;
+use rustc_lint::LateContext;
+use rustc_middle::mir::Mutability;
+use rustc_middle::ty;
+use rustc_middle::ty::adjustment::Adjust;
+use rustc_semver::RustcVersion;
+use rustc_span::symbol::Ident;
+use rustc_span::{sym, Span};
+
+use super::MAP_CLONE;
+
+pub(super) fn check<'tcx>(
+    cx: &LateContext<'_>,
+    e: &hir::Expr<'_>,
+    recv: &hir::Expr<'_>,
+    arg: &'tcx hir::Expr<'_>,
+    msrv: Option<RustcVersion>,
+) {
+    if_chain! {
+        if let Some(method_id) = cx.typeck_results().type_dependent_def_id(e.hir_id);
+        if cx.tcx.impl_of_method(method_id)
+            .map_or(false, |id| is_type_diagnostic_item(cx, cx.tcx.type_of(id), sym::Option))
+            || is_diag_trait_item(cx, method_id, sym::Iterator);
+        if let hir::ExprKind::Closure(&hir::Closure{ body, .. }) = arg.kind;
+        then {
+            let closure_body = cx.tcx.hir().body(body);
+            let closure_expr = peel_blocks(&closure_body.value);
+            match closure_body.params[0].pat.kind {
+                hir::PatKind::Ref(inner, hir::Mutability::Not) => if let hir::PatKind::Binding(
+                    hir::BindingAnnotation::Unannotated, .., name, None
+                ) = inner.kind {
+                    if ident_eq(name, closure_expr) {
+                        lint_explicit_closure(cx, e.span, recv.span, true, msrv);
+                    }
+                },
+                hir::PatKind::Binding(hir::BindingAnnotation::Unannotated, .., name, None) => {
+                    match closure_expr.kind {
+                        hir::ExprKind::Unary(hir::UnOp::Deref, inner) => {
+                            if ident_eq(name, inner) {
+                                if let ty::Ref(.., Mutability::Not) = cx.typeck_results().expr_ty(inner).kind() {
+                                    lint_explicit_closure(cx, e.span, recv.span, true, msrv);
+                                }
+                            }
+                        },
+                        hir::ExprKind::MethodCall(method, [obj], _) => if_chain! {
+                            if ident_eq(name, obj) && method.ident.name == sym::clone;
+                            if let Some(fn_id) = cx.typeck_results().type_dependent_def_id(closure_expr.hir_id);
+                            if let Some(trait_id) = cx.tcx.trait_of_item(fn_id);
+                            if cx.tcx.lang_items().clone_trait().map_or(false, |id| id == trait_id);
+                            // no autoderefs
+                            if !cx.typeck_results().expr_adjustments(obj).iter()
+                                .any(|a| matches!(a.kind, Adjust::Deref(Some(..))));
+                            then {
+                                let obj_ty = cx.typeck_results().expr_ty(obj);
+                                if let ty::Ref(_, ty, mutability) = obj_ty.kind() {
+                                    if matches!(mutability, Mutability::Not) {
+                                        let copy = is_copy(cx, *ty);
+                                        lint_explicit_closure(cx, e.span, recv.span, copy, msrv);
+                                    }
+                                } else {
+                                    lint_needless_cloning(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
+    } else {
+        false
+    }
+}
+
+fn lint_needless_cloning(cx: &LateContext<'_>, root: Span, receiver: Span) {
+    span_lint_and_sugg(
+        cx,
+        MAP_CLONE,
+        root.trim_start(receiver).unwrap(),
+        "you are needlessly cloning iterator elements",
+        "remove the `map` call",
+        String::new(),
+        Applicability::MachineApplicable,
+    );
+}
+
+fn lint_explicit_closure(cx: &LateContext<'_>, replace: Span, root: Span, is_copy: bool, msrv: Option<RustcVersion>) {
+    let mut applicability = Applicability::MachineApplicable;
+
+    let (message, sugg_method) = if is_copy && meets_msrv(msrv, msrvs::ITERATOR_COPIED) {
+        ("you are using an explicit closure for copying elements", "copied")
+    } else {
+        ("you are using an explicit closure for cloning elements", "cloned")
+    };
+
+    span_lint_and_sugg(
+        cx,
+        MAP_CLONE,
+        replace,
+        message,
+        &format!("consider calling the dedicated `{}` method", sugg_method),
+        format!(
+            "{}.{}()",
+            snippet_with_applicability(cx, root, "..", &mut applicability),
+            sugg_method,
+        ),
+        applicability,
+    );
+}
diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs
index 132f6040a51..02592da1aae 100644
--- a/clippy_lints/src/methods/mod.rs
+++ b/clippy_lints/src/methods/mod.rs
@@ -45,6 +45,7 @@ mod iterator_step_by_zero;
 mod manual_ok_or;
 mod manual_saturating_arithmetic;
 mod manual_str_repeat;
+mod map_clone;
 mod map_collect_result_unit;
 mod map_flatten;
 mod map_identity;
@@ -2511,6 +2512,35 @@ declare_clippy_lint! {
     "finds patterns that can be encoded more concisely with `Option::ok_or`"
 }
 
+declare_clippy_lint! {
+    /// ### What it does
+    /// Checks for usage of `map(|x| x.clone())` or
+    /// dereferencing closures for `Copy` types, on `Iterator` or `Option`,
+    /// and suggests `cloned()` or `copied()` instead
+    ///
+    /// ### Why is this bad?
+    /// Readability, this can be written more concisely
+    ///
+    /// ### 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();
+    /// ```
+    #[clippy::version = "pre 1.29.0"]
+    pub MAP_CLONE,
+    style,
+    "using `iterator.map(|x| x.clone())`, or dereferencing closures for `Copy` types"
+}
+
 pub struct Methods {
     avoid_breaking_exported_api: bool,
     msrv: Option<RustcVersion>,
@@ -2620,6 +2650,7 @@ impl_lint_pass!(Methods => [
     CASE_SENSITIVE_FILE_EXTENSION_COMPARISONS,
     GET_FIRST,
     MANUAL_OK_OR,
+    MAP_CLONE,
 ]);
 
 /// Extracts a method call name, args, and `Span` of the method name.
@@ -2949,6 +2980,9 @@ impl Methods {
                     }
                 },
                 (name @ ("map" | "map_err"), [m_arg]) => {
+                    if name == "map" {
+                        map_clone::check(cx, expr, recv, m_arg, self.msrv);
+                    }
                     if let Some((name, [recv2, args @ ..], span2)) = method_call(recv) {
                         match (name, args) {
                             ("as_mut", []) => option_as_ref_deref::check(cx, expr, recv2, m_arg, true, self.msrv),