Merge commit '1e8fdf492808a25d78a97e1242b835ace9924e4d' into clippyup

5 files changed, 224 insertions, 130 deletions

diff --git a/clippy_lints/src/methods/expect_used.rs b/clippy_lints/src/methods/expect_used.rs
deleted file mode 100644
index 614610335a1..00000000000
--- a/clippy_lints/src/methods/expect_used.rs
+++ /dev/null
@@ -1,44 +0,0 @@
-use clippy_utils::diagnostics::span_lint_and_help;
-use clippy_utils::ty::is_type_diagnostic_item;
-use clippy_utils::{is_in_cfg_test, is_in_test_function};
-use rustc_hir as hir;
-use rustc_lint::LateContext;
-use rustc_span::sym;
-
-use super::EXPECT_USED;
-
-/// lint use of `expect()` or `expect_err` for `Result` and `expect()` for `Option`.
-pub(super) fn check(
-    cx: &LateContext<'_>,
-    expr: &hir::Expr<'_>,
-    recv: &hir::Expr<'_>,
-    is_err: bool,
-    allow_expect_in_tests: bool,
-) {
-    let obj_ty = cx.typeck_results().expr_ty(recv).peel_refs();
-
-    let mess = if is_type_diagnostic_item(cx, obj_ty, sym::Option) && !is_err {
-        Some((EXPECT_USED, "an `Option`", "None", ""))
-    } else if is_type_diagnostic_item(cx, obj_ty, sym::Result) {
-        Some((EXPECT_USED, "a `Result`", if is_err { "Ok" } else { "Err" }, "an "))
-    } else {
-        None
-    };
-
-    let method = if is_err { "expect_err" } else { "expect" };
-
-    if allow_expect_in_tests && (is_in_test_function(cx.tcx, expr.hir_id) || is_in_cfg_test(cx.tcx, expr.hir_id)) {
-        return;
-    }
-
-    if let Some((lint, kind, none_value, none_prefix)) = mess {
-        span_lint_and_help(
-            cx,
-            lint,
-            expr.span,
-            &format!("used `{method}()` on {kind} value"),
-            None,
-            &format!("if this value is {none_prefix}`{none_value}`, it will panic"),
-        );
-    }
-}
diff --git a/clippy_lints/src/methods/filter_map_bool_then.rs b/clippy_lints/src/methods/filter_map_bool_then.rs
new file mode 100644
index 00000000000..fafc9709770
--- /dev/null
+++ b/clippy_lints/src/methods/filter_map_bool_then.rs
@@ -0,0 +1,53 @@
+use super::FILTER_MAP_BOOL_THEN;
+use clippy_utils::diagnostics::span_lint_and_sugg;
+use clippy_utils::paths::BOOL_THEN;
+use clippy_utils::source::snippet_opt;
+use clippy_utils::ty::is_copy;
+use clippy_utils::{is_from_proc_macro, is_trait_method, match_def_path, peel_blocks};
+use rustc_errors::Applicability;
+use rustc_hir::{Expr, ExprKind};
+use rustc_lint::{LateContext, LintContext};
+use rustc_middle::lint::in_external_macro;
+use rustc_middle::ty::Binder;
+use rustc_span::{sym, Span};
+
+pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>, arg: &Expr<'_>, call_span: Span) {
+    if !in_external_macro(cx.sess(), expr.span)
+        && is_trait_method(cx, expr, sym::Iterator)
+        && let ExprKind::Closure(closure) = arg.kind
+        && let body = cx.tcx.hir().body(closure.body)
+        && let value = peel_blocks(body.value)
+        // Indexing should be fine as `filter_map` always has 1 input, we unfortunately need both
+        // `inputs` and `params` here as we need both the type and the span
+        && let param_ty = closure.fn_decl.inputs[0]
+        && let param = body.params[0]
+        // Issue #11309
+        && let param_ty = cx.tcx.liberate_late_bound_regions(
+            closure.def_id.to_def_id(),
+            Binder::bind_with_vars(
+                cx.typeck_results().node_type(param_ty.hir_id),
+                cx.tcx.late_bound_vars(cx.tcx.hir().local_def_id_to_hir_id(closure.def_id)),
+            ),
+        )
+        && is_copy(cx, param_ty)
+        && let ExprKind::MethodCall(_, recv, [then_arg], _) = value.kind
+        && let ExprKind::Closure(then_closure) = then_arg.kind
+        && let then_body = peel_blocks(cx.tcx.hir().body(then_closure.body).value)
+        && let Some(def_id) = cx.typeck_results().type_dependent_def_id(value.hir_id)
+        && match_def_path(cx, def_id, &BOOL_THEN)
+        && !is_from_proc_macro(cx, expr)
+        && let Some(param_snippet) = snippet_opt(cx, param.span)
+        && let Some(filter) = snippet_opt(cx, recv.span)
+        && let Some(map) = snippet_opt(cx, then_body.span)
+    {
+        span_lint_and_sugg(
+            cx,
+            FILTER_MAP_BOOL_THEN,
+            call_span,
+            "usage of `bool::then` in `filter_map`",
+            "use `filter` then `map` instead",
+            format!("filter(|&{param_snippet}| {filter}).map(|{param_snippet}| {map})"),
+            Applicability::MachineApplicable,
+        );
+    }
+}
diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs
index 28a8978973f..42756b27d01 100644
--- a/clippy_lints/src/methods/mod.rs
+++ b/clippy_lints/src/methods/mod.rs
@@ -17,10 +17,10 @@ mod collapsible_str_replace;
 mod drain_collect;
 mod err_expect;
 mod expect_fun_call;
