unnecessary_map_or: lint .map_or(true, …) as well

This commit is contained in:
Samuel Tardieu 2024-11-04 19:04:30 +01:00
parent ca963b653e
commit de03a05bc8
5 changed files with 75 additions and 17 deletions

View file

@ -4107,24 +4107,32 @@ declare_clippy_lint! {
/// ### Why is this bad?
/// Calls such as `opt.map_or(false, |val| val == 5)` are needlessly long and cumbersome,
/// and can be reduced to, for example, `opt == Some(5)` assuming `opt` implements `PartialEq`.
/// Also, calls such as `opt.map_or(true, |val| val == 5)` can be reduced to
/// `opt.is_none_or(|val| val == 5)`.
/// This lint offers readability and conciseness improvements.
///
/// ### Example
/// ```no_run
/// pub fn a(x: Option<i32>) -> bool {
/// x.map_or(false, |n| n == 5)
/// pub fn a(x: Option<i32>) -> (bool, bool) {
/// (
/// x.map_or(false, |n| n == 5),
/// x.map_or(true, |n| n > 5),
/// )
/// }
/// ```
/// Use instead:
/// ```no_run
/// pub fn a(x: Option<i32>) -> bool {
/// x == Some(5)
/// pub fn a(x: Option<i32>) -> (bool, bool) {
/// (
/// x == Some(5),
/// x.is_none_or(|n| n > 5),
/// )
/// }
/// ```
#[clippy::version = "1.75.0"]
pub UNNECESSARY_MAP_OR,
style,
"reduce unnecessary pattern matching for constructs that implement `PartialEq`"
"reduce unnecessary calls to `.map_or(bool, …)`"
}
declare_clippy_lint! {

View file

@ -60,7 +60,7 @@ pub(super) fn check<'a>(
Some(_) | None => return,
};
let (sugg, method) = if let ExprKind::Closure(map_closure) = map.kind
let (sugg, method, applicability) = if let ExprKind::Closure(map_closure) = map.kind
&& let closure_body = cx.tcx.hir().body(map_closure.body)
&& let closure_body_value = closure_body.value.peel_blocks()
&& let ExprKind::Binary(op, l, r) = closure_body_value.kind
@ -100,7 +100,7 @@ pub(super) fn check<'a>(
.maybe_par()
.into_string();
(binop, "a standard comparison")
(binop, "a standard comparison", Applicability::MaybeIncorrect)
} else if !def_bool
&& msrv.meets(msrvs::OPTION_RESULT_IS_VARIANT_AND)
&& let Some(recv_callsite) = snippet_opt(cx, recv.span.source_callsite())
@ -110,6 +110,18 @@ pub(super) fn check<'a>(
(
format!("{recv_callsite}.{suggested_name}({span_callsite})",),
suggested_name,
Applicability::MachineApplicable,
)
} else if def_bool
&& matches!(variant, Variant::Some)
&& msrv.meets(msrvs::IS_NONE_OR)
&& let Some(recv_callsite) = snippet_opt(cx, recv.span.source_callsite())
&& let Some(span_callsite) = snippet_opt(cx, map.span.source_callsite())
{
(
format!("{recv_callsite}.is_none_or({span_callsite})"),
"is_none_or",
Applicability::MachineApplicable,
)
} else {
return;
@ -126,6 +138,6 @@ pub(super) fn check<'a>(
"this `map_or` is redundant",
format!("use {method} instead"),
sugg,
Applicability::MaybeIncorrect,
applicability,
);
}

View file

