user0/rust
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Unify manual_unwrap_or and manual_unwrap_or_default code (#14332)

Browse source 
Both lints share a lot of characteristics but were implemented in
unrelated ways. This unifies them, saving around 100 SLOC in the
process, and making one more test trigger the lint. Also, this removes
useless blocks in suggestions.

Close #12956

changelog: none
This commit is contained in:
Manish Goregaokar 2025-03-25 17:34:00 +00:00 • committed by GitHub
commit 1893e1e5e7
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
13 changed files with 328 additions and 355 deletions
Download patch file Download diff file Expand all files Collapse all files

2
clippy_lints/src/declared_lints.rs
View file

@ -335,7 +335,6 @@ pub static LINTS: &[&crate::LintInfo] = &[
crate::manual_slice_size_calculation::MANUAL_SLICE_SIZE_CALCULATION_INFO,
crate::manual_string_new::MANUAL_STRING_NEW_INFO,
crate::manual_strip::MANUAL_STRIP_INFO,
crate::manual_unwrap_or_default::MANUAL_UNWRAP_OR_DEFAULT_INFO,
crate::map_unit_fn::OPTION_MAP_UNIT_FN_INFO,
crate::map_unit_fn::RESULT_MAP_UNIT_FN_INFO,
crate::match_result_ok::MATCH_RESULT_OK_INFO,
@ -345,6 +344,7 @@ pub static LINTS: &[&crate::LintInfo] = &[
crate::matches::MANUAL_MAP_INFO,
crate::matches::MANUAL_OK_ERR_INFO,
crate::matches::MANUAL_UNWRAP_OR_INFO,
crate::matches::MANUAL_UNWRAP_OR_DEFAULT_INFO,
crate::matches::MATCH_AS_REF_INFO,
crate::matches::MATCH_BOOL_INFO,
crate::matches::MATCH_LIKE_MATCHES_MACRO_INFO,

2
clippy_lints/src/lib.rs
View file

@ -226,7 +226,6 @@ mod manual_rotate;
mod manual_slice_size_calculation;
mod manual_string_new;
mod manual_strip;
mod manual_unwrap_or_default;
mod map_unit_fn;
mod match_result_ok;
mod matches;
@ -960,7 +959,6 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) {
store.register_early_pass(|| Box::new(multiple_bound_locations::MultipleBoundLocations));
store.register_late_pass(move |_| Box::new(assigning_clones::AssigningClones::new(conf)));
store.register_late_pass(|_| Box::new(zero_repeat_side_effects::ZeroRepeatSideEffects));
store.register_late_pass(|_| Box::new(manual_unwrap_or_default::ManualUnwrapOrDefault));
store.register_late_pass(|_| Box::new(integer_division_remainder_used::IntegerDivisionRemainderUsed));
store.register_late_pass(move |_| Box::new(macro_metavars_in_unsafe::ExprMetavarsInUnsafe::new(conf)));
store.register_late_pass(move |_| Box::new(string_patterns::StringPatterns::new(conf)));

212
clippy_lints/src/manual_unwrap_or_default.rs
View file

@ -1,212 +0,0 @@
use rustc_errors::Applicability;
use rustc_hir::def::Res;
use rustc_hir::{Arm, Expr, ExprKind, HirId, LangItem, MatchSource, Pat, PatExpr, PatExprKind, PatKind, QPath};
use rustc_lint::{LateContext, LateLintPass, LintContext};
use rustc_middle::ty::GenericArgKind;
use rustc_session::declare_lint_pass;
use rustc_span::sym;
use clippy_utils::diagnostics::span_lint_and_sugg;
use clippy_utils::higher::IfLetOrMatch;
use clippy_utils::sugg::Sugg;
use clippy_utils::ty::{expr_type_is_certain, implements_trait};
use clippy_utils::{is_default_equivalent, is_in_const_context, path_res, peel_blocks, span_contains_comment};
declare_clippy_lint! {
/// ### What it does
/// Checks if a `match` or `if let` expression can be simplified using
/// `.unwrap_or_default()`.
///
/// ### Why is this bad?
/// It can be done in one call with `.unwrap_or_default()`.
///
/// ### Example
/// ```no_run
/// let x: Option<String> = Some(String::new());
/// let y: String = match x {
/// Some(v) => v,
/// None => String::new(),
/// };
///
/// let x: Option<Vec<String>> = Some(Vec::new());
/// let y: Vec<String> = if let Some(v) = x {
/// v
/// } else {
/// Vec::new()
/// };
/// ```
/// Use instead:
/// ```no_run
/// let x: Option<String> = Some(String::new());
/// let y: String = x.unwrap_or_default();
///
/// let x: Option<Vec<String>> = Some(Vec::new());
/// let y: Vec<String> = x.unwrap_or_default();
/// ```
#[clippy::version = "1.79.0"]
pub MANUAL_UNWRAP_OR_DEFAULT,
suspicious,
"check if a `match` or `if let` can be simplified with `unwrap_or_default`"
}
declare_lint_pass!(ManualUnwrapOrDefault => [MANUAL_UNWRAP_OR_DEFAULT]);
fn get_some<'tcx>(cx: &LateContext<'tcx>, pat: &Pat<'tcx>) -> Option<HirId> {
if let PatKind::TupleStruct(QPath::Resolved(_, path), &[pat], _) = pat.kind
&& let PatKind::Binding(_, pat_id, _, _) = pat.kind
&& let Some(def_id) = path.res.opt_def_id()
// Since it comes from a pattern binding, we need to get the parent to actually match
// against it.
&& let Some(def_id) = cx.tcx.opt_parent(def_id)
&& (cx.tcx.lang_items().get(LangItem::OptionSome) == Some(def_id)
|| cx.tcx.lang_items().get(LangItem::ResultOk) == Some(def_id))
{
Some(pat_id)
} else {
None
}
}
fn get_none<'tcx>(cx: &LateContext<'tcx>, arm: &Arm<'tcx>) -> Option<&'tcx Expr<'tcx>> {
if let PatKind::Expr(PatExpr { kind: PatExprKind::Path(QPath::Resolved(_, path)), .. }) = arm.pat.kind
&& let Some(def_id) = path.res.opt_def_id()
// Since it comes from a pattern binding, we need to get the parent to actually match
// against it.
&& let Some(def_id) = cx.tcx.opt_parent(def_id)
&& cx.tcx.lang_items().get(LangItem::OptionNone) == Some(def_id)
{
Some(arm.body)
} else if let PatKind::TupleStruct(QPath::Resolved(_, path), _, _)= arm.pat.kind
&& let Some(def_id) = path.res.opt_def_id()
// Since it comes from a pattern binding, we need to get the parent to actually match
// against it.
&& let Some(def_id) = cx.tcx.opt_parent(def_id)
&& cx.tcx.lang_items().get(LangItem::ResultErr) == Some(def_id)
{
Some(arm.body)
} else if let PatKind::Wild = arm.pat.kind {
// We consider that the `Some` check will filter it out if it's not right.
Some(arm.body)
} else {
None
}
}
fn get_some_and_none_bodies<'tcx>(
cx: &LateContext<'tcx>,
arm1: &'tcx Arm<'tcx>,
arm2: &'tcx Arm<'tcx>,
) -> Option<((&'tcx Expr<'tcx>, HirId), &'tcx Expr<'tcx>)> {
if let Some(binding_id) = get_some(cx, arm1.pat)
&& let Some(body_none) = get_none(cx, arm2)
{
Some(((arm1.body, binding_id), body_none))
} else if let Some(binding_id) = get_some(cx, arm2.pat)
&& let Some(body_none) = get_none(cx, arm1)
{
Some(((arm2.body, binding_id), body_none))
} else {
None
}
}
#[allow(clippy::needless_pass_by_value)]
fn handle<'tcx>(cx: &LateContext<'tcx>, if_let_or_match: IfLetOrMatch<'tcx>, expr: &'tcx Expr<'tcx>) {
// Get expr_name ("if let" or "match" depending on kind of expression), the condition, the body for
// the some arm, the body for the none arm and the binding id of the some arm
let (expr_name, condition, body_some, body_none, binding_id) = match if_let_or_match {
IfLetOrMatch::Match(condition, [arm1, arm2], MatchSource::Normal | MatchSource::ForLoopDesugar)
// Make sure there are no guards to keep things simple
if arm1.guard.is_none()
&& arm2.guard.is_none()
// Get the some and none bodies and the binding id of the some arm
&& let Some(((body_some, binding_id), body_none)) = get_some_and_none_bodies(cx, arm1, arm2) =>
{
("match", condition, body_some, body_none, binding_id)
},
IfLetOrMatch::IfLet(condition, pat, if_expr, Some(else_expr), _)
if let Some(binding_id) = get_some(cx, pat) =>
{
("if let", condition, if_expr, else_expr, binding_id)
},
_ => {
// All other cases (match with number of arms != 2, if let without else, etc.)
return;
},
};
// We check if the return type of the expression implements Default.
let expr_type = cx.typeck_results().expr_ty(expr);
if let Some(default_trait_id) = cx.tcx.get_diagnostic_item(sym::Default)
&& implements_trait(cx, expr_type, default_trait_id, &[])
// We check if the initial condition implements Default.
&& let Some(condition_ty) = cx.typeck_results().expr_ty(condition).walk().nth(1)
&& let GenericArgKind::Type(condition_ty) = condition_ty.unpack()
&& implements_trait(cx, condition_ty, default_trait_id, &[])
// We check that the `Some(x) => x` doesn't do anything apart "returning" the value in `Some`.
&& let ExprKind::Path(QPath::Resolved(_, path)) = peel_blocks(body_some).kind
&& let Res::Local(local_id) = path.res
&& local_id == binding_id
// We now check the `None` arm is calling a method equivalent to `Default::default`.
&& let body_none = peel_blocks(body_none)
&& is_default_equivalent(cx, body_none)
&& let Some(receiver) = Sugg::hir_opt(cx, condition).map(Sugg::maybe_paren)
{
// Machine applicable only if there are no comments present
let applicability = if span_contains_comment(cx.sess().source_map(), expr.span) {
Applicability::MaybeIncorrect
} else {
Applicability::MachineApplicable
};
// We now check if the condition is a None variant, in which case we need to specify the type
if path_res(cx, condition)
.opt_def_id()
.is_some_and(|id| Some(cx.tcx.parent(id)) == cx.tcx.lang_items().option_none_variant())
{
return span_lint_and_sugg(
cx,
MANUAL_UNWRAP_OR_DEFAULT,
expr.span,
format!("{expr_name} can be simplified with `.unwrap_or_default()`"),
"replace it with",
format!("{receiver}::<{expr_type}>.unwrap_or_default()"),
applicability,
);
}
// We check if the expression type is still uncertain, in which case we ask the user to specify it
if !expr_type_is_certain(cx, condition) {
return span_lint_and_sugg(
cx,
MANUAL_UNWRAP_OR_DEFAULT,
expr.span,
format!("{expr_name} can be simplified with `.unwrap_or_default()`"),
format!("ascribe the type {expr_type} and replace your expression with"),
format!("{receiver}.unwrap_or_default()"),
Applicability::Unspecified,
);
}
span_lint_and_sugg(
cx,
MANUAL_UNWRAP_OR_DEFAULT,
expr.span,
format!("{expr_name} can be simplified with `.unwrap_or_default()`"),
"replace it with",
format!("{receiver}.unwrap_or_default()"),
applicability,
);
}
}
impl<'tcx> LateLintPass<'tcx> for ManualUnwrapOrDefault {
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) {
if let Some(if_let_or_match) = IfLetOrMatch::parse(cx, expr)
&& !expr.span.from_expansion()
&& !is_in_const_context(cx)
{
handle(cx, if_let_or_match, expr);
}
}
}

