Removed checks for user-defined identity functions, added checks for std::convert::identity

Author: theo-lw

clippy_lints/src/map_identity.rs

@@ -1,9 +1,9 @@
 use crate::utils::{
-    is_adjusted, is_type_diagnostic_item, match_trait_method, match_var, paths, remove_blocks, span_lint_and_sugg,
+    is_adjusted, is_type_diagnostic_item, match_path, match_trait_method, match_var, paths, remove_blocks, span_lint_and_sugg,
 };
 use if_chain::if_chain;
 use rustc_errors::Applicability;
-use rustc_hir::{def, Body, Expr, ExprKind, Pat, PatKind, Path, QPath, StmtKind};
+use rustc_hir::{Body, Expr, ExprKind, Pat, PatKind, QPath, StmtKind};
 use rustc_lint::{LateContext, LateLintPass};
 use rustc_session::{declare_lint_pass, declare_tool_lint};
 
@@ -40,8 +40,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for MapIdentity {
 
         if_chain! {
             if let Some([caller, func]) = get_map_argument(cx, expr);
-            if let Some(body) = get_body(cx, func);
-            if is_identity_function(cx, body);
+            if is_expr_identity_function(cx, func);
             then {
                 span_lint_and_sugg(
                     cx,
@@ -75,8 +74,19 @@ fn get_map_argument<'a>(cx: &LateContext<'_, '_>, expr: &'a Expr<'a>) -> Option<
     }
 }
 
-/// Determines if a function is the identity function (in a naive way)
-fn is_identity_function(cx: &LateContext<'_, '_>, func: &Body<'_>) -> bool {
+/// Checks if an expression represents the identity function
+/// Only examines closures and std::convert::identity
+fn is_expr_identity_function(cx: &LateContext<'_, '_>, expr: &Expr<'_>) -> bool {
+    match expr.kind {
+        ExprKind::Closure(_, _, body_id, _, _) => is_body_identity_function(cx, cx.tcx.hir().body(body_id)),
+        ExprKind::Path(QPath::Resolved(_, ref path)) => match_path(path, &paths::STD_CONVERT_IDENTITY),
+        _ => false,
+    }
+}
+
+/// Checks if a function's body represents the identity function
+/// Looks for bodies of the form |x| x, |x| return x, |x| return x;, |x| { return x } or |x| { return x; }
+fn is_body_identity_function(cx: &LateContext<'_, '_>, func: &Body<'_>) -> bool {
     let params = func.params;
     let body = remove_blocks(&func.value);
 
@@ -104,29 +114,7 @@ fn is_identity_function(cx: &LateContext<'_, '_>, func: &Body<'_>) -> bool {
     }
 }
 
-/// Returns an associated function/closure body from an expression
-fn get_body<'a>(cx: &LateContext<'a, '_>, expr: &'a Expr<'a>) -> Option<&'a Body<'a>> {
-    match expr.kind {
-        ExprKind::Closure(_, _, body_id, _, _) => Some(cx.tcx.hir().body(body_id)),
-        ExprKind::Path(QPath::Resolved(_, ref path)) => path_to_body(cx, path),
-        _ => None,
-    }
-}
-
-/// Returns the function body associated with a path
-fn path_to_body<'a>(cx: &LateContext<'a, '_>, path: &'a Path<'a>) -> Option<&'a Body<'a>> {
-    if let def::Res::Def(_, def_id) = path.res {
-        def_id
-            .as_local()
-            .and_then(|local_id| cx.tcx.hir().opt_local_def_id_to_hir_id(local_id))
-            .and_then(|hir_id| cx.tcx.hir().maybe_body_owned_by(hir_id))
-            .map(|body_id| cx.tcx.hir().body(body_id))
-    } else {
-        None
-    }
-}
-
-/// Determines if an expression (syntactically) returns the same thing as a parameter's pattern
+/// Returns true iff an expression returns the same thing as a parameter's pattern
 fn match_expr_param(cx: &LateContext<'_, '_>, expr: &Expr<'_>, pat: &Pat<'_>) -> bool {
     if let PatKind::Binding(_, _, ident, _) = pat.kind {
         match_var(expr, ident.name) && !(cx.tables.hir_owner == Some(expr.hir_id.owner) && is_adjusted(cx, expr))

tests/ui/map_identity.fixed

@@ -18,11 +18,6 @@ fn main() {
     });
 }
 
-#[allow(dead_code)]
-fn identity(x: &u16) -> &u16 {
-    return x;
-}
-
 fn not_identity(x: &u16) -> u16 {
     *x
 }

tests/ui/map_identity.rs

@@ -6,7 +6,7 @@ fn main() {
     let x: [u16; 3] = [1, 2, 3];
     // should lint
     let _: Vec<_> = x.iter().map(not_identity).map(|x| return x).collect();
-    let _: Vec<_> = x.iter().map(identity).map(|y| y).collect();
+    let _: Vec<_> = x.iter().map(std::convert::identity).map(|y| y).collect();
     let _: Option<u8> = Some(3).map(|x| x);
     let _: Result<i8, f32> = Ok(-3).map(|x| {
         return x;
@@ -20,11 +20,6 @@ fn main() {
     });
 }
 
-#[allow(dead_code)]
-fn identity(x: &u16) -> &u16 {
-    return x;
-}
-
 fn not_identity(x: &u16) -> u16 {
     *x
 }

tests/ui/map_identity.stderr

@@ -7,16 +7,16 @@ LL |     let _: Vec<_> = x.iter().map(not_identity).map(|x| return x).collect();
    = note: `-D clippy::map-identity` implied by `-D warnings`
 
 error: unnecessary map of the identity function
-  --> $DIR/map_identity.rs:9:43
+  --> $DIR/map_identity.rs:9:57
    |
-LL |     let _: Vec<_> = x.iter().map(identity).map(|y| y).collect();
-   |                                           ^^^^^^^^^^^ help: remove the call to `map`
+LL |     let _: Vec<_> = x.iter().map(std::convert::identity).map(|y| y).collect();
+   |                                                         ^^^^^^^^^^^ help: remove the call to `map`
 
 error: unnecessary map of the identity function
   --> $DIR/map_identity.rs:9:29
    |
-LL |     let _: Vec<_> = x.iter().map(identity).map(|y| y).collect();
-   |                             ^^^^^^^^^^^^^^ help: remove the call to `map`
+LL |     let _: Vec<_> = x.iter().map(std::convert::identity).map(|y| y).collect();
+   |                             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: remove the call to `map`
 
 error: unnecessary map of the identity function
   --> $DIR/map_identity.rs:10:32