return_and_then: only lint returning expressions (#14783)

If an expression is not going to return from the current function of
closure, it should not get linted.

This also allows `return` expression to be linted, in addition to the
final expression. Those require blockification and proper indentation.

changelog: [`return_and_then`]: only lint returning expressions

Fixes #14781

Author: llogiq

clippy_lints/src/methods/return_and_then.rs

@@ -9,7 +9,7 @@ use clippy_utils::diagnostics::span_lint_and_sugg;
 use clippy_utils::source::{indent_of, reindent_multiline, snippet_with_applicability};
 use clippy_utils::ty::get_type_diagnostic_name;
 use clippy_utils::visitors::for_each_unconsumed_temporary;
-use clippy_utils::{is_expr_final_block_expr, peel_blocks};
+use clippy_utils::{get_parent_expr, peel_blocks};
 
 use super::RETURN_AND_THEN;
 
@@ -21,7 +21,7 @@ pub(super) fn check<'tcx>(
     recv: &'tcx hir::Expr<'tcx>,
     arg: &'tcx hir::Expr<'_>,
 ) {
-    if !is_expr_final_block_expr(cx.tcx, expr) {
+    if cx.tcx.hir_get_fn_id_for_return_block(expr.hir_id).is_none() {
         return;
     }
 
@@ -55,12 +55,24 @@ pub(super) fn check<'tcx>(
         None => &body_snip,
     };
 
-    let sugg = format!(
-        "let {} = {}?;\n{}",
-        arg_snip,
-        recv_snip,
-        reindent_multiline(inner, false, indent_of(cx, expr.span))
-    );
+    // If suggestion is going to get inserted as part of a `return` expression, it must be blockified.
+    let sugg = if let Some(parent_expr) = get_parent_expr(cx, expr) {
+        let base_indent = indent_of(cx, parent_expr.span);
+        let inner_indent = base_indent.map(|i| i + 4);
+        format!(
+            "{}\n{}\n{}",
+            reindent_multiline(&format!("{{\nlet {arg_snip} = {recv_snip}?;"), true, inner_indent),
+            reindent_multiline(inner, false, inner_indent),
+            reindent_multiline("}", false, base_indent),
+        )
+    } else {
+        format!(
+            "let {} = {}?;\n{}",
+            arg_snip,
+            recv_snip,
+            reindent_multiline(inner, false, indent_of(cx, expr.span))
+        )
+    };
 
     span_lint_and_sugg(
         cx,

tests/ui/return_and_then.fixed

@@ -67,8 +67,60 @@ fn main() {
             .first() // creates temporary reference
             .and_then(|x| test_opt_block(Some(*x)))
     }
+
+    fn in_closure() -> bool {
+        let _ = || {
+            let x = Some("")?;
+            if x.len() > 2 { Some(3) } else { None }
+            //~^ return_and_then
+        };
+        true
+    }
+
+    fn with_return(shortcut: bool) -> Option<i32> {
+        if shortcut {
+            return {
+                let x = Some("")?;
+                if x.len() > 2 { Some(3) } else { None }
+            };
+            //~^ return_and_then
+        };
+        None
+    }
+
+    fn with_return_multiline(shortcut: bool) -> Option<i32> {
+        if shortcut {
+            return {
+                let mut x = Some("")?;
+                let x = format!("{x}.");
+                if x.len() > 2 { Some(3) } else { None }
+            };
+            //~^^^^ return_and_then
+        };
+        None
+    }
 }
 
 fn gen_option(n: i32) -> Option<i32> {
     Some(n)
 }
+
+mod issue14781 {
+    fn foo(_: &str, _: (u32, u32)) -> Result<(u32, u32), ()> {
+        Ok((1, 1))
+    }
+
+    fn bug(_: Option<&str>) -> Result<(), ()> {
+        let year: Option<&str> = None;
+        let month: Option<&str> = None;
+        let day: Option<&str> = None;
+
+        let _day = if let (Some(year), Some(month)) = (year, month) {
+            day.and_then(|day| foo(day, (1, 31)).ok())
+        } else {
+            None
+        };
+
+        Ok(())
+    }
+}

tests/ui/return_and_then.rs

@@ -63,8 +63,55 @@ fn main() {
             .first() // creates temporary reference
             .and_then(|x| test_opt_block(Some(*x)))
     }
+
+    fn in_closure() -> bool {
+        let _ = || {
+            Some("").and_then(|x| if x.len() > 2 { Some(3) } else { None })
+            //~^ return_and_then
+        };
+        true
+    }
+
+    fn with_return(shortcut: bool) -> Option<i32> {
+        if shortcut {
+            return Some("").and_then(|x| if x.len() > 2 { Some(3) } else { None });
+            //~^ return_and_then
+        };
+        None
+    }
+
+    fn with_return_multiline(shortcut: bool) -> Option<i32> {
+        if shortcut {
+            return Some("").and_then(|mut x| {
+                let x = format!("{x}.");
+                if x.len() > 2 { Some(3) } else { None }
+            });
+            //~^^^^ return_and_then
+        };
+        None
+    }
 }
 
 fn gen_option(n: i32) -> Option<i32> {
     Some(n)
 }
+
+mod issue14781 {
+    fn foo(_: &str, _: (u32, u32)) -> Result<(u32, u32), ()> {
+        Ok((1, 1))
+    }
+
+    fn bug(_: Option<&str>) -> Result<(), ()> {
+        let year: Option<&str> = None;
+        let month: Option<&str> = None;
+        let day: Option<&str> = None;
+
+        let _day = if let (Some(year), Some(month)) = (year, month) {
+            day.and_then(|day| foo(day, (1, 31)).ok())
+        } else {
+            None
+        };
+
+        Ok(())
+    }
+}

tests/ui/return_and_then.stderr

@@ -101,5 +101,50 @@ LL +         })?;
 LL +         if x.len() > 2 { Some(3) } else { None }
    |
 
-error: aborting due to 7 previous errors
+error: use the `?` operator instead of an `and_then` call
+  --> tests/ui/return_and_then.rs:69:13
+   |
+LL |             Some("").and_then(|x| if x.len() > 2 { Some(3) } else { None })
+   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+help: try
+   |
+LL ~             let x = Some("")?;
+LL +             if x.len() > 2 { Some(3) } else { None }
+   |
+
+error: use the `?` operator instead of an `and_then` call
+  --> tests/ui/return_and_then.rs:77:20
+   |
+LL |             return Some("").and_then(|x| if x.len() > 2 { Some(3) } else { None });
+   |                    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+help: try
+   |
+LL ~             return {
+LL +                 let x = Some("")?;
+LL +                 if x.len() > 2 { Some(3) } else { None }
+LL ~             };
+   |
+
+error: use the `?` operator instead of an `and_then` call
+  --> tests/ui/return_and_then.rs:85:20
+   |
+LL |               return Some("").and_then(|mut x| {
+   |  ____________________^
+LL | |                 let x = format!("{x}.");
+LL | |                 if x.len() > 2 { Some(3) } else { None }
+LL | |             });
+   | |______________^
+   |
+help: try
+   |
+LL ~             return {
+LL +                 let mut x = Some("")?;
+LL +                 let x = format!("{x}.");
+LL +                 if x.len() > 2 { Some(3) } else { None }
+LL ~             };
+   |
+
+error: aborting due to 10 previous errors