Skip to content

Commit 3479e94

Browse files
JarchoJarcho
committed
Split out match_single_binding 
1 parent 6836941 commit 3479e94

File tree

2 files changed

+171
-163
lines changed

2 files changed

+171
-163
lines changed

‎clippy_lints/src/matches/match_single_binding.rs

+166
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,166 @@
1+
use clippy_utils::diagnostics::span_lint_and_sugg;
2+
use clippy_utils::source::{indent_of, snippet_block, snippet_opt, snippet_with_applicability};
3+
use clippy_utils::sugg::Sugg;
4+
use clippy_utils::{get_parent_expr, is_refutable, peel_blocks};
5+
use rustc_errors::Applicability;
6+
use rustc_hir::{Arm, Expr, ExprKind, Local, Node, PatKind};
7+
use rustc_lint::LateContext;
8+
9+
use super::MATCH_SINGLE_BINDING;
10+
11+
#[allow(clippy::too_many_lines)]
12+
pub(crate) fn check<'a>(cx: &LateContext<'a>, ex: &Expr<'a>, arms: &[Arm<'_>], expr: &Expr<'_>) {
13+
if expr.span.from_expansion() || arms.len() != 1 || is_refutable(cx, arms[0].pat) {
14+
return;
15+
}
16+
17+
// HACK:
18+
// This is a hack to deal with arms that are excluded by macros like `#[cfg]`. It is only used here
19+
// to prevent false positives as there is currently no better way to detect if code was excluded by
20+
// a macro. See PR #6435
21+
if_chain! {
22+
if let Some(match_snippet) = snippet_opt(cx, expr.span);
23+
if let Some(arm_snippet) = snippet_opt(cx, arms[0].span);
24+
if let Some(ex_snippet) = snippet_opt(cx, ex.span);
25+
let rest_snippet = match_snippet.replace(&arm_snippet, "").replace(&ex_snippet, "");
26+
if rest_snippet.contains("=>");
27+
then {
28+
// The code it self contains another thick arrow "=>"
29+
// -> Either another arm or a comment
30+
return;
31+
}
32+
}
33+
34+
let matched_vars = ex.span;
35+
let bind_names = arms[0].pat.span;
36+
let match_body = peel_blocks(arms[0].body);
37+
let mut snippet_body = if match_body.span.from_expansion() {
38+
Sugg::hir_with_macro_callsite(cx, match_body, "..").to_string()
39+
} else {
40+
snippet_block(cx, match_body.span, "..", Some(expr.span)).to_string()
41+
};
42+
43+
// Do we need to add ';' to suggestion ?
44+
match match_body.kind {
45+
ExprKind::Block(block, _) => {
46+
// macro + expr_ty(body) == ()
47+
if block.span.from_expansion() && cx.typeck_results().expr_ty(match_body).is_unit() {
48+
snippet_body.push(';');
49+
}
50+
},
51+
_ => {
52+
// expr_ty(body) == ()
53+
if cx.typeck_results().expr_ty(match_body).is_unit() {
54+
snippet_body.push(';');
55+
}
56+
},
57+
}
58+
59+
let mut applicability = Applicability::MaybeIncorrect;
60+
match arms[0].pat.kind {
61+
PatKind::Binding(..) | PatKind::Tuple(_, _) | PatKind::Struct(..) => {
62+
// If this match is in a local (`let`) stmt
63+
let (target_span, sugg) = if let Some(parent_let_node) = opt_parent_let(cx, ex) {
64+
(
65+
parent_let_node.span,
66+
format!(
67+
"let {} = {};\n{}let {} = {};",
68+
snippet_with_applicability(cx, bind_names, "..", &mut applicability),
69+
snippet_with_applicability(cx, matched_vars, "..", &mut applicability),
70+
" ".repeat(indent_of(cx, expr.span).unwrap_or(0)),
71+
snippet_with_applicability(cx, parent_let_node.pat.span, "..", &mut applicability),
72+
snippet_body
73+
),
74+
)
75+
} else {
76+
// If we are in closure, we need curly braces around suggestion
77+
let mut indent = " ".repeat(indent_of(cx, ex.span).unwrap_or(0));
78+
let (mut cbrace_start, mut cbrace_end) = ("".to_string(), "".to_string());
79+
if let Some(parent_expr) = get_parent_expr(cx, expr) {
80+
if let ExprKind::Closure(..) = parent_expr.kind {
81+
cbrace_end = format!("\n{}}}", indent);
82+
// Fix body indent due to the closure
83+
indent = " ".repeat(indent_of(cx, bind_names).unwrap_or(0));
84+
cbrace_start = format!("{{\n{}", indent);
85+
}
86+
}
87+
// If the parent is already an arm, and the body is another match statement,
88+
// we need curly braces around suggestion
89+
let parent_node_id = cx.tcx.hir().get_parent_node(expr.hir_id);
90+
if let Node::Arm(arm) = &cx.tcx.hir().get(parent_node_id) {
91+
if let ExprKind::Match(..) = arm.body.kind {
92+
cbrace_end = format!("\n{}}}", indent);
93+
// Fix body indent due to the match
94+
indent = " ".repeat(indent_of(cx, bind_names).unwrap_or(0));
95+
cbrace_start = format!("{{\n{}", indent);
96+
}
97+
}
98+
(
99+
expr.span,
100+
format!(
101+
"{}let {} = {};\n{}{}{}",
102+
cbrace_start,
103+
snippet_with_applicability(cx, bind_names, "..", &mut applicability),
104+
snippet_with_applicability(cx, matched_vars, "..", &mut applicability),
105+
indent,
106+
snippet_body,
107+
cbrace_end
108+
),
109+
)
110+
};
111+
span_lint_and_sugg(
112+
cx,
113+
MATCH_SINGLE_BINDING,
114+
target_span,
115+
"this match could be written as a `let` statement",
116+
"consider using `let` statement",
117+
sugg,
118+
applicability,
119+
);
120+
},
121+
PatKind::Wild => {
122+
if ex.can_have_side_effects() {
123+
let indent = " ".repeat(indent_of(cx, expr.span).unwrap_or(0));
124+
let sugg = format!(
125+
"{};\n{}{}",
126+
snippet_with_applicability(cx, ex.span, "..", &mut applicability),
127+
indent,
128+
snippet_body
129+
);
130+
span_lint_and_sugg(
131+
cx,
132+
MATCH_SINGLE_BINDING,
133+
expr.span,
134+
"this match could be replaced by its scrutinee and body",
135+
"consider using the scrutinee and body instead",
136+
sugg,
137+
applicability,
138+
);
139+
} else {
140+
span_lint_and_sugg(
141+
cx,
142+
MATCH_SINGLE_BINDING,
143+
expr.span,
144+
"this match could be replaced by its body itself",
145+
"consider using the match body instead",
146+
snippet_body,
147+
Applicability::MachineApplicable,
148+
);
149+
}
150+
},
151+
_ => (),
152+
}
153+
}
154+
155+
/// Returns true if the `ex` match expression is in a local (`let`) statement
156+
fn opt_parent_let<'a>(cx: &LateContext<'a>, ex: &Expr<'a>) -> Option<&'a Local<'a>> {
157+
let map = &cx.tcx.hir();
158+
if_chain! {
159+
if let Some(Node::Expr(parent_arm_expr)) = map.find(map.get_parent_node(ex.hir_id));
160+
if let Some(Node::Local(parent_let_expr)) = map.find(map.get_parent_node(parent_arm_expr.hir_id));
161+
then {
162+
return Some(parent_let_expr);
163+
}
164+
}
165+
None
166+
}

