feat: new lint for and_then when returning Option or Result (#14051)

close #6436

changelog: [`return_and_then`]: new lint

CHANGELOG.md

@@ -6026,6 +6026,7 @@ Released 2018-09-13
 [`result_map_unwrap_or_else`]: https://rust-lang.github.io/rust-clippy/master/index.html#result_map_unwrap_or_else
 [`result_unit_err`]: https://rust-lang.github.io/rust-clippy/master/index.html#result_unit_err
 [`result_unwrap_used`]: https://rust-lang.github.io/rust-clippy/master/index.html#result_unwrap_used
+[`return_and_then`]: https://rust-lang.github.io/rust-clippy/master/index.html#return_and_then
 [`return_self_not_must_use`]: https://rust-lang.github.io/rust-clippy/master/index.html#return_self_not_must_use
 [`reverse_range_loop`]: https://rust-lang.github.io/rust-clippy/master/index.html#reverse_range_loop
 [`reversed_empty_ranges`]: https://rust-lang.github.io/rust-clippy/master/index.html#reversed_empty_ranges

clippy_lints/src/declared_lints.rs

@@ -463,6 +463,7 @@ pub static LINTS: &[&crate::LintInfo] = &[
     crate::methods::REPEAT_ONCE_INFO,
     crate::methods::RESULT_FILTER_MAP_INFO,
     crate::methods::RESULT_MAP_OR_INTO_OPTION_INFO,
+    crate::methods::RETURN_AND_THEN_INFO,
     crate::methods::SEARCH_IS_SOME_INFO,
     crate::methods::SEEK_FROM_CURRENT_INFO,
     crate::methods::SEEK_TO_START_INSTEAD_OF_REWIND_INFO,

clippy_lints/src/methods/mod.rs

@@ -95,6 +95,7 @@ mod readonly_write_lock;
 mod redundant_as_str;
 mod repeat_once;
 mod result_map_or_else_none;
+mod return_and_then;
 mod search_is_some;
 mod seek_from_current;
 mod seek_to_start_instead_of_rewind;
@@ -4392,6 +4393,46 @@ declare_clippy_lint! {
      "slicing a string and immediately calling as_bytes is less efficient and can lead to panics"
 }
 
+declare_clippy_lint! {
+    /// ### What it does
+    ///
+    /// Detect functions that end with `Option::and_then` or `Result::and_then`, and suggest using a question mark (`?`) instead.
+    ///
+    /// ### Why is this bad?
+    ///
+    /// The `and_then` method is used to chain a computation that returns an `Option` or a `Result`.
+    /// This can be replaced with the `?` operator, which is more concise and idiomatic.
+    ///
+    /// ### Example
+    ///
+    /// ```no_run
+    /// fn test(opt: Option<i32>) -> Option<i32> {
+    ///     opt.and_then(|n| {
+    ///         if n > 1 {
+    ///             Some(n + 1)
+    ///         } else {
+    ///             None
+    ///        }
+    ///     })
+    /// }
+    /// ```
+    /// Use instead:
+    /// ```no_run
+    /// fn test(opt: Option<i32>) -> Option<i32> {
+    ///     let n = opt?;
+    ///     if n > 1 {
+    ///         Some(n + 1)
+    ///     } else {
+    ///         None
+    ///     }
+    /// }
+    /// ```
+    #[clippy::version = "1.86.0"]
+    pub RETURN_AND_THEN,
+    restriction,
+    "using `Option::and_then` or `Result::and_then` to chain a computation that returns an `Option` or a `Result`"
+}
+
 pub struct Methods {
     avoid_breaking_exported_api: bool,
     msrv: Msrv,
@@ -4561,6 +4602,7 @@ impl_lint_pass!(Methods => [
     USELESS_NONZERO_NEW_UNCHECKED,
     MANUAL_REPEAT_N,
     SLICED_STRING_AS_BYTES,
+    RETURN_AND_THEN,
 ]);
 
 /// Extracts a method call name, args, and `Span` of the method name.
@@ -4790,7 +4832,10 @@ impl Methods {
                     let biom_option_linted = bind_instead_of_map::check_and_then_some(cx, expr, recv, arg);
                     let biom_result_linted = bind_instead_of_map::check_and_then_ok(cx, expr, recv, arg);
                     if !biom_option_linted && !biom_result_linted {
-                        unnecessary_lazy_eval::check(cx, expr, recv, arg, "and");
+                        let ule_and_linted = unnecessary_lazy_eval::check(cx, expr, recv, arg, "and");
+                        if !ule_and_linted {
+                            return_and_then::check(cx, expr, recv, arg);
+                        }
                     }
                 },
                 ("any", [arg]) => {
@@ -5004,7 +5049,9 @@ impl Methods {
                     get_first::check(cx, expr, recv, arg);
                     get_last_with_len::check(cx, expr, recv, arg);
                 },
-                ("get_or_insert_with", [arg]) => unnecessary_lazy_eval::check(cx, expr, recv, arg, "get_or_insert"),
+                ("get_or_insert_with", [arg]) => {
+                    unnecessary_lazy_eval::check(cx, expr, recv, arg, "get_or_insert");
+                },
                 ("hash", [arg]) => {
                     unit_hash::check(cx, expr, recv, arg);
                 },
@@ -5145,7 +5192,9 @@ impl Methods {
                     },
                     _ => iter_nth_zero::check(cx, expr, recv, n_arg),
                 },
-                ("ok_or_else", [arg]) => unnecessary_lazy_eval::check(cx, expr, recv, arg, "ok_or"),
+                ("ok_or_else", [arg]) => {
+                    unnecessary_lazy_eval::check(cx, expr, recv, arg, "ok_or");
+                },
                 ("open", [_]) => {
                     open_options::check(cx, expr, recv);
                 },

clippy_lints/src/methods/return_and_then.rs

@@ -0,0 +1,67 @@
+use rustc_errors::Applicability;
+use rustc_hir as hir;
+use rustc_lint::LateContext;
+use rustc_middle::ty::{self, GenericArg, Ty};
+use rustc_span::sym;
+use std::ops::ControlFlow;
+
+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 super::RETURN_AND_THEN;
+
+/// lint if `and_then` is the last expression in a block, and
+/// there are no references or temporaries in the receiver
+pub(super) fn check<'tcx>(
+    cx: &LateContext<'tcx>,
+    expr: &hir::Expr<'_>,
+    recv: &'tcx hir::Expr<'tcx>,
+    arg: &'tcx hir::Expr<'_>,
+) {
+    if !is_expr_final_block_expr(cx.tcx, expr) {
+        return;
+    }
+
+    let recv_type = cx.typeck_results().expr_ty(recv);
+    if !matches!(get_type_diagnostic_name(cx, recv_type), Some(sym::Option | sym::Result)) {
+        return;
+    }
+
+    let has_ref_type = matches!(recv_type.kind(), ty::Adt(_, args) if args
+        .first()
+        .and_then(|arg0: &GenericArg<'tcx>| GenericArg::as_type(*arg0))
+        .is_some_and(Ty::is_ref));
+    let has_temporaries = for_each_unconsumed_temporary(cx, recv, |_| ControlFlow::Break(())).is_break();
+    if has_ref_type && has_temporaries {
+        return;
+    }
+
+    let hir::ExprKind::Closure(&hir::Closure { body, fn_decl, .. }) = arg.kind else {
+        return;
+    };
+
+    let closure_arg = fn_decl.inputs[0];
+    let closure_expr = peel_blocks(cx.tcx.hir().body(body).value);
+
+    let mut applicability = Applicability::MachineApplicable;
+    let arg_snip = snippet_with_applicability(cx, closure_arg.span, "_", &mut applicability);
+    let recv_snip = snippet_with_applicability(cx, recv.span, "_", &mut applicability);
+    let body_snip = snippet_with_applicability(cx, closure_expr.span, "..", &mut applicability);
+    let inner = match body_snip.strip_prefix('{').and_then(|s| s.strip_suffix('}')) {
+        Some(s) => s.trim_start_matches('\n').trim_end(),
+        None => &body_snip,
+    };
+
+    let msg = "use the question mark operator instead of an `and_then` call";
+    let sugg = format!(
+        "let {} = {}?;\n{}",
+        arg_snip,
+        recv_snip,
+        reindent_multiline(inner.into(), false, indent_of(cx, expr.span))
+    );
+
+    span_lint_and_sugg(cx, RETURN_AND_THEN, expr.span, msg, "try", sugg, applicability);
+}

clippy_lints/src/methods/unnecessary_lazy_eval.rs

@@ -18,7 +18,7 @@ pub(super) fn check<'tcx>(
     recv: &'tcx hir::Expr<'_>,
     arg: &'tcx hir::Expr<'_>,
     simplify_using: &str,
-) {
+) -> bool {
     let is_option = is_type_diagnostic_item(cx, cx.typeck_results().expr_ty(recv), sym::Option);
     let is_result = is_type_diagnostic_item(cx, cx.typeck_results().expr_ty(recv), sym::Result);
     let is_bool = cx.typeck_results().expr_ty(recv).is_bool();
@@ -29,7 +29,7 @@ pub(super) fn check<'tcx>(
             let body_expr = &body.value;
 
             if usage::BindingUsageFinder::are_params_used(cx, body) || is_from_proc_macro(cx, expr) {
-                return;
+                return false;
             }
 
             if eager_or_lazy::switch_to_eager_eval(cx, body_expr) {
@@ -71,8 +71,10 @@ pub(super) fn check<'tcx>(
                             applicability,
                         );
                     });
+                    return true;
                 }
             }
         }
     }
