manual_is_variant_of: detect .map_or(false, f) as well

Author: samueltardieu

clippy_lints/src/methods/manual_is_variant_and.rs

@@ -15,6 +15,7 @@ pub(super) fn check<'tcx>(
     map_arg: &'tcx rustc_hir::Expr<'_>,
     map_span: Span,
     msrv: &Msrv,
+    is_map_or: bool,
 ) {
     // Don't lint if:
 
@@ -23,15 +24,15 @@ pub(super) fn check<'tcx>(
         return;
     }
 
-    // 2. the caller of `map()` is neither `Option` nor `Result`
+    // 2. the caller of `map()` or `map_or()` 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() {
+    if !is_map_or && !cx.typeck_results().expr_ty(expr).is_bool() {
         return;
     }
 
@@ -40,12 +41,17 @@ pub(super) fn check<'tcx>(
         return;
     }
 
-    let lint_msg = if is_option {
-        "called `map(<f>).unwrap_or_default()` on an `Option` value"
+    let call_msg = if is_map_or {
+        "map_or(false, <f>)"
     } else {
-        "called `map(<f>).unwrap_or_default()` on a `Result` value"
+        "map(<f>).unwrap_or_default()"
     };
-    let suggestion = if is_option { "is_some_and" } else { "is_ok_and" };
+    let (ty, suggestion) = if is_option {
+        ("an `Option`", "is_some_and")
+    } else {
+        ("a `Result`", "is_ok_and")
+    };
+    let lint_msg = format!("called `{call_msg}` on {ty} value");
 
     span_lint_and_sugg(
         cx,

clippy_lints/src/methods/mod.rs

@@ -140,6 +140,7 @@ use clippy_utils::macros::FormatArgsStorage;
 use clippy_utils::ty::{contains_ty_adt_constructor_opaque, implements_trait, is_copy, is_type_diagnostic_item};
 use clippy_utils::{contains_return, is_bool, is_trait_method, iter_input_pats, peel_blocks, return_ty};
 pub use path_ends_with_ext::DEFAULT_ALLOWED_DOTFILES;
+use rustc_ast::ast;
 use rustc_data_structures::fx::FxHashSet;
 use rustc_hir as hir;
 use rustc_hir::{Expr, ExprKind, Node, Stmt, StmtKind, TraitItem, TraitItemKind};
@@ -3889,6 +3890,8 @@ declare_clippy_lint! {
     /// # let result: Result<usize, ()> = Ok(1);
     /// option.map(|a| a > 10).unwrap_or_default();
     /// result.map(|a| a > 10).unwrap_or_default();
+    /// option.map_or(false, |a| a > 10);
+    /// result.map_or(false, |a| a > 10);
     /// ```
     /// Use instead:
     /// ```no_run
@@ -3900,7 +3903,7 @@ declare_clippy_lint! {
     #[clippy::version = "1.77.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)`"
+    "calling `.map(f).unwrap_or_default()` or `.map_or(false, f)` instead of `is_some_and(f)` or `is_ok_and(f)`"
 }
 
 declare_clippy_lint! {
@@ -4839,6 +4842,11 @@ impl Methods {
                     option_map_or_none::check(cx, expr, recv, def, map);
                     manual_ok_or::check(cx, expr, recv, def, map);
                     option_map_or_err_ok::check(cx, expr, recv, def, map);
+                    if let ExprKind::Lit(lit) = def.kind
+                        && lit.node == ast::LitKind::Bool(false)
+                    {
+                        manual_is_variant_and::check(cx, expr, recv, map, span, &self.msrv, true);
+                    }
                 },
                 ("map_or_else", [def, map]) => {
                     result_map_or_else_none::check(cx, expr, recv, def, map);
@@ -5052,7 +5060,7 @@ impl Methods {
                 },
                 ("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);
+                        manual_is_variant_and::check(cx, expr, m_recv, arg, span, &self.msrv, false);
                     }
                     unnecessary_literal_unwrap::check(cx, expr, recv, name, args);
                 },

tests/ui/manual_is_variant_and.fixed

@@ -19,6 +19,8 @@ fn option_methods() {
     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();
 
@@ -37,6 +39,8 @@ fn result_methods() {
     });
     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();
 

tests/ui/manual_is_variant_and.rs

@@ -23,6 +23,8 @@ fn option_methods() {
         .map(|x| x > 1)
         .unwrap_or_default();
 
+    let _ = opt.map_or(false, |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();
 
@@ -43,6 +45,8 @@ fn result_methods() {
     let _ = res.map(|x| x > 1)
         .unwrap_or_default();
 
+    let _ = res.map_or(false, |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();
 

tests/ui/manual_is_variant_and.stderr

@@ -41,14 +41,20 @@ LL |           .map(|x| x > 1)
 LL | |         .unwrap_or_default();
    | |____________________________^ help: use: `is_some_and(|x| x > 1)`
 
+error: called `map_or(false, <f>)` on an `Option` value
+  --> tests/ui/manual_is_variant_and.rs:26:17
+   |
+LL |     let _ = opt.map_or(false, |x| x > 1);
+   |                 ^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `is_some_and(|x| x > 1)`
+
 error: called `map(<f>).unwrap_or_default()` on an `Option` value
-  --> tests/ui/manual_is_variant_and.rs:30:18
+  --> tests/ui/manual_is_variant_and.rs:32: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
-  --> tests/ui/manual_is_variant_and.rs:39:17
+  --> tests/ui/manual_is_variant_and.rs:41:17
    |
 LL |       let _ = res.map(|x| {
    |  _________________^
@@ -65,18 +71,24 @@ LL ~     });
    |
 
 error: called `map(<f>).unwrap_or_default()` on a `Result` value
-  --> tests/ui/manual_is_variant_and.rs:43:17
+  --> tests/ui/manual_is_variant_and.rs:45:17
    |
 LL |       let _ = res.map(|x| x > 1)
    |  _________________^
 LL | |         .unwrap_or_default();
    | |____________________________^ help: use: `is_ok_and(|x| x > 1)`
 
+error: called `map_or(false, <f>)` on a `Result` value
+  --> tests/ui/manual_is_variant_and.rs:48:17
+   |
+LL |     let _ = res.map_or(false, |x| x > 1);
+   |                 ^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `is_ok_and(|x| x > 1)`
+
 error: called `map(<f>).unwrap_or_default()` on a `Result` value
-  --> tests/ui/manual_is_variant_and.rs:50:18
+  --> tests/ui/manual_is_variant_and.rs:54: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
+error: aborting due to 10 previous errors