Merge remote-tracking branch 'upstream/master' into rustup

clippy_lints/src/matches/manual_filter.rs

@@ -34,7 +34,7 @@ fn get_cond_expr<'tcx>(
             needs_negated: is_none_expr(cx, then_expr), /* if the `then_expr` resolves to `None`, need to negate the
                                                          * cond */
         });
-    };
+    }
     None
 }
 
@@ -45,7 +45,7 @@ fn peels_blocks_incl_unsafe_opt<'a>(expr: &'a Expr<'a>) -> Option<&'a Expr<'a>>
         if block.stmts.is_empty() {
             return block.expr;
         }
-    };
+    }
     None
 }
 
@@ -68,14 +68,14 @@ fn is_some_expr(cx: &LateContext<'_>, target: HirId, ctxt: SyntaxContext, expr:
                 && is_res_lang_ctor(cx, path_res(cx, callee), OptionSome)
                 && path_to_local_id(arg, target);
         }
-    };
+    }
     false
 }
 
 fn is_none_expr(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool {
     if let Some(inner_expr) = peels_blocks_incl_unsafe_opt(expr) {
         return is_res_lang_ctor(cx, path_res(cx, inner_expr), OptionNone);
-    };
+    }
     false
 }
 

clippy_lints/src/matches/manual_ok_err.rs

@@ -0,0 +1,135 @@
+use clippy_utils::diagnostics::span_lint_and_sugg;
+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 rustc_ast::BindingMode;
+use rustc_errors::Applicability;
+use rustc_hir::LangItem::{OptionNone, OptionSome, ResultErr};
+use rustc_hir::def::{DefKind, Res};
+use rustc_hir::{Arm, Expr, ExprKind, Pat, PatKind, Path, QPath};
+use rustc_lint::{LateContext, LintContext};
+use rustc_middle::ty::Ty;
+use rustc_span::symbol::Ident;
+
+use super::MANUAL_OK_ERR;
+
+pub(crate) fn check_if_let(
+    cx: &LateContext<'_>,
+    expr: &Expr<'_>,
+    let_pat: &Pat<'_>,
+    let_expr: &Expr<'_>,
+    if_then: &Expr<'_>,
+    else_expr: &Expr<'_>,
+) {
+    if let Some(inner_expr_ty) = option_arg_ty(cx, cx.typeck_results().expr_ty(expr))
+        && let Some((is_ok, ident)) = is_ok_or_err(cx, let_pat)
+        && is_some_ident(cx, if_then, ident, inner_expr_ty)
+        && is_none(cx, else_expr)
+    {
+        apply_lint(cx, expr, let_expr, is_ok);
+    }
+}
+
+pub(crate) fn check_match(cx: &LateContext<'_>, expr: &Expr<'_>, scrutinee: &Expr<'_>, arms: &[Arm<'_>]) {
+    if let Some(inner_expr_ty) = option_arg_ty(cx, cx.typeck_results().expr_ty(expr))
+        && arms.len() == 2
+        && arms.iter().all(|arm| arm.guard.is_none())
+        && let Some((idx, is_ok)) = arms.iter().enumerate().find_map(|(arm_idx, arm)| {
+            // Check if the arm is a `Ok(x) => x` or `Err(x) => x` alternative.
+            // In this case, return its index and whether it uses `Ok` or `Err`.
+             if let Some((is_ok, ident)) = is_ok_or_err(cx, arm.pat)
+                && is_some_ident(cx, arm.body, ident, inner_expr_ty)
+            {
+                Some((arm_idx, is_ok))
+            } else {
+                None
+            }
+        })
+        // Accept wildcard only as the second arm
+        && is_variant_or_wildcard(cx, arms[1-idx].pat, idx == 0, is_ok)
+        // Check that the body of the non `Ok`/`Err` arm is `None`
+        && is_none(cx, arms[1 - idx].body)
+    {
+        apply_lint(cx, expr, scrutinee, is_ok);
+    }
+}
+
+/// Check that `pat` applied to a `Result` only matches `Ok(_)`, `Err(_)`, not a subset or a
+/// superset of it. If `can_be_wild` is `true`, wildcards are also accepted. In the case of
+/// a non-wildcard, `must_match_err` indicates whether the `Err` or the `Ok` variant should be
+/// accepted.
+fn is_variant_or_wildcard(cx: &LateContext<'_>, pat: &Pat<'_>, can_be_wild: bool, must_match_err: bool) -> bool {
+    match pat.kind {
+        PatKind::Wild | PatKind::Path(..) | PatKind::Binding(_, _, _, None) if can_be_wild => true,
+        PatKind::TupleStruct(qpath, ..) => {
+            is_res_lang_ctor(cx, cx.qpath_res(&qpath, pat.hir_id), ResultErr) == must_match_err
+        },
+        PatKind::Binding(_, _, _, Some(pat)) | PatKind::Ref(pat, _) => {
+            is_variant_or_wildcard(cx, pat, can_be_wild, must_match_err)
+        },
+        _ => false,
+    }
+}
+
+/// Return `Some((true, IDENT))` if `pat` contains `Ok(IDENT)`, `Some((false, IDENT))` if it
+/// contains `Err(IDENT)`, `None` otherwise.
+fn is_ok_or_err<'hir>(cx: &LateContext<'_>, pat: &Pat<'hir>) -> Option<(bool, &'hir Ident)> {
+    if let PatKind::TupleStruct(qpath, [arg], _) = &pat.kind
+        && let PatKind::Binding(BindingMode::NONE, _, ident, _) = &arg.kind
+        && let res = cx.qpath_res(qpath, pat.hir_id)
+        && let Res::Def(DefKind::Ctor(..), id) = res
+        && let id @ Some(_) = cx.tcx.opt_parent(id)
+    {
+        let lang_items = cx.tcx.lang_items();
+        if id == lang_items.result_ok_variant() {
+            return Some((true, ident));
+        } else if id == lang_items.result_err_variant() {
+            return Some((false, ident));
+        }
+    }
+    None
+}
+
+/// Check if `expr` contains `Some(ident)`, possibly as a block
+fn is_some_ident<'tcx>(cx: &LateContext<'tcx>, expr: &Expr<'_>, ident: &Ident, ty: Ty<'tcx>) -> bool {
+    if let ExprKind::Call(body_callee, [body_arg]) = peel_blocks(expr).kind
+        && is_res_lang_ctor(cx, path_res(cx, body_callee), OptionSome)
+        && cx.typeck_results().expr_ty(body_arg) == ty
+        && let ExprKind::Path(QPath::Resolved(
+            _,
+            Path {
+                segments: [segment], ..
+            },
+        )) = body_arg.kind
+    {
+        segment.ident.name == ident.name
+    } else {
+        false
+    }
+}
+
+/// Check if `expr` is `None`, possibly as a block
+fn is_none(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool {
+    is_res_lang_ctor(cx, path_res(cx, peel_blocks(expr)), OptionNone)
+}
+
+/// Suggest replacing `expr` by `scrutinee.METHOD()`, where `METHOD` is either `ok` or
+/// `err`, depending on `is_ok`.
+fn apply_lint(cx: &LateContext<'_>, expr: &Expr<'_>, scrutinee: &Expr<'_>, is_ok: bool) {
+    let method = if is_ok { "ok" } else { "err" };
+    let mut app = if span_contains_comment(cx.sess().source_map(), expr.span) {
+        Applicability::MaybeIncorrect
+    } else {
+        Applicability::MachineApplicable
+    };
+    let scrut = Sugg::hir_with_applicability(cx, scrutinee, "..", &mut app).maybe_par();
+    span_lint_and_sugg(
+        cx,
+        MANUAL_OK_ERR,
+        expr.span,
+        format!("manual implementation of `{method}`"),
+        "replace with",
+        format!("{scrut}.{method}()"),
+        app,
+    );
+}

clippy_lints/src/matches/manual_utils.rs

@@ -109,7 +109,7 @@ where
             }
         },
         None => return None,
