Skip to content

Commit

Permalink
unnecessary_map_or: do not consume the comparison value if it does …
Browse files Browse the repository at this point in the history
…not implement `Copy` (#14207)

Fix #14201

changelog: [`unnecessary_map_or`]: do not consume the comparison value
if it does not implement `Copy`
  • Loading branch information
Jarcho authored Feb 15, 2025
2 parents 823b818 + f826193 commit 4e899e1
Show file tree
Hide file tree
Showing 4 changed files with 41 additions and 3 deletions.
6 changes: 4 additions & 2 deletions clippy_lints/src/methods/unnecessary_map_or.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use clippy_utils::diagnostics::span_lint_and_then;
use clippy_utils::eager_or_lazy::switch_to_eager_eval;
use clippy_utils::msrvs::{self, Msrv};
use clippy_utils::sugg::{Sugg, make_binop};
use clippy_utils::ty::{get_type_diagnostic_name, implements_trait};
use clippy_utils::ty::{get_type_diagnostic_name, implements_trait, is_copy};
use clippy_utils::visitors::is_local_used;
use clippy_utils::{get_parent_expr, is_from_proc_macro, path_to_local_id};
use rustc_ast::LitKind::Bool;
Expand Down Expand Up @@ -81,9 +81,11 @@ pub(super) fn check<'a>(
&& (path_to_local_id(l, hir_id) ^ path_to_local_id(r, hir_id))
&& !is_local_used(cx, non_binding_location, hir_id)
&& let typeck_results = cx.typeck_results()
&& typeck_results.expr_ty(l) == typeck_results.expr_ty(r)
&& let l_ty = typeck_results.expr_ty(l)
&& l_ty == typeck_results.expr_ty(r)
&& let Some(partial_eq) = cx.tcx.get_diagnostic_item(sym::PartialEq)
&& implements_trait(cx, recv_ty, partial_eq, &[recv_ty.into()])
&& is_copy(cx, l_ty)
{
let wrap = variant.variant_name();

Expand Down
6 changes: 6 additions & 0 deletions tests/ui/unnecessary_map_or.fixed
Original file line number Diff line number Diff line change
Expand Up @@ -99,3 +99,9 @@ impl std::ops::Deref for S {
fn with_deref(o: &S) -> bool {
o.is_none_or(|n| n > 5)
}

fn issue14201(a: Option<String>, b: Option<String>, s: &String) -> bool {
let x = a.is_some_and(|a| a == *s);
let y = b.is_none_or(|b| b == *s);
x && y
}
6 changes: 6 additions & 0 deletions tests/ui/unnecessary_map_or.rs
Original file line number Diff line number Diff line change
Expand Up @@ -102,3 +102,9 @@ impl std::ops::Deref for S {
fn with_deref(o: &S) -> bool {
o.map_or(true, |n| n > 5)
}

fn issue14201(a: Option<String>, b: Option<String>, s: &String) -> bool {
let x = a.map_or(false, |a| a == *s);
let y = b.map_or(true, |b| b == *s);
x && y
}
26 changes: 25 additions & 1 deletion tests/ui/unnecessary_map_or.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -287,5 +287,29 @@ LL - o.map_or(true, |n| n > 5)
LL + o.is_none_or(|n| n > 5)
|

error: aborting due to 24 previous errors
error: this `map_or` can be simplified
--> tests/ui/unnecessary_map_or.rs:107:13
|
LL | let x = a.map_or(false, |a| a == *s);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
help: use is_some_and instead
|
LL - let x = a.map_or(false, |a| a == *s);
LL + let x = a.is_some_and(|a| a == *s);
|

error: this `map_or` can be simplified
--> tests/ui/unnecessary_map_or.rs:108:13
|
LL | let y = b.map_or(true, |b| b == *s);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
help: use is_none_or instead
|
LL - let y = b.map_or(true, |b| b == *s);
LL + let y = b.is_none_or(|b| b == *s);
|

error: aborting due to 26 previous errors

0 comments on commit 4e899e1

Please sign in to comment.