@ -23,10 +23,9 @@ fn main() {
let _ = Ok::<Vec<i32>, i32>(vec![5]).is_ok_and(|n| n == [5]);
let _ = (Ok::<i32, i32>(5) == Ok(5));
let _ = (Some(5) == Some(5)).then(|| 1);
let _ = Some(5).is_none_or(|n| n == 5);
let _ = Some(5).is_none_or(|n| 5 == n);
// shouldnt trigger
let _ = Some(5).map_or(true, |n| n == 5);
let _ = Some(5).map_or(true, |n| 5 == n);
macro_rules! x {
() => {
Some(1)
@ -56,11 +55,16 @@ fn main() {
let r: Result<i32, S> = Ok(3);
let _ = r.is_ok_and(func);
let _ = Some(5).is_some_and(func);
let _ = Some(5).is_none_or(func);
#[derive(PartialEq)]
struct S2;
let r: Result<i32, S2> = Ok(4);
let _ = (r == Ok(8));
// do not lint `Result::map_or(true, …)`
let r: Result<i32, S2> = Ok(4);
let _ = r.map_or(true, |x| x == 8);
}
#[clippy::msrv = "1.69.0"]
@ -68,3 +72,9 @@ fn msrv_1_69() {
// is_some_and added in 1.70.0
let _ = Some(5).map_or(false, |n| n == if 2 > 1 { n } else { 0 });
}
#[clippy::msrv = "1.81.0"]
fn msrv_1_81() {
// is_none_or added in 1.82.0
let _ = Some(5).map_or(true, |n| n == if 2 > 1 { n } else { 0 });
}

View file

@ -26,10 +26,9 @@ fn main() {
let _ = Ok::<Vec<i32>, i32>(vec![5]).map_or(false, |n| n == [5]);
let _ = Ok::<i32, i32>(5).map_or(false, |n| n == 5);
let _ = Some(5).map_or(false, |n| n == 5).then(|| 1);
// shouldnt trigger
let _ = Some(5).map_or(true, |n| n == 5);
let _ = Some(5).map_or(true, |n| 5 == n);
macro_rules! x {
() => {
Some(1)
@ -59,11 +58,16 @@ fn main() {
let r: Result<i32, S> = Ok(3);
let _ = r.map_or(false, func);
let _ = Some(5).map_or(false, func);
let _ = Some(5).map_or(true, func);
#[derive(PartialEq)]
struct S2;
let r: Result<i32, S2> = Ok(4);
let _ = r.map_or(false, |x| x == 8);
// do not lint `Result::map_or(true, …)`
let r: Result<i32, S2> = Ok(4);
let _ = r.map_or(true, |x| x == 8);
}
#[clippy::msrv = "1.69.0"]
@ -71,3 +75,9 @@ fn msrv_1_69() {
// is_some_and added in 1.70.0
let _ = Some(5).map_or(false, |n| n == if 2 > 1 { n } else { 0 });
}
#[clippy::msrv = "1.81.0"]
fn msrv_1_81() {
// is_none_or added in 1.82.0
let _ = Some(5).map_or(true, |n| n == if 2 > 1 { n } else { 0 });
}

View file

@ -84,28 +84,46 @@ LL | let _ = Some(5).map_or(false, |n| n == 5).then(|| 1);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use a standard comparison instead: `(Some(5) == Some(5))`
error: this `map_or` is redundant
--> tests/ui/unnecessary_map_or.rs:55:13
--> tests/ui/unnecessary_map_or.rs:29:13
|
LL | let _ = Some(5).map_or(true, |n| n == 5);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use is_none_or instead: `Some(5).is_none_or(|n| n == 5)`
error: this `map_or` is redundant
--> tests/ui/unnecessary_map_or.rs:30:13
|
LL | let _ = Some(5).map_or(true, |n| 5 == n);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use is_none_or instead: `Some(5).is_none_or(|n| 5 == n)`
error: this `map_or` is redundant
--> tests/ui/unnecessary_map_or.rs:54:13
|
LL | let _ = r.map_or(false, |x| x == 7);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use is_ok_and instead: `r.is_ok_and(|x| x == 7)`
error: this `map_or` is redundant
--> tests/ui/unnecessary_map_or.rs:60:13
--> tests/ui/unnecessary_map_or.rs:59:13
|
LL | let _ = r.map_or(false, func);
| ^^^^^^^^^^^^^^^^^^^^^ help: use is_ok_and instead: `r.is_ok_and(func)`
error: this `map_or` is redundant
--> tests/ui/unnecessary_map_or.rs:61:13
--> tests/ui/unnecessary_map_or.rs:60:13
|
LL | let _ = Some(5).map_or(false, func);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use is_some_and instead: `Some(5).is_some_and(func)`
error: this `map_or` is redundant
--> tests/ui/unnecessary_map_or.rs:61:13
|
LL | let _ = Some(5).map_or(true, func);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use is_none_or instead: `Some(5).is_none_or(func)`
error: this `map_or` is redundant
--> tests/ui/unnecessary_map_or.rs:66:13
|
LL | let _ = r.map_or(false, |x| x == 8);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use a standard comparison instead: `(r == Ok(8))`
error: aborting due to 15 previous errors
error: aborting due to 18 previous errors