+    false
 }

tests/ui/return_and_then.fixed

@@ -0,0 +1,67 @@
+#![warn(clippy::return_and_then)]
+
+fn main() {
+    fn test_opt_block(opt: Option<i32>) -> Option<i32> {
+        let n = opt?;
+        let mut ret = n + 1;
+        ret += n;
+        if n > 1 { Some(ret) } else { None }
+    }
+
+    fn test_opt_func(opt: Option<i32>) -> Option<i32> {
+        let n = opt?;
+        test_opt_block(Some(n))
+    }
+
+    fn test_call_chain() -> Option<i32> {
+        let n = gen_option(1)?;
+        test_opt_block(Some(n))
+    }
+
+    fn test_res_block(opt: Result<i32, i32>) -> Result<i32, i32> {
+        let n = opt?;
+        if n > 1 { Ok(n + 1) } else { Err(n) }
+    }
+
+    fn test_res_func(opt: Result<i32, i32>) -> Result<i32, i32> {
+        let n = opt?;
+        test_res_block(Ok(n))
+    }
+
+    fn test_ref_only() -> Option<i32> {
+        // ref: empty string
+        let x = Some("")?;
+        if x.len() > 2 { Some(3) } else { None }
+    }
+
+    fn test_tmp_only() -> Option<i32> {
+        // unused temporary: vec![1, 2, 4]
+        let x = Some(match (vec![1, 2, 3], vec![1, 2, 4]) {
+            (a, _) if a.len() > 1 => a,
+            (_, b) => b,
+        })?;
+        if x.len() > 2 { Some(3) } else { None }
+    }
+
+    // should not lint
+    fn test_tmp_ref() -> Option<String> {
+        String::from("<BOOM>")
+            .strip_prefix("<")
+            .and_then(|s| s.strip_suffix(">").map(String::from))
+    }
+
+    // should not lint
+    fn test_unconsumed_tmp() -> Option<i32> {
+        [1, 2, 3]
+            .iter()
+            .map(|x| x + 1)
+            .collect::<Vec<_>>() // temporary Vec created here
+            .as_slice() // creates temporary slice
+            .first() // creates temporary reference
+            .and_then(|x| test_opt_block(Some(*x)))
+    }
+}
+
+fn gen_option(n: i32) -> Option<i32> {
+    Some(n)
+}

