@@ -15,7 +15,8 @@ use if_chain::if_chain;
1515use rustc_ast:: ast;
1616use rustc_errors:: Applicability ;
1717use rustc_hir as hir;
18- use rustc_hir:: { TraitItem , TraitItemKind } ;
18+ use rustc_hir:: def:: Res ;
19+ use rustc_hir:: { Expr , ExprKind , PatKind , QPath , TraitItem , TraitItemKind , UnOp } ;
1920use rustc_lint:: { LateContext , LateLintPass , Lint , LintContext } ;
2021use rustc_middle:: lint:: in_external_macro;
2122use rustc_middle:: ty:: { self , TraitRef , Ty , TyS } ;
@@ -450,6 +451,58 @@ declare_clippy_lint! {
450451 "using combinations of `filter`, `map`, `filter_map` and `flat_map` which can usually be written as a single method call"
451452}
452453
454+ declare_clippy_lint ! {
455+ /// **What it does:** Checks for usage of `_.filter(_).map(_)` that can be written more simply
456+ /// as `filter_map(_)`.
457+ ///
458+ /// **Why is this bad?** Redundant code in the `filter` and `map` operations is poor style and
459+ /// less performant.
460+ ///
461+ /// **Known problems:** None.
462+ ///
463+ /// **Example:**
464+ /// Bad:
465+ /// ```rust
466+ /// (0_i32..10)
467+ /// .filter(|n| n.checked_add(1).is_some())
468+ /// .map(|n| n.checked_add(1).unwrap());
469+ /// ```
470+ ///
471+ /// Good:
472+ /// ```rust
473+ /// (0_i32..10).filter_map(|n| n.checked_add(1));
474+ /// ```
475+ pub MANUAL_FILTER_MAP ,
476+ complexity,
477+ "using `_.filter(_).map(_)` in a way that can be written more simply as `filter_map(_)`"
478+ }
479+
480+ declare_clippy_lint ! {
481+ /// **What it does:** Checks for usage of `_.find(_).map(_)` that can be written more simply
482+ /// as `find_map(_)`.
483+ ///
484+ /// **Why is this bad?** Redundant code in the `find` and `map` operations is poor style and
485+ /// less performant.
486+ ///
487+ /// **Known problems:** None.
488+ ///
489+ /// **Example:**
490+ /// Bad:
491+ /// ```rust
492+ /// (0_i32..10)
493+ /// .find(|n| n.checked_add(1).is_some())
494+ /// .map(|n| n.checked_add(1).unwrap());
495+ /// ```
496+ ///
497+ /// Good:
498+ /// ```rust
499+ /// (0_i32..10).find_map(|n| n.checked_add(1));
500+ /// ```
501+ pub MANUAL_FIND_MAP ,
502+ complexity,
503+ "using `_.find(_).map(_)` in a way that can be written more simply as `find_map(_)`"
504+ }
505+
453506declare_clippy_lint ! {
454507 /// **What it does:** Checks for usage of `_.filter_map(_).next()`.
455508///
@@ -494,28 +547,6 @@ declare_clippy_lint! {
494547 "call to `flat_map` where `flatten` is sufficient"
495548}
496549
497- declare_clippy_lint ! {
498- /// **What it does:** Checks for usage of `_.find(_).map(_)`.
499- ///
500- /// **Why is this bad?** Readability, this can be written more concisely as
501- /// `_.find_map(_)`.
502- ///
503- /// **Known problems:** Often requires a condition + Option/Iterator creation
504- /// inside the closure.
505- ///
506- /// **Example:**
507- /// ```rust
508- /// (0..3).find(|x| *x == 2).map(|x| x * 2);
509- /// ```
510- /// Can be written as
511- /// ```rust
512- /// (0..3).find_map(|x| if x == 2 { Some(x * 2) } else { None });
513- /// ```
514- pub FIND_MAP ,
515- pedantic,
516- "using a combination of `find` and `map` can usually be written as a single method call"
517- }
518-
519550declare_clippy_lint ! {
520551 /// **What it does:** Checks for an iterator or string search (such as `find()`,
521552/// `position()`, or `rposition()`) followed by a call to `is_some()`.
@@ -1473,9 +1504,10 @@ impl_lint_pass!(Methods => [
14731504 FILTER_NEXT ,
14741505 SKIP_WHILE_NEXT ,
14751506 FILTER_MAP ,
1507+ MANUAL_FILTER_MAP ,
1508+ MANUAL_FIND_MAP ,
14761509 FILTER_MAP_NEXT ,
14771510 FLAT_MAP_IDENTITY ,
1478- FIND_MAP ,
14791511 MAP_FLATTEN ,
14801512 ITERATOR_STEP_BY_ZERO ,
14811513 ITER_NEXT_SLICE ,
@@ -1540,10 +1572,10 @@ impl<'tcx> LateLintPass<'tcx> for Methods {
15401572 [ "next" , "filter" ] => lint_filter_next ( cx, expr, arg_lists[ 1 ] ) ,
15411573 [ "next" , "skip_while" ] => lint_skip_while_next ( cx, expr, arg_lists[ 1 ] ) ,
15421574 [ "next" , "iter" ] => lint_iter_next ( cx, expr, arg_lists[ 1 ] ) ,
1543- [ "map" , "filter" ] => lint_filter_map ( cx, expr, arg_lists [ 1 ] , arg_lists [ 0 ] ) ,
1575+ [ "map" , "filter" ] => lint_filter_map ( cx, expr, false ) ,
15441576 [ "map" , "filter_map" ] => lint_filter_map_map ( cx, expr, arg_lists[ 1 ] , arg_lists[ 0 ] ) ,
15451577 [ "next" , "filter_map" ] => lint_filter_map_next ( cx, expr, arg_lists[ 1 ] , self . msrv . as_ref ( ) ) ,
1546- [ "map" , "find" ] => lint_find_map ( cx, expr, arg_lists [ 1 ] , arg_lists [ 0 ] ) ,
1578+ [ "map" , "find" ] => lint_filter_map ( cx, expr, true ) ,
15471579 [ "flat_map" , "filter" ] => lint_filter_flat_map ( cx, expr, arg_lists[ 1 ] , arg_lists[ 0 ] ) ,
15481580 [ "flat_map" , "filter_map" ] => lint_filter_map_flat_map ( cx, expr, arg_lists[ 1 ] , arg_lists[ 0 ] ) ,
15491581 [ "flat_map" , ..] => lint_flat_map_identity ( cx, expr, arg_lists[ 0 ] , method_spans[ 0 ] ) ,
@@ -2988,18 +3020,79 @@ fn lint_skip_while_next<'tcx>(
29883020 }
29893021}
29903022
2991- /// lint use of `filter().map()` for `Iterators`
2992- fn lint_filter_map < ' tcx > (
2993- cx : & LateContext < ' tcx > ,
2994- expr : & ' tcx hir:: Expr < ' _ > ,
2995- _filter_args : & ' tcx [ hir:: Expr < ' _ > ] ,
2996- _map_args : & ' tcx [ hir:: Expr < ' _ > ] ,
2997- ) {
2998- // lint if caller of `.filter().map()` is an Iterator
2999- if match_trait_method ( cx, expr, & paths:: ITERATOR ) {
3000- let msg = "called `filter(..).map(..)` on an `Iterator`" ;
3001- let hint = "this is more succinctly expressed by calling `.filter_map(..)` instead" ;
3002- span_lint_and_help ( cx, FILTER_MAP , expr. span , msg, None , hint) ;
3023+ /// lint use of `filter().map()` or `find().map()` for `Iterators`
3024+ fn lint_filter_map < ' tcx > ( cx : & LateContext < ' tcx > , expr : & ' tcx hir:: Expr < ' _ > , is_find : bool ) {
3025+ if_chain ! {
3026+ if let ExprKind :: MethodCall ( _, _, [ map_recv, map_arg] , map_span) = expr. kind;
3027+ if let ExprKind :: MethodCall ( _, _, [ _, filter_arg] , filter_span) = map_recv. kind;
3028+ if match_trait_method( cx, map_recv, & paths:: ITERATOR ) ;
3029+
3030+ // filter(|x| ...is_some())...
3031+ if let ExprKind :: Closure ( _, _, filter_body_id, ..) = filter_arg. kind;
3032+ let filter_body = cx. tcx. hir( ) . body( filter_body_id) ;
3033+ if let [ filter_param] = filter_body. params;
3034+ // optional ref pattern: `filter(|&x| ..)`
3035+ let ( filter_pat, is_filter_param_ref) = if let PatKind :: Ref ( ref_pat, _) = filter_param. pat. kind {
3036+ ( ref_pat, true )
3037+ } else {
3038+ ( filter_param. pat, false )
3039+ } ;
3040+ // closure ends with is_some() or is_ok()
3041+ if let PatKind :: Binding ( _, filter_param_id, _, None ) = filter_pat. kind;
3042+ if let ExprKind :: MethodCall ( path, _, [ filter_arg] , _) = filter_body. value. kind;
3043+ if let Some ( opt_ty) = cx. typeck_results( ) . expr_ty( filter_arg) . ty_adt_def( ) ;
3044+ if let Some ( is_result) = if cx. tcx. is_diagnostic_item( sym:: option_type, opt_ty. did) {
3045+ Some ( false )
3046+ } else if cx. tcx. is_diagnostic_item( sym:: result_type, opt_ty. did) {
3047+ Some ( true )
3048+ } else {
3049+ None
3050+ } ;
3051+ if path. ident. name. as_str( ) == if is_result { "is_ok" } else { "is_some" } ;
3052+
3053+ // ...map(|x| ...unwrap())
3054+ if let ExprKind :: Closure ( _, _, map_body_id, ..) = map_arg. kind;
3055+ let map_body = cx. tcx. hir( ) . body( map_body_id) ;
3056+ if let [ map_param] = map_body. params;
3057+ if let PatKind :: Binding ( _, map_param_id, map_param_ident, None ) = map_param. pat. kind;
3058+ // closure ends with expect() or unwrap()
3059+ if let ExprKind :: MethodCall ( seg, _, [ map_arg, ..] , _) = map_body. value. kind;
3060+ if matches!( seg. ident. name, sym:: expect | sym:: unwrap | sym:: unwrap_or) ;
3061+
3062+ let eq_fallback = |a: & Expr <' _>, b: & Expr <' _>| {
3063+ // in `filter(|x| ..)`, replace `*x` with `x`
3064+ let a_path = if_chain! {
3065+ if !is_filter_param_ref;
3066+ if let ExprKind :: Unary ( UnOp :: UnDeref , expr_path) = a. kind;
3067+ then { expr_path } else { a }
3068+ } ;
3069+ // let the filter closure arg and the map closure arg be equal
3070+ if_chain! {
3071+ if let ExprKind :: Path ( QPath :: Resolved ( None , a_path) ) = a_path. kind;
3072+ if let ExprKind :: Path ( QPath :: Resolved ( None , b_path) ) = b. kind;
3073+ if a_path. res == Res :: Local ( filter_param_id) ;
3074+ if b_path. res == Res :: Local ( map_param_id) ;
3075+ if TyS :: same_type( cx. typeck_results( ) . expr_ty_adjusted( a) , cx. typeck_results( ) . expr_ty_adjusted( b) ) ;
3076+ then {
3077+ return true ;
3078+ }
3079+ }
3080+ false
3081+ } ;
3082+ if SpanlessEq :: new( cx) . expr_fallback( eq_fallback) . eq_expr( filter_arg, map_arg) ;
3083+ then {
3084+ let span = filter_span. to( map_span) ;
3085+ let ( filter_name, lint) = if is_find {
3086+ ( "find" , MANUAL_FIND_MAP )
3087+ } else {
3088+ ( "filter" , MANUAL_FILTER_MAP )
3089+ } ;
3090+ let msg = format!( "`{}(..).map(..)` can be simplified as `{0}_map(..)`" , filter_name) ;
3091+ let to_opt = if is_result { ".ok()" } else { "" } ;
3092+ let sugg = format!( "{}_map(|{}| {}{})" , filter_name, map_param_ident,
3093+ snippet( cx, map_arg. span, ".." ) , to_opt) ;
3094+ span_lint_and_sugg( cx, lint, span, & msg, "try" , sugg, Applicability :: MachineApplicable ) ;
3095+ }
30033096 }
30043097}
30053098
@@ -3037,29 +3130,14 @@ fn lint_filter_map_next<'tcx>(
30373130 }
30383131}
30393132
3040- /// lint use of `find().map()` for `Iterators`
3041- fn lint_find_map < ' tcx > (
3042- cx : & LateContext < ' tcx > ,
3043- expr : & ' tcx hir:: Expr < ' _ > ,
3044- _find_args : & ' tcx [ hir:: Expr < ' _ > ] ,
3045- map_args : & ' tcx [ hir:: Expr < ' _ > ] ,
3046- ) {
3047- // lint if caller of `.filter().map()` is an Iterator
3048- if match_trait_method ( cx, & map_args[ 0 ] , & paths:: ITERATOR ) {
3049- let msg = "called `find(..).map(..)` on an `Iterator`" ;
3050- let hint = "this is more succinctly expressed by calling `.find_map(..)` instead" ;
3051- span_lint_and_help ( cx, FIND_MAP , expr. span , msg, None , hint) ;
3052- }
3053- }
3054-
30553133/// lint use of `filter_map().map()` for `Iterators`
30563134fn lint_filter_map_map < ' tcx > (
30573135 cx : & LateContext < ' tcx > ,
30583136 expr : & ' tcx hir:: Expr < ' _ > ,
30593137 _filter_args : & ' tcx [ hir:: Expr < ' _ > ] ,
30603138 _map_args : & ' tcx [ hir:: Expr < ' _ > ] ,
30613139) {
3062- // lint if caller of `.filter ().map()` is an Iterator
3140+ // lint if caller of `.filter_map ().map()` is an Iterator
30633141 if match_trait_method ( cx, expr, & paths:: ITERATOR ) {
30643142 let msg = "called `filter_map(..).map(..)` on an `Iterator`" ;
30653143 let hint = "this is more succinctly expressed by only calling `.filter_map(..)` instead" ;
0 commit comments