Fix manual_map suggestion for if let.. else ... if let.. else chain

Author: Jarcho

clippy_lints/src/manual_map.rs

@@ -12,7 +12,7 @@ use rustc_errors::Applicability;
 use rustc_hir::{
     def::Res,
     intravisit::{walk_expr, ErasedMap, NestedVisitorMap, Visitor},
-    Arm, BindingAnnotation, Block, Expr, ExprKind, Mutability, Pat, PatKind, Path, QPath,
+    Arm, BindingAnnotation, Block, Expr, ExprKind, MatchSource, Mutability, Node, Pat, PatKind, Path, QPath,
 };
 use rustc_lint::{LateContext, LateLintPass, LintContext};
 use rustc_middle::lint::in_external_macro;
@@ -55,8 +55,11 @@ impl LateLintPass<'_> for ManualMap {
             return;
         }
 
-        if let ExprKind::Match(scrutinee, [arm1 @ Arm { guard: None, .. }, arm2 @ Arm { guard: None, .. }], _) =
-            expr.kind
+        if let ExprKind::Match(
+            scrutinee,
+            [arm1 @ Arm { guard: None, .. }, arm2 @ Arm { guard: None, .. }],
+            match_kind,
+        ) = expr.kind
         {
             let (scrutinee_ty, ty_ref_count, ty_mutability) =
                 peel_mid_ty_refs_is_mutable(cx.typeck_results().expr_ty(scrutinee));
@@ -182,13 +185,36 @@ impl LateLintPass<'_> for ManualMap {
                 expr.span,
                 "manual implementation of `Option::map`",
                 "try this",
-                format!("{}{}.map({})", scrutinee_str, as_ref_str, body_str),
+                if matches!(match_kind, MatchSource::IfLetDesugar { .. }) && is_else_of_if_let_else(cx, expr) {
+                    format!("{{ {}{}.map({}) }}", scrutinee_str, as_ref_str, body_str)
+                } else {
+                    format!("{}{}.map({})", scrutinee_str, as_ref_str, body_str)
+                },
                 app,
             );
         }
     }
 }
 
+fn is_else_of_if_let_else(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool {
+    let map = cx.tcx.hir();
+    let mut iter = map.parent_iter(expr.hir_id);
+    let arm_id = match iter.next() {
+        Some((id, Node::Arm(..))) => id,
+        _ => return false,
+    };
+    match iter.next() {
+        Some((
+            _,
+            Node::Expr(Expr {
+                kind: ExprKind::Match(_, [_, else_arm], kind),
+                ..
+            }),
+        )) => else_arm.hir_id == arm_id && matches!(kind, MatchSource::IfLetDesugar { .. }),
+        _ => false,
+    }
+}
+
 // Checks if the expression can be moved into a closure as is.
 fn can_move_expr_to_closure(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) -> bool {
     struct V<'cx, 'tcx> {

tests/ui/manual_map_option.fixed

@@ -128,4 +128,9 @@ fn main() {
             None => None,
         };
     }
+
+    // #6847
+    if let Some(x) = Some(0) {
+        Some(0)
+    } else { Some(0).map(|x| x + 1) };
 }

tests/ui/manual_map_option.rs

@@ -186,4 +186,13 @@ fn main() {
             None => None,
         };
     }
+
+    // #6847
+    if let Some(x) = Some(0) {
+        Some(0)
+    } else if let Some(x) = Some(0) {
+        Some(x + 1)
+    } else {
+        None
+    };
 }

tests/ui/manual_map_option.stderr

@@ -172,5 +172,16 @@ LL | |         None => None,
 LL | |     };
    | |_____^ help: try this: `option_env!("").map(String::from)`
 
-error: aborting due to 19 previous errors
+error: manual implementation of `Option::map`
+  --> $DIR/manual_map_option.rs:193:12
+   |
+LL |       } else if let Some(x) = Some(0) {
+   |  ____________^
+LL | |         Some(x + 1)
+LL | |     } else {
+LL | |         None
+LL | |     };
+   | |_____^ help: try this: `{ Some(0).map(|x| x + 1) }`
+
+error: aborting due to 20 previous errors