-mod expect_used;
 mod extend_with_drain;
 mod filetype_is_file;
 mod filter_map;
+mod filter_map_bool_then;
 mod filter_map_identity;
 mod filter_map_next;
 mod filter_next;
@@ -104,7 +104,7 @@ mod unnecessary_lazy_eval;
 mod unnecessary_literal_unwrap;
 mod unnecessary_sort_by;
 mod unnecessary_to_owned;
-mod unwrap_used;
+mod unwrap_expect_used;
 mod useless_asref;
 mod utils;
 mod vec_resize_to_zero;
@@ -3478,6 +3478,37 @@ declare_clippy_lint! {
 
 declare_clippy_lint! {
     /// ### What it does
+    /// Checks for usage of `bool::then` in `Iterator::filter_map`.
+    ///
+    /// ### Why is this bad?
+    /// This can be written with `filter` then `map` instead, which would reduce nesting and
+    /// separates the filtering from the transformation phase. This comes with no cost to
+    /// performance and is just cleaner.
+    ///
+    /// ### Limitations
+    /// Does not lint `bool::then_some`, as it eagerly evaluates its arguments rather than lazily.
+    /// This can create differing behavior, so better safe than sorry.
+    ///
+    /// ### Example
+    /// ```rust
+    /// # fn really_expensive_fn(i: i32) -> i32 { i }
+    /// # let v = vec![];
+    /// _ = v.into_iter().filter_map(|i| (i % 2 == 0).then(|| really_expensive_fn(i)));
+    /// ```
+    /// Use instead:
+    /// ```rust
+    /// # fn really_expensive_fn(i: i32) -> i32 { i }
+    /// # let v = vec![];
+    /// _ = v.into_iter().filter(|i| i % 2 == 0).map(|i| really_expensive_fn(i));
+    /// ```
+    #[clippy::version = "1.72.0"]
+    pub FILTER_MAP_BOOL_THEN,
+    style,
+    "checks for usage of `bool::then` in `Iterator::filter_map`"
+}
+
+declare_clippy_lint! {
+    /// ### What it does
     /// Looks for calls to `RwLock::write` where the lock is only used for reading.
     ///
     /// ### Why is this bad?
@@ -3644,6 +3675,7 @@ impl_lint_pass!(Methods => [
     FORMAT_COLLECT,
     STRING_LIT_CHARS_ANY,
     ITER_SKIP_ZERO,
+    FILTER_MAP_BOOL_THEN,
     READONLY_WRITE_LOCK
 ]);
 
@@ -3848,6 +3880,13 @@ impl Methods {
                         unnecessary_lazy_eval::check(cx, expr, recv, arg, "and");
                     }
                 },
+                ("any", [arg]) if let ExprKind::Closure(arg) = arg.kind
+                    && let body = cx.tcx.hir().body(arg.body)
+                    && let [param] = body.params
+                    && let Some(("chars", recv, _, _, _)) = method_call(recv) =>
+                {
+                    string_lit_chars_any::check(cx, expr, recv, param, peel_blocks(body.value), &self.msrv);
+                }
                 ("arg", [arg]) => {
                     suspicious_command_arg_space::check(cx, recv, arg, span);
                 }
@@ -3908,13 +3947,27 @@ impl Methods {
                     match method_call(recv) {
                         Some(("ok", recv, [], _, _)) => ok_expect::check(cx, expr, recv),
                         Some(("err", recv, [], err_span, _)) => err_expect::check(cx, expr, recv, span, err_span, &self.msrv),
-                        _ => expect_used::check(cx, expr, recv, false, self.allow_expect_in_tests),
+                        _ => unwrap_expect_used::check(
+                            cx,
+                            expr,
+                            recv,
+                            false,
+                            self.allow_expect_in_tests,
+                            unwrap_expect_used::Variant::Expect,
+                        ),
                     }
                     unnecessary_literal_unwrap::check(cx, expr, recv, name, args);
                 },
                 ("expect_err", [_]) => {
                     unnecessary_literal_unwrap::check(cx, expr, recv, name, args);
-                    expect_used::check(cx, expr, recv, true, self.allow_expect_in_tests);
+                    unwrap_expect_used::check(
+                        cx,
+                        expr,
+                        recv,
+                        true,
+                        self.allow_expect_in_tests,
+                        unwrap_expect_used::Variant::Expect,
+                    );
                 },
                 ("extend", [arg]) => {
                     string_extend_chars::check(cx, expr, recv, arg);
@@ -3922,6 +3975,7 @@ impl Methods {
                 },
                 ("filter_map", [arg]) => {
                     unnecessary_filter_map::check(cx, expr, arg, name);
+                    filter_map_bool_then::check(cx, expr, arg, call_span);
                     filter_map_identity::check(cx, expr, arg, span);
                 },
                 ("find_map", [arg]) => {
@@ -3965,20 +4019,9 @@ impl Methods {
                         unnecessary_join::check(cx, expr, recv, join_arg, span);
                     }
                 },
-                ("skip", [arg]) => {
-                    iter_skip_zero::check(cx, expr, arg);
-
-                    if let Some((name2, recv2, args2, _span2, _)) = method_call(recv) {
-                        if let ("cloned", []) = (name2, args2) {
-                            iter_overeager_cloned::check(cx, expr, recv, recv2, false, false);
-                        }
-                    }
-                }
                 ("last", []) => {
-                    if let Some((name2, recv2, args2, _span2, _)) = method_call(recv) {
-                        if let ("cloned", []) = (name2, args2) {
-                            iter_overeager_cloned::check(cx, expr, recv, recv2, false, false);
-                        }
+                    if let Some(("cloned", recv2, [], _span2, _)) = method_call(recv) {
+                        iter_overeager_cloned::check(cx, expr, recv, recv2, false, false);
                     }
                 },
                 ("lock", []) => {
@@ -4026,13 +4069,6 @@ impl Methods {
                         }
                     }
                 },
-                ("any", [arg]) if let ExprKind::Closure(arg) = arg.kind
-                    && let body = cx.tcx.hir().body(arg.body)
-                    && let [param] = body.params
-                    && let Some(("chars", recv, _, _, _)) = method_call(recv) =>
-                {
-                    string_lit_chars_any::check(cx, expr, recv, param, peel_blocks(body.value), &self.msrv);
-                }
                 ("nth", [n_arg]) => match method_call(recv) {
                     Some(("bytes", recv2, [], _, _)) => bytes_nth::check(cx, expr, recv2, n_arg),
                     Some(("cloned", recv2, [], _, _)) => iter_overeager_cloned::check(cx, expr, recv, recv2, false, false),
@@ -4086,6 +4122,13 @@ impl Methods {
                         seek_to_start_instead_of_rewind::check(cx, expr, recv, arg, span);
                     }
                 },
+                ("skip", [arg]) => {
+                    iter_skip_zero::check(cx, expr, arg);
+
+                    if let Some(("cloned", recv2, [], _span2, _)) = method_call(recv) {
+                        iter_overeager_cloned::check(cx, expr, recv, recv2, false, false);
+                    }
+                }
                 ("sort", []) => {
                     stable_sort_primitive::check(cx, expr, recv);
                 },
@@ -4108,10 +4151,8 @@ impl Methods {
                 },
                 ("step_by", [arg]) => iterator_step_by_zero::check(cx, expr, arg),
                 ("take", [_arg]) => {
-                    if let Some((name2, recv2, args2, _span2, _)) = method_call(recv) {
-                        if let ("cloned", []) = (name2, args2) {
-                            iter_overeager_cloned::check(cx, expr, recv, recv2, false, false);
-                        }
+                    if let Some(("cloned", recv2, [], _span2, _)) = method_call(recv) {
+                        iter_overeager_cloned::check(cx, expr, recv, recv2, false, false);
                     }
                 },
                 ("take", []) => needless_option_take::check(cx, expr, recv),
@@ -4146,11 +4187,25 @@ impl Methods {
                         _ => {},
                     }
                     unnecessary_literal_unwrap::check(cx, expr, recv, name, args);
-                    unwrap_used::check(cx, expr, recv, false, self.allow_unwrap_in_tests);
+                    unwrap_expect_used::check(
+                        cx,
+                        expr,
+                        recv,
+                        false,
+                        self.allow_unwrap_in_tests,
+                        unwrap_expect_used::Variant::Unwrap,
+                    );
                 },
                 ("unwrap_err", []) => {
                     unnecessary_literal_unwrap::check(cx, expr, recv, name, args);
-                    unwrap_used::check(cx, expr, recv, true, self.allow_unwrap_in_tests);
+                    unwrap_expect_used::check(
+                        cx,
+                        expr,
+                        recv,
+                        true,
+                        self.allow_unwrap_in_tests,
+                        unwrap_expect_used::Variant::Unwrap,
+                    );
                 },
                 ("unwrap_or", [u_arg]) => {
                     match method_call(recv) {
@@ -4180,6 +4235,9 @@ impl Methods {
                     }
                     unnecessary_literal_unwrap::check(cx, expr, recv, name, args);
                 },
+                ("write", []) => {
+                    readonly_write_lock::check(cx, expr, recv);
+                }
                 ("zip", [arg]) => {
                     if let ExprKind::MethodCall(name, iter_recv, [], _) = recv.kind
                         && name.ident.name == sym::iter
@@ -4187,9 +4245,6 @@ impl Methods {
                         range_zip_with_len::check(cx, expr, iter_recv, arg);
                     }
                 },
-                ("write", []) => {
-                    readonly_write_lock::check(cx, expr, recv);
-                }
                 _ => {},
             }
         }
diff --git a/clippy_lints/src/methods/unwrap_expect_used.rs b/clippy_lints/src/methods/unwrap_expect_used.rs
new file mode 100644
index 00000000000..7bd16b473ce
--- /dev/null
+++ b/clippy_lints/src/methods/unwrap_expect_used.rs
@@ -0,0 +1,83 @@
+use clippy_utils::diagnostics::span_lint_and_then;
+use clippy_utils::ty::{is_never_like, is_type_diagnostic_item};
+use clippy_utils::{is_in_cfg_test, is_in_test_function, is_lint_allowed};
+use rustc_hir::Expr;
+use rustc_lint::{LateContext, Lint};
+use rustc_middle::ty;
+use rustc_span::sym;
+
+use super::{EXPECT_USED, UNWRAP_USED};
+
+#[derive(Clone, Copy, Eq, PartialEq)]
+pub(super) enum Variant {
+    Unwrap,
+    Expect,
+}
+
+impl Variant {
+    fn method_name(self, is_err: bool) -> &'static str {
+        match (self, is_err) {
+            (Variant::Unwrap, true) => "unwrap_err",
+            (Variant::Unwrap, false) => "unwrap",
+            (Variant::Expect, true) => "expect_err",
+            (Variant::Expect, false) => "expect",
+        }
+    }
+
+    fn lint(self) -> &'static Lint {
+        match self {
+            Variant::Unwrap => UNWRAP_USED,
+            Variant::Expect => EXPECT_USED,
+        }
+    }
+}
+
+/// Lint usage of `unwrap` or `unwrap_err` for `Result` and `unwrap()` for `Option` (and their
+/// `expect` counterparts).
+pub(super) fn check(
+    cx: &LateContext<'_>,
+    expr: &Expr<'_>,
+    recv: &Expr<'_>,
+    is_err: bool,
+    allow_unwrap_in_tests: bool,
+    variant: Variant,
+) {
+    let ty = cx.typeck_results().expr_ty(recv).peel_refs();
+
+    let (kind, none_value, none_prefix) = if is_type_diagnostic_item(cx, ty, sym::Option) && !is_err {
+        ("an `Option`", "None", "")
+    } else if is_type_diagnostic_item(cx, ty, sym::Result)
+        && let ty::Adt(_, substs) = ty.kind()
+        && let Some(t_or_e_ty) = substs[usize::from(!is_err)].as_type()
+    {
+        if is_never_like(t_or_e_ty) {
+            return;
+        }
+
+        ("a `Result`", if is_err { "Ok" } else { "Err" }, "an ")
+    } else {
+        return;
+    };
+
+    let method_suffix = if is_err { "_err" } else { "" };
+
+    if allow_unwrap_in_tests && (is_in_test_function(cx.tcx, expr.hir_id) || is_in_cfg_test(cx.tcx, expr.hir_id)) {
+        return;
+    }
+
+    span_lint_and_then(
+        cx,
+        variant.lint(),
+        expr.span,
+        &format!("used `{}()` on {kind} value", variant.method_name(is_err)),
+        |diag| {
+            diag.note(format!("if this value is {none_prefix}`{none_value}`, it will panic"));
+
+            if variant == Variant::Unwrap && is_lint_allowed(cx, EXPECT_USED, expr.hir_id) {
+                diag.help(format!(
+                    "consider using `expect{method_suffix}()` to provide a better panic message"
+                ));
+            }
+        },
+    );
+}
diff --git a/clippy_lints/src/methods/unwrap_used.rs b/clippy_lints/src/methods/unwrap_used.rs
deleted file mode 100644
index 5e4c3daee64..00000000000
--- a/clippy_lints/src/methods/unwrap_used.rs
+++ /dev/null
@@ -1,53 +0,0 @@
-use clippy_utils::diagnostics::span_lint_and_help;
-use clippy_utils::ty::is_type_diagnostic_item;
-use clippy_utils::{is_in_cfg_test, is_in_test_function, is_lint_allowed};
-use rustc_hir as hir;
-use rustc_lint::LateContext;
-use rustc_span::sym;
-
-use super::{EXPECT_USED, UNWRAP_USED};
-
-/// lint use of `unwrap()` or `unwrap_err` for `Result` and `unwrap()` for `Option`.
-pub(super) fn check(
-    cx: &LateContext<'_>,
-    expr: &hir::Expr<'_>,
-    recv: &hir::Expr<'_>,
-    is_err: bool,
-    allow_unwrap_in_tests: bool,
-) {
-    let obj_ty = cx.typeck_results().expr_ty(recv).peel_refs();
-
-    let mess = if is_type_diagnostic_item(cx, obj_ty, sym::Option) && !is_err {
-        Some((UNWRAP_USED, "an `Option`", "None", ""))
-    } else if is_type_diagnostic_item(cx, obj_ty, sym::Result) {
-        Some((UNWRAP_USED, "a `Result`", if is_err { "Ok" } else { "Err" }, "an "))
-    } else {
-        None
-    };
-
-    let method_suffix = if is_err { "_err" } else { "" };
-
-    if allow_unwrap_in_tests && (is_in_test_function(cx.tcx, expr.hir_id) || is_in_cfg_test(cx.tcx, expr.hir_id)) {
-        return;
-    }
-
-    if let Some((lint, kind, none_value, none_prefix)) = mess {
-        let help = if is_lint_allowed(cx, EXPECT_USED, expr.hir_id) {
-            format!(
-                "if you don't want to handle the `{none_value}` case gracefully, consider \
-                using `expect{method_suffix}()` to provide a better panic message"
-            )
-        } else {
-            format!("if this value is {none_prefix}`{none_value}`, it will panic")
-        };
-
-        span_lint_and_help(
-            cx,
-            lint,
-            expr.span,
-            &format!("used `unwrap{method_suffix}()` on {kind} value"),
-            None,
-            &help,
-        );
-    }
-}