306
clippy_lints/src/matches/manual_unwrap_or.rs
View file

@ -1,133 +1,219 @@
use clippy_utils::consts::ConstEvalCtxt;
use clippy_utils::diagnostics::span_lint_and_sugg;
use clippy_utils::source::{SpanRangeExt, indent_of, reindent_multiline};
use clippy_utils::ty::is_type_diagnostic_item;
use clippy_utils::usage::contains_return_break_continue_macro;
use clippy_utils::{is_res_lang_ctor, path_to_local_id, peel_blocks, sugg};
use clippy_utils::source::{SpanRangeExt as _, indent_of, reindent_multiline};
use rustc_errors::Applicability;
use rustc_hir::LangItem::{OptionNone, ResultErr};
use rustc_hir::def::{DefKind, Res};
use rustc_hir::{Arm, Expr, Pat, PatExpr, PatExprKind, PatKind};
use rustc_lint::LateContext;
use rustc_middle::ty::Ty;
use rustc_hir::def::Res;
use rustc_hir::{Arm, Expr, ExprKind, HirId, LangItem, Pat, PatExpr, PatExprKind, PatKind, QPath};
use rustc_lint::{LateContext, LintContext};
use rustc_middle::ty::{GenericArgKind, Ty};
use rustc_span::sym;
use super::MANUAL_UNWRAP_OR;
use clippy_utils::diagnostics::span_lint_and_sugg;
use clippy_utils::sugg::Sugg;
use clippy_utils::ty::{expr_type_is_certain, get_type_diagnostic_name, implements_trait};
use clippy_utils::{is_default_equivalent, is_lint_allowed, path_res, peel_blocks, span_contains_comment};
pub(super) fn check_match<'tcx>(
cx: &LateContext<'tcx>,
expr: &'tcx Expr<'tcx>,
scrutinee: &'tcx Expr<'_>,
arms: &'tcx [Arm<'_>],
) {
let ty = cx.typeck_results().expr_ty(scrutinee);
if let Some((or_arm, unwrap_arm)) = applicable_or_arm(cx, arms) {
check_and_lint(cx, expr, unwrap_arm.pat, scrutinee, unwrap_arm.body, or_arm.body, ty);
use super::{MANUAL_UNWRAP_OR, MANUAL_UNWRAP_OR_DEFAULT};
fn get_some(cx: &LateContext<'_>, pat: &Pat<'_>) -> Option<HirId> {
if let PatKind::TupleStruct(QPath::Resolved(_, path), &[pat], _) = pat.kind
&& let PatKind::Binding(_, pat_id, _, _) = pat.kind
&& let Some(def_id) = path.res.opt_def_id()
// Since it comes from a pattern binding, we need to get the parent to actually match
// against it.
&& let Some(def_id) = cx.tcx.opt_parent(def_id)
&& let Some(lang_item) = cx.tcx.lang_items().from_def_id(def_id)
&& matches!(lang_item, LangItem::OptionSome | LangItem::ResultOk)
{
Some(pat_id)
} else {
None
}
}
pub(super) fn check_if_let<'tcx>(
cx: &LateContext<'tcx>,
expr: &'tcx Expr<'_>,
let_pat: &'tcx Pat<'_>,
let_expr: &'tcx Expr<'_>,
then_expr: &'tcx Expr<'_>,
else_expr: &'tcx Expr<'_>,
) {
let ty = cx.typeck_results().expr_ty(let_expr);
let then_ty = cx.typeck_results().expr_ty(then_expr);
// The signature is `fn unwrap_or<T>(self: Option<T>, default: T) -> T`.
// When `expr_adjustments(then_expr).is_empty()`, `T` should equate to `default`'s type.
// Otherwise, type error will occur.
if cx.typeck_results().expr_adjustments(then_expr).is_empty()
&& let rustc_middle::ty::Adt(_did, args) = ty.kind()
&& let Some(some_ty) = args.first().and_then(|arg| arg.as_type())
&& some_ty != then_ty
fn get_none<'tcx>(cx: &LateContext<'_>, arm: &Arm<'tcx>) -> Option<&'tcx Expr<'tcx>> {
if let PatKind::Expr(PatExpr { kind: PatExprKind::Path(QPath::Resolved(_, path)), .. }) = arm.pat.kind
&& let Some(def_id) = path.res.opt_def_id()
// Since it comes from a pattern binding, we need to get the parent to actually match
// against it.
&& let Some(def_id) = cx.tcx.opt_parent(def_id)
&& cx.tcx.lang_items().get(LangItem::OptionNone) == Some(def_id)
{
Some(arm.body)
} else if let PatKind::TupleStruct(QPath::Resolved(_, path), _, _)= arm.pat.kind
&& let Some(def_id) = path.res.opt_def_id()
// Since it comes from a pattern binding, we need to get the parent to actually match
// against it.
&& let Some(def_id) = cx.tcx.opt_parent(def_id)
&& cx.tcx.lang_items().get(LangItem::ResultErr) == Some(def_id)
{
Some(arm.body)
} else if let PatKind::Wild = arm.pat.kind {
// We consider that the `Some` check will filter it out if it's not right.
Some(arm.body)
} else {
None
}
}
fn get_some_and_none_bodies<'tcx>(
cx: &LateContext<'tcx>,
arm1: &'tcx Arm<'tcx>,
arm2: &'tcx Arm<'tcx>,
) -> Option<((&'tcx Expr<'tcx>, HirId), &'tcx Expr<'tcx>)> {
if let Some(binding_id) = get_some(cx, arm1.pat)
&& let Some(body_none) = get_none(cx, arm2)
{
Some(((arm1.body, binding_id), body_none))
} else if let Some(binding_id) = get_some(cx, arm2.pat)
&& let Some(body_none) = get_none(cx, arm1)
{
Some(((arm2.body, binding_id), body_none))
} else {
None
}
}
fn handle(
cx: &LateContext<'_>,
expr: &Expr<'_>,
expr_name: &'static str,
condition: &Expr<'_>,
body_some: &Expr<'_>,
body_none: &Expr<'_>,
binding_id: HirId,
) {
// Only deal with situations where both alternatives return the same non-adjusted type.
if cx.typeck_results().expr_ty(body_some) != cx.typeck_results().expr_ty(body_none) {
return;
}
check_and_lint(cx, expr, let_pat, let_expr, then_expr, peel_blocks(else_expr), ty);
}
fn check_and_lint<'tcx>(
cx: &LateContext<'tcx>,
expr: &'tcx Expr<'_>,
let_pat: &'tcx Pat<'_>,
let_expr: &'tcx Expr<'_>,
then_expr: &'tcx Expr<'_>,
else_expr: &'tcx Expr<'_>,
ty: Ty<'tcx>,
) {
if let PatKind::TupleStruct(ref qpath, [unwrap_pat], _) = let_pat.kind
&& let Res::Def(DefKind::Ctor(..), ctor_id) = cx.qpath_res(qpath, let_pat.hir_id)
&& let Some(variant_id) = cx.tcx.opt_parent(ctor_id)
&& (cx.tcx.lang_items().option_some_variant() == Some(variant_id)
|| cx.tcx.lang_items().result_ok_variant() == Some(variant_id))
&& let PatKind::Binding(_, binding_hir_id, ..) = unwrap_pat.kind
&& path_to_local_id(peel_blocks(then_expr), binding_hir_id)
&& cx.typeck_results().expr_adjustments(then_expr).is_empty()
&& let Some(ty_name) = find_type_name(cx, ty)
&& let Some(or_body_snippet) = else_expr.span.get_source_text(cx)
&& let Some(indent) = indent_of(cx, expr.span)
&& ConstEvalCtxt::new(cx).eval_simple(else_expr).is_some()
let expr_type = cx.typeck_results().expr_ty(expr);
// We check that the `Some(x) => x` doesn't do anything apart "returning" the value in `Some`.
if let ExprKind::Path(QPath::Resolved(_, path)) = peel_blocks(body_some).kind
&& let Res::Local(local_id) = path.res
&& local_id == binding_id
{
lint(cx, expr, let_expr, ty_name, &or_body_snippet, indent);
// Machine applicable only if there are no comments present
let mut applicability = if span_contains_comment(cx.sess().source_map(), expr.span) {
Applicability::MaybeIncorrect
} else {
Applicability::MachineApplicable
};
let receiver = Sugg::hir_with_applicability(cx, condition, "_", &mut applicability).maybe_paren();
// We now check the `None` arm is calling a method equivalent to `Default::default`.
if !is_lint_allowed(cx, MANUAL_UNWRAP_OR_DEFAULT, expr.hir_id)
// We check if the return type of the expression implements Default.
&& let Some(default_trait_id) = cx.tcx.get_diagnostic_item(sym::Default)
&& implements_trait(cx, expr_type, default_trait_id, &[])
// We check if the initial condition implements Default.
&& let Some(condition_ty) = cx.typeck_results().expr_ty(condition).walk().nth(1)
&& let GenericArgKind::Type(condition_ty) = condition_ty.unpack()
&& implements_trait(cx, condition_ty, default_trait_id, &[])
&& is_default_equivalent(cx, peel_blocks(body_none))
{
// We now check if the condition is a None variant, in which case we need to specify the type
if path_res(cx, condition)
.opt_def_id()
.is_some_and(|id| Some(cx.tcx.parent(id)) == cx.tcx.lang_items().option_none_variant())
{
return span_lint_and_sugg(
cx,
MANUAL_UNWRAP_OR_DEFAULT,
expr.span,
format!("{expr_name} can be simplified with `.unwrap_or_default()`"),
"replace it with",
format!("{receiver}::<{expr_type}>.unwrap_or_default()"),
applicability,
);
}
// We check if the expression type is still uncertain, in which case we ask the user to specify it
if !expr_type_is_certain(cx, condition) {
return span_lint_and_sugg(
cx,
MANUAL_UNWRAP_OR_DEFAULT,
expr.span,
format!("{expr_name} can be simplified with `.unwrap_or_default()`"),
format!("ascribe the type {expr_type} and replace your expression with"),
format!("{receiver}.unwrap_or_default()"),
Applicability::Unspecified,
);
}
span_lint_and_sugg(
cx,
MANUAL_UNWRAP_OR_DEFAULT,
expr.span,
format!("{expr_name} can be simplified with `.unwrap_or_default()`"),
"replace it with",
format!("{receiver}.unwrap_or_default()"),
applicability,
);
} else if let Some(ty_name) = find_type_name(cx, cx.typeck_results().expr_ty(condition))
&& cx.typeck_results().expr_adjustments(body_some).is_empty()
&& let Some(or_body_snippet) = peel_blocks(body_none).span.get_source_text(cx)
&& let Some(indent) = indent_of(cx, expr.span)
&& ConstEvalCtxt::new(cx).eval_simple(body_none).is_some()
{
let reindented_or_body = reindent_multiline(&or_body_snippet, true, Some(indent));
let mut app = Applicability::MachineApplicable;
let suggestion = Sugg::hir_with_context(cx, condition, expr.span.ctxt(), "..", &mut app).maybe_paren();
span_lint_and_sugg(
cx,
MANUAL_UNWRAP_OR,
expr.span,
format!("this pattern reimplements `{ty_name}::unwrap_or`"),
"replace with",
format!("{suggestion}.unwrap_or({reindented_or_body})",),
app,
);
}
}
}
fn find_type_name<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>) -> Option<&'static str> {
if is_type_diagnostic_item(cx, ty, sym::Option) {
Some("Option")
} else if is_type_diagnostic_item(cx, ty, sym::Result) {
Some("Result")
} else {
None
match get_type_diagnostic_name(cx, ty)? {
sym::Option => Some("Option"),
sym::Result => Some("Result"),
_ => None,
}
}
fn applicable_or_arm<'a>(cx: &LateContext<'_>, arms: &'a [Arm<'a>]) -> Option<(&'a Arm<'a>, &'a Arm<'a>)> {
if arms.len() == 2
&& arms.iter().all(|arm| arm.guard.is_none())
&& let Some((idx, or_arm)) = arms.iter().enumerate().find(|(_, arm)| match arm.pat.kind {
PatKind::Expr(PatExpr {
hir_id,
kind: PatExprKind::Path(qpath),
..
}) => is_res_lang_ctor(cx, cx.qpath_res(qpath, *hir_id), OptionNone),
PatKind::TupleStruct(ref qpath, [pat], _) => {
matches!(pat.kind, PatKind::Wild)
&& is_res_lang_ctor(cx, cx.qpath_res(qpath, arm.pat.hir_id), ResultErr)
},
_ => false,
})
&& let unwrap_arm = &arms[1 - idx]
&& !contains_return_break_continue_macro(or_arm.body)
{
Some((or_arm, unwrap_arm))
} else {
None
}
}
fn lint<'tcx>(
pub fn check_match<'tcx>(
cx: &LateContext<'tcx>,
expr: &Expr<'tcx>,
scrutinee: &'tcx Expr<'_>,
ty_name: &str,
or_body_snippet: &str,
indent: usize,
expr: &'tcx Expr<'tcx>,
scrutinee: &'tcx Expr<'tcx>,
arms: &'tcx [Arm<'tcx>],
) {
let reindented_or_body = reindent_multiline(or_body_snippet, true, Some(indent));
let mut app = Applicability::MachineApplicable;
let suggestion = sugg::Sugg::hir_with_context(cx, scrutinee, expr.span.ctxt(), "..", &mut app).maybe_paren();
span_lint_and_sugg(
cx,
MANUAL_UNWRAP_OR,
expr.span,
format!("this pattern reimplements `{ty_name}::unwrap_or`"),
"replace with",
format!("{suggestion}.unwrap_or({reindented_or_body})",),
app,
);
if let [arm1, arm2] = arms
// Make sure there are no guards to keep things simple
&& arm1.guard.is_none()
&& arm2.guard.is_none()
// Get the some and none bodies and the binding id of the some arm
&& let Some(((body_some, binding_id), body_none)) = get_some_and_none_bodies(cx, arm1, arm2)
{
handle(cx, expr, "match", scrutinee, body_some, body_none, binding_id);
}
}
pub fn check_if_let<'tcx>(
cx: &LateContext<'tcx>,
expr: &'tcx Expr<'tcx>,
pat: &'tcx Pat<'tcx>,
scrutinee: &'tcx Expr<'tcx>,
then_expr: &'tcx Expr<'tcx>,
else_expr: &'tcx Expr<'tcx>,
) {
if let Some(binding_id) = get_some(cx, pat) {
handle(
cx,
expr,
"if let",
scrutinee,
peel_blocks(then_expr),
peel_blocks(else_expr),
binding_id,
);
}
}

