From 9b7d8d1dc7cee007ec90a8c82e159bb504655615 Mon Sep 17 00:00:00 2001 From: darklyspaced Date: Mon, 26 Jun 2023 13:13:42 +0800 Subject: [PATCH] suggests `is_some_and` over `map().unwrap_or(false)` --- clippy_lints/src/methods/mod.rs | 2 ++ .../src/methods/option_map_unwrap_or.rs | 28 +++++++++++++++++-- tests/ui/map_unwrap_or.rs | 4 +++ tests/ui/map_unwrap_or.stderr | 20 ++++++++++--- 4 files changed, 47 insertions(+), 7 deletions(-) diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs index 99c984ba6..403fac123 100644 --- a/clippy_lints/src/methods/mod.rs +++ b/clippy_lints/src/methods/mod.rs @@ -513,6 +513,7 @@ declare_clippy_lint! { /// # let result: Result = Ok(1); /// # fn some_function(foo: ()) -> usize { 1 } /// option.map(|a| a + 1).unwrap_or(0); + /// option.map(|a| a > 10).unwrap_or(false); /// result.map(|a| a + 1).unwrap_or_else(some_function); /// ``` /// @@ -522,6 +523,7 @@ declare_clippy_lint! { /// # let result: Result = Ok(1); /// # fn some_function(foo: ()) -> usize { 1 } /// option.map_or(0, |a| a + 1); + /// option.is_some_and(|a| a > 10); /// result.map_or_else(some_function, |a| a + 1); /// ``` #[clippy::version = "1.45.0"] diff --git a/clippy_lints/src/methods/option_map_unwrap_or.rs b/clippy_lints/src/methods/option_map_unwrap_or.rs index e931c6277..6a4d288ab 100644 --- a/clippy_lints/src/methods/option_map_unwrap_or.rs +++ b/clippy_lints/src/methods/option_map_unwrap_or.rs @@ -74,16 +74,32 @@ pub(super) fn check<'tcx>( return; } + let mut suggest_is_some_and = false; + // argument to `unwrap_or` is false; should suggest using `is_some_and` + if let ExprKind::Lit(unwrap_lit) = &unwrap_arg.kind { + if let rustc_ast::LitKind::Bool(false) = unwrap_lit.node { + suggest_is_some_and = true; + } + } + let mut applicability = Applicability::MachineApplicable; // get snippet for unwrap_or() let unwrap_snippet = snippet_with_applicability(cx, unwrap_arg.span, "..", &mut applicability); // lint message // comparing the snippet from source to raw text ("None") below is safe // because we already have checked the type. - let arg = if unwrap_snippet == "None" { "None" } else { "" }; + let arg = if unwrap_snippet == "None" { + "None" + } else if suggest_is_some_and { + "false" + } else { + "" + }; let unwrap_snippet_none = unwrap_snippet == "None"; let suggest = if unwrap_snippet_none { "and_then()" + } else if suggest_is_some_and { + "is_some_and()" } else { "map_or(, )" }; @@ -98,12 +114,18 @@ pub(super) fn check<'tcx>( let mut suggestion = vec![ ( map_span, - String::from(if unwrap_snippet_none { "and_then" } else { "map_or" }), + String::from(if unwrap_snippet_none { + "and_then" + } else if suggest_is_some_and { + "is_some_and" + } else { + "map_or" + }), ), (expr.span.with_lo(unwrap_recv.span.hi()), String::new()), ]; - if !unwrap_snippet_none { + if !unwrap_snippet_none && !suggest_is_some_and { suggestion.push((map_arg_span.with_hi(map_arg_span.lo()), format!("{unwrap_snippet}, "))); } diff --git a/tests/ui/map_unwrap_or.rs b/tests/ui/map_unwrap_or.rs index 0cd49dd77..51b88c7cf 100644 --- a/tests/ui/map_unwrap_or.rs +++ b/tests/ui/map_unwrap_or.rs @@ -56,6 +56,10 @@ fn option_methods() { .unwrap_or_else(|| 0 ); + + // Check for `map(f).unwrap_or(false)` use. + let _ = opt.map(|x| x > 5).unwrap_or(false); + } #[rustfmt::skip] diff --git a/tests/ui/map_unwrap_or.stderr b/tests/ui/map_unwrap_or.stderr index 41781b050..6f23d2e4f 100644 --- a/tests/ui/map_unwrap_or.stderr +++ b/tests/ui/map_unwrap_or.stderr @@ -126,8 +126,20 @@ LL | | 0 LL | | ); | |_________^ +error: called `map().unwrap_or(false)` on an `Option` value. This can be done more directly by calling `is_some_and()` instead + --> $DIR/map_unwrap_or.rs:61:13 + | +LL | let _ = opt.map(|x| x > 5).unwrap_or(false); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: use `is_some_and()` instead + | +LL - let _ = opt.map(|x| x > 5).unwrap_or(false); +LL + let _ = opt.is_some_and(|x| x > 5); + | + error: called `map().unwrap_or_else()` on a `Result` value. This can be done more directly by calling `.map_or_else(, )` instead - --> $DIR/map_unwrap_or.rs:67:13 + --> $DIR/map_unwrap_or.rs:71:13 | LL | let _ = res.map(|x| { | _____________^ @@ -137,7 +149,7 @@ LL | | ).unwrap_or_else(|_e| 0); | |____________________________^ error: called `map().unwrap_or_else()` on a `Result` value. This can be done more directly by calling `.map_or_else(, )` instead - --> $DIR/map_unwrap_or.rs:71:13 + --> $DIR/map_unwrap_or.rs:75:13 | LL | let _ = res.map(|x| x + 1) | _____________^ @@ -147,10 +159,10 @@ LL | | }); | |__________^ error: called `map().unwrap_or_else()` on a `Result` value. This can be done more directly by calling `.map_or_else(, )` instead - --> $DIR/map_unwrap_or.rs:95:13 + --> $DIR/map_unwrap_or.rs:99:13 | LL | let _ = res.map(|x| x + 1).unwrap_or_else(|_e| 0); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `res.map_or_else(|_e| 0, |x| x + 1)` -error: aborting due to 12 previous errors +error: aborting due to 13 previous errors