manual_ok_err: blockify the replacement of an else if …

If the part being replaced is an `if` expression following an `else`,
the replacement expression must be blockified.

Author: samueltardieu

clippy_lints/src/matches/manual_ok_err.rs

@@ -1,7 +1,8 @@
 use clippy_utils::diagnostics::span_lint_and_sugg;
+use clippy_utils::source::{indent_of, reindent_multiline};
 use clippy_utils::sugg::Sugg;
 use clippy_utils::ty::option_arg_ty;
-use clippy_utils::{is_res_lang_ctor, path_res, peel_blocks, span_contains_comment};
+use clippy_utils::{get_parent_expr, is_res_lang_ctor, path_res, peel_blocks, span_contains_comment};
 use rustc_ast::BindingMode;
 use rustc_errors::Applicability;
 use rustc_hir::LangItem::{OptionNone, OptionSome, ResultErr};
@@ -132,13 +133,23 @@ fn apply_lint(cx: &LateContext<'_>, expr: &Expr<'_>, scrutinee: &Expr<'_>, is_ok
         Applicability::MachineApplicable
     };
     let scrut = Sugg::hir_with_applicability(cx, scrutinee, "..", &mut app).maybe_par();
+    let sugg = format!("{scrut}.{method}()");
+    // If the expression being expanded is the `if …` part of an `else if …`, it must be blockified.
+    let sugg = if let Some(parent_expr) = get_parent_expr(cx, expr)
+        && let ExprKind::If(_, _, Some(else_part)) = parent_expr.kind
+        && else_part.hir_id == expr.hir_id
+    {
+        reindent_multiline(&format!("{{\n    {sugg}\n}}"), true, indent_of(cx, parent_expr.span))
+    } else {
+        sugg
+    };
     span_lint_and_sugg(
         cx,
         MANUAL_OK_ERR,
         expr.span,
         format!("manual implementation of `{method}`"),
         "replace with",
-        format!("{scrut}.{method}()"),
+        sugg,
         app,
     );
 }

tests/ui/manual_ok_err.fixed

@@ -89,3 +89,12 @@ const fn cf(x: Result<u32, &'static str>) -> Option<u32> {
         Err(_) => None,
     }
 }
+
+fn issue14239() {
+    let _ = if false {
+        None
+    } else {
+        "1".parse::<u8>().ok()
+    };
+    //~^^^^^ manual_ok_err
+}

tests/ui/manual_ok_err.rs

@@ -125,3 +125,14 @@ const fn cf(x: Result<u32, &'static str>) -> Option<u32> {
         Err(_) => None,
     }
 }
+
+fn issue14239() {
+    let _ = if false {
+        None
+    } else if let Ok(n) = "1".parse::<u8>() {
+        Some(n)
+    } else {
+        None
+    };
+    //~^^^^^ manual_ok_err
+}

tests/ui/manual_ok_err.stderr

@@ -93,5 +93,23 @@ LL | |         _ => None,
 LL | |     };
    | |_____^ help: replace with: `(-S).ok()`
 
-error: aborting due to 8 previous errors
+error: manual implementation of `ok`
+  --> tests/ui/manual_ok_err.rs:132:12
+   |
+LL |       } else if let Ok(n) = "1".parse::<u8>() {
+   |  ____________^
+LL | |         Some(n)
+LL | |     } else {
+LL | |         None
+LL | |     };
+   | |_____^
+   |
+help: replace with
+   |
+LL ~     } else {
+LL +         "1".parse::<u8>().ok()
+LL ~     };
+   |
+
+error: aborting due to 9 previous errors