38
clippy_lints/src/matches/mod.rs
View file

@ -722,6 +722,43 @@ declare_clippy_lint! {
"finds patterns that can be encoded more concisely with `Option::unwrap_or` or `Result::unwrap_or`"
}
declare_clippy_lint! {
/// ### What it does
/// Checks if a `match` or `if let` expression can be simplified using
/// `.unwrap_or_default()`.
///
/// ### Why is this bad?
/// It can be done in one call with `.unwrap_or_default()`.
///
/// ### Example
/// ```no_run
/// let x: Option<String> = Some(String::new());
/// let y: String = match x {
/// Some(v) => v,
/// None => String::new(),
/// };
///
/// let x: Option<Vec<String>> = Some(Vec::new());
/// let y: Vec<String> = if let Some(v) = x {
/// v
/// } else {
/// Vec::new()
/// };
/// ```
/// Use instead:
/// ```no_run
/// let x: Option<String> = Some(String::new());
/// let y: String = x.unwrap_or_default();
///
/// let x: Option<Vec<String>> = Some(Vec::new());
/// let y: Vec<String> = x.unwrap_or_default();
/// ```
#[clippy::version = "1.79.0"]
pub MANUAL_UNWRAP_OR_DEFAULT,
suspicious,
"check if a `match` or `if let` can be simplified with `unwrap_or_default`"
}
declare_clippy_lint! {
/// ### What it does
/// Checks for `match vec[idx]` or `match vec[n..m]`.
@ -1040,6 +1077,7 @@ impl_lint_pass!(Matches => [
NEEDLESS_MATCH,
COLLAPSIBLE_MATCH,
MANUAL_UNWRAP_OR,
MANUAL_UNWRAP_OR_DEFAULT,
MATCH_ON_VEC_ITEMS,
MATCH_STR_CASE_MISMATCH,
SIGNIFICANT_DROP_IN_SCRUTINEE,

6
clippy_utils/src/higher.rs
View file

@ -176,6 +176,12 @@ impl<'hir> IfLetOrMatch<'hir> {
),
}
}
pub fn scrutinee(&self) -> &'hir Expr<'hir> {
match self {
Self::Match(scrutinee, _, _) | Self::IfLet(scrutinee, _, _, _, _) => scrutinee,
}
}
}
/// An `if` or `if let` expression

