From 307b8cd82537b5ac46da2f04cced4e4362835757 Mon Sep 17 00:00:00 2001 From: Alex Macleod Date: Mon, 6 Jun 2022 15:09:37 +0000 Subject: [PATCH] Lint simple expressions in manual_filter_map, manual_find_map --- clippy_lints/src/methods/filter_map.rs | 107 +++++++++---------- tests/ui/manual_filter_map.fixed | 34 +++++++ tests/ui/manual_filter_map.rs | 38 +++++++ tests/ui/manual_filter_map.stderr | 120 ++++++++++++++++++++-- tests/ui/manual_find_map.fixed | 37 +++++++ tests/ui/manual_find_map.rs | 41 ++++++++ tests/ui/manual_find_map.stderr | 136 +++++++++++++++++++++++-- 7 files changed, 440 insertions(+), 73 deletions(-) diff --git a/clippy_lints/src/methods/filter_map.rs b/clippy_lints/src/methods/filter_map.rs index 58c3e52e138..b694a5a7948 100644 --- a/clippy_lints/src/methods/filter_map.rs +++ b/clippy_lints/src/methods/filter_map.rs @@ -8,6 +8,7 @@ use rustc_hir as hir; use rustc_hir::def::Res; use rustc_hir::{Expr, ExprKind, PatKind, PathSegment, QPath, UnOp}; use rustc_lint::LateContext; +use rustc_middle::ty::adjustment::Adjust; use rustc_span::source_map::Span; use rustc_span::symbol::{sym, Symbol}; use std::borrow::Cow; @@ -49,35 +50,18 @@ fn is_option_filter_map<'tcx>(cx: &LateContext<'tcx>, filter_arg: &hir::Expr<'_> is_method(cx, map_arg, sym::unwrap) && is_method(cx, filter_arg, sym!(is_some)) } -/// lint use of `filter().map()` for `Iterators` -fn lint_filter_some_map_unwrap( +/// is `filter(|x| x.is_some()).map(|x| x.unwrap())` +fn is_filter_some_map_unwrap( cx: &LateContext<'_>, expr: &hir::Expr<'_>, filter_recv: &hir::Expr<'_>, filter_arg: &hir::Expr<'_>, map_arg: &hir::Expr<'_>, - target_span: Span, - methods_span: Span, -) { +) -> bool { let iterator = is_trait_method(cx, expr, sym::Iterator); let option = is_type_diagnostic_item(cx, cx.typeck_results().expr_ty(filter_recv), sym::Option); - if (iterator || option) && is_option_filter_map(cx, filter_arg, map_arg) { - let msg = "`filter` for `Some` followed by `unwrap`"; - let help = "consider using `flatten` instead"; - let sugg = format!( - "{}", - reindent_multiline(Cow::Borrowed("flatten()"), true, indent_of(cx, target_span),) - ); - span_lint_and_sugg( - cx, - OPTION_FILTER_MAP, - methods_span, - msg, - help, - sugg, - Applicability::MachineApplicable, - ); - } + + (iterator || option) && is_option_filter_map(cx, filter_arg, map_arg) } /// lint use of `filter().map()` or `find().map()` for `Iterators` @@ -93,15 +77,20 @@ pub(super) fn check<'tcx>( map_span: Span, is_find: bool, ) { - lint_filter_some_map_unwrap( - cx, - expr, - filter_recv, - filter_arg, - map_arg, - map_span, - filter_span.with_hi(expr.span.hi()), - ); + if is_filter_some_map_unwrap(cx, expr, filter_recv, filter_arg, map_arg) { + span_lint_and_sugg( + cx, + OPTION_FILTER_MAP, + filter_span.with_hi(expr.span.hi()), + "`filter` for `Some` followed by `unwrap`", + "consider using `flatten` instead", + reindent_multiline(Cow::Borrowed("flatten()"), true, indent_of(cx, map_span)).into_owned(), + Applicability::MachineApplicable, + ); + + return; + } + if_chain! { if is_trait_method(cx, map_recv, sym::Iterator); @@ -118,7 +107,7 @@ pub(super) fn check<'tcx>( // closure ends with is_some() or is_ok() if let PatKind::Binding(_, filter_param_id, _, None) = filter_pat.kind; if let ExprKind::MethodCall(path, [filter_arg], _) = filter_body.value.kind; - if let Some(opt_ty) = cx.typeck_results().expr_ty(filter_arg).ty_adt_def(); + if let Some(opt_ty) = cx.typeck_results().expr_ty(filter_arg).peel_refs().ty_adt_def(); if let Some(is_result) = if cx.tcx.is_diagnostic_item(sym::Option, opt_ty.did()) { Some(false) } else if cx.tcx.is_diagnostic_item(sym::Result, opt_ty.did()) { @@ -137,6 +126,19 @@ pub(super) fn check<'tcx>( if let ExprKind::MethodCall(seg, [map_arg, ..], _) = map_body.value.kind; if matches!(seg.ident.name, sym::expect | sym::unwrap | sym::unwrap_or); + // .filter(..).map(|y| f(y).copied().unwrap()) + // ~~~~ + let map_arg_peeled = match map_arg.kind { + ExprKind::MethodCall(method, [original_arg], _) if acceptable_methods(method) => { + original_arg + }, + _ => map_arg, + }; + + // .filter(|x| x.is_some()).map(|y| y[.acceptable_method()].unwrap()) + let simple_equal = path_to_local_id(filter_arg, filter_param_id) + && path_to_local_id(map_arg_peeled, map_param_id); + let eq_fallback = |a: &Expr<'_>, b: &Expr<'_>| { // in `filter(|x| ..)`, replace `*x` with `x` let a_path = if_chain! { @@ -145,25 +147,12 @@ pub(super) fn check<'tcx>( then { expr_path } else { a } }; // let the filter closure arg and the map closure arg be equal - if_chain! { - if path_to_local_id(a_path, filter_param_id); - if path_to_local_id(b, map_param_id); - if cx.typeck_results().expr_ty_adjusted(a) == cx.typeck_results().expr_ty_adjusted(b); - then { - return true; - } - } - false - }; - - if match map_arg.kind { - ExprKind::MethodCall(method, [original_arg], _) => { - acceptable_methods(method) - && SpanlessEq::new(cx).expr_fallback(eq_fallback).eq_expr(filter_arg, original_arg) - }, - _ => SpanlessEq::new(cx).expr_fallback(eq_fallback).eq_expr(filter_arg, map_arg) + path_to_local_id(a_path, filter_param_id) + && path_to_local_id(b, map_param_id) + && cx.typeck_results().expr_ty_adjusted(a) == cx.typeck_results().expr_ty_adjusted(b) }; + if simple_equal || SpanlessEq::new(cx).expr_fallback(eq_fallback).eq_expr(filter_arg, map_arg_peeled); then { let span = filter_span.with_hi(expr.span.hi()); let (filter_name, lint) = if is_find { @@ -171,10 +160,22 @@ pub(super) fn check<'tcx>( } else { ("filter", MANUAL_FILTER_MAP) }; - let msg = format!("`{}(..).map(..)` can be simplified as `{0}_map(..)`", filter_name); - let to_opt = if is_result { ".ok()" } else { "" }; - let sugg = format!("{}_map(|{}| {}{})", filter_name, map_param_ident, - snippet(cx, map_arg.span, ".."), to_opt); + let msg = format!("`{filter_name}(..).map(..)` can be simplified as `{filter_name}_map(..)`"); + let (to_opt, deref) = if is_result { + (".ok()", String::new()) + } else { + let derefs = cx.typeck_results() + .expr_adjustments(map_arg) + .iter() + .filter(|adj| matches!(adj.kind, Adjust::Deref(_))) + .count(); + + ("", "*".repeat(derefs)) + }; + let sugg = format!( + "{filter_name}_map(|{map_param_ident}| {deref}{}{to_opt})", + snippet(cx, map_arg.span, ".."), + ); span_lint_and_sugg(cx, lint, span, &msg, "try", sugg, Applicability::MachineApplicable); } } diff --git a/tests/ui/manual_filter_map.fixed b/tests/ui/manual_filter_map.fixed index de0d8614889..4936dc9b2e0 100644 --- a/tests/ui/manual_filter_map.fixed +++ b/tests/ui/manual_filter_map.fixed @@ -12,6 +12,32 @@ fn main() { // is_ok(), unwrap_or() let _ = (0..).filter_map(|a| to_res(a).ok()); + + let _ = (1..5) + .filter_map(|y| *to_ref(to_opt(y))); + let _ = (1..5) + .filter_map(|y| *to_ref(to_opt(y))); + + let _ = (1..5) + .filter_map(|y| to_ref(to_res(y)).ok()); + let _ = (1..5) + .filter_map(|y| to_ref(to_res(y)).ok()); +} + +#[rustfmt::skip] +fn simple_equal() { + iter::>().find_map(|x| x.cloned()); + iter::<&Option<&u8>>().find_map(|x| x.cloned()); + iter::<&Option>().find_map(|x| x.as_deref()); + iter::>().find_map(|y| to_ref(y).cloned()); + + iter::>().find_map(|x| x.ok()); + iter::<&Result>().find_map(|x| x.ok()); + iter::<&&Result>().find_map(|x| x.ok()); + iter::>().find_map(|x| x.cloned().ok()); + iter::<&Result<&u8, ()>>().find_map(|x| x.cloned().ok()); + iter::<&Result>().find_map(|x| x.as_deref().ok()); + iter::>().find_map(|y| to_ref(y).cloned().ok()); } fn no_lint() { @@ -28,6 +54,10 @@ fn no_lint() { .map(|a| to_opt(a).unwrap()); } +fn iter() -> impl Iterator { + std::iter::empty() +} + fn to_opt(_: T) -> Option { unimplemented!() } @@ -36,6 +66,10 @@ fn to_res(_: T) -> Result { unimplemented!() } +fn to_ref<'a, T>(_: T) -> &'a T { + unimplemented!() +} + struct Issue8920<'a> { option_field: Option, result_field: Result, diff --git a/tests/ui/manual_filter_map.rs b/tests/ui/manual_filter_map.rs index bd6516f038b..8c67e827b4c 100644 --- a/tests/ui/manual_filter_map.rs +++ b/tests/ui/manual_filter_map.rs @@ -12,6 +12,36 @@ fn main() { // is_ok(), unwrap_or() let _ = (0..).filter(|&n| to_res(n).is_ok()).map(|a| to_res(a).unwrap_or(1)); + + let _ = (1..5) + .filter(|&x| to_ref(to_opt(x)).is_some()) + .map(|y| to_ref(to_opt(y)).unwrap()); + let _ = (1..5) + .filter(|x| to_ref(to_opt(*x)).is_some()) + .map(|y| to_ref(to_opt(y)).unwrap()); + + let _ = (1..5) + .filter(|&x| to_ref(to_res(x)).is_ok()) + .map(|y| to_ref(to_res(y)).unwrap()); + let _ = (1..5) + .filter(|x| to_ref(to_res(*x)).is_ok()) + .map(|y| to_ref(to_res(y)).unwrap()); +} + +#[rustfmt::skip] +fn simple_equal() { + iter::>().find(|x| x.is_some()).map(|x| x.cloned().unwrap()); + iter::<&Option<&u8>>().find(|x| x.is_some()).map(|x| x.cloned().unwrap()); + iter::<&Option>().find(|x| x.is_some()).map(|x| x.as_deref().unwrap()); + iter::>().find(|&x| to_ref(x).is_some()).map(|y| to_ref(y).cloned().unwrap()); + + iter::>().find(|x| x.is_ok()).map(|x| x.unwrap()); + iter::<&Result>().find(|x| x.is_ok()).map(|x| x.unwrap()); + iter::<&&Result>().find(|x| x.is_ok()).map(|x| x.unwrap()); + iter::>().find(|x| x.is_ok()).map(|x| x.cloned().unwrap()); + iter::<&Result<&u8, ()>>().find(|x| x.is_ok()).map(|x| x.cloned().unwrap()); + iter::<&Result>().find(|x| x.is_ok()).map(|x| x.as_deref().unwrap()); + iter::>().find(|&x| to_ref(x).is_ok()).map(|y| to_ref(y).cloned().unwrap()); } fn no_lint() { @@ -28,6 +58,10 @@ fn no_lint() { .map(|a| to_opt(a).unwrap()); } +fn iter() -> impl Iterator { + std::iter::empty() +} + fn to_opt(_: T) -> Option { unimplemented!() } @@ -36,6 +70,10 @@ fn to_res(_: T) -> Result { unimplemented!() } +fn to_ref<'a, T>(_: T) -> &'a T { + unimplemented!() +} + struct Issue8920<'a> { option_field: Option, result_field: Result, diff --git a/tests/ui/manual_filter_map.stderr b/tests/ui/manual_filter_map.stderr index 465f1b19110..6e5bbe8f2aa 100644 --- a/tests/ui/manual_filter_map.stderr +++ b/tests/ui/manual_filter_map.stderr @@ -19,7 +19,107 @@ LL | let _ = (0..).filter(|&n| to_res(n).is_ok()).map(|a| to_res(a).unwrap_o | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `filter_map(|a| to_res(a).ok())` error: `filter(..).map(..)` can be simplified as `filter_map(..)` - --> $DIR/manual_filter_map.rs:54:10 + --> $DIR/manual_filter_map.rs:17:10 + | +LL | .filter(|&x| to_ref(to_opt(x)).is_some()) + | __________^ +LL | | .map(|y| to_ref(to_opt(y)).unwrap()); + | |____________________________________________^ help: try: `filter_map(|y| *to_ref(to_opt(y)))` + +error: `filter(..).map(..)` can be simplified as `filter_map(..)` + --> $DIR/manual_filter_map.rs:20:10 + | +LL | .filter(|x| to_ref(to_opt(*x)).is_some()) + | __________^ +LL | | .map(|y| to_ref(to_opt(y)).unwrap()); + | |____________________________________________^ help: try: `filter_map(|y| *to_ref(to_opt(y)))` + +error: `filter(..).map(..)` can be simplified as `filter_map(..)` + --> $DIR/manual_filter_map.rs:24:10 + | +LL | .filter(|&x| to_ref(to_res(x)).is_ok()) + | __________^ +LL | | .map(|y| to_ref(to_res(y)).unwrap()); + | |____________________________________________^ help: try: `filter_map(|y| to_ref(to_res(y)).ok())` + +error: `filter(..).map(..)` can be simplified as `filter_map(..)` + --> $DIR/manual_filter_map.rs:27:10 + | +LL | .filter(|x| to_ref(to_res(*x)).is_ok()) + | __________^ +LL | | .map(|y| to_ref(to_res(y)).unwrap()); + | |____________________________________________^ help: try: `filter_map(|y| to_ref(to_res(y)).ok())` + +error: `find(..).map(..)` can be simplified as `find_map(..)` + --> $DIR/manual_filter_map.rs:33:27 + | +LL | iter::>().find(|x| x.is_some()).map(|x| x.cloned().unwrap()); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `find_map(|x| x.cloned())` + | + = note: `-D clippy::manual-find-map` implied by `-D warnings` + +error: `find(..).map(..)` can be simplified as `find_map(..)` + --> $DIR/manual_filter_map.rs:34:28 + | +LL | iter::<&Option<&u8>>().find(|x| x.is_some()).map(|x| x.cloned().unwrap()); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `find_map(|x| x.cloned())` + +error: `find(..).map(..)` can be simplified as `find_map(..)` + --> $DIR/manual_filter_map.rs:35:31 + | +LL | iter::<&Option>().find(|x| x.is_some()).map(|x| x.as_deref().unwrap()); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `find_map(|x| x.as_deref())` + +error: `find(..).map(..)` can be simplified as `find_map(..)` + --> $DIR/manual_filter_map.rs:36:31 + | +LL | iter::>().find(|&x| to_ref(x).is_some()).map(|y| to_ref(y).cloned().unwrap()); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `find_map(|y| to_ref(y).cloned())` + +error: `find(..).map(..)` can be simplified as `find_map(..)` + --> $DIR/manual_filter_map.rs:38:30 + | +LL | iter::>().find(|x| x.is_ok()).map(|x| x.unwrap()); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `find_map(|x| x.ok())` + +error: `find(..).map(..)` can be simplified as `find_map(..)` + --> $DIR/manual_filter_map.rs:39:31 + | +LL | iter::<&Result>().find(|x| x.is_ok()).map(|x| x.unwrap()); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `find_map(|x| x.ok())` + +error: `find(..).map(..)` can be simplified as `find_map(..)` + --> $DIR/manual_filter_map.rs:40:32 + | +LL | iter::<&&Result>().find(|x| x.is_ok()).map(|x| x.unwrap()); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `find_map(|x| x.ok())` + +error: `find(..).map(..)` can be simplified as `find_map(..)` + --> $DIR/manual_filter_map.rs:41:31 + | +LL | iter::>().find(|x| x.is_ok()).map(|x| x.cloned().unwrap()); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `find_map(|x| x.cloned().ok())` + +error: `find(..).map(..)` can be simplified as `find_map(..)` + --> $DIR/manual_filter_map.rs:42:32 + | +LL | iter::<&Result<&u8, ()>>().find(|x| x.is_ok()).map(|x| x.cloned().unwrap()); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `find_map(|x| x.cloned().ok())` + +error: `find(..).map(..)` can be simplified as `find_map(..)` + --> $DIR/manual_filter_map.rs:43:35 + | +LL | iter::<&Result>().find(|x| x.is_ok()).map(|x| x.as_deref().unwrap()); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `find_map(|x| x.as_deref().ok())` + +error: `find(..).map(..)` can be simplified as `find_map(..)` + --> $DIR/manual_filter_map.rs:44:35 + | +LL | iter::>().find(|&x| to_ref(x).is_ok()).map(|y| to_ref(y).cloned().unwrap()); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `find_map(|y| to_ref(y).cloned().ok())` + +error: `filter(..).map(..)` can be simplified as `filter_map(..)` + --> $DIR/manual_filter_map.rs:92:10 | LL | .filter(|f| f.option_field.is_some()) | __________^ @@ -27,7 +127,7 @@ LL | | .map(|f| f.option_field.clone().unwrap()); | |_________________________________________________^ help: try: `filter_map(|f| f.option_field.clone())` error: `filter(..).map(..)` can be simplified as `filter_map(..)` - --> $DIR/manual_filter_map.rs:59:10 + --> $DIR/manual_filter_map.rs:97:10 | LL | .filter(|f| f.ref_field.is_some()) | __________^ @@ -35,7 +135,7 @@ LL | | .map(|f| f.ref_field.cloned().unwrap()); | |_______________________________________________^ help: try: `filter_map(|f| f.ref_field.cloned())` error: `filter(..).map(..)` can be simplified as `filter_map(..)` - --> $DIR/manual_filter_map.rs:64:10 + --> $DIR/manual_filter_map.rs:102:10 | LL | .filter(|f| f.ref_field.is_some()) | __________^ @@ -43,7 +143,7 @@ LL | | .map(|f| f.ref_field.copied().unwrap()); | |_______________________________________________^ help: try: `filter_map(|f| f.ref_field.copied())` error: `filter(..).map(..)` can be simplified as `filter_map(..)` - --> $DIR/manual_filter_map.rs:69:10 + --> $DIR/manual_filter_map.rs:107:10 | LL | .filter(|f| f.result_field.is_ok()) | __________^ @@ -51,7 +151,7 @@ LL | | .map(|f| f.result_field.clone().unwrap()); | |_________________________________________________^ help: try: `filter_map(|f| f.result_field.clone().ok())` error: `filter(..).map(..)` can be simplified as `filter_map(..)` - --> $DIR/manual_filter_map.rs:74:10 + --> $DIR/manual_filter_map.rs:112:10 | LL | .filter(|f| f.result_field.is_ok()) | __________^ @@ -59,7 +159,7 @@ LL | | .map(|f| f.result_field.as_ref().unwrap()); | |__________________________________________________^ help: try: `filter_map(|f| f.result_field.as_ref().ok())` error: `filter(..).map(..)` can be simplified as `filter_map(..)` - --> $DIR/manual_filter_map.rs:79:10 + --> $DIR/manual_filter_map.rs:117:10 | LL | .filter(|f| f.result_field.is_ok()) | __________^ @@ -67,7 +167,7 @@ LL | | .map(|f| f.result_field.as_deref().unwrap()); | |____________________________________________________^ help: try: `filter_map(|f| f.result_field.as_deref().ok())` error: `filter(..).map(..)` can be simplified as `filter_map(..)` - --> $DIR/manual_filter_map.rs:84:10 + --> $DIR/manual_filter_map.rs:122:10 | LL | .filter(|f| f.result_field.is_ok()) | __________^ @@ -75,7 +175,7 @@ LL | | .map(|f| f.result_field.as_mut().unwrap()); | |__________________________________________________^ help: try: `filter_map(|f| f.result_field.as_mut().ok())` error: `filter(..).map(..)` can be simplified as `filter_map(..)` - --> $DIR/manual_filter_map.rs:89:10 + --> $DIR/manual_filter_map.rs:127:10 | LL | .filter(|f| f.result_field.is_ok()) | __________^ @@ -83,12 +183,12 @@ LL | | .map(|f| f.result_field.as_deref_mut().unwrap()); | |________________________________________________________^ help: try: `filter_map(|f| f.result_field.as_deref_mut().ok())` error: `filter(..).map(..)` can be simplified as `filter_map(..)` - --> $DIR/manual_filter_map.rs:94:10 + --> $DIR/manual_filter_map.rs:132:10 | LL | .filter(|f| f.result_field.is_ok()) | __________^ LL | | .map(|f| f.result_field.to_owned().unwrap()); | |____________________________________________________^ help: try: `filter_map(|f| f.result_field.to_owned().ok())` -error: aborting due to 12 previous errors +error: aborting due to 27 previous errors diff --git a/tests/ui/manual_find_map.fixed b/tests/ui/manual_find_map.fixed index d69b6c1dcf3..54302beceff 100644 --- a/tests/ui/manual_find_map.fixed +++ b/tests/ui/manual_find_map.fixed @@ -12,6 +12,35 @@ fn main() { // is_ok(), unwrap_or() let _ = (0..).find_map(|a| to_res(a).ok()); + + let _ = (1..5) + .find_map(|y| *to_ref(to_opt(y))); + let _ = (1..5) + .find_map(|y| *to_ref(to_opt(y))); + + let _ = (1..5) + .find_map(|y| to_ref(to_res(y)).ok()); + let _ = (1..5) + .find_map(|y| to_ref(to_res(y)).ok()); +} + +#[rustfmt::skip] +fn simple_equal() { + iter::>().find_map(|x| x); + iter::<&Option>().find_map(|x| *x); + iter::<&&Option>().find_map(|x| **x); + iter::>().find_map(|x| x.cloned()); + iter::<&Option<&u8>>().find_map(|x| x.cloned()); + iter::<&Option>().find_map(|x| x.as_deref()); + iter::>().find_map(|y| to_ref(y).cloned()); + + iter::>().find_map(|x| x.ok()); + iter::<&Result>().find_map(|x| x.ok()); + iter::<&&Result>().find_map(|x| x.ok()); + iter::>().find_map(|x| x.cloned().ok()); + iter::<&Result<&u8, ()>>().find_map(|x| x.cloned().ok()); + iter::<&Result>().find_map(|x| x.as_deref().ok()); + iter::>().find_map(|y| to_ref(y).cloned().ok()); } fn no_lint() { @@ -28,6 +57,10 @@ fn no_lint() { .map(|a| to_opt(a).unwrap()); } +fn iter() -> impl Iterator { + std::iter::empty() +} + fn to_opt(_: T) -> Option { unimplemented!() } @@ -36,6 +69,10 @@ fn to_res(_: T) -> Result { unimplemented!() } +fn to_ref<'a, T>(_: T) -> &'a T { + unimplemented!() +} + struct Issue8920<'a> { option_field: Option, result_field: Result, diff --git a/tests/ui/manual_find_map.rs b/tests/ui/manual_find_map.rs index 1c4e18e31c8..afcc1825a9a 100644 --- a/tests/ui/manual_find_map.rs +++ b/tests/ui/manual_find_map.rs @@ -12,6 +12,39 @@ fn main() { // is_ok(), unwrap_or() let _ = (0..).find(|&n| to_res(n).is_ok()).map(|a| to_res(a).unwrap_or(1)); + + let _ = (1..5) + .find(|&x| to_ref(to_opt(x)).is_some()) + .map(|y| to_ref(to_opt(y)).unwrap()); + let _ = (1..5) + .find(|x| to_ref(to_opt(*x)).is_some()) + .map(|y| to_ref(to_opt(y)).unwrap()); + + let _ = (1..5) + .find(|&x| to_ref(to_res(x)).is_ok()) + .map(|y| to_ref(to_res(y)).unwrap()); + let _ = (1..5) + .find(|x| to_ref(to_res(*x)).is_ok()) + .map(|y| to_ref(to_res(y)).unwrap()); +} + +#[rustfmt::skip] +fn simple_equal() { + iter::>().find(|x| x.is_some()).map(|x| x.unwrap()); + iter::<&Option>().find(|x| x.is_some()).map(|x| x.unwrap()); + iter::<&&Option>().find(|x| x.is_some()).map(|x| x.unwrap()); + iter::>().find(|x| x.is_some()).map(|x| x.cloned().unwrap()); + iter::<&Option<&u8>>().find(|x| x.is_some()).map(|x| x.cloned().unwrap()); + iter::<&Option>().find(|x| x.is_some()).map(|x| x.as_deref().unwrap()); + iter::>().find(|&x| to_ref(x).is_some()).map(|y| to_ref(y).cloned().unwrap()); + + iter::>().find(|x| x.is_ok()).map(|x| x.unwrap()); + iter::<&Result>().find(|x| x.is_ok()).map(|x| x.unwrap()); + iter::<&&Result>().find(|x| x.is_ok()).map(|x| x.unwrap()); + iter::>().find(|x| x.is_ok()).map(|x| x.cloned().unwrap()); + iter::<&Result<&u8, ()>>().find(|x| x.is_ok()).map(|x| x.cloned().unwrap()); + iter::<&Result>().find(|x| x.is_ok()).map(|x| x.as_deref().unwrap()); + iter::>().find(|&x| to_ref(x).is_ok()).map(|y| to_ref(y).cloned().unwrap()); } fn no_lint() { @@ -28,6 +61,10 @@ fn no_lint() { .map(|a| to_opt(a).unwrap()); } +fn iter() -> impl Iterator { + std::iter::empty() +} + fn to_opt(_: T) -> Option { unimplemented!() } @@ -36,6 +73,10 @@ fn to_res(_: T) -> Result { unimplemented!() } +fn to_ref<'a, T>(_: T) -> &'a T { + unimplemented!() +} + struct Issue8920<'a> { option_field: Option, result_field: Result, diff --git a/tests/ui/manual_find_map.stderr b/tests/ui/manual_find_map.stderr index 9dea42b7686..c1ac499f7c6 100644 --- a/tests/ui/manual_find_map.stderr +++ b/tests/ui/manual_find_map.stderr @@ -19,7 +19,123 @@ LL | let _ = (0..).find(|&n| to_res(n).is_ok()).map(|a| to_res(a).unwrap_or( | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `find_map(|a| to_res(a).ok())` error: `find(..).map(..)` can be simplified as `find_map(..)` - --> $DIR/manual_find_map.rs:54:10 + --> $DIR/manual_find_map.rs:17:10 + | +LL | .find(|&x| to_ref(to_opt(x)).is_some()) + | __________^ +LL | | .map(|y| to_ref(to_opt(y)).unwrap()); + | |____________________________________________^ help: try: `find_map(|y| *to_ref(to_opt(y)))` + +error: `find(..).map(..)` can be simplified as `find_map(..)` + --> $DIR/manual_find_map.rs:20:10 + | +LL | .find(|x| to_ref(to_opt(*x)).is_some()) + | __________^ +LL | | .map(|y| to_ref(to_opt(y)).unwrap()); + | |____________________________________________^ help: try: `find_map(|y| *to_ref(to_opt(y)))` + +error: `find(..).map(..)` can be simplified as `find_map(..)` + --> $DIR/manual_find_map.rs:24:10 + | +LL | .find(|&x| to_ref(to_res(x)).is_ok()) + | __________^ +LL | | .map(|y| to_ref(to_res(y)).unwrap()); + | |____________________________________________^ help: try: `find_map(|y| to_ref(to_res(y)).ok())` + +error: `find(..).map(..)` can be simplified as `find_map(..)` + --> $DIR/manual_find_map.rs:27:10 + | +LL | .find(|x| to_ref(to_res(*x)).is_ok()) + | __________^ +LL | | .map(|y| to_ref(to_res(y)).unwrap()); + | |____________________________________________^ help: try: `find_map(|y| to_ref(to_res(y)).ok())` + +error: `find(..).map(..)` can be simplified as `find_map(..)` + --> $DIR/manual_find_map.rs:33:26 + | +LL | iter::>().find(|x| x.is_some()).map(|x| x.unwrap()); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `find_map(|x| x)` + +error: `find(..).map(..)` can be simplified as `find_map(..)` + --> $DIR/manual_find_map.rs:34:27 + | +LL | iter::<&Option>().find(|x| x.is_some()).map(|x| x.unwrap()); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `find_map(|x| *x)` + +error: `find(..).map(..)` can be simplified as `find_map(..)` + --> $DIR/manual_find_map.rs:35:28 + | +LL | iter::<&&Option>().find(|x| x.is_some()).map(|x| x.unwrap()); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `find_map(|x| **x)` + +error: `find(..).map(..)` can be simplified as `find_map(..)` + --> $DIR/manual_find_map.rs:36:27 + | +LL | iter::>().find(|x| x.is_some()).map(|x| x.cloned().unwrap()); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `find_map(|x| x.cloned())` + +error: `find(..).map(..)` can be simplified as `find_map(..)` + --> $DIR/manual_find_map.rs:37:28 + | +LL | iter::<&Option<&u8>>().find(|x| x.is_some()).map(|x| x.cloned().unwrap()); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `find_map(|x| x.cloned())` + +error: `find(..).map(..)` can be simplified as `find_map(..)` + --> $DIR/manual_find_map.rs:38:31 + | +LL | iter::<&Option>().find(|x| x.is_some()).map(|x| x.as_deref().unwrap()); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `find_map(|x| x.as_deref())` + +error: `find(..).map(..)` can be simplified as `find_map(..)` + --> $DIR/manual_find_map.rs:39:31 + | +LL | iter::>().find(|&x| to_ref(x).is_some()).map(|y| to_ref(y).cloned().unwrap()); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `find_map(|y| to_ref(y).cloned())` + +error: `find(..).map(..)` can be simplified as `find_map(..)` + --> $DIR/manual_find_map.rs:41:30 + | +LL | iter::>().find(|x| x.is_ok()).map(|x| x.unwrap()); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `find_map(|x| x.ok())` + +error: `find(..).map(..)` can be simplified as `find_map(..)` + --> $DIR/manual_find_map.rs:42:31 + | +LL | iter::<&Result>().find(|x| x.is_ok()).map(|x| x.unwrap()); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `find_map(|x| x.ok())` + +error: `find(..).map(..)` can be simplified as `find_map(..)` + --> $DIR/manual_find_map.rs:43:32 + | +LL | iter::<&&Result>().find(|x| x.is_ok()).map(|x| x.unwrap()); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `find_map(|x| x.ok())` + +error: `find(..).map(..)` can be simplified as `find_map(..)` + --> $DIR/manual_find_map.rs:44:31 + | +LL | iter::>().find(|x| x.is_ok()).map(|x| x.cloned().unwrap()); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `find_map(|x| x.cloned().ok())` + +error: `find(..).map(..)` can be simplified as `find_map(..)` + --> $DIR/manual_find_map.rs:45:32 + | +LL | iter::<&Result<&u8, ()>>().find(|x| x.is_ok()).map(|x| x.cloned().unwrap()); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `find_map(|x| x.cloned().ok())` + +error: `find(..).map(..)` can be simplified as `find_map(..)` + --> $DIR/manual_find_map.rs:46:35 + | +LL | iter::<&Result>().find(|x| x.is_ok()).map(|x| x.as_deref().unwrap()); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `find_map(|x| x.as_deref().ok())` + +error: `find(..).map(..)` can be simplified as `find_map(..)` + --> $DIR/manual_find_map.rs:47:35 + | +LL | iter::>().find(|&x| to_ref(x).is_ok()).map(|y| to_ref(y).cloned().unwrap()); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `find_map(|y| to_ref(y).cloned().ok())` + +error: `find(..).map(..)` can be simplified as `find_map(..)` + --> $DIR/manual_find_map.rs:95:10 | LL | .find(|f| f.option_field.is_some()) | __________^ @@ -27,7 +143,7 @@ LL | | .map(|f| f.option_field.clone().unwrap()); | |_________________________________________________^ help: try: `find_map(|f| f.option_field.clone())` error: `find(..).map(..)` can be simplified as `find_map(..)` - --> $DIR/manual_find_map.rs:59:10 + --> $DIR/manual_find_map.rs:100:10 | LL | .find(|f| f.ref_field.is_some()) | __________^ @@ -35,7 +151,7 @@ LL | | .map(|f| f.ref_field.cloned().unwrap()); | |_______________________________________________^ help: try: `find_map(|f| f.ref_field.cloned())` error: `find(..).map(..)` can be simplified as `find_map(..)` - --> $DIR/manual_find_map.rs:64:10 + --> $DIR/manual_find_map.rs:105:10 | LL | .find(|f| f.ref_field.is_some()) | __________^ @@ -43,7 +159,7 @@ LL | | .map(|f| f.ref_field.copied().unwrap()); | |_______________________________________________^ help: try: `find_map(|f| f.ref_field.copied())` error: `find(..).map(..)` can be simplified as `find_map(..)` - --> $DIR/manual_find_map.rs:69:10 + --> $DIR/manual_find_map.rs:110:10 | LL | .find(|f| f.result_field.is_ok()) | __________^ @@ -51,7 +167,7 @@ LL | | .map(|f| f.result_field.clone().unwrap()); | |_________________________________________________^ help: try: `find_map(|f| f.result_field.clone().ok())` error: `find(..).map(..)` can be simplified as `find_map(..)` - --> $DIR/manual_find_map.rs:74:10 + --> $DIR/manual_find_map.rs:115:10 | LL | .find(|f| f.result_field.is_ok()) | __________^ @@ -59,7 +175,7 @@ LL | | .map(|f| f.result_field.as_ref().unwrap()); | |__________________________________________________^ help: try: `find_map(|f| f.result_field.as_ref().ok())` error: `find(..).map(..)` can be simplified as `find_map(..)` - --> $DIR/manual_find_map.rs:79:10 + --> $DIR/manual_find_map.rs:120:10 | LL | .find(|f| f.result_field.is_ok()) | __________^ @@ -67,7 +183,7 @@ LL | | .map(|f| f.result_field.as_deref().unwrap()); | |____________________________________________________^ help: try: `find_map(|f| f.result_field.as_deref().ok())` error: `find(..).map(..)` can be simplified as `find_map(..)` - --> $DIR/manual_find_map.rs:84:10 + --> $DIR/manual_find_map.rs:125:10 | LL | .find(|f| f.result_field.is_ok()) | __________^ @@ -75,7 +191,7 @@ LL | | .map(|f| f.result_field.as_mut().unwrap()); | |__________________________________________________^ help: try: `find_map(|f| f.result_field.as_mut().ok())` error: `find(..).map(..)` can be simplified as `find_map(..)` - --> $DIR/manual_find_map.rs:89:10 + --> $DIR/manual_find_map.rs:130:10 | LL | .find(|f| f.result_field.is_ok()) | __________^ @@ -83,12 +199,12 @@ LL | | .map(|f| f.result_field.as_deref_mut().unwrap()); | |________________________________________________________^ help: try: `find_map(|f| f.result_field.as_deref_mut().ok())` error: `find(..).map(..)` can be simplified as `find_map(..)` - --> $DIR/manual_find_map.rs:94:10 + --> $DIR/manual_find_map.rs:135:10 | LL | .find(|f| f.result_field.is_ok()) | __________^ LL | | .map(|f| f.result_field.to_owned().unwrap()); | |____________________________________________________^ help: try: `find_map(|f| f.result_field.to_owned().ok())` -error: aborting due to 12 previous errors +error: aborting due to 30 previous errors