‎clippy_lints/src/matches/mod.rs

+5-163
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,11 @@
11
use clippy_utils::diagnostics::{multispan_sugg, span_lint_and_help, span_lint_and_sugg, span_lint_and_then};
2-
use clippy_utils::source::{indent_of, snippet, snippet_block, snippet_opt, snippet_with_applicability};
2+
use clippy_utils::source::{snippet, snippet_with_applicability};
33
use clippy_utils::sugg::Sugg;
4-
use clippy_utils::{
5-
get_parent_expr, is_refutable, is_wild, meets_msrv, msrvs, path_to_local_id, peel_blocks, strip_pat_refs,
6-
};
4+
use clippy_utils::{is_wild, meets_msrv, msrvs, path_to_local_id, peel_blocks, strip_pat_refs};
75
use core::iter::once;
86
use if_chain::if_chain;
97
use rustc_errors::Applicability;
10-
use rustc_hir::{Arm, BorrowKind, Expr, ExprKind, Local, MatchSource, Mutability, Node, Pat, PatKind, QPath};
8+
use rustc_hir::{Arm, BorrowKind, Expr, ExprKind, Local, MatchSource, Mutability, Pat, PatKind, QPath};
119
use rustc_lint::{LateContext, LateLintPass};
1210
use rustc_middle::ty;
1311
use rustc_semver::RustcVersion;
@@ -17,6 +15,7 @@ mod match_as_ref;
1715
mod match_bool;
1816
mod match_like_matches;
1917
mod match_same_arms;
18+
mod match_single_binding;
2019
mod match_wild_enum;
2120
mod match_wild_err_arm;
2221
mod overlapping_arms;
@@ -630,7 +629,7 @@ impl<'tcx> LateLintPass<'tcx> for Matches {
630629
if self.infallible_destructuring_match_linted {
631630
self.infallible_destructuring_match_linted = false;
632631
} else {
633-
check_match_single_binding(cx, ex, arms, expr);
632+
match_single_binding::check(cx, ex, arms, expr);
634633
}
635634
}
636635
if let ExprKind::Match(ex, arms, _) = expr.kind {
@@ -753,163 +752,6 @@ fn check_wild_in_or_pats(cx: &LateContext<'_>, arms: &[Arm<'_>]) {
753752
}
754753
}
755754