1
clippy_utils/src/lib.rs
View file

@ -1027,6 +1027,7 @@ pub fn is_default_equivalent(cx: &LateContext<'_>, e: &Expr<'_>) -> bool {
ExprKind::Call(from_func, [arg]) => is_default_equivalent_from(cx, from_func, arg),
ExprKind::Path(qpath) => is_res_lang_ctor(cx, cx.qpath_res(qpath, e.hir_id), OptionNone),
ExprKind::AddrOf(rustc_hir::BorrowKind::Ref, _, expr) => matches!(expr.kind, ExprKind::Array([])),
ExprKind::Block(Block { stmts: [], expr, .. }, _) => expr.is_some_and(|e| is_default_equivalent(cx, e)),
_ => false,
}
}

30
tests/ui/manual_unwrap_or.fixed
View file

@ -18,11 +18,9 @@ fn option_unwrap_or() {
// multiline case
#[rustfmt::skip]
Some(1).unwrap_or({
42 + 42
+ 42 + 42 + 42
+ 42 + 42 + 42
});
Some(1).unwrap_or(42 + 42
+ 42 + 42 + 42
+ 42 + 42 + 42);
// string case
Some("Bob").unwrap_or("Alice");
@ -125,11 +123,9 @@ fn result_unwrap_or() {
// multiline case
#[rustfmt::skip]
Ok::<i32, &str>(1).unwrap_or({
42 + 42
+ 42 + 42 + 42
+ 42 + 42 + 42
});
Ok::<i32, &str>(1).unwrap_or(42 + 42
+ 42 + 42 + 42
+ 42 + 42 + 42);
// string case
Ok::<&str, &str>("Bob").unwrap_or("Alice");
@ -159,11 +155,7 @@ fn result_unwrap_or() {
Ok(s) => s,
Err(s) => s,
};
// could lint, but unused_variables takes care of it
match Ok::<&str, &str>("Alice") {
Ok(s) => s,
Err(s) => "Bob",
};
Ok::<&str, &str>("Alice").unwrap_or("Bob");
Ok::<i32, i32>(1).unwrap_or(42);
@ -250,4 +242,12 @@ mod issue_13018 {
}
}
fn implicit_deref(v: Vec<String>) {
let _ = if let Some(s) = v.first() { s } else { "" };
}
fn allowed_manual_unwrap_or_zero() -> u32 {
Some(42).unwrap_or(0)
}
fn main() {}

15
tests/ui/manual_unwrap_or.rs
View file

@ -216,8 +216,8 @@ fn result_unwrap_or() {
Ok(s) => s,
Err(s) => s,
};
// could lint, but unused_variables takes care of it
match Ok::<&str, &str>("Alice") {
//~^ manual_unwrap_or
Ok(s) => s,
Err(s) => "Bob",
};
@ -316,4 +316,17 @@ mod issue_13018 {
}
}
fn implicit_deref(v: Vec<String>) {
let _ = if let Some(s) = v.first() { s } else { "" };
}
fn allowed_manual_unwrap_or_zero() -> u32 {
if let Some(x) = Some(42) {
//~^ manual_unwrap_or
x
} else {
0
}
}
fn main() {}