tests/ui/return_and_then.rs

@@ -0,0 +1,63 @@
+#![warn(clippy::return_and_then)]
+
+fn main() {
+    fn test_opt_block(opt: Option<i32>) -> Option<i32> {
+        opt.and_then(|n| {
+            let mut ret = n + 1;
+            ret += n;
+            if n > 1 { Some(ret) } else { None }
+        })
+    }
+
+    fn test_opt_func(opt: Option<i32>) -> Option<i32> {
+        opt.and_then(|n| test_opt_block(Some(n)))
+    }
+
+    fn test_call_chain() -> Option<i32> {
+        gen_option(1).and_then(|n| test_opt_block(Some(n)))
+    }
+
+    fn test_res_block(opt: Result<i32, i32>) -> Result<i32, i32> {
+        opt.and_then(|n| if n > 1 { Ok(n + 1) } else { Err(n) })
+    }
+
+    fn test_res_func(opt: Result<i32, i32>) -> Result<i32, i32> {
+        opt.and_then(|n| test_res_block(Ok(n)))
+    }
+
+    fn test_ref_only() -> Option<i32> {
+        // ref: empty string
+        Some("").and_then(|x| if x.len() > 2 { Some(3) } else { None })
+    }
+
+    fn test_tmp_only() -> Option<i32> {
+        // unused temporary: vec![1, 2, 4]
+        Some(match (vec![1, 2, 3], vec![1, 2, 4]) {
+            (a, _) if a.len() > 1 => a,
+            (_, b) => b,
+        })
+        .and_then(|x| if x.len() > 2 { Some(3) } else { None })
+    }
+
+    // should not lint
+    fn test_tmp_ref() -> Option<String> {
+        String::from("<BOOM>")
+            .strip_prefix("<")
+            .and_then(|s| s.strip_suffix(">").map(String::from))
+    }
+
+    // should not lint
+    fn test_unconsumed_tmp() -> Option<i32> {
+        [1, 2, 3]
+            .iter()
+            .map(|x| x + 1)
+            .collect::<Vec<_>>() // temporary Vec created here
+            .as_slice() // creates temporary slice
+            .first() // creates temporary reference
+            .and_then(|x| test_opt_block(Some(*x)))
+    }
+}
+
+fn gen_option(n: i32) -> Option<i32> {
+    Some(n)
+}