diff --git a/clippy_lints/src/methods/filter_map_identity.rs b/clippy_lints/src/methods/filter_map_identity.rs index 3337b250c0e7..20878f1e4dfe 100644 --- a/clippy_lints/src/methods/filter_map_identity.rs +++ b/clippy_lints/src/methods/filter_map_identity.rs @@ -1,5 +1,5 @@ use clippy_utils::diagnostics::span_lint_and_sugg; -use clippy_utils::{is_expr_identity_function, is_trait_method}; +use clippy_utils::{is_expr_untyped_identity_function, is_trait_method}; use rustc_errors::Applicability; use rustc_hir as hir; use rustc_lint::LateContext; @@ -9,7 +9,7 @@ use rustc_span::sym; use super::FILTER_MAP_IDENTITY; pub(super) fn check(cx: &LateContext<'_>, expr: &hir::Expr<'_>, filter_map_arg: &hir::Expr<'_>, filter_map_span: Span) { - if is_trait_method(cx, expr, sym::Iterator) && is_expr_identity_function(cx, filter_map_arg) { + if is_trait_method(cx, expr, sym::Iterator) && is_expr_untyped_identity_function(cx, filter_map_arg) { span_lint_and_sugg( cx, FILTER_MAP_IDENTITY, diff --git a/clippy_lints/src/methods/flat_map_identity.rs b/clippy_lints/src/methods/flat_map_identity.rs index 84a21de0ac86..8849a4f49420 100644 --- a/clippy_lints/src/methods/flat_map_identity.rs +++ b/clippy_lints/src/methods/flat_map_identity.rs @@ -1,5 +1,5 @@ use clippy_utils::diagnostics::span_lint_and_sugg; -use clippy_utils::{is_expr_identity_function, is_trait_method}; +use clippy_utils::{is_expr_untyped_identity_function, is_trait_method}; use rustc_errors::Applicability; use rustc_hir as hir; use rustc_lint::LateContext; @@ -15,7 +15,7 @@ pub(super) fn check<'tcx>( flat_map_arg: &'tcx hir::Expr<'_>, flat_map_span: Span, ) { - if is_trait_method(cx, expr, sym::Iterator) && is_expr_identity_function(cx, flat_map_arg) { + if is_trait_method(cx, expr, sym::Iterator) && is_expr_untyped_identity_function(cx, flat_map_arg) { span_lint_and_sugg( cx, FLAT_MAP_IDENTITY, diff --git a/clippy_lints/src/methods/map_identity.rs b/clippy_lints/src/methods/map_identity.rs index 7be1ce483f63..57581363cfa0 100644 --- a/clippy_lints/src/methods/map_identity.rs +++ b/clippy_lints/src/methods/map_identity.rs @@ -1,6 +1,6 @@ use clippy_utils::diagnostics::span_lint_and_sugg; use clippy_utils::ty::is_type_diagnostic_item; -use clippy_utils::{is_expr_identity_function, is_trait_method}; +use clippy_utils::{is_expr_untyped_identity_function, is_trait_method}; use rustc_errors::Applicability; use rustc_hir as hir; use rustc_lint::LateContext; @@ -23,7 +23,7 @@ pub(super) fn check( if is_trait_method(cx, expr, sym::Iterator) || is_type_diagnostic_item(cx, caller_ty, sym::Result) || is_type_diagnostic_item(cx, caller_ty, sym::Option); - if is_expr_identity_function(cx, map_arg); + if is_expr_untyped_identity_function(cx, map_arg); if let Some(sugg_span) = expr.span.trim_start(caller.span); then { span_lint_and_sugg( diff --git a/clippy_utils/src/lib.rs b/clippy_utils/src/lib.rs index 7e2e4f74ca1d..dc1b5a0a3244 100644 --- a/clippy_utils/src/lib.rs +++ b/clippy_utils/src/lib.rs @@ -2027,32 +2027,31 @@ pub fn is_must_use_func_call(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool { did.map_or(false, |did| cx.tcx.has_attr(did, sym::must_use)) } -/// Checks if an expression represents the identity function -/// Only examines closures and `std::convert::identity` -pub fn is_expr_identity_function(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool { - /// 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; }` - 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; - } - }; +/// 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; }` +/// +/// 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; + } + }; - let mut expr = func.value; - loop { - match expr.kind { - #[rustfmt::skip] + let mut expr = func.value; + loop { + match expr.kind { + #[rustfmt::skip] ExprKind::Block(&Block { stmts: [], expr: Some(e), .. }, _, ) | ExprKind::Ret(Some(e)) => expr = e, - #[rustfmt::skip] + #[rustfmt::skip] ExprKind::Block(&Block { stmts: [stmt], expr: None, .. }, _) => { if_chain! { if let StmtKind::Semi(e) | StmtKind::Expr(e) = stmt.kind; @@ -2064,11 +2063,41 @@ pub fn is_expr_identity_function(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool } } }, - _ => return path_to_local_id(expr, id) && cx.typeck_results().expr_adjustments(expr).is_empty(), - } + _ => return path_to_local_id(expr, id) && cx.typeck_results().expr_adjustments(expr).is_empty(), } } +} +/// This is the same as [`is_expr_identity_function`], but does not consider closures +/// with type annotations for its bindings (or similar) as identity functions: +/// * `|x: u8| x` +/// * `std::convert::identity::` +pub fn is_expr_untyped_identity_function(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool { + match expr.kind { + ExprKind::Closure(&Closure { body, fn_decl, .. }) + if fn_decl.inputs.iter().all(|ty| matches!(ty.kind, TyKind::Infer)) => + { + is_body_identity_function(cx, cx.tcx.hir().body(body)) + }, + ExprKind::Path(QPath::Resolved(_, path)) + if path.segments.iter().all(|seg| seg.infer_args) + && let Some(did) = path.res.opt_def_id() => + { + match_def_path(cx, did, &paths::CONVERT_IDENTITY) + }, + _ => false, + } +} + +/// Checks if an expression represents the identity function +/// Only examines closures and `std::convert::identity` +/// +/// NOTE: If you want to use this function to find out if a closure is unnecessary, you likely want +/// to call [`is_expr_untyped_identity_function`] instead, which makes sure that the closure doesn't +/// have type annotations. This is important because removing a closure with bindings can +/// remove type information that helped type inference before, which can then lead to compile +/// errors. +pub fn is_expr_identity_function(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool { match expr.kind { ExprKind::Closure(&Closure { body, .. }) => is_body_identity_function(cx, cx.tcx.hir().body(body)), _ => path_def_id(cx, expr).map_or(false, |id| match_def_path(cx, id, &paths::CONVERT_IDENTITY)), diff --git a/tests/ui/map_identity.fixed b/tests/ui/map_identity.fixed index cc40b1620587..e756d9b59354 100644 --- a/tests/ui/map_identity.fixed +++ b/tests/ui/map_identity.fixed @@ -17,6 +17,9 @@ fn main() { }); let _: Result = Ok(1); let _: Result = Ok(1).map_err(|a: u32| a * 42); + // : u32 guides type inference + let _ = Ok(1).map_err(|a: u32| a); + let _ = Ok(1).map_err(std::convert::identity::); } fn not_identity(x: &u16) -> u16 { diff --git a/tests/ui/map_identity.rs b/tests/ui/map_identity.rs index 97a91aea6dc4..74cbaade4054 100644 --- a/tests/ui/map_identity.rs +++ b/tests/ui/map_identity.rs @@ -19,6 +19,9 @@ fn main() { }); let _: Result = Ok(1).map_err(|a| a); let _: Result = Ok(1).map_err(|a: u32| a * 42); + // : u32 guides type inference + let _ = Ok(1).map_err(|a: u32| a); + let _ = Ok(1).map_err(std::convert::identity::); } fn not_identity(x: &u16) -> u16 {