39
tests/ui/manual_unwrap_or.stderr
View file

@ -44,11 +44,9 @@ LL | | };
|
help: replace with
|
LL ~ Some(1).unwrap_or({
LL + 42 + 42
LL + + 42 + 42 + 42
LL + + 42 + 42 + 42
LL ~ });
LL ~ Some(1).unwrap_or(42 + 42
LL + + 42 + 42 + 42
LL ~ + 42 + 42 + 42);
|
error: this pattern reimplements `Option::unwrap_or`
@ -145,11 +143,9 @@ LL | | };
|
help: replace with
|
LL ~ Ok::<i32, &str>(1).unwrap_or({
LL + 42 + 42
LL + + 42 + 42 + 42
LL + + 42 + 42 + 42
LL ~ });
LL ~ Ok::<i32, &str>(1).unwrap_or(42 + 42
LL + + 42 + 42 + 42
LL ~ + 42 + 42 + 42);
|
error: this pattern reimplements `Result::unwrap_or`
@ -162,6 +158,16 @@ LL | | Err(_) => "Alice",
LL | | };
| |_____^ help: replace with: `Ok::<&str, &str>("Bob").unwrap_or("Alice")`
error: this pattern reimplements `Result::unwrap_or`
--> tests/ui/manual_unwrap_or.rs:219:5
|
LL | / match Ok::<&str, &str>("Alice") {
LL | |
LL | | Ok(s) => s,
LL | | Err(s) => "Bob",
LL | | };
| |_____^ help: replace with: `Ok::<&str, &str>("Alice").unwrap_or("Bob")`
error: this pattern reimplements `Result::unwrap_or`
--> tests/ui/manual_unwrap_or.rs:225:5
|
@ -184,5 +190,16 @@ LL | | None => 0,
LL | | };
| |_________^ help: replace with: `some_macro!().unwrap_or(0)`
error: aborting due to 16 previous errors
error: this pattern reimplements `Option::unwrap_or`
--> tests/ui/manual_unwrap_or.rs:324:5
|
LL | / if let Some(x) = Some(42) {
LL | |
LL | | x
LL | | } else {
LL | | 0
LL | | }
| |_____^ help: replace with: `Some(42).unwrap_or(0)`
error: aborting due to 18 previous errors

