feat: new lint for and_then when returning Option or Result

CHANGELOG.md

@@ -6024,6 +6024,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/booleans.rs

@@ -403,29 +403,28 @@ fn simplify_not(cx: &LateContext<'_>, curr_msrv: &Msrv, expr: &Expr<'_>) -> Opti
                 return None;
             }
 
-            match binop.node {
+            let op = match binop.node {
                 BinOpKind::Eq => Some(" != "),
                 BinOpKind::Ne => Some(" == "),
                 BinOpKind::Lt => Some(" >= "),
                 BinOpKind::Gt => Some(" <= "),
                 BinOpKind::Le => Some(" > "),
                 BinOpKind::Ge => Some(" < "),
                 _ => None,
-            }
-            .and_then(|op| {
-                let lhs_snippet = lhs.span.get_source_text(cx)?;
-                let rhs_snippet = rhs.span.get_source_text(cx)?;
-
-                if !(lhs_snippet.starts_with('(') && lhs_snippet.ends_with(')')) {
-                    if let (ExprKind::Cast(..), BinOpKind::Ge) = (&lhs.kind, binop.node) {
-                        // e.g. `(a as u64) < b`. Without the parens the `<` is
-                        // interpreted as a start of generic arguments for `u64`
-                        return Some(format!("({lhs_snippet}){op}{rhs_snippet}"));
-                    }
+            }?;
+
+            let lhs_snippet = lhs.span.get_source_text(cx)?;
+            let rhs_snippet = rhs.span.get_source_text(cx)?;
+
+            if !(lhs_snippet.starts_with('(') && lhs_snippet.ends_with(')')) {
+                if let (ExprKind::Cast(..), BinOpKind::Ge) = (&lhs.kind, binop.node) {
+                    // e.g. `(a as u64) < b`. Without the parens the `<` is
+                    // interpreted as a start of generic arguments for `u64`
+                    return Some(format!("({lhs_snippet}){op}{rhs_snippet}"));
                 }
+            }
 
-                Some(format!("{lhs_snippet}{op}{rhs_snippet}"))
-            })
+            Some(format!("{lhs_snippet}{op}{rhs_snippet}"))
         },
         ExprKind::MethodCall(path, receiver, args, _) => {
             let type_of_receiver = cx.typeck_results().expr_ty(receiver);
@@ -434,22 +433,20 @@ fn simplify_not(cx: &LateContext<'_>, curr_msrv: &Msrv, expr: &Expr<'_>) -> Opti
             {
                 return None;
             }
-            METHODS_WITH_NEGATION
+            let (_, _, neg_method) = METHODS_WITH_NEGATION
                 .iter()
                 .copied()
                 .flat_map(|(msrv, a, b)| vec![(msrv, a, b), (msrv, b, a)])
-                .find(|&(msrv, a, _)| msrv.is_none_or(|msrv| curr_msrv.meets(msrv)) && a == path.ident.name.as_str())
-                .and_then(|(_, _, neg_method)| {
-                    let negated_args = args
-                        .iter()
-                        .map(|arg| simplify_not(cx, curr_msrv, arg))
-                        .collect::<Option<Vec<_>>>()?
-                        .join(", ");
-                    Some(format!(
-                        "{}.{neg_method}({negated_args})",
-                        receiver.span.get_source_text(cx)?
-                    ))
-                })
+                .find(|&(msrv, a, _)| msrv.is_none_or(|msrv| curr_msrv.meets(msrv)) && a == path.ident.name.as_str())?;
+            let negated_args = args
+                .iter()
+                .map(|arg| simplify_not(cx, curr_msrv, arg))
+                .collect::<Option<Vec<_>>>()?
+                .join(", ");
+            Some(format!(
+                "{}.{neg_method}({negated_args})",
+                receiver.span.get_source_text(cx)?
+            ))
         },
         ExprKind::Closure(closure) => {
             let body = cx.tcx.hir().body(closure.body);

clippy_lints/src/checked_conversions.rs

@@ -72,9 +72,8 @@ impl LateLintPass<'_> for CheckedConversions {
                 None => check_upper_bound(lt1, gt1).filter(|cv| cv.cvt == ConversionType::FromUnsigned),
                 Some((lt2, gt2)) => {
                     let upper_lower = |lt1, gt1, lt2, gt2| {
-                        check_upper_bound(lt1, gt1)
-                            .zip(check_lower_bound(lt2, gt2))
-                            .and_then(|(l, r)| l.combine(r, cx))
+                        let (l, r) = check_upper_bound(lt1, gt1).zip(check_lower_bound(lt2, gt2))?;
+                        l.combine(r, cx)
                     };
                     upper_lower(lt1, gt1, lt2, gt2).or_else(|| upper_lower(lt2, gt2, lt1, gt1))
                 },

clippy_lints/src/declared_lints.rs

@@ -462,6 +462,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/loops/manual_memcpy.rs

@@ -55,10 +55,10 @@ pub(super) fn check<'tcx>(
             let big_sugg = assignments
                 // The only statements in the for loops can be indexed assignments from
                 // indexed retrievals (except increments of loop counters).
-                .map(|o| {
-                    o.and_then(|(lhs, rhs)| {
-                        let rhs = fetch_cloned_expr(rhs);
-                        if let ExprKind::Index(base_left, idx_left, _) = lhs.kind
+                .map(|assignment| {
+                    let (lhs, rhs) = assignment?;
+                    let rhs = fetch_cloned_expr(rhs);
+                    if let ExprKind::Index(base_left, idx_left, _) = lhs.kind
                             && let ExprKind::Index(base_right, idx_right, _) = rhs.kind
                             && let Some(ty) = get_slice_like_element_ty(cx, cx.typeck_results().expr_ty(base_left))
                             && get_slice_like_element_ty(cx, cx.typeck_results().expr_ty(base_right)).is_some()
@@ -68,24 +68,23 @@ pub(super) fn check<'tcx>(
                             && !local_used_in(cx, canonical_id, base_right)
 							// Source and destination must be different
                             && path_to_local(base_left) != path_to_local(base_right)
-                        {
-                            Some((
-                                ty,
-                                IndexExpr {
-                                    base: base_left,
-                                    idx: start_left,
-                                    idx_offset: offset_left,
-                                },
-                                IndexExpr {
-                                    base: base_right,
-                                    idx: start_right,
-                                    idx_offset: offset_right,
-                                },
-                            ))
-                        } else {
-                            None
-                        }
-                    })
+                    {
+                        Some((
+                            ty,
+                            IndexExpr {
+                                base: base_left,
+                                idx: start_left,
+                                idx_offset: offset_left,
+                            },
+                            IndexExpr {
+                                base: base_right,
+                                idx: start_right,
+                                idx_offset: offset_right,
+                            },
+                        ))
+                    } else {
+                        None
+                    }
                 })
                 .map(|o| o.map(|(ty, dst, src)| build_manual_memcpy_suggestion(cx, start, end, limits, ty, &dst, &src)))
                 .collect::<Option<Vec<_>>>()
@@ -380,7 +379,8 @@ fn get_details_from_idx<'tcx>(
                 offset_opt.map(|(s, o)| (s, Offset::positive(o)))
             },
             BinOpKind::Sub => {
-                get_start(lhs, starts).and_then(|s| get_offset(cx, rhs, starts).map(|o| (s, Offset::negative(o))))
+                let start = get_start(lhs, starts)?;
+                get_offset(cx, rhs, starts).map(|o| (start, Offset::negative(o)))
             },
             _ => None,
         },
@@ -442,20 +442,19 @@ fn get_loop_counters<'a, 'tcx>(
 
     // For each candidate, check the parent block to see if
     // it's initialized to zero at the start of the loop.
-    get_enclosing_block(cx, expr.hir_id).and_then(|block| {
-        increment_visitor
-            .into_results()
-            .filter_map(move |var_id| {
-                let mut initialize_visitor = InitializeVisitor::new(cx, expr, var_id);
-                walk_block(&mut initialize_visitor, block);
-
-                initialize_visitor.get_result().map(|(_, _, initializer)| Start {
-                    id: var_id,
-                    kind: StartKind::Counter { initializer },
-                })
+    let block = get_enclosing_block(cx, expr.hir_id)?;
+    increment_visitor
+        .into_results()
+        .filter_map(move |var_id| {
+            let mut initialize_visitor = InitializeVisitor::new(cx, expr, var_id);
+            walk_block(&mut initialize_visitor, block);
+
+            initialize_visitor.get_result().map(|(_, _, initializer)| Start {
+                id: var_id,
+                kind: StartKind::Counter { initializer },
             })
-            .into()
-    })
+        })
+        .into()
 }
 
 fn is_array_length_equal_to_range(cx: &LateContext<'_>, start: &Expr<'_>, end: &Expr<'_>, arr: &Expr<'_>) -> bool {

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;
@@ -4363,6 +4364,46 @@ declare_clippy_lint! {
     "detect `repeat().take()` that can be replaced with `repeat_n()`"
 }
 
+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.85.0"]
+    pub RETURN_AND_THEN,
+    style,
+    "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,
@@ -4531,6 +4572,7 @@ impl_lint_pass!(Methods => [
     DOUBLE_ENDED_ITERATOR_LAST,
     USELESS_NONZERO_NEW_UNCHECKED,
     MANUAL_REPEAT_N,
+    RETURN_AND_THEN,
 ]);
 
 /// Extracts a method call name, args, and `Span` of the method name.
@@ -4760,7 +4802,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]) => {
@@ -4973,7 +5018,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);
                 },
@@ -5114,7 +5161,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,69 @@
+use clippy_utils::diagnostics::span_lint_and_sugg;
+use clippy_utils::source::{indent_of, reindent_multiline, snippet};
+use clippy_utils::ty::is_type_diagnostic_item;
+use clippy_utils::{is_expr_final_block_expr, is_expr_used_or_unified, peel_blocks};
+use rustc_errors::Applicability;
+use rustc_hir as hir;
+use rustc_lint::LateContext;
+use rustc_span::sym;
+
+use super::RETURN_AND_THEN;
+
+fn is_final_call(cx: &LateContext<'_>, expr: &hir::Expr<'_>) -> bool {
+    if !is_expr_used_or_unified(cx.tcx, expr) {
+        return false;
+    }
+    is_expr_final_block_expr(cx.tcx, expr)
+}
+
+/// lint if `and_then` is the last call in the function
+pub(super) fn check<'tcx>(
+    cx: &LateContext<'_>,
+    expr: &hir::Expr<'_>,
+    recv: &'tcx hir::Expr<'_>,
+    arg: &'tcx hir::Expr<'_>,
+) {
+    if !is_final_call(cx, expr) {
+        return;
+    }
+
+    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);
+
+    if !is_option && !is_result {
+        return;
+    }
+
+    let hir::ExprKind::Closure(&hir::Closure { body, fn_decl, .. }) = arg.kind else {
+        return;
+    };
+
+    let closure_arg = fn_decl.inputs[0];
+    let closure_body = cx.tcx.hir().body(body);
+    let closure_expr = peel_blocks(closure_body.value);
+
+    let msg = "use the question mark operator instead of an `and_then` call";
+    let body_snip = snippet(cx, closure_expr.span, "..");
+    let inner = if body_snip.starts_with('{') {
+        body_snip[1..body_snip.len() - 1].trim()
+    } else {
+        body_snip.trim()
+    };
+
+    let sugg = format!(
+        "let {} = {}?;\n{}",
+        snippet(cx, closure_arg.span, "_"),
+        snippet(cx, recv.span, ".."),
+        reindent_multiline(inner.into(), false, indent_of(cx, expr.span))
+    );
+
+    span_lint_and_sugg(
+        cx,
+        RETURN_AND_THEN,
+        expr.span,
+        msg,
+        "try",
+        sugg,
+        Applicability::MachineApplicable,
+    );
+}