Skip to content

Commit 81debac

Browse files
borsbors
committed
Auto merge of rust-lang#12440 - GuillaumeGomez:add-match_option_and_default, r=llogiq
Add new `manual_unwrap_or_default` lint This adds a new lint checking if a `match` or a `if let` can be replaced with `unwrap_or_default`. ---- changelog: Add new [`manual_unwrap_or_default`] lint
2 parents 7ee75f8 + 98ac5f1 commit 81debac

21 files changed

+413
-98
lines changed

‎CHANGELOG.md

+1
Original file line numberDiff line numberDiff line change
@@ -5377,6 +5377,7 @@ Released 2018-09-13
53775377
[`manual_swap`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_swap
53785378
[`manual_try_fold`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_try_fold
53795379
[`manual_unwrap_or`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_unwrap_or
5380+
[`manual_unwrap_or_default`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_unwrap_or_default
53805381
[`manual_while_let_some`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_while_let_some
53815382
[`many_single_char_names`]: https://rust-lang.github.io/rust-clippy/master/index.html#many_single_char_names
53825383
[`map_clone`]: https://rust-lang.github.io/rust-clippy/master/index.html#map_clone

‎clippy_lints/src/declared_lints.rs

+1
Original file line numberDiff line numberDiff line change
@@ -310,6 +310,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
310310
crate::manual_slice_size_calculation::MANUAL_SLICE_SIZE_CALCULATION_INFO,
311311
crate::manual_string_new::MANUAL_STRING_NEW_INFO,
312312
crate::manual_strip::MANUAL_STRIP_INFO,
313+
crate::manual_unwrap_or_default::MANUAL_UNWRAP_OR_DEFAULT_INFO,
313314
crate::map_unit_fn::OPTION_MAP_UNIT_FN_INFO,
314315
crate::map_unit_fn::RESULT_MAP_UNIT_FN_INFO,
315316
crate::match_result_ok::MATCH_RESULT_OK_INFO,

‎clippy_lints/src/lib.rs

+2
Original file line numberDiff line numberDiff line change
@@ -211,6 +211,7 @@ mod manual_retain;
211211
mod manual_slice_size_calculation;
212212
mod manual_string_new;
213213
mod manual_strip;
214+
mod manual_unwrap_or_default;
214215
mod map_unit_fn;
215216
mod match_result_ok;
216217
mod matches;
@@ -1122,6 +1123,7 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) {
11221123
store.register_early_pass(|| Box::new(multiple_bound_locations::MultipleBoundLocations));
11231124
store.register_late_pass(|_| Box::new(assigning_clones::AssigningClones));
11241125
store.register_late_pass(|_| Box::new(zero_repeat_side_effects::ZeroRepeatSideEffects));
1126+
store.register_late_pass(|_| Box::new(manual_unwrap_or_default::ManualUnwrapOrDefault));
11251127
// add lints here, do not remove this comment, it's used in `new_lint`
11261128
}
11271129

‎clippy_lints/src/manual_unwrap_or_default.rs

+181
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,181 @@
1+
use rustc_errors::Applicability;
2+
use rustc_hir::def::Res;
3+
use rustc_hir::{Arm, Expr, ExprKind, HirId, LangItem, MatchSource, Pat, PatKind, QPath};
4+
use rustc_lint::{LateContext, LateLintPass};
5+
use rustc_session::declare_lint_pass;
6+
use rustc_span::sym;
7+
8+
use clippy_utils::diagnostics::span_lint_and_sugg;
9+
use clippy_utils::is_default_equivalent;
10+
use clippy_utils::source::snippet_opt;
11+
use clippy_utils::ty::implements_trait;
12+
13+
declare_clippy_lint! {
14+
/// ### What it does
15+
/// Checks if a `match` or `if let` expression can be simplified using
16+
/// `.unwrap_or_default()`.
17+
///
18+
/// ### Why is this bad?
19+
/// It can be done in one call with `.unwrap_or_default()`.
20+
///
21+
/// ### Example
22+
/// ```no_run
23+
/// let x: Option<String> = Some(String::new());
24+
/// let y: String = match x {
25+
/// Some(v) => v,
26+
/// None => String::new(),
27+
/// };
28+
///
29+
/// let x: Option<Vec<String>> = Some(Vec::new());
30+
/// let y: Vec<String> = if let Some(v) = x {
31+
/// v
32+
/// } else {
33+
/// Vec::new()
34+
/// };
35+
/// ```
36+
/// Use instead:
37+
/// ```no_run
38+
/// let x: Option<String> = Some(String::new());
39+
/// let y: String = x.unwrap_or_default();
40+
///
41+
/// let x: Option<Vec<String>> = Some(Vec::new());
42+
/// let y: Vec<String> = x.unwrap_or_default();
43+
/// ```
44+
#[clippy::version = "1.78.0"]
45+
pub MANUAL_UNWRAP_OR_DEFAULT,
46+
suspicious,
47+
"check if a `match` or `if let` can be simplified with `unwrap_or_default`"
48+
}
49+
50+
declare_lint_pass!(ManualUnwrapOrDefault => [MANUAL_UNWRAP_OR_DEFAULT]);
51+
52+
fn get_some<'tcx>(cx: &LateContext<'tcx>, pat: &Pat<'tcx>) -> Option<HirId> {
53+
if let PatKind::TupleStruct(QPath::Resolved(_, path), &[pat], _) = pat.kind
54+
&& let Some(def_id) = path.res.opt_def_id()
55+
// Since it comes from a pattern binding, we need to get the parent to actually match
56+
// against it.
57+
&& let Some(def_id) = cx.tcx.opt_parent(def_id)
58+
&& cx.tcx.lang_items().get(LangItem::OptionSome) == Some(def_id)
59+
{
60+
let mut bindings = Vec::new();
61+
pat.each_binding(|_, id, _, _| bindings.push(id));
62+
if let &[id] = bindings.as_slice() {
63+
Some(id)
64+
} else {
65+
None
66+
}
67+
} else {
68+
None
69+
}
70+
}
71+
72+
fn get_none<'tcx>(cx: &LateContext<'tcx>, arm: &Arm<'tcx>) -> Option<&'tcx Expr<'tcx>> {
73+
if let PatKind::Path(QPath::Resolved(_, path)) = arm.pat.kind
74+
&& let Some(def_id) = path.res.opt_def_id()
75+
// Since it comes from a pattern binding, we need to get the parent to actually match
76+
// against it.
77+
&& let Some(def_id) = cx.tcx.opt_parent(def_id)
78+
&& cx.tcx.lang_items().get(LangItem::OptionNone) == Some(def_id)
79+
{
80+
Some(arm.body)
81+
} else if let PatKind::Wild = arm.pat.kind {
82+
// We consider that the `Some` check will filter it out if it's not right.
83+
Some(arm.body)
84+
} else {
85+
None
86+
}
87+
}
88+
89+
fn get_some_and_none_bodies<'tcx>(
90+
cx: &LateContext<'tcx>,
91+
arm1: &'tcx Arm<'tcx>,
92+
arm2: &'tcx Arm<'tcx>,
93+
) -> Option<((&'tcx Expr<'tcx>, HirId), &'tcx Expr<'tcx>)> {
94+
if let Some(binding_id) = get_some(cx, arm1.pat)
95+
&& let Some(body_none) = get_none(cx, arm2)
96+
{
97+
Some(((arm1.body, binding_id), body_none))
98+
} else if let Some(binding_id) = get_some(cx, arm2.pat)
99+
&& let Some(body_none) = get_none(cx, arm1)
100+
{
101+
Some(((arm2.body, binding_id), body_none))
102+
} else {
103+
None
104+
}
105+
}
106+
107+
fn handle_match<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) -> bool {
108+
let ExprKind::Match(match_expr, [arm1, arm2], MatchSource::Normal | MatchSource::ForLoopDesugar) = expr.kind else {
109+
return false;
110+
};
111+
// We don't want conditions on the arms to simplify things.
112+
if arm1.guard.is_none()
113+
&& arm2.guard.is_none()
114+
// We check that the returned type implements the `Default` trait.
115+
&& let match_ty = cx.typeck_results().expr_ty(expr)
116+
&& let Some(default_trait_id) = cx.tcx.get_diagnostic_item(sym::Default)
117+
&& implements_trait(cx, match_ty, default_trait_id, &[])
118+
// We now get the bodies for both the `Some` and `None` arms.
119+
&& let Some(((body_some, binding_id), body_none)) = get_some_and_none_bodies(cx, arm1, arm2)
120+
// We check that the `Some(x) => x` doesn't do anything apart "returning" the value in `Some`.
121+
&& let ExprKind::Path(QPath::Resolved(_, path)) = body_some.peel_blocks().kind
122+
&& let Res::Local(local_id) = path.res
123+
&& local_id == binding_id
124+
// We now check the `None` arm is calling a method equivalent to `Default::default`.
125+
&& let body_none = body_none.peel_blocks()
126+
&& is_default_equivalent(cx, body_none)
127+
&& let Some(match_expr_snippet) = snippet_opt(cx, match_expr.span)
128+
{
129+
span_lint_and_sugg(
130+
cx,
131+
MANUAL_UNWRAP_OR_DEFAULT,
132+
expr.span,
133+
"match can be simplified with `.unwrap_or_default()`",
134+
"replace it with",
135+
format!("{match_expr_snippet}.unwrap_or_default()"),
136+
Applicability::MachineApplicable,
137+
);
138+
}
139+
true
140+
}
141+
142+
fn handle_if_let<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) {
143+
if let ExprKind::If(cond, if_block, Some(else_expr)) = expr.kind
144+
&& let ExprKind::Let(let_) = cond.kind
145+
&& let ExprKind::Block(_, _) = else_expr.kind
146+
// We check that the returned type implements the `Default` trait.
147+
&& let match_ty = cx.typeck_results().expr_ty(expr)
148+
&& let Some(default_trait_id) = cx.tcx.get_diagnostic_item(sym::Default)
149+
&& implements_trait(cx, match_ty, default_trait_id, &[])
150+
&& let Some(binding_id) = get_some(cx, let_.pat)
151+
// We check that the `Some(x) => x` doesn't do anything apart "returning" the value in `Some`.
152+
&& let ExprKind::Path(QPath::Resolved(_, path)) = if_block.peel_blocks().kind
153+
&& let Res::Local(local_id) = path.res
154+
&& local_id == binding_id
155+
// We now check the `None` arm is calling a method equivalent to `Default::default`.
156+
&& let body_else = else_expr.peel_blocks()
157+
&& is_default_equivalent(cx, body_else)
158+
&& let Some(if_let_expr_snippet) = snippet_opt(cx, let_.init.span)
159+
{
160+
span_lint_and_sugg(
161+
cx,
162+
MANUAL_UNWRAP_OR_DEFAULT,
163+
expr.span,
164+
"if let can be simplified with `.unwrap_or_default()`",
165+
"replace it with",
166+
format!("{if_let_expr_snippet}.unwrap_or_default()"),
167+
Applicability::MachineApplicable,
168+
);
169+
}
170+
}
171+
172+
impl<'tcx> LateLintPass<'tcx> for ManualUnwrapOrDefault {
173+
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) {
174+
if expr.span.from_expansion() {
175+
return;
176+
}
177+
if !handle_match(cx, expr) {
178+
handle_if_let(cx, expr);
179+
}
180+
}
181+
}

‎tests/ui/manual_let_else.rs

+2-1
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,8 @@
88
clippy::never_loop,
99
clippy::needless_if,
1010
clippy::diverging_sub_expression,
11-
clippy::single_match
11+
clippy::single_match,
12+
clippy::manual_unwrap_or_default
1213
)]
1314
#![warn(clippy::manual_let_else)]
1415
//@no-rustfix

0 commit comments

Comments
 (0)