-    };
+    }
 
     let mut app = Applicability::MachineApplicable;
 

clippy_lints/src/matches/match_bool.rs

@@ -1,6 +1,6 @@
 use clippy_utils::diagnostics::span_lint_and_then;
 use clippy_utils::is_unit_expr;
-use clippy_utils::source::{expr_block, snippet};
+use clippy_utils::source::expr_block;
 use clippy_utils::sugg::Sugg;
 use rustc_ast::LitKind;
 use rustc_errors::Applicability;
@@ -17,17 +17,28 @@ pub(crate) fn check(cx: &LateContext<'_>, scrutinee: &Expr<'_>, arms: &[Arm<'_>]
             cx,
             MATCH_BOOL,
             expr.span,
-            "you seem to be trying to match on a boolean expression",
+            "`match` on a boolean expression",
             move |diag| {
                 if arms.len() == 2 {
-                    // no guards
-                    let exprs = if let PatKind::Expr(arm_bool) = arms[0].pat.kind {
+                    let mut app = Applicability::MachineApplicable;
+                    let test_sugg = if let PatKind::Expr(arm_bool) = arms[0].pat.kind {
+                        let test = Sugg::hir_with_applicability(cx, scrutinee, "_", &mut app);
                         if let PatExprKind::Lit { lit, .. } = arm_bool.kind {
-                            match lit.node {
-                                LitKind::Bool(true) => Some((arms[0].body, arms[1].body)),
-                                LitKind::Bool(false) => Some((arms[1].body, arms[0].body)),
+                            match &lit.node {
+                                LitKind::Bool(true) => Some(test),
+                                LitKind::Bool(false) => Some(!test),
                                 _ => None,
                             }
+                            .map(|test| {
+                                if let Some(guard) = &arms[0]
+                                    .guard
+                                    .map(|g| Sugg::hir_with_applicability(cx, g, "_", &mut app))
+                                {
+                                    test.and(guard)
+                                } else {
+                                    test
+                                }
+                            })
                         } else {
                             None
                         }
@@ -35,39 +46,31 @@ pub(crate) fn check(cx: &LateContext<'_>, scrutinee: &Expr<'_>, arms: &[Arm<'_>]
                         None
                     };
 
-                    if let Some((true_expr, false_expr)) = exprs {
-                        let mut app = Applicability::HasPlaceholders;
+                    if let Some(test_sugg) = test_sugg {
                         let ctxt = expr.span.ctxt();
+                        let (true_expr, false_expr) = (arms[0].body, arms[1].body);
                         let sugg = match (is_unit_expr(true_expr), is_unit_expr(false_expr)) {
                             (false, false) => Some(format!(
                                 "if {} {} else {}",
-                                snippet(cx, scrutinee.span, "b"),
+                                test_sugg,
                                 expr_block(cx, true_expr, ctxt, "..", Some(expr.span), &mut app),
                                 expr_block(cx, false_expr, ctxt, "..", Some(expr.span), &mut app)
                             )),
                             (false, true) => Some(format!(
                                 "if {} {}",
-                                snippet(cx, scrutinee.span, "b"),
+                                test_sugg,
                                 expr_block(cx, true_expr, ctxt, "..", Some(expr.span), &mut app)
                             )),
-                            (true, false) => {
-                                let test = Sugg::hir(cx, scrutinee, "..");
-                                Some(format!(
-                                    "if {} {}",
-                                    !test,
-                                    expr_block(cx, false_expr, ctxt, "..", Some(expr.span), &mut app)
-                                ))
-                            },
+                            (true, false) => Some(format!(
+                                "if {} {}",
+                                !test_sugg,
+                                expr_block(cx, false_expr, ctxt, "..", Some(expr.span), &mut app)
+                            )),
                             (true, true) => None,
                         };
 
                         if let Some(sugg) = sugg {
-                            diag.span_suggestion(
-                                expr.span,
-                                "consider using an `if`/`else` expression",
-                                sugg,
-                                Applicability::HasPlaceholders,
-                            );
+                            diag.span_suggestion(expr.span, "consider using an `if`/`else` expression", sugg, app);
                         }
                     }
                 }

clippy_lints/src/matches/match_like_matches.rs

@@ -117,7 +117,7 @@ where
             if let ty::Ref(..) = cx.typeck_results().expr_ty(ex_inner).kind() {
                 ex_new = ex_inner;
             }
-        };
+        }
         span_lint_and_sugg(
             cx,
             MATCH_LIKE_MATCHES_MACRO,

clippy_lints/src/matches/match_str_case_mismatch.rs

@@ -5,7 +5,7 @@ use clippy_utils::ty::is_type_lang_item;
 use rustc_ast::ast::LitKind;
 use rustc_errors::Applicability;
 use rustc_hir::intravisit::{Visitor, walk_expr};
-use rustc_hir::{Arm, Expr, ExprKind, PatExpr, PatExprKind, LangItem, PatKind};
+use rustc_hir::{Arm, Expr, ExprKind, LangItem, PatExpr, PatExprKind, PatKind};
 use rustc_lint::LateContext;
 use rustc_middle::ty;
 use rustc_span::Span;

clippy_lints/src/matches/match_wild_enum.rs

@@ -170,7 +170,7 @@ pub(crate) fn check(cx: &LateContext<'_>, ex: &Expr<'_>, arms: &[Arm<'_>]) {
                 );
             });
         },
-    };
+    }
 }
 
 enum CommonPrefixSearcher<'a> {

clippy_lints/src/matches/mod.rs

@@ -2,6 +2,7 @@ mod collapsible_match;
 mod infallible_destructuring_match;
 mod manual_filter;
 mod manual_map;
+mod manual_ok_err;
 mod manual_unwrap_or;
 mod manual_utils;
 mod match_as_ref;
@@ -972,6 +973,40 @@ declare_clippy_lint! {
     "checks for unnecessary guards in match expressions"
 }
 
+declare_clippy_lint! {
+    /// ### What it does
+    /// Checks for manual implementation of `.ok()` or `.err()`
+    /// on `Result` values.
+    ///
+    /// ### Why is this bad?
+    /// Using `.ok()` or `.err()` rather than a `match` or
+    /// `if let` is less complex and more readable.
+    ///
+    /// ### Example
+    /// ```no_run
+    /// # fn func() -> Result<u32, &'static str> { Ok(0) }
+    /// let a = match func() {
+    ///     Ok(v) => Some(v),
+    ///     Err(_) => None,
+    /// };
+    /// let b = if let Err(v) = func() {
+    ///     Some(v)
+    /// } else {
+    ///     None
+    /// };
+    /// ```
+    /// Use instead:
+    /// ```no_run
+    /// # fn func() -> Result<u32, &'static str> { Ok(0) }
+    /// let a = func().ok();
+    /// let b = func().err();
+    /// ```
+    #[clippy::version = "1.86.0"]
+    pub MANUAL_OK_ERR,
+    complexity,
+    "find manual implementations of `.ok()` or `.err()` on `Result`"
+}
+
 pub struct Matches {
     msrv: Msrv,
     infallible_destructuring_match_linted: bool,
@@ -1013,6 +1048,7 @@ impl_lint_pass!(Matches => [
     MANUAL_MAP,
     MANUAL_FILTER,
     REDUNDANT_GUARDS,
+    MANUAL_OK_ERR,
 ]);
 
 impl<'tcx> LateLintPass<'tcx> for Matches {
@@ -1091,6 +1127,7 @@ impl<'tcx> LateLintPass<'tcx> for Matches {
                         manual_unwrap_or::check_match(cx, expr, ex, arms);
                         manual_map::check_match(cx, expr, ex, arms);
                         manual_filter::check_match(cx, ex, arms, expr);
+                        manual_ok_err::check_match(cx, expr, ex, arms);
                     }
 
                     if self.infallible_destructuring_match_linted {
@@ -1134,6 +1171,14 @@ impl<'tcx> LateLintPass<'tcx> for Matches {
                             if_let.if_then,
                             else_expr,
                         );
+                        manual_ok_err::check_if_let(
+                            cx,
+                            expr,
+                            if_let.let_pat,
+                            if_let.let_expr,
+                            if_let.if_then,
+                            else_expr,
+                        );
                     }
                 }
                 redundant_pattern_match::check_if_let(

clippy_lints/src/matches/significant_drop_in_scrutinee.rs

@@ -2,7 +2,7 @@ use std::ops::ControlFlow;
 
 use crate::FxHashSet;
 use clippy_utils::diagnostics::span_lint_and_then;
-use clippy_utils::source::{indent_of, snippet};
+use clippy_utils::source::{first_line_of_span, indent_of, snippet};
 use clippy_utils::ty::{for_each_top_level_late_bound_region, is_copy};
 use clippy_utils::{get_attr, is_lint_allowed};
 use itertools::Itertools;
@@ -152,7 +152,7 @@ fn set_suggestion<'tcx>(diag: &mut Diag<'_, ()>, cx: &LateContext<'tcx>, expr: &
     diag.multipart_suggestion(
         suggestion_message,
         vec![
-            (expr.span.shrink_to_lo(), replacement),
+            (first_line_of_span(cx, expr.span).shrink_to_lo(), replacement),
             (found.found_span, scrutinee_replacement),
         ],
         Applicability::MaybeIncorrect,
@@ -441,8 +441,9 @@ impl<'tcx> Visitor<'tcx> for SigDropHelper<'_, 'tcx> {
         let parent_expr_before = self.parent_expr.replace(ex);
 
         match ex.kind {
-            // Skip blocks because values in blocks will be dropped as usual.
-            ExprKind::Block(..) => (),
+            // Skip blocks because values in blocks will be dropped as usual, and await
+            // desugaring because temporary insides the future will have been dropped.
+            ExprKind::Block(..) | ExprKind::Match(_, _, MatchSource::AwaitDesugar) => (),
             _ => walk_expr(self, ex),
         }
 

clippy_lints/src/matches/try_err.rs

@@ -46,7 +46,7 @@ pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, scrutine
             err_ty = ty;
         } else {
             return;
-        };
+        }
 
         span_lint_and_then(
             cx,

clippy_lints/src/matches/wild_in_or_pats.rs

@@ -13,7 +13,7 @@ pub(crate) fn check(cx: &LateContext<'_>, expr: &Expr<'_>, arms: &[Arm<'_>]) {
         && has_non_exhaustive_attr(cx.tcx, *adt_def)
     {
         return;
-    };
+    }
     for arm in arms {
         if let PatKind::Or(fields) = arm.pat.kind {
             // look for multiple fields in this arm that contains at least one Wild pattern