Auto merge of #11865 - yuxqiu:map_unwrap_or_default, r=Jarcho

feat: add `manual_is_variant_and` lint

changelog: add a new lint [`manual_is_variant_and`].
- Replace `option.map(f).unwrap_or_default()` and `result.map(f).unwrap_or_default()` with `option.is_some_and(f)` and `result.is_ok_and(f)` where `f` is a function or closure that returns `bool`.
- MSRV is set to 1.70.0 for this lint; when `is_some_and` and `is_ok_and` was stabilised

---

For example, for the following code:

```rust
let opt = Some(0);
opt.map(|x| x > 1).unwrap_or_default();
```

It suggests to instead write:

```rust
let opt = Some(0);
opt.is_some_and(|x| x > 1)
```

CHANGELOG.md

@@ -5294,6 +5294,7 @@ Released 2018-09-13
 [`manual_is_ascii_check`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_is_ascii_check
 [`manual_is_finite`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_is_finite
 [`manual_is_infinite`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_is_infinite
+[`manual_is_variant_and`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_is_variant_and
 [`manual_let_else`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_let_else
 [`manual_main_separator_str`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_main_separator_str
 [`manual_map`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_map

clippy_config/src/msrvs.rs

@@ -17,7 +17,7 @@ macro_rules! msrv_aliases {
 // names may refer to stabilized feature flags or library items
 msrv_aliases! {
     1,71,0 { TUPLE_ARRAY_CONVERSIONS, BUILD_HASHER_HASH_ONE }
-    1,70,0 { OPTION_IS_SOME_AND, BINARY_HEAP_RETAIN }
+    1,70,0 { OPTION_RESULT_IS_VARIANT_AND, BINARY_HEAP_RETAIN }
     1,68,0 { PATH_MAIN_SEPARATOR_STR }
     1,65,0 { LET_ELSE, POINTER_CAST_CONSTNESS }
     1,62,0 { BOOL_THEN_SOME, DEFAULT_ENUM_ATTRIBUTE }

clippy_lints/src/declared_lints.rs

@@ -385,6 +385,7 @@
     crate::methods::JOIN_ABSOLUTE_PATHS_INFO,
     crate::methods::MANUAL_FILTER_MAP_INFO,
     crate::methods::MANUAL_FIND_MAP_INFO,
+    crate::methods::MANUAL_IS_VARIANT_AND_INFO,
     crate::methods::MANUAL_NEXT_BACK_INFO,
     crate::methods::MANUAL_OK_OR_INFO,
     crate::methods::MANUAL_SATURATING_ARITHMETIC_INFO,

clippy_lints/src/lines_filter_map_ok.rs

@@ -96,8 +96,7 @@ fn should_lint(cx: &LateContext<'_>, args: &[Expr<'_>], method_str: &str) -> boo
                 ExprKind::Path(qpath) => cx
                     .qpath_res(qpath, fm_arg.hir_id)
                     .opt_def_id()
-                    .map(|did| match_def_path(cx, did, &paths::CORE_RESULT_OK_METHOD))
+                    .is_some_and(|did| match_def_path(cx, did, &paths::CORE_RESULT_OK_METHOD)),
-                    .unwrap_or_default(),
                 // Detect `|x| x.ok()`
                 ExprKind::Closure(Closure { body, .. }) => {
                     if let Body {

clippy_lints/src/methods/manual_is_variant_and.rs

@@ -0,0 +1,59 @@
+use clippy_config::msrvs::{Msrv, OPTION_RESULT_IS_VARIANT_AND};
+use clippy_utils::diagnostics::span_lint_and_sugg;
+use clippy_utils::source::snippet;
+use clippy_utils::ty::is_type_diagnostic_item;
+use rustc_errors::Applicability;
+use rustc_lint::LateContext;
+use rustc_span::{sym, Span};
+
+use super::MANUAL_IS_VARIANT_AND;
+
+pub(super) fn check<'tcx>(
+    cx: &LateContext<'_>,
+    expr: &'tcx rustc_hir::Expr<'_>,
+    map_recv: &'tcx rustc_hir::Expr<'_>,
+    map_arg: &'tcx rustc_hir::Expr<'_>,
+    map_span: Span,
+    msrv: &Msrv,
+) {
+    // Don't lint if:
+
+    // 1. the `expr` is generated by a macro
+    if expr.span.from_expansion() {
+        return;
+    }
+
+    // 2. the caller of `map()` is neither `Option` nor `Result`
+    let is_option = is_type_diagnostic_item(cx, cx.typeck_results().expr_ty(map_recv), sym::Option);
+    let is_result = is_type_diagnostic_item(cx, cx.typeck_results().expr_ty(map_recv), sym::Result);
+    if !is_option && !is_result {
+        return;
+    }
+
+    // 3. the caller of `unwrap_or_default` is neither `Option<bool>` nor `Result<bool, _>`
+    if !cx.typeck_results().expr_ty(expr).is_bool() {
+        return;
+    }
+
+    // 4. msrv doesn't meet `OPTION_RESULT_IS_VARIANT_AND`
+    if !msrv.meets(OPTION_RESULT_IS_VARIANT_AND) {
+        return;
+    }
+
+    let lint_msg = if is_option {
+        "called `map(<f>).unwrap_or_default()` on an `Option` value"
+    } else {
+        "called `map(<f>).unwrap_or_default()` on a `Result` value"
+    };
+    let suggestion = if is_option { "is_some_and" } else { "is_ok_and" };
+
+    span_lint_and_sugg(
+        cx,
+        MANUAL_IS_VARIANT_AND,
+        expr.span.with_lo(map_span.lo()),
+        lint_msg,
+        "use",
+        format!("{}({})", suggestion, snippet(cx, map_arg.span, "..")),
+        Applicability::MachineApplicable,
+    );
+}

clippy_lints/src/methods/mod.rs

@@ -51,6 +51,7 @@
 mod iter_with_drain;
 mod iterator_step_by_zero;
 mod join_absolute_paths;
+mod manual_is_variant_and;
 mod manual_next_back;
 mod manual_ok_or;
 mod manual_saturating_arithmetic;
@@ -3829,6 +3830,32 @@
     "filtering an iterator over `Result`s for `Ok` can be achieved with `flatten`"
 }
 
+declare_clippy_lint! {
+    /// Checks for usage of `option.map(f).unwrap_or_default()` and `result.map(f).unwrap_or_default()` where f is a function or closure that returns the `bool` type.
+    ///
+    /// ### Why is this bad?
+    /// Readability. These can be written more concisely as `option.is_some_and(f)` and `result.is_ok_and(f)`.
+    ///
+    /// ### Example
+    /// ```no_run
+    /// # let option = Some(1);
+    /// # let result: Result<usize, ()> = Ok(1);
+    /// option.map(|a| a > 10).unwrap_or_default();
+    /// result.map(|a| a > 10).unwrap_or_default();
+    /// ```
+    /// Use instead:
+    /// ```no_run
+    /// # let option = Some(1);
+    /// # let result: Result<usize, ()> = Ok(1);
+    /// option.is_some_and(|a| a > 10);
+    /// result.is_ok_and(|a| a > 10);
+    /// ```
+    #[clippy::version = "1.76.0"]
+    pub MANUAL_IS_VARIANT_AND,
+    pedantic,
+    "using `.map(f).unwrap_or_default()`, which is more succinctly expressed as `is_some_and(f)` or `is_ok_and(f)`"
+}
+
 pub struct Methods {
     avoid_breaking_exported_api: bool,
     msrv: Msrv,
@@ -3983,6 +4010,7 @@ pub fn new(
     RESULT_FILTER_MAP,
     ITER_FILTER_IS_SOME,
     ITER_FILTER_IS_OK,
+    MANUAL_IS_VARIANT_AND,
 ]);
 
 /// Extracts a method call name, args, and `Span` of the method name.
@@ -4664,7 +4692,13 @@ fn check_methods<'tcx>(&self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
                     }
                     unnecessary_literal_unwrap::check(cx, expr, recv, name, args);
                 },
-                ("unwrap_or_default" | "unwrap_unchecked" | "unwrap_err_unchecked", []) => {
+                ("unwrap_or_default", []) => {
+                    if let Some(("map", m_recv, [arg], span, _)) = method_call(recv) {
+                        manual_is_variant_and::check(cx, expr, m_recv, arg, span, &self.msrv);
+                    }
+                    unnecessary_literal_unwrap::check(cx, expr, recv, name, args);
+                },
+                ("unwrap_unchecked" | "unwrap_err_unchecked", []) => {
                     unnecessary_literal_unwrap::check(cx, expr, recv, name, args);
                 },
                 ("unwrap_or_else", [u_arg]) => {

clippy_lints/src/methods/option_map_unwrap_or.rs

@@ -72,7 +72,7 @@ pub(super) fn check<'tcx>(
         }
 
         // is_some_and is stabilised && `unwrap_or` argument is false; suggest `is_some_and` instead
-        let suggest_is_some_and = msrv.meets(msrvs::OPTION_IS_SOME_AND)
+        let suggest_is_some_and = msrv.meets(msrvs::OPTION_RESULT_IS_VARIANT_AND)
             && matches!(&unwrap_arg.kind, ExprKind::Lit(lit)
             if matches!(lit.node, rustc_ast::LitKind::Bool(false)));
 

clippy_lints/src/methods/path_ends_with_ext.rs

@@ -33,7 +33,7 @@ pub(super) fn check(
         && path.chars().all(char::is_alphanumeric)
     {
         let mut sugg = snippet(cx, recv.span, "..").into_owned();
-        if msrv.meets(msrvs::OPTION_IS_SOME_AND) {
+        if msrv.meets(msrvs::OPTION_RESULT_IS_VARIANT_AND) {
             let _ = write!(sugg, r#".extension().is_some_and(|ext| ext == "{path}")"#);
         } else {
             let _ = write!(sugg, r#".extension().map_or(false, |ext| ext == "{path}")"#);

tests/ui/manual_is_variant_and.fixed

@@ -0,0 +1,51 @@
+//@aux-build:option_helpers.rs
+#![warn(clippy::manual_is_variant_and)]
+
+#[macro_use]
+extern crate option_helpers;
+
+#[rustfmt::skip]
+fn option_methods() {
+    let opt = Some(1);
+
+    // Check for `option.map(_).unwrap_or_default()` use.
+    // Single line case.
+    let _ = opt.is_some_and(|x| x > 1);
+    // Multi-line cases.
+    let _ = opt.is_some_and(|x| {
+        x > 1
+    });
+    let _ = opt.is_some_and(|x| x > 1);
+    let _ = opt
+        .is_some_and(|x| x > 1);
+
+    // won't fix because the return type of the closure is not `bool`
+    let _ = opt.map(|x| x + 1).unwrap_or_default();
+
+    let opt2 = Some('a');
+    let _ = opt2.is_some_and(char::is_alphanumeric); // should lint
+    let _ = opt_map!(opt2, |x| x == 'a').unwrap_or_default(); // should not lint
+}
+
+#[rustfmt::skip]
+fn result_methods() {
+    let res: Result<i32, ()> = Ok(1);
+
+    // multi line cases
+    let _ = res.is_ok_and(|x| {
+        x > 1
+    });
+    let _ = res.is_ok_and(|x| x > 1);
+
+    // won't fix because the return type of the closure is not `bool`
+    let _ = res.map(|x| x + 1).unwrap_or_default();
+
+    let res2: Result<char, ()> = Ok('a');
+    let _ = res2.is_ok_and(char::is_alphanumeric); // should lint
+    let _ = opt_map!(res2, |x| x == 'a').unwrap_or_default(); // should not lint
+}
+
+fn main() {
+    option_methods();
+    result_methods();
+}

tests/ui/manual_is_variant_and.rs

@@ -0,0 +1,57 @@
+//@aux-build:option_helpers.rs
+#![warn(clippy::manual_is_variant_and)]
+
+#[macro_use]
+extern crate option_helpers;
+
+#[rustfmt::skip]
+fn option_methods() {
+    let opt = Some(1);
+
+    // Check for `option.map(_).unwrap_or_default()` use.
+    // Single line case.
+    let _ = opt.map(|x| x > 1)
+        // Should lint even though this call is on a separate line.
+        .unwrap_or_default();
+    // Multi-line cases.
+    let _ = opt.map(|x| {
+        x > 1
+    }
+    ).unwrap_or_default();
+    let _ = opt.map(|x| x > 1).unwrap_or_default();
+    let _ = opt
+        .map(|x| x > 1)
+        .unwrap_or_default();
+
+    // won't fix because the return type of the closure is not `bool`
+    let _ = opt.map(|x| x + 1).unwrap_or_default();
+
+    let opt2 = Some('a');
+    let _ = opt2.map(char::is_alphanumeric).unwrap_or_default(); // should lint
+    let _ = opt_map!(opt2, |x| x == 'a').unwrap_or_default(); // should not lint
+}
+
+#[rustfmt::skip]
+fn result_methods() {
+    let res: Result<i32, ()> = Ok(1);
+
+    // multi line cases
+    let _ = res.map(|x| {
+        x > 1
+    }
+    ).unwrap_or_default();
+    let _ = res.map(|x| x > 1)
+        .unwrap_or_default();
+
+    // won't fix because the return type of the closure is not `bool`
+    let _ = res.map(|x| x + 1).unwrap_or_default();
+
+    let res2: Result<char, ()> = Ok('a');
+    let _ = res2.map(char::is_alphanumeric).unwrap_or_default(); // should lint
+    let _ = opt_map!(res2, |x| x == 'a').unwrap_or_default(); // should not lint
+}
+
+fn main() {
+    option_methods();
+    result_methods();
+}

tests/ui/manual_is_variant_and.stderr

@@ -0,0 +1,82 @@
+error: called `map(<f>).unwrap_or_default()` on an `Option` value
+  --> $DIR/manual_is_variant_and.rs:13:17
+   |
+LL |       let _ = opt.map(|x| x > 1)
+   |  _________________^
+LL | |         // Should lint even though this call is on a separate line.
+LL | |         .unwrap_or_default();
+   | |____________________________^ help: use: `is_some_and(|x| x > 1)`
+   |
+   = note: `-D clippy::manual-is-variant-and` implied by `-D warnings`
+   = help: to override `-D warnings` add `#[allow(clippy::manual_is_variant_and)]`
+
+error: called `map(<f>).unwrap_or_default()` on an `Option` value
+  --> $DIR/manual_is_variant_and.rs:17:17
+   |
+LL |       let _ = opt.map(|x| {
+   |  _________________^
+LL | |         x > 1
+LL | |     }
+LL | |     ).unwrap_or_default();
+   | |_________________________^
+   |
+help: use
+   |
+LL ~     let _ = opt.is_some_and(|x| {
+LL +         x > 1
+LL ~     });
+   |
+
+error: called `map(<f>).unwrap_or_default()` on an `Option` value
+  --> $DIR/manual_is_variant_and.rs:21:17
+   |
+LL |     let _ = opt.map(|x| x > 1).unwrap_or_default();
+   |                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `is_some_and(|x| x > 1)`
+
+error: called `map(<f>).unwrap_or_default()` on an `Option` value
+  --> $DIR/manual_is_variant_and.rs:23:10
+   |
+LL |           .map(|x| x > 1)
+   |  __________^
+LL | |         .unwrap_or_default();
+   | |____________________________^ help: use: `is_some_and(|x| x > 1)`
+
+error: called `map(<f>).unwrap_or_default()` on an `Option` value
+  --> $DIR/manual_is_variant_and.rs:30:18
+   |
+LL |     let _ = opt2.map(char::is_alphanumeric).unwrap_or_default(); // should lint
+   |                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `is_some_and(char::is_alphanumeric)`
+
+error: called `map(<f>).unwrap_or_default()` on a `Result` value
+  --> $DIR/manual_is_variant_and.rs:39:17
+   |
+LL |       let _ = res.map(|x| {
+   |  _________________^
+LL | |         x > 1
+LL | |     }
+LL | |     ).unwrap_or_default();
+   | |_________________________^
+   |
+help: use
+   |
+LL ~     let _ = res.is_ok_and(|x| {
+LL +         x > 1
+LL ~     });
+   |
+
+error: called `map(<f>).unwrap_or_default()` on a `Result` value
+  --> $DIR/manual_is_variant_and.rs:43:17
+   |
+LL |       let _ = res.map(|x| x > 1)
+   |  _________________^
+LL | |         .unwrap_or_default();
+   | |____________________________^ help: use: `is_ok_and(|x| x > 1)`
+
+error: called `map(<f>).unwrap_or_default()` on a `Result` value
+  --> $DIR/manual_is_variant_and.rs:50:18
+   |
+LL |     let _ = res2.map(char::is_alphanumeric).unwrap_or_default(); // should lint
+   |                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `is_ok_and(char::is_alphanumeric)`
+
+error: aborting due to 8 previous errors
+