From f826193aec2d58f64c4592818dd2d855a5bbc597 Mon Sep 17 00:00:00 2001 From: Samuel Tardieu Date: Wed, 12 Feb 2025 20:44:22 +0100 Subject: [PATCH] `unnecessary_map_or`: do not consume the non-`Copy` comparison value --- .../src/methods/unnecessary_map_or.rs | 6 +++-- tests/ui/unnecessary_map_or.fixed | 6 +++++ tests/ui/unnecessary_map_or.rs | 6 +++++ tests/ui/unnecessary_map_or.stderr | 26 ++++++++++++++++++- 4 files changed, 41 insertions(+), 3 deletions(-) diff --git a/clippy_lints/src/methods/unnecessary_map_or.rs b/clippy_lints/src/methods/unnecessary_map_or.rs index 6dea1506d0e3..c307e8c951d0 100644 --- a/clippy_lints/src/methods/unnecessary_map_or.rs +++ b/clippy_lints/src/methods/unnecessary_map_or.rs @@ -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; @@ -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(); diff --git a/tests/ui/unnecessary_map_or.fixed b/tests/ui/unnecessary_map_or.fixed index 5a6e77a06b8f..27532373a7b9 100644 --- a/tests/ui/unnecessary_map_or.fixed +++ b/tests/ui/unnecessary_map_or.fixed @@ -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, b: Option, s: &String) -> bool { + let x = a.is_some_and(|a| a == *s); + let y = b.is_none_or(|b| b == *s); + x && y +} diff --git a/tests/ui/unnecessary_map_or.rs b/tests/ui/unnecessary_map_or.rs index 5ba63121659f..6496defe1300 100644 --- a/tests/ui/unnecessary_map_or.rs +++ b/tests/ui/unnecessary_map_or.rs @@ -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, b: Option, s: &String) -> bool { + let x = a.map_or(false, |a| a == *s); + let y = b.map_or(true, |b| b == *s); + x && y +} diff --git a/tests/ui/unnecessary_map_or.stderr b/tests/ui/unnecessary_map_or.stderr index 2ae327f0bf88..f672f55dd04d 100644 --- a/tests/ui/unnecessary_map_or.stderr +++ b/tests/ui/unnecessary_map_or.stderr @@ -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