From 875de5b8986375e68cc27d000236006eea40e1e9 Mon Sep 17 00:00:00 2001 From: y21 <30553356+y21@users.noreply.github.com> Date: Tue, 13 Jun 2023 15:21:14 +0200 Subject: [PATCH] [`map_identity`]: recognize tuples --- clippy_utils/src/lib.rs | 27 +++++++++++++++++++-------- lintcheck/src/main.rs | 2 +- tests/ui/map_identity.fixed | 24 ++++++++++++++++++++++++ tests/ui/map_identity.rs | 26 ++++++++++++++++++++++++++ tests/ui/map_identity.stderr | 29 ++++++++++++++++++++++++++++- 5 files changed, 98 insertions(+), 10 deletions(-) diff --git a/clippy_utils/src/lib.rs b/clippy_utils/src/lib.rs index dc1b5a0a3244..707044be0cc3 100644 --- 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; @@ -2063,7 +2072,9 @@ 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 index 3a022b343a43..58cb42316fd2 100644 --- 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 index e756d9b59354..ec2d5627d592 100644 --- 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::); } +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(); + let _ = y + .iter() + .map(|(x, y, (z, (w,))): &(i32, i32, (i32, (i32,)))| (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))); +} + fn not_identity(x: &u16) -> u16 { *x } diff --git a/tests/ui/map_identity.rs b/tests/ui/map_identity.rs index 74cbaade4054..e18862c7dad2 100644 --- 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::); } +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,)))); + let _ = y + .iter() + .map(|(x, y, (z, (w,))): &(i32, i32, (i32, (i32,)))| (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))); +} + fn not_identity(x: &u16) -> u16 { *x } diff --git a/tests/ui/map_identity.stderr b/tests/ui/map_identity.stderr index 8942fd7c0d26..4ca24b0b04c4 100644 --- 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 = 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