7
tests/ui/manual_unwrap_or_default.fixed
View file

@ -1,5 +1,5 @@
#![warn(clippy::manual_unwrap_or_default)]
#![allow(clippy::unnecessary_literal_unwrap, clippy::manual_unwrap_or)]
#![allow(clippy::unnecessary_literal_unwrap)]
fn main() {
let x: Option<Vec<String>> = None;
@ -99,3 +99,8 @@ fn issue_12928() {
let y = if let Some(Y(a, _)) = x { a } else { 0 };
let y = if let Some(Y(a, ..)) = x { a } else { 0 };
}
// For symetry with `manual_unwrap_or` test
fn allowed_manual_unwrap_or_zero() -> u32 {
Some(42).unwrap_or_default()
}

12
tests/ui/manual_unwrap_or_default.rs
View file

@ -1,5 +1,5 @@
#![warn(clippy::manual_unwrap_or_default)]
#![allow(clippy::unnecessary_literal_unwrap, clippy::manual_unwrap_or)]
#![allow(clippy::unnecessary_literal_unwrap)]
fn main() {
let x: Option<Vec<String>> = None;
@ -135,3 +135,13 @@ fn issue_12928() {
let y = if let Some(Y(a, _)) = x { a } else { 0 };
let y = if let Some(Y(a, ..)) = x { a } else { 0 };
}
// For symetry with `manual_unwrap_or` test
fn allowed_manual_unwrap_or_zero() -> u32 {
if let Some(x) = Some(42) {
//~^ manual_unwrap_or_default
x
} else {
0
}
}

13
tests/ui/manual_unwrap_or_default.stderr
View file

@ -86,5 +86,16 @@ LL | | _ => 0,
LL | | },
| |_________^ help: replace it with: `(*b).unwrap_or_default()`
error: aborting due to 8 previous errors
error: if let can be simplified with `.unwrap_or_default()`
--> tests/ui/manual_unwrap_or_default.rs:141:5
|
LL | / if let Some(x) = Some(42) {
LL | |
LL | | x
LL | | } else {
LL | | 0
LL | | }
| |_____^ help: replace it with: `Some(42).unwrap_or_default()`
error: aborting due to 9 previous errors