clean-up unnecessary_map_or and manual_is_variant_and (#16386)

This is the first commit of
https://github.com/rust-lang/rust-clippy/pull/16383

changelog: none
This commit is contained in:
Jason Newcomb 2026-01-12 13:36:01 +00:00 committed by GitHub
commit 5ef494c536
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
6 changed files with 111 additions and 99 deletions

View file

@ -1,12 +1,13 @@
use clippy_utils::diagnostics::span_lint_and_sugg;
use clippy_utils::diagnostics::{span_lint_and_sugg, span_lint_and_then};
use clippy_utils::msrvs::{self, Msrv};
use clippy_utils::res::MaybeDef;
use clippy_utils::source::{snippet, snippet_with_applicability};
use clippy_utils::source::snippet_with_applicability;
use clippy_utils::sugg::Sugg;
use clippy_utils::{get_parent_expr, sym};
use rustc_ast::LitKind;
use rustc_errors::Applicability;
use rustc_hir::def::{CtorKind, CtorOf, DefKind, Res};
use rustc_hir::def_id::DefId;
use rustc_hir::{BinOpKind, Closure, Expr, ExprKind, QPath};
use rustc_lint::LateContext;
use rustc_middle::ty;
@ -14,7 +15,37 @@ use rustc_span::{Span, Symbol};
use super::MANUAL_IS_VARIANT_AND;
pub(super) fn check(
#[derive(Clone, Copy, PartialEq)]
enum Flavor {
Option,
Result,
}
impl Flavor {
fn new(cx: &LateContext<'_>, def_id: DefId) -> Option<Self> {
match cx.tcx.get_diagnostic_name(def_id)? {
sym::Option => Some(Self::Option),
sym::Result => Some(Self::Result),
_ => None,
}
}
const fn diag_sym(self) -> Symbol {
match self {
Self::Option => sym::Option,
Self::Result => sym::Result,
}
}
const fn positive_variant_name(self) -> Symbol {
match self {
Self::Option => sym::Some,
Self::Result => sym::Ok,
}
}
}
pub(super) fn check_map_unwrap_or_default(
cx: &LateContext<'_>,
expr: &Expr<'_>,
map_recv: &Expr<'_>,
@ -30,11 +61,13 @@ pub(super) fn check(
}
// 2. the caller of `map()` is neither `Option` nor `Result`
let is_option = cx.typeck_results().expr_ty(map_recv).is_diag_item(cx, sym::Option);
let is_result = cx.typeck_results().expr_ty(map_recv).is_diag_item(cx, sym::Result);
if !is_option && !is_result {
let Some(flavor) = (cx.typeck_results())
.expr_ty(map_recv)
.opt_def_id()
.and_then(|did| Flavor::new(cx, did))
else {
return;
}
};
// 3. the caller of `unwrap_or_default` is neither `Option<bool>` nor `Result<bool, _>`
if !cx.typeck_results().expr_ty(expr).is_bool() {
@ -46,44 +79,23 @@ pub(super) fn check(
return;
}
let lint_msg = if is_option {
"called `map(<f>).unwrap_or_default()` on an `Option` value"
} else {
"called `map(<f>).unwrap_or_default()` on a `Result` value"
let lint_span = expr.span.with_lo(map_span.lo());
let lint_msg = match flavor {
Flavor::Option => "called `map(<f>).unwrap_or_default()` on an `Option` value",
Flavor::Result => "called `map(<f>).unwrap_or_default()` on a `Result` value",
};
let suggestion = if is_option { "is_some_and" } else { "is_ok_and" };
span_lint_and_sugg(
cx,
MANUAL_IS_VARIANT_AND,
expr.span.with_lo(map_span.lo()),
lint_msg,
"use",
format!("{}({})", suggestion, snippet(cx, map_arg.span, "..")),
Applicability::MachineApplicable,
);
}
span_lint_and_then(cx, MANUAL_IS_VARIANT_AND, lint_span, lint_msg, |diag| {
let method = match flavor {
Flavor::Option => "is_some_and",
Flavor::Result => "is_ok_and",
};
#[derive(Clone, Copy, PartialEq)]
enum Flavor {
Option,
Result,
}
let mut app = Applicability::MachineApplicable;
let map_arg_snippet = snippet_with_applicability(cx, map_arg.span, "_", &mut app);
impl Flavor {
const fn symbol(self) -> Symbol {
match self {
Self::Option => sym::Option,
Self::Result => sym::Result,
}
}
const fn positive(self) -> Symbol {
match self {
Self::Option => sym::Some,
Self::Result => sym::Ok,
}
}
diag.span_suggestion(lint_span, "use", format!("{method}({map_arg_snippet})"), app);
});
}
#[derive(Clone, Copy, PartialEq)]
@ -178,7 +190,7 @@ fn emit_lint<'tcx>(
cx,
MANUAL_IS_VARIANT_AND,
span,
format!("called `.map() {op} {pos}()`", pos = flavor.positive(),),
format!("called `.map() {op} {pos}()`", pos = flavor.positive_variant_name()),
"use",
format!(
"{inversion}{recv}.{method}({body})",
@ -195,24 +207,23 @@ pub(super) fn check_map(cx: &LateContext<'_>, expr: &Expr<'_>) {
&& op.span.eq_ctxt(expr.span)
&& let Ok(op) = Op::try_from(op.node)
{
// Check `left` and `right` expression in any order, and for `Option` and `Result`
// Check `left` and `right` expression in any order
for (expr1, expr2) in [(left, right), (right, left)] {
for flavor in [Flavor::Option, Flavor::Result] {
if let ExprKind::Call(call, [arg]) = expr1.kind
&& let ExprKind::Lit(lit) = arg.kind
&& let LitKind::Bool(bool_cst) = lit.node
&& let ExprKind::Path(QPath::Resolved(_, path)) = call.kind
&& let Res::Def(DefKind::Ctor(CtorOf::Variant, CtorKind::Fn), _) = path.res
&& let ty = cx.typeck_results().expr_ty(expr1)
&& let ty::Adt(adt, args) = ty.kind()
&& cx.tcx.is_diagnostic_item(flavor.symbol(), adt.did())
&& args.type_at(0).is_bool()
&& let ExprKind::MethodCall(_, recv, [map_expr], _) = expr2.kind
&& cx.typeck_results().expr_ty(recv).is_diag_item(cx, flavor.symbol())
&& let Ok(map_func) = MapFunc::try_from(map_expr)
{
return emit_lint(cx, parent_expr.span, op, flavor, bool_cst, map_func, recv);
}
if let ExprKind::Call(call, [arg]) = expr1.kind
&& let ExprKind::Lit(lit) = arg.kind
&& let LitKind::Bool(bool_cst) = lit.node
&& let ExprKind::Path(QPath::Resolved(_, path)) = call.kind
&& let Res::Def(DefKind::Ctor(CtorOf::Variant, CtorKind::Fn), _) = path.res
&& let ExprKind::MethodCall(_, recv, [map_expr], _) = expr2.kind
&& let ty = cx.typeck_results().expr_ty(expr1)
&& let ty::Adt(adt, args) = ty.kind()
&& let Some(flavor) = Flavor::new(cx, adt.did())
&& args.type_at(0).is_bool()
&& cx.typeck_results().expr_ty(recv).is_diag_item(cx, flavor.diag_sym())
&& let Ok(map_func) = MapFunc::try_from(map_expr)
{
emit_lint(cx, parent_expr.span, op, flavor, bool_cst, map_func, recv);
return;
}
}
}

View file

@ -3933,7 +3933,8 @@ declare_clippy_lint! {
declare_clippy_lint! {
/// ### What it does
/// Checks for usage of `option.map(f).unwrap_or_default()` and `result.map(f).unwrap_or_default()` where f is a function or closure that returns the `bool` type.
/// Checks for usage of `option.map(f).unwrap_or_default()` and `result.map(f).unwrap_or_default()` where `f` is a function or closure that returns the `bool` type.
///
/// Also checks for equality comparisons like `option.map(f) == Some(true)` and `result.map(f) == Ok(true)`.
///
/// ### Why is this bad?
@ -5629,7 +5630,7 @@ impl Methods {
manual_saturating_arithmetic::check_sub_unwrap_or_default(cx, expr, lhs, rhs);
},
Some((sym::map, m_recv, [arg], span, _)) => {
manual_is_variant_and::check(cx, expr, m_recv, arg, span, self.msrv);
manual_is_variant_and::check_map_unwrap_or_default(cx, expr, m_recv, arg, span, self.msrv);
},
Some((then_method @ (sym::then | sym::then_some), t_recv, [t_arg], _, _)) => {
obfuscated_if_else::check(

View file

@ -8,7 +8,7 @@ use clippy_utils::sugg::{Sugg, make_binop};
use clippy_utils::ty::{implements_trait, is_copy};
use clippy_utils::visitors::is_local_used;
use clippy_utils::{get_parent_expr, is_from_proc_macro};
use rustc_ast::LitKind::Bool;
use rustc_ast::LitKind;
use rustc_errors::Applicability;
use rustc_hir::{BinOpKind, Expr, ExprKind, PatKind};
use rustc_lint::LateContext;
@ -48,13 +48,14 @@ pub(super) fn check<'a>(
let ExprKind::Lit(def_kind) = def.kind else {
return;
};
let recv_ty = cx.typeck_results().expr_ty_adjusted(recv);
let Bool(def_bool) = def_kind.node else {
let LitKind::Bool(def_bool) = def_kind.node else {
return;
};
let typeck = cx.typeck_results();
let recv_ty = typeck.expr_ty_adjusted(recv);
let variant = match recv_ty.opt_diag_name(cx) {
Some(sym::Option) => Variant::Some,
Some(sym::Result) => Variant::Ok,
@ -63,12 +64,12 @@ pub(super) fn check<'a>(
let ext_def_span = def.span.until(map.span);
let (sugg, method, applicability) = if cx.typeck_results().expr_adjustments(recv).is_empty()
let (sugg, method, applicability): (_, Cow<'_, _>, _) = if typeck.expr_adjustments(recv).is_empty()
&& 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
&& let Some(param) = closure_body.params.first()
&& let [param] = closure_body.params
&& let PatKind::Binding(_, hir_id, _, _) = param.pat.kind
// checking that map_or is one of the following:
// .map_or(false, |x| x == y)
@ -78,14 +79,13 @@ pub(super) fn check<'a>(
&& ((BinOpKind::Eq == op.node && !def_bool) || (BinOpKind::Ne == op.node && def_bool))
&& let non_binding_location = if l.res_local_id() == Some(hir_id) { r } else { l }
&& switch_to_eager_eval(cx, non_binding_location)
// if its both then that's a strange edge case and
// if it's both then that's a strange edge case and
// we can just ignore it, since by default clippy will error on this
&& (l.res_local_id() == Some(hir_id)) != (r.res_local_id() == Some(hir_id))
&& !is_local_used(cx, non_binding_location, hir_id)
&& let typeck_results = cx.typeck_results()
&& 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)
&& let l_ty = typeck.expr_ty(l)
&& l_ty == typeck.expr_ty(r)
&& let Some(partial_eq) = cx.tcx.lang_items().eq_trait()
&& implements_trait(cx, recv_ty, partial_eq, &[recv_ty.into()])
&& is_copy(cx, l_ty)
{
@ -126,18 +126,18 @@ pub(super) fn check<'a>(
}
.into_string();
(vec![(expr.span, sugg)], "a standard comparison", app)
(vec![(expr.span, sugg)], "a standard comparison".into(), app)
} else if !def_bool && msrv.meets(cx, msrvs::OPTION_RESULT_IS_VARIANT_AND) {
let suggested_name = variant.method_name();
(
vec![(method_span, suggested_name.into()), (ext_def_span, String::default())],
suggested_name,
vec![(method_span, suggested_name.into()), (ext_def_span, String::new())],
format!("`{suggested_name}`").into(),
Applicability::MachineApplicable,
)
} else if def_bool && matches!(variant, Variant::Some) && msrv.meets(cx, msrvs::IS_NONE_OR) {
(
vec![(method_span, "is_none_or".into()), (ext_def_span, String::default())],
"is_none_or",
vec![(method_span, "is_none_or".into()), (ext_def_span, String::new())],
"`is_none_or`".into(),
Applicability::MachineApplicable,
)
} else {

View file

@ -70,7 +70,7 @@ fn main() {
let _ = r.is_ok_and(|x| x == 7);
//~^ unnecessary_map_or
// lint constructs that are not comparaisons as well
// lint constructs that are not comparisons as well
let func = |_x| true;
let r: Result<i32, S> = Ok(3);
let _ = r.is_ok_and(func);

View file

@ -74,7 +74,7 @@ fn main() {
let _ = r.map_or(false, |x| x == 7);
//~^ unnecessary_map_or
// lint constructs that are not comparaisons as well
// lint constructs that are not comparisons as well
let func = |_x| true;
let r: Result<i32, S> = Ok(3);
let _ = r.map_or(false, func);

View file

@ -56,7 +56,7 @@ LL | | 6 >= 5
LL | | });
| |______^
|
help: use is_some_and instead
help: use `is_some_and` instead
|
LL - let _ = Some(5).map_or(false, |n| {
LL + let _ = Some(5).is_some_and(|n| {
@ -68,7 +68,7 @@ error: this `map_or` can be simplified
LL | let _ = Some(vec![5]).map_or(false, |n| n == [5]);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
help: use is_some_and instead
help: use `is_some_and` instead
|
LL - let _ = Some(vec![5]).map_or(false, |n| n == [5]);
LL + let _ = Some(vec![5]).is_some_and(|n| n == [5]);
@ -80,7 +80,7 @@ error: this `map_or` can be simplified
LL | let _ = Some(vec![1]).map_or(false, |n| vec![2] == n);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
help: use is_some_and instead
help: use `is_some_and` instead
|
LL - let _ = Some(vec![1]).map_or(false, |n| vec![2] == n);
LL + let _ = Some(vec![1]).is_some_and(|n| vec![2] == n);
@ -92,7 +92,7 @@ error: this `map_or` can be simplified
LL | let _ = Some(5).map_or(false, |n| n == n);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
help: use is_some_and instead
help: use `is_some_and` instead
|
LL - let _ = Some(5).map_or(false, |n| n == n);
LL + let _ = Some(5).is_some_and(|n| n == n);
@ -104,7 +104,7 @@ error: this `map_or` can be simplified
LL | let _ = Some(5).map_or(false, |n| n == if 2 > 1 { n } else { 0 });
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
help: use is_some_and instead
help: use `is_some_and` instead
|
LL - let _ = Some(5).map_or(false, |n| n == if 2 > 1 { n } else { 0 });
LL + let _ = Some(5).is_some_and(|n| n == if 2 > 1 { n } else { 0 });
@ -116,7 +116,7 @@ error: this `map_or` can be simplified
LL | let _ = Ok::<Vec<i32>, i32>(vec![5]).map_or(false, |n| n == [5]);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
help: use is_ok_and instead
help: use `is_ok_and` instead
|
LL - let _ = Ok::<Vec<i32>, i32>(vec![5]).map_or(false, |n| n == [5]);
LL + let _ = Ok::<Vec<i32>, i32>(vec![5]).is_ok_and(|n| n == [5]);
@ -152,7 +152,7 @@ error: this `map_or` can be simplified
LL | let _ = Some(5).map_or(true, |n| n == 5);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
help: use is_none_or instead
help: use `is_none_or` instead
|
LL - let _ = Some(5).map_or(true, |n| n == 5);
LL + let _ = Some(5).is_none_or(|n| n == 5);
@ -164,7 +164,7 @@ error: this `map_or` can be simplified
LL | let _ = Some(5).map_or(true, |n| 5 == n);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
help: use is_none_or instead
help: use `is_none_or` instead
|
LL - let _ = Some(5).map_or(true, |n| 5 == n);
LL + let _ = Some(5).is_none_or(|n| 5 == n);
@ -212,7 +212,7 @@ error: this `map_or` can be simplified
LL | let _ = r.map_or(false, |x| x == 7);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
help: use is_ok_and instead
help: use `is_ok_and` instead
|
LL - let _ = r.map_or(false, |x| x == 7);
LL + let _ = r.is_ok_and(|x| x == 7);
@ -224,7 +224,7 @@ error: this `map_or` can be simplified
LL | let _ = r.map_or(false, func);
| ^^^^^^^^^^^^^^^^^^^^^
|
help: use is_ok_and instead
help: use `is_ok_and` instead
|
LL - let _ = r.map_or(false, func);
LL + let _ = r.is_ok_and(func);
@ -236,7 +236,7 @@ error: this `map_or` can be simplified
LL | let _ = Some(5).map_or(false, func);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
help: use is_some_and instead
help: use `is_some_and` instead
|
LL - let _ = Some(5).map_or(false, func);
LL + let _ = Some(5).is_some_and(func);
@ -248,7 +248,7 @@ error: this `map_or` can be simplified
LL | let _ = Some(5).map_or(true, func);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^
|
help: use is_none_or instead
help: use `is_none_or` instead
|
LL - let _ = Some(5).map_or(true, func);
LL + let _ = Some(5).is_none_or(func);
@ -272,7 +272,7 @@ error: this `map_or` can be simplified
LL | o.map_or(true, |n| n > 5) || (o as &Option<u32>).map_or(true, |n| n < 5)
| ^^^^^^^^^^^^^^^^^^^^^^^^^
|
help: use is_none_or instead
help: use `is_none_or` instead
|
LL - o.map_or(true, |n| n > 5) || (o as &Option<u32>).map_or(true, |n| n < 5)
LL + o.is_none_or(|n| n > 5) || (o as &Option<u32>).map_or(true, |n| n < 5)
@ -284,7 +284,7 @@ error: this `map_or` can be simplified
LL | o.map_or(true, |n| n > 5) || (o as &Option<u32>).map_or(true, |n| n < 5)
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
help: use is_none_or instead
help: use `is_none_or` instead
|
LL - o.map_or(true, |n| n > 5) || (o as &Option<u32>).map_or(true, |n| n < 5)
LL + o.map_or(true, |n| n > 5) || (o as &Option<u32>).is_none_or(|n| n < 5)
@ -296,7 +296,7 @@ error: this `map_or` can be simplified
LL | o.map_or(true, |n| n > 5)
| ^^^^^^^^^^^^^^^^^^^^^^^^^
|
help: use is_none_or instead
help: use `is_none_or` instead
|
LL - o.map_or(true, |n| n > 5)
LL + o.is_none_or(|n| n > 5)
@ -308,7 +308,7 @@ error: this `map_or` can be simplified
LL | let x = a.map_or(false, |a| a == *s);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
help: use is_some_and instead
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);
@ -320,7 +320,7 @@ error: this `map_or` can be simplified
LL | let y = b.map_or(true, |b| b == *s);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
help: use is_none_or instead
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);
@ -356,7 +356,7 @@ error: this `map_or` can be simplified
LL | _ = s.lock().unwrap().map_or(false, |s| s == "foo");
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
help: use is_some_and instead
help: use `is_some_and` instead
|
LL - _ = s.lock().unwrap().map_or(false, |s| s == "foo");
LL + _ = s.lock().unwrap().is_some_and(|s| s == "foo");
@ -368,7 +368,7 @@ error: this `map_or` can be simplified
LL | _ = s.map_or(false, |s| s == "foo");
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
help: use is_some_and instead
help: use `is_some_and` instead
|
LL - _ = s.map_or(false, |s| s == "foo");
LL + _ = s.is_some_and(|s| s == "foo");