Merge branch 'rust-lang:master' into split_assert

Author: jcarey9149

clippy_lints/src/methods/mod.rs

@@ -134,9 +134,8 @@ mod unnecessary_join;
 mod unnecessary_lazy_eval;
 mod unnecessary_literal_unwrap;
 mod unnecessary_map_or;
+mod unnecessary_map_or_else;
 mod unnecessary_min_or_max;
-mod unnecessary_option_map_or_else;
-mod unnecessary_result_map_or_else;
 mod unnecessary_sort_by;
 mod unnecessary_to_owned;
 mod unwrap_expect_used;
@@ -4347,6 +4346,7 @@ declare_clippy_lint! {
 }
 
 declare_clippy_lint! {
+    /// ### What it does
     /// Checks for usage of `.map_or_else()` "map closure" for `Option` type.
     ///
     /// ### Why is this bad?
@@ -5449,8 +5449,7 @@ impl Methods {
                 },
                 (sym::map_or_else, [def, map]) => {
                     result_map_or_else_none::check(cx, expr, recv, def, map);
-                    unnecessary_option_map_or_else::check(cx, expr, recv, def, map);
-                    unnecessary_result_map_or_else::check(cx, expr, recv, def, map);
+                    unnecessary_map_or_else::check(cx, expr, recv, def, map, call_span);
                 },
                 (sym::next, []) => {
                     if let Some((name2, recv2, args2, _, _)) = method_call(recv) {

clippy_lints/src/methods/unnecessary_map_or_else.rs

@@ -0,0 +1,39 @@
+use clippy_utils::diagnostics::span_lint_and_then;
+use clippy_utils::is_expr_identity_function;
+use clippy_utils::res::MaybeDef;
+use clippy_utils::source::snippet_with_applicability;
+use rustc_errors::Applicability;
+use rustc_hir::Expr;
+use rustc_lint::LateContext;
+use rustc_span::Span;
+use rustc_span::symbol::sym;
+
+use super::{UNNECESSARY_OPTION_MAP_OR_ELSE, UNNECESSARY_RESULT_MAP_OR_ELSE};
+
+/// lint use of `_.map_or_else(|err| err, |n| n)` for `Result`s and `Option`s.
+pub(super) fn check(
+    cx: &LateContext<'_>,
+    expr: &Expr<'_>,
+    recv: &Expr<'_>,
+    def_arg: &Expr<'_>,
+    map_arg: &Expr<'_>,
+    call_span: Span,
+) {
+    let (symbol, lint) = match cx.typeck_results().expr_ty(recv).opt_diag_name(cx) {
+        Some(x @ sym::Result) => (x, UNNECESSARY_RESULT_MAP_OR_ELSE),
+        Some(x @ sym::Option) => (x, UNNECESSARY_OPTION_MAP_OR_ELSE),
+        _ => return,
+    };
+
+    if is_expr_identity_function(cx, map_arg) {
+        let msg = format!("unused \"map closure\" when calling `{symbol}::map_or_else` value");
+
+        span_lint_and_then(cx, lint, expr.span, msg, |diag| {
+            let mut applicability = Applicability::MachineApplicable;
+            let err_snippet = snippet_with_applicability(cx, def_arg.span, "..", &mut applicability);
+            let sugg = format!("unwrap_or_else({err_snippet})");
+
+            diag.span_suggestion_verbose(call_span, "consider using `unwrap_or_else`", sugg, applicability);
+        });
+    }
+}

clippy_lints/src/methods/unnecessary_option_map_or_else.rs

@@ -501,7 +501,8 @@ fn check_if_let_some_or_err_and_early_return<'tcx>(cx: &LateContext<'tcx>, expr:
 
         let mut applicability = Applicability::MachineApplicable;
         let receiver_str = snippet_with_applicability(cx, let_expr.span, "..", &mut applicability);
-        let requires_semi = matches!(cx.tcx.parent_hir_node(expr.hir_id), Node::Stmt(_));
+        let parent = cx.tcx.parent_hir_node(expr.hir_id);
+        let requires_semi = matches!(parent, Node::Stmt(_)) || cx.typeck_results().expr_ty(expr).is_unit();
         let method_call_str = match by_ref {
             ByRef::Yes(_, Mutability::Mut) => ".as_mut()",
             ByRef::Yes(_, Mutability::Not) => ".as_ref()",
@@ -512,7 +513,7 @@ fn check_if_let_some_or_err_and_early_return<'tcx>(cx: &LateContext<'tcx>, expr:
             "{receiver_str}{method_call_str}?{}",
             if requires_semi { ";" } else { "" }
         );
-        if is_else_clause(cx.tcx, expr) {
+        if is_else_clause(cx.tcx, expr) || (requires_semi && !matches!(parent, Node::Stmt(_) | Node::Block(_))) {
             sugg = format!("{{ {sugg} }}");
         }
 

clippy_lints/src/methods/unnecessary_result_map_or_else.rs

@@ -1840,13 +1840,38 @@ pub fn is_must_use_func_call(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool {
 /// * `|[x, y]| [x, y]`
 /// * `|Foo(bar, baz)| Foo(bar, baz)`
 /// * `|Foo { bar, baz }| Foo { bar, baz }`
+/// * `|x| { let y = x; ...; let z = y; z }`
+/// * `|x| { let y = x; ...; let z = y; return z }`
 ///
 /// Consider calling [`is_expr_untyped_identity_function`] or [`is_expr_identity_function`] instead.
-fn is_body_identity_function(cx: &LateContext<'_>, func: &Body<'_>) -> bool {
+fn is_body_identity_function<'hir>(cx: &LateContext<'_>, func: &Body<'hir>) -> bool {
     let [param] = func.params else {
         return false;
     };
 
+    let mut param_pat = param.pat;
+
+    // Given a sequence of `Stmt`s of the form `let p = e` where `e` is an expr identical to the
+    // current `param_pat`, advance the current `param_pat` to `p`.
+    //
+    // Note: This is similar to `clippy_utils::get_last_chain_binding_hir_id`, but it works
+    // directly over a `Pattern` rather than a `HirId`. And it checks for compatibility via
+    // `is_expr_identity_of_pat` rather than `HirId` equality
+    let mut advance_param_pat_over_stmts = |stmts: &[Stmt<'hir>]| {
+        for stmt in stmts {
+            if let StmtKind::Let(local) = stmt.kind
+                && let Some(init) = local.init
+                && is_expr_identity_of_pat(cx, param_pat, init, true)
+            {
+                param_pat = local.pat;
+            } else {
+                return false;
+            }
+        }
+
+        true
+    };
+
     let mut expr = func.value;
     loop {
         match expr.kind {
@@ -1875,7 +1900,30 @@ fn is_body_identity_function(cx: &LateContext<'_>, func: &Body<'_>) -> bool {
                     return false;
                 }
             },
-            _ => return is_expr_identity_of_pat(cx, param.pat, expr, true),
+            ExprKind::Block(
+                &Block {
+                    stmts, expr: Some(e), ..
+                },
+                _,
+            ) => {
+                if !advance_param_pat_over_stmts(stmts) {
+                    return false;
+                }
+
+                expr = e;
+            },
+            ExprKind::Block(&Block { stmts, expr: None, .. }, _) => {
+                if let Some((last_stmt, stmts)) = stmts.split_last()
+                    && advance_param_pat_over_stmts(stmts)
+                    && let StmtKind::Semi(e) | StmtKind::Expr(e) = last_stmt.kind
+                    && let ExprKind::Ret(Some(ret_val)) = e.kind
+                {
+                    expr = ret_val;
+                } else {
+                    return false;
+                }
+            },
+            _ => return is_expr_identity_of_pat(cx, param_pat, expr, true),
         }
     }
 }

clippy_lints/src/question_mark.rs

@@ -13,7 +13,8 @@
     clippy::uninlined_format_args,
     clippy::useless_vec,
     clippy::unnecessary_map_on_constructor,
-    clippy::needless_lifetimes
+    clippy::needless_lifetimes,
+    clippy::unnecessary_option_map_or_else
 )]
 
 use std::path::{Path, PathBuf};

clippy_utils/src/lib.rs

@@ -13,7 +13,8 @@
     clippy::uninlined_format_args,
     clippy::useless_vec,
     clippy::unnecessary_map_on_constructor,
-    clippy::needless_lifetimes
+    clippy::needless_lifetimes,
+    clippy::unnecessary_option_map_or_else
 )]
 
 use std::path::{Path, PathBuf};