756-
#[allow(clippy::too_many_lines)]
757-
fn check_match_single_binding<'a>(cx: &LateContext<'a>, ex: &Expr<'a>, arms: &[Arm<'_>], expr: &Expr<'_>) {
758-
if expr.span.from_expansion() || arms.len() != 1 || is_refutable(cx, arms[0].pat) {
759-
return;
760-
}
761-
762-
// HACK:
763-
// This is a hack to deal with arms that are excluded by macros like `#[cfg]`. It is only used here
764-
// to prevent false positives as there is currently no better way to detect if code was excluded by
765-
// a macro. See PR #6435
766-
if_chain! {
767-
if let Some(match_snippet) = snippet_opt(cx, expr.span);
768-
if let Some(arm_snippet) = snippet_opt(cx, arms[0].span);
769-
if let Some(ex_snippet) = snippet_opt(cx, ex.span);
770-
let rest_snippet = match_snippet.replace(&arm_snippet, "").replace(&ex_snippet, "");
771-
if rest_snippet.contains("=>");
772-
then {
773-
// The code it self contains another thick arrow "=>"
774-
// -> Either another arm or a comment
775-
return;
776-
}
777-
}
778-
779-
let matched_vars = ex.span;
780-
let bind_names = arms[0].pat.span;
781-
let match_body = peel_blocks(arms[0].body);
782-
let mut snippet_body = if match_body.span.from_expansion() {
783-
Sugg::hir_with_macro_callsite(cx, match_body, "..").to_string()
784-
} else {
785-
snippet_block(cx, match_body.span, "..", Some(expr.span)).to_string()
786-
};
787-
788-
// Do we need to add ';' to suggestion ?
789-
match match_body.kind {
790-
ExprKind::Block(block, _) => {
791-
// macro + expr_ty(body) == ()
792-
if block.span.from_expansion() && cx.typeck_results().expr_ty(match_body).is_unit() {
793-
snippet_body.push(';');
794-
}
795-
},
796-
_ => {
797-
// expr_ty(body) == ()
798-
if cx.typeck_results().expr_ty(match_body).is_unit() {
799-
snippet_body.push(';');
800-
}
801-
},
802-
}
803-
804-
let mut applicability = Applicability::MaybeIncorrect;
805-
match arms[0].pat.kind {
806-
PatKind::Binding(..) | PatKind::Tuple(_, _) | PatKind::Struct(..) => {
807-
// If this match is in a local (`let`) stmt
808-
let (target_span, sugg) = if let Some(parent_let_node) = opt_parent_let(cx, ex) {
809-
(
810-
parent_let_node.span,
811-
format!(
812-
"let {} = {};\n{}let {} = {};",
813-
snippet_with_applicability(cx, bind_names, "..", &mut applicability),
814-
snippet_with_applicability(cx, matched_vars, "..", &mut applicability),
815-
" ".repeat(indent_of(cx, expr.span).unwrap_or(0)),
816-
snippet_with_applicability(cx, parent_let_node.pat.span, "..", &mut applicability),
817-
snippet_body
818-
),
819-
)
820-
} else {
821-
// If we are in closure, we need curly braces around suggestion
822-
let mut indent = " ".repeat(indent_of(cx, ex.span).unwrap_or(0));
823-
let (mut cbrace_start, mut cbrace_end) = ("".to_string(), "".to_string());
824-
if let Some(parent_expr) = get_parent_expr(cx, expr) {
825-
if let ExprKind::Closure(..) = parent_expr.kind {
826-
cbrace_end = format!("\n{}}}", indent);
827-
// Fix body indent due to the closure
828-
indent = " ".repeat(indent_of(cx, bind_names).unwrap_or(0));
829-
cbrace_start = format!("{{\n{}", indent);
830-
}
831-
}
832-
// If the parent is already an arm, and the body is another match statement,
833-
// we need curly braces around suggestion
834-
let parent_node_id = cx.tcx.hir().get_parent_node(expr.hir_id);
835-
if let Node::Arm(arm) = &cx.tcx.hir().get(parent_node_id) {
836-
if let ExprKind::Match(..) = arm.body.kind {
837-
cbrace_end = format!("\n{}}}", indent);
838-
// Fix body indent due to the match
839-
indent = " ".repeat(indent_of(cx, bind_names).unwrap_or(0));
840-
cbrace_start = format!("{{\n{}", indent);
841-
}
842-
}
843-
(
844-
expr.span,
845-
format!(
846-
"{}let {} = {};\n{}{}{}",
847-
cbrace_start,
848-
snippet_with_applicability(cx, bind_names, "..", &mut applicability),
849-
snippet_with_applicability(cx, matched_vars, "..", &mut applicability),
850-
indent,
851-
snippet_body,
852-
cbrace_end
853-
),
854-
)
855-
};
856-
span_lint_and_sugg(
857-
cx,
858-
MATCH_SINGLE_BINDING,
859-
target_span,
860-
"this match could be written as a `let` statement",
861-
"consider using `let` statement",
862-
sugg,
863-
applicability,
864-
);
865-
},
866-
PatKind::Wild => {
867-
if ex.can_have_side_effects() {
868-
let indent = " ".repeat(indent_of(cx, expr.span).unwrap_or(0));
869-
let sugg = format!(
870-
"{};\n{}{}",
871-
snippet_with_applicability(cx, ex.span, "..", &mut applicability),
872-
indent,
873-
snippet_body
874-
);
875-
span_lint_and_sugg(
876-
cx,
877-
MATCH_SINGLE_BINDING,
878-
expr.span,
879-
"this match could be replaced by its scrutinee and body",
880-
"consider using the scrutinee and body instead",
881-
sugg,
882-
applicability,
883-
);
884-
} else {
885-
span_lint_and_sugg(
886-
cx,
887-
MATCH_SINGLE_BINDING,
888-
expr.span,
889-
"this match could be replaced by its body itself",
890-
"consider using the match body instead",
891-
snippet_body,
892-
Applicability::MachineApplicable,
893-
);
894-
}
895-
},
896-
_ => (),
897-
}
898-
}
899-
900-
/// Returns true if the `ex` match expression is in a local (`let`) statement
901-
fn opt_parent_let<'a>(cx: &LateContext<'a>, ex: &Expr<'a>) -> Option<&'a Local<'a>> {
902-
let map = &cx.tcx.hir();
903-
if_chain! {
904-
if let Some(Node::Expr(parent_arm_expr)) = map.find(map.get_parent_node(ex.hir_id));
905-
if let Some(Node::Local(parent_let_expr)) = map.find(map.get_parent_node(parent_arm_expr.hir_id));
906-
then {
907-
return Some(parent_let_expr);
908-
}
909-
}
910-
None
911-
}
912-
913755
fn has_multiple_ref_pats<'a, 'b, I>(pats: I) -> bool
914756
where
915757
'b: 'a,

0 commit comments

Comments
 (0)