Auto merge of #10943 - y21:issue7189, r=llogiq

[`map_identity`]: recognize tuple identity function Fixes #7189 This lint now recognizes `.map(|(a, b)| (a, b))` as a useless `map` call. changelog: [`map_identity`]: recognize tuple identity function
rust-lang · Oct 21, 2023 · 56c8235 · 56c8235
2 parents 2b030eb + d6fc606
commit 56c8235
Show file tree

Hide file tree

Showing 5 changed files with 96 additions and 10 deletions.
diff --git a/clippy_utils/src/lib.rs b/clippy_utils/src/lib.rs
@@ -2032,17 +2032,26 @@ pub fn is_must_use_func_call(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool {
 /// * `|x| return x`
 /// * `|x| { return x }`
 /// * `|x| { return x; }`
+/// * `|(x, y)| (x, y)`
 ///
 /// Consider calling [`is_expr_untyped_identity_function`] or [`is_expr_identity_function`] instead.
 fn is_body_identity_function(cx: &LateContext<'_>, func: &Body<'_>) -> bool {
-    let id = if_chain! {
-        if let [param] = func.params;
-        if let PatKind::Binding(_, id, _, _) = param.pat.kind;
-        then {
-            id
-        } else {
-            return false;
+    fn check_pat(cx: &LateContext<'_>, pat: &Pat<'_>, expr: &Expr<'_>) -> bool {
+        match (pat.kind, expr.kind) {
+            (PatKind::Binding(_, id, _, _), _) => {
+                path_to_local_id(expr, id) && cx.typeck_results().expr_adjustments(expr).is_empty()
+            },
+            (PatKind::Tuple(pats, dotdot), ExprKind::Tup(tup))
+                if dotdot.as_opt_usize().is_none() && pats.len() == tup.len() =>
+            {
+                pats.iter().zip(tup).all(|(pat, expr)| check_pat(cx, pat, expr))
+            },
+            _ => false,
         }
+    }
+
+    let [param] = func.params else {
+        return false;
     };
 
     let mut expr = func.value;
@@ -2075,7 +2084,7 @@ fn is_body_identity_function(cx: &LateContext<'_>, func: &Body<'_>) -> bool {
                     }
                 }
             },
-            _ => return path_to_local_id(expr, id) && cx.typeck_results().expr_adjustments(expr).is_empty(),
+            _ => return check_pat(cx, param.pat, expr),
         }
     }
 }

diff --git a/lintcheck/src/main.rs b/lintcheck/src/main.rs
@@ -523,7 +523,7 @@ fn gather_stats(clippy_warnings: &[ClippyWarning]) -> (String, HashMap<&String,
         .for_each(|wrn| *counter.entry(&wrn.lint_type).or_insert(0) += 1);
 
     // collect into a tupled list for sorting
-    let mut stats: Vec<(&&String, &usize)> = counter.iter().map(|(lint, count)| (lint, count)).collect();
+    let mut stats: Vec<(&&String, &usize)> = counter.iter().collect();
     // sort by "000{count} {clippy::lintname}"
     // to not have a lint with 200 and 2 warnings take the same spot
     stats.sort_by_key(|(lint, count)| format!("{count:0>4}, {lint}"));

diff --git a/tests/ui/map_identity.fixed b/tests/ui/map_identity.fixed
@@ -22,6 +22,30 @@ fn main() {
     let _ = Ok(1).map_err(std::convert::identity::<u32>);
 }
 
+fn issue7189() {
+    // should lint
+    let x = [(1, 2), (3, 4)];
+    let _ = x.iter();
+    let _ = x.iter();
+    let _ = x.iter();
+
+    let y = [(1, 2, (3, (4,))), (5, 6, (7, (8,)))];
+    let _ = y.iter();
+
+    // should not lint
+    let _ = x.iter().map(|(x, y)| (x, y, y));
+    let _ = x.iter().map(|(x, _y)| (x,));
+    let _ = x.iter().map(|(x, _)| (x,));
+    let _ = x.iter().map(|(x, ..)| (x,));
+    let _ = y.iter().map(|(x, y, (z, _))| (x, y, (z, z)));
+    let _ = y
+        .iter()
+        .map(|(x, y, (z, _)): &(i32, i32, (i32, (i32,)))| (x, y, (z, z)));
+    let _ = y
+        .iter()
+        .map(|(x, y, (z, (w,))): &(i32, i32, (i32, (i32,)))| (x, y, (z, (w,))));
+}
+
 fn not_identity(x: &u16) -> u16 {
     *x
 }
diff --git a/tests/ui/map_identity.rs b/tests/ui/map_identity.rs
@@ -24,6 +24,32 @@ fn main() {
     let _ = Ok(1).map_err(std::convert::identity::<u32>);
 }
 
+fn issue7189() {
+    // should lint
+    let x = [(1, 2), (3, 4)];
+    let _ = x.iter().map(|(x, y)| (x, y));
+    let _ = x.iter().map(|(x, y)| {
+        return (x, y);
+    });
+    let _ = x.iter().map(|(x, y)| return (x, y));
+
+    let y = [(1, 2, (3, (4,))), (5, 6, (7, (8,)))];
+    let _ = y.iter().map(|(x, y, (z, (w,)))| (x, y, (z, (w,))));
+
+    // should not lint
+    let _ = x.iter().map(|(x, y)| (x, y, y));
+    let _ = x.iter().map(|(x, _y)| (x,));
+    let _ = x.iter().map(|(x, _)| (x,));
+    let _ = x.iter().map(|(x, ..)| (x,));
+    let _ = y.iter().map(|(x, y, (z, _))| (x, y, (z, z)));
+    let _ = y
+        .iter()
+        .map(|(x, y, (z, _)): &(i32, i32, (i32, (i32,)))| (x, y, (z, z)));
+    let _ = y
+        .iter()
+        .map(|(x, y, (z, (w,))): &(i32, i32, (i32, (i32,)))| (x, y, (z, (w,))));
+}
+
 fn not_identity(x: &u16) -> u16 {
     *x
 }
diff --git a/tests/ui/map_identity.stderr b/tests/ui/map_identity.stderr
@@ -40,5 +40,32 @@ error: unnecessary map of the identity function
 LL |     let _: Result<u32, u32> = Ok(1).map_err(|a| a);
    |                                    ^^^^^^^^^^^^^^^ help: remove the call to `map_err`
 
-error: aborting due to 6 previous errors
+error: unnecessary map of the identity function
+  --> $DIR/map_identity.rs:30:21
+   |
+LL |     let _ = x.iter().map(|(x, y)| (x, y));
+   |                     ^^^^^^^^^^^^^^^^^^^^^ help: remove the call to `map`
+
+error: unnecessary map of the identity function
+  --> $DIR/map_identity.rs:31:21
+   |
+LL |       let _ = x.iter().map(|(x, y)| {
+   |  _____________________^
+LL | |         return (x, y);
+LL | |     });
+   | |______^ help: remove the call to `map`
+
+error: unnecessary map of the identity function
+  --> $DIR/map_identity.rs:34:21
+   |
+LL |     let _ = x.iter().map(|(x, y)| return (x, y));
+   |                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: remove the call to `map`
+
+error: unnecessary map of the identity function
+  --> $DIR/map_identity.rs:37:21
+   |
+LL |     let _ = y.iter().map(|(x, y, (z, (w,)))| (x, y, (z, (w,))));
+   |                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: remove the call to `map`
+
+error: aborting due to 10 previous errors