Skip to content

Commit f908909

Browse files
JarchoJarcho
committed
Refactor single_match 
1 parent 3da21b0 commit f908909

File tree

1 file changed

+60
-92
lines changed

1 file changed

+60
-92
lines changed

‎clippy_lints/src/matches/single_match.rs

Lines changed: 60 additions & 92 deletions
Original file line numberDiff line numberDiff line change
@@ -29,56 +29,41 @@ fn empty_arm_has_comment(cx: &LateContext<'_>, span: Span) -> bool {
2929

3030
#[rustfmt::skip]
3131
pub(crate) fn check(cx: &LateContext<'_>, ex: &Expr<'_>, arms: &[Arm<'_>], expr: &Expr<'_>) {
32-
if arms.len() == 2 && arms[0].guard.is_none() && arms[1].guard.is_none() {
33-
if expr.span.from_expansion() {
34-
// Don't lint match expressions present in
35-
// macro_rules! block
36-
return;
37-
}
38-
if let PatKind::Or(..) = arms[0].pat.kind {
39-
// don't lint for or patterns for now, this makes
40-
// the lint noisy in unnecessary situations
41-
return;
42-
}
43-
let els = arms[1].body;
44-
let els = if is_unit_expr(peel_blocks(els)) && !empty_arm_has_comment(cx, els.span) {
32+
if let [arm1, arm2] = arms
33+
&& arm1.guard.is_none()
34+
&& arm2.guard.is_none()
35+
&& !expr.span.from_expansion()
36+
// don't lint for or patterns for now, this makes
37+
// the lint noisy in unnecessary situations
38+
&& !matches!(arm1.pat.kind, PatKind::Or(..))
39+
{
40+
let els = if is_unit_expr(peel_blocks(arm2.body)) && !empty_arm_has_comment(cx, arm2.body.span) {
4541
None
46-
} else if let ExprKind::Block(Block { stmts, expr: block_expr, .. }, _) = els.kind {
47-
if stmts.len() == 1 && block_expr.is_none() || stmts.is_empty() && block_expr.is_some() {
42+
} else if let ExprKind::Block(block, _) = arm2.body.kind {
43+
if matches!((block.stmts, block.expr), ([], Some(_)) | ([_], None)) {
4844
// single statement/expr "else" block, don't lint
4945
return;
5046
}
5147
// block with 2+ statements or 1 expr and 1+ statement
52-
Some(els)
48+
Some(arm2.body)
5349
} else {
54-
// not a block or an emtpy block w/ comments, don't lint
50+
// not a block or an empty block w/ comments, don't lint
5551
return;
5652
};
5753

5854
let ty = cx.typeck_results().expr_ty(ex);
59-
if *ty.kind() != ty::Bool || is_lint_allowed(cx, MATCH_BOOL, ex.hir_id) {
60-
check_single_pattern(cx, ex, arms, expr, els);
61-
check_opt_like(cx, ex, arms, expr, ty, els);
55+
if (*ty.kind() != ty::Bool || is_lint_allowed(cx, MATCH_BOOL, ex.hir_id))
56+
&& (is_wild(arm2.pat) || form_exhaustive_matches(cx, ty, arm1.pat, arm2.pat))
57+
{
58+
report_single_pattern(cx, ex, arm1, expr, els);
6259
}
6360
}
6461
}
6562

66-
fn check_single_pattern(
67-
cx: &LateContext<'_>,
68-
ex: &Expr<'_>,
69-
arms: &[Arm<'_>],
70-
expr: &Expr<'_>,
71-
els: Option<&Expr<'_>>,
72-
) {
73-
if is_wild(arms[1].pat) {
74-
report_single_pattern(cx, ex, arms, expr, els);
75-
}
76-
}
77-
7863
fn report_single_pattern(
7964
cx: &LateContext<'_>,
8065
ex: &Expr<'_>,
81-
arms: &[Arm<'_>],
66+
arm: &Arm<'_>,
8267
expr: &Expr<'_>,
8368
els: Option<&Expr<'_>>,
8469
) {
@@ -89,71 +74,54 @@ fn report_single_pattern(
8974
format!(" else {}", expr_block(cx, els, ctxt, "..", Some(expr.span), &mut app))
9075
});
9176

92-
let (pat, pat_ref_count) = peel_hir_pat_refs(arms[0].pat);
93-
let (msg, sugg) = if_chain! {
94-
if let PatKind::Path(_) | PatKind::Lit(_) = pat.kind;
95-
let (ty, ty_ref_count) = peel_mid_ty_refs(cx.typeck_results().expr_ty(ex));
96-
if let Some(spe_trait_id) = cx.tcx.lang_items().structural_peq_trait();
97-
if let Some(pe_trait_id) = cx.tcx.lang_items().eq_trait();
98-
if ty.is_integral() || ty.is_char() || ty.is_str()
77+
let (pat, pat_ref_count) = peel_hir_pat_refs(arm.pat);
78+
let (msg, sugg) = if let PatKind::Path(_) | PatKind::Lit(_) = pat.kind
79+
&& let (ty, ty_ref_count) = peel_mid_ty_refs(cx.typeck_results().expr_ty(ex))
80+
&& let Some(spe_trait_id) = cx.tcx.lang_items().structural_peq_trait()
81+
&& let Some(pe_trait_id) = cx.tcx.lang_items().eq_trait()
82+
&& (ty.is_integral() || ty.is_char() || ty.is_str()
9983
|| (implements_trait(cx, ty, spe_trait_id, &[])
100-
&& implements_trait(cx, ty, pe_trait_id, &[ty.into()]));
101-
then {
102-
// scrutinee derives PartialEq and the pattern is a constant.
103-
let pat_ref_count = match pat.kind {
104-
// string literals are already a reference.
105-
PatKind::Lit(Expr { kind: ExprKind::Lit(lit), .. }) if lit.node.is_str() => pat_ref_count + 1,
106-
_ => pat_ref_count,
107-
};
108-
// References are only implicitly added to the pattern, so no overflow here.
109-
// e.g. will work: match &Some(_) { Some(_) => () }
110-
// will not: match Some(_) { &Some(_) => () }
111-
let ref_count_diff = ty_ref_count - pat_ref_count;
112-
113-
// Try to remove address of expressions first.
114-
let (ex, removed) = peel_n_hir_expr_refs(ex, ref_count_diff);
115-
let ref_count_diff = ref_count_diff - removed;
116-
117-
let msg = "you seem to be trying to use `match` for an equality check. Consider using `if`";
118-
let sugg = format!(
119-
"if {} == {}{} {}{els_str}",
120-
snippet(cx, ex.span, ".."),
121-
// PartialEq for different reference counts may not exist.
122-
"&".repeat(ref_count_diff),
123-
snippet(cx, arms[0].pat.span, ".."),
124-
expr_block(cx, arms[0].body, ctxt, "..", Some(expr.span), &mut app),
125-
);
126-
(msg, sugg)
127-
} else {
128-
let msg = "you seem to be trying to use `match` for destructuring a single pattern. Consider using `if let`";
129-
let sugg = format!(
130-
"if let {} = {} {}{els_str}",
131-
snippet(cx, arms[0].pat.span, ".."),
132-
snippet(cx, ex.span, ".."),
133-
expr_block(cx, arms[0].body, ctxt, "..", Some(expr.span), &mut app),
134-
);
135-
(msg, sugg)
136-
}
84+
&& implements_trait(cx, ty, pe_trait_id, &[ty.into()])))
85+
{
86+
// scrutinee derives PartialEq and the pattern is a constant.
87+
let pat_ref_count = match pat.kind {
88+
// string literals are already a reference.
89+
PatKind::Lit(Expr { kind: ExprKind::Lit(lit), .. }) if lit.node.is_str() => pat_ref_count + 1,
90+
_ => pat_ref_count,
91+
};
92+
// References are only implicitly added to the pattern, so no overflow here.
93+
// e.g. will work: match &Some(_) { Some(_) => () }
94+
// will not: match Some(_) { &Some(_) => () }
95+
let ref_count_diff = ty_ref_count - pat_ref_count;
96+
97+
// Try to remove address of expressions first.
98+
let (ex, removed) = peel_n_hir_expr_refs(ex, ref_count_diff);
99+
let ref_count_diff = ref_count_diff - removed;
100+
101+
let msg = "you seem to be trying to use `match` for an equality check. Consider using `if`";
102+
let sugg = format!(
103+
"if {} == {}{} {}{els_str}",
104+
snippet(cx, ex.span, ".."),
105+
// PartialEq for different reference counts may not exist.
106+
"&".repeat(ref_count_diff),
107+
snippet(cx, arm.pat.span, ".."),
108+
expr_block(cx, arm.body, ctxt, "..", Some(expr.span), &mut app),
109+
);
110+
(msg, sugg)
111+
} else {
112+
let msg = "you seem to be trying to use `match` for destructuring a single pattern. Consider using `if let`";
113+
let sugg = format!(
114+
"if let {} = {} {}{els_str}",
115+
snippet(cx, arm.pat.span, ".."),
116+
snippet(cx, ex.span, ".."),
117+
expr_block(cx, arm.body, ctxt, "..", Some(expr.span), &mut app),
118+
);
119+
(msg, sugg)
137120
};
138121

139122
span_lint_and_sugg(cx, lint, expr.span, msg, "try", sugg, app);
140123
}
141124

142-
fn check_opt_like<'a>(
143-
cx: &LateContext<'a>,
144-
ex: &Expr<'_>,
145-
arms: &[Arm<'_>],
146-
expr: &Expr<'_>,
147-
ty: Ty<'a>,
148-
els: Option<&Expr<'_>>,
149-
) {
150-
// We don't want to lint if the second arm contains an enum which could
151-
// have more variants in the future.
152-
if form_exhaustive_matches(cx, ty, arms[0].pat, arms[1].pat) {
153-
report_single_pattern(cx, ex, arms, expr, els);
154-
}
155-
}
156-
157125
/// Returns `true` if all of the types in the pattern are enums which we know
158126
/// won't be expanded in the future
159127
fn pat_in_candidate_enum<'a>(cx: &LateContext<'a>, ty: Ty<'a>, pat: &Pat<'_>) -> bool {

0 commit comments

Comments
 (0)