Skip to content

Commit ac18c24

Browse files
committed
Use IfLetOrMatch to avoid repetition of code
1 parent 6a6a917 commit ac18c24

File tree

1 file changed

+40
-62
lines changed

1 file changed

+40
-62
lines changed

clippy_lints/src/manual_unwrap_or_default.rs

Lines changed: 40 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,3 @@
1-
use clippy_utils::sugg::Sugg;
21
use rustc_errors::Applicability;
32
use rustc_hir::def::Res;
43
use rustc_hir::{Arm, Expr, ExprKind, HirId, LangItem, MatchSource, Pat, PatKind, QPath};
@@ -8,7 +7,8 @@ use rustc_session::declare_lint_pass;
87
use rustc_span::sym;
98

109
use clippy_utils::diagnostics::span_lint_and_sugg;
11-
use clippy_utils::source::snippet_opt;
10+
use clippy_utils::higher::IfLetOrMatch;
11+
use clippy_utils::sugg::Sugg;
1212
use clippy_utils::ty::implements_trait;
1313
use clippy_utils::{in_constant, is_default_equivalent, peel_blocks, span_contains_comment};
1414

@@ -106,32 +106,49 @@ fn get_some_and_none_bodies<'tcx>(
106106
}
107107
}
108108

109-
fn handle_match<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) -> bool {
110-
let ExprKind::Match(match_expr, [arm1, arm2], MatchSource::Normal | MatchSource::ForLoopDesugar) = expr.kind else {
111-
return false;
109+
#[allow(clippy::needless_pass_by_value)]
110+
fn handle<'tcx>(cx: &LateContext<'tcx>, if_let_or_match: IfLetOrMatch<'tcx>, expr: &'tcx Expr<'tcx>) {
111+
// Get expr_name ("if let" or "match" depending on kind of expression), the condition, the body for
112+
// the some arm, the body for the none arm and the binding id of the some arm
113+
let (expr_name, condition, body_some, body_none, binding_id) = match if_let_or_match {
114+
IfLetOrMatch::Match(condition, [arm1, arm2], MatchSource::Normal | MatchSource::ForLoopDesugar)
115+
// Make sure there are no guards to keep things simple
116+
if arm1.guard.is_none()
117+
&& arm2.guard.is_none()
118+
// Get the some and none bodies and the binding id of the some arm
119+
&& let Some(((body_some, binding_id), body_none)) = get_some_and_none_bodies(cx, arm1, arm2) =>
120+
{
121+
("match", condition, body_some, body_none, binding_id)
122+
},
123+
IfLetOrMatch::IfLet(condition, pat, if_expr, Some(else_expr), _)
124+
if let Some(binding_id) = get_some(cx, pat) =>
125+
{
126+
("if let", condition, if_expr, else_expr, binding_id)
127+
},
128+
_ => {
129+
// All other cases (match with number of arms != 2, if let without else, etc.)
130+
return;
131+
},
112132
};
113-
// We don't want conditions on the arms to simplify things.
114-
if arm1.guard.is_none()
115-
&& arm2.guard.is_none()
116-
// We check that the returned type implements the `Default` trait.
117-
&& let match_ty = cx.typeck_results().expr_ty(expr)
118-
&& let Some(default_trait_id) = cx.tcx.get_diagnostic_item(sym::Default)
119-
&& implements_trait(cx, match_ty, default_trait_id, &[])
120-
// We now get the bodies for both the `Some` and `None` arms.
121-
&& let Some(((body_some, binding_id), body_none)) = get_some_and_none_bodies(cx, arm1, arm2)
122-
// We check that the initial expression also implies the `Default` trait.
123-
&& let Some(match_expr_ty) = cx.typeck_results().expr_ty(match_expr).walk().nth(1)
124-
&& let GenericArgKind::Type(match_expr_ty) = match_expr_ty.unpack()
125-
&& implements_trait(cx, match_expr_ty, default_trait_id, &[])
133+
134+
// We check if the return type of the expression implements Default.
135+
let expr_type = cx.typeck_results().expr_ty(expr);
136+
if let Some(default_trait_id) = cx.tcx.get_diagnostic_item(sym::Default)
137+
&& implements_trait(cx, expr_type, default_trait_id, &[])
138+
// We check if the initial condition implements Default.
139+
&& let Some(condition_ty) = cx.typeck_results().expr_ty(condition).walk().nth(1)
140+
&& let GenericArgKind::Type(condition_ty) = condition_ty.unpack()
141+
&& implements_trait(cx, condition_ty, default_trait_id, &[])
126142
// We check that the `Some(x) => x` doesn't do anything apart "returning" the value in `Some`.
127143
&& let ExprKind::Path(QPath::Resolved(_, path)) = peel_blocks(body_some).kind
128144
&& let Res::Local(local_id) = path.res
129145
&& local_id == binding_id
130146
// We now check the `None` arm is calling a method equivalent to `Default::default`.
131147
&& let body_none = peel_blocks(body_none)
132148
&& is_default_equivalent(cx, body_none)
133-
&& let Some(receiver) = Sugg::hir_opt(cx, match_expr).map(Sugg::maybe_par)
149+
&& let Some(receiver) = Sugg::hir_opt(cx, condition).map(Sugg::maybe_par)
134150
{
151+
// Machine applicable only if there are no comments present
135152
let applicability = if span_contains_comment(cx.sess().source_map(), expr.span) {
136153
Applicability::MaybeIncorrect
137154
} else {
@@ -141,61 +158,22 @@ fn handle_match<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) -> bool {
141158
cx,
142159
MANUAL_UNWRAP_OR_DEFAULT,
143160
expr.span,
144-
"match can be simplified with `.unwrap_or_default()`",
161+
format!("{expr_name} can be simplified with `.unwrap_or_default()`"),
145162
"replace it with",
146163
format!("{receiver}.unwrap_or_default()"),
147164
applicability,
148165
);
149166
}
150-
true
151-
}
152-
153-
fn handle_if_let<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) {
154-
if let ExprKind::If(cond, if_block, Some(else_expr)) = expr.kind
155-
&& let ExprKind::Let(let_) = cond.kind
156-
&& let ExprKind::Block(_, _) = else_expr.kind
157-
// We check that the returned type implements the `Default` trait.
158-
&& let match_ty = cx.typeck_results().expr_ty(expr)
159-
&& let Some(default_trait_id) = cx.tcx.get_diagnostic_item(sym::Default)
160-
&& implements_trait(cx, match_ty, default_trait_id, &[])
161-
&& let Some(binding_id) = get_some(cx, let_.pat)
162-
// We check that the initial expression also implies the `Default` trait.
163-
&& let Some(let_ty) = cx.typeck_results().expr_ty(let_.init).walk().nth(1)
164-
&& let GenericArgKind::Type(let_ty) = let_ty.unpack()
165-
&& implements_trait(cx, let_ty, default_trait_id, &[])
166-
// We check that the `Some(x) => x` doesn't do anything apart "returning" the value in `Some`.
167-
&& let ExprKind::Path(QPath::Resolved(_, path)) = peel_blocks(if_block).kind
168-
&& let Res::Local(local_id) = path.res
169-
&& local_id == binding_id
170-
// We now check the `None` arm is calling a method equivalent to `Default::default`.
171-
&& let body_else = peel_blocks(else_expr)
172-
&& is_default_equivalent(cx, body_else)
173-
&& let Some(if_let_expr_snippet) = snippet_opt(cx, let_.init.span)
174-
{
175-
let applicability = if span_contains_comment(cx.sess().source_map(), expr.span) {
176-
Applicability::MaybeIncorrect
177-
} else {
178-
Applicability::MachineApplicable
179-
};
180-
span_lint_and_sugg(
181-
cx,
182-
MANUAL_UNWRAP_OR_DEFAULT,
183-
expr.span,
184-
"if let can be simplified with `.unwrap_or_default()`",
185-
"replace it with",
186-
format!("{if_let_expr_snippet}.unwrap_or_default()"),
187-
applicability,
188-
);
189-
}
190167
}
191168

192169
impl<'tcx> LateLintPass<'tcx> for ManualUnwrapOrDefault {
193170
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) {
194171
if expr.span.from_expansion() || in_constant(cx, expr.hir_id) {
195172
return;
196173
}
197-
if !handle_match(cx, expr) {
198-
handle_if_let(cx, expr);
174+
// Call handle only if the expression is `if let` or `match`
175+
if let Some(if_let_or_match) = IfLetOrMatch::parse(cx, expr) {
176+
handle(cx, if_let_or_match, expr);
199177
}
200178
}
201179
}

0 commit comments

Comments
 (0)