Skip to content

Commit 3b7c888

Browse files
CYBAICYBAI
committed
Add lint for redundant pattern matching for explicit return boolean
1 parent dae7abb commit 3b7c888

File tree

3 files changed

+241
-37
lines changed

3 files changed

+241
-37
lines changed

‎clippy_lints/src/if_let_redundant_pattern_matching.rs

Lines changed: 156 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,8 @@
1111
use crate::rustc::lint::{LateContext, LateLintPass, LintArray, LintPass};
1212
use crate::rustc::{declare_tool_lint, lint_array};
1313
use crate::rustc::hir::*;
14+
use crate::syntax::ptr::P;
15+
use crate::syntax::ast::LitKind;
1416
use crate::utils::{match_qpath, paths, snippet, span_lint_and_then};
1517
use crate::rustc_errors::Applicability;
1618

@@ -58,46 +60,164 @@ impl LintPass for Pass {
5860
impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Pass {
5961
#[allow(clippy::similar_names)]
6062
fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr) {
61-
if let ExprKind::Match(ref op, ref arms, MatchSource::IfLetDesugar { .. }) = expr.node {
62-
if arms[0].pats.len() == 1 {
63-
let good_method = match arms[0].pats[0].node {
64-
PatKind::TupleStruct(ref path, ref pats, _) if pats.len() == 1 => {
65-
if let PatKind::Wild = pats[0].node {
66-
if match_qpath(path, &paths::RESULT_OK) {
67-
"is_ok()"
68-
} else if match_qpath(path, &paths::RESULT_ERR) {
69-
"is_err()"
70-
} else if match_qpath(path, &paths::OPTION_SOME) {
71-
"is_some()"
72-
} else {
73-
return;
74-
}
75-
} else {
76-
return;
77-
}
78-
},
63+
if let ExprKind::Match(ref op, ref arms, ref match_source) = expr.node {
64+
match match_source {
65+
MatchSource::Normal => find_sugg_for_match(cx, expr, op, arms),
66+
MatchSource::IfLetDesugar { .. } => find_sugg_for_if_let(cx, expr, op, arms),
67+
_ => return,
68+
}
69+
}
70+
}
71+
}
7972

80-
PatKind::Path(ref path) if match_qpath(path, &paths::OPTION_NONE) => "is_none()",
73+
fn find_sugg_for_if_let<'a, 'tcx>(
74+
cx: &LateContext<'a, 'tcx>,
75+
expr: &'tcx Expr,
76+
op: &P<Expr>,
77+
arms: &HirVec<Arm>
78+
) {
79+
if arms[0].pats.len() == 1 {
80+
let good_method = match arms[0].pats[0].node {
81+
PatKind::TupleStruct(ref path, ref pats, _) if pats.len() == 1 => {
82+
if let PatKind::Wild = pats[0].node {
83+
if match_qpath(path, &paths::RESULT_OK) {
84+
"is_ok()"
85+
} else if match_qpath(path, &paths::RESULT_ERR) {
86+
"is_err()"
87+
} else if match_qpath(path, &paths::OPTION_SOME) {
88+
"is_some()"
89+
} else {
90+
return;
91+
}
92+
} else {
93+
return;
94+
}
95+
},
8196

82-
_ => return,
83-
};
97+
PatKind::Path(ref path) if match_qpath(path, &paths::OPTION_NONE) => "is_none()",
8498

85-
span_lint_and_then(
86-
cx,
87-
IF_LET_REDUNDANT_PATTERN_MATCHING,
88-
arms[0].pats[0].span,
89-
&format!("redundant pattern matching, consider using `{}`", good_method),
90-
|db| {
91-
let span = expr.span.to(op.span);
92-
db.span_suggestion_with_applicability(
93-
span,
94-
"try this",
95-
format!("if {}.{}", snippet(cx, op.span, "_"), good_method),
96-
Applicability::MachineApplicable, // snippet
97-
);
98-
},
99+
_ => return,
100+
};
101+
102+
span_lint_and_then(
103+
cx,
104+
IF_LET_REDUNDANT_PATTERN_MATCHING,
105+
arms[0].pats[0].span,
106+
&format!("redundant pattern matching, consider using `{}`", good_method),
107+
|db| {
108+
let span = expr.span.to(op.span);
109+
db.span_suggestion_with_applicability(
110+
span,
111+
"try this",
112+
format!("if {}.{}", snippet(cx, op.span, "_"), good_method),
113+
Applicability::MachineApplicable, // snippet
99114
);
100-
}
115+
},
116+
);
117+
} else {
118+
return;
119+
}
120+
}
121+
122+
fn find_sugg_for_match<'a, 'tcx>(
123+
cx: &LateContext<'a, 'tcx>,
124+
expr: &'tcx Expr,
125+
op: &P<Expr>,
126+
arms: &HirVec<Arm>
127+
) {
128+
if arms.len() == 2 {
129+
let node_pair = (&arms[0].pats[0].node, &arms[1].pats[0].node);
130+
131+
let found_good_method = match node_pair {
132+
(
133+
PatKind::TupleStruct(ref path_left, ref pats_left, _),
134+
PatKind::TupleStruct(ref path_right, ref pats_right, _)
135+
) if pats_left.len() == 1 && pats_right.len() == 1 => {
136+
if let (PatKind::Wild, PatKind::Wild) = (&pats_left[0].node, &pats_right[0].node) {
137+
find_good_method_for_match(
138+
arms,
139+
path_left,
140+
path_right,
141+
&paths::RESULT_OK,
142+
&paths::RESULT_ERR,
143+
"is_ok()",
144+
"is_err()"
145+
)
146+
} else {
147+
None
148+
}
149+
},
150+
(
151+
PatKind::TupleStruct(ref path_left, ref pats, _),
152+
PatKind::Path(ref path_right)
153+
) | (
154+
PatKind::Path(ref path_left),
155+
PatKind::TupleStruct(ref path_right, ref pats, _)
156+
) if pats.len() == 1 => {
157+
if let PatKind::Wild = pats[0].node {
158+
find_good_method_for_match(
159+
arms,
160+
path_left,
161+
path_right,
162+
&paths::OPTION_SOME,
163+
&paths::OPTION_NONE,
164+
"is_some()",
165+
"is_none()"
166+
)
167+
} else {
168+
None
169+
}
170+
},
171+
_ => None,
172+
};
173+
174+
if let Some(good_method) = found_good_method {
175+
span_lint_and_then(
176+
cx,
177+
IF_LET_REDUNDANT_PATTERN_MATCHING,
178+
expr.span,
179+
&format!("redundant pattern matching, consider using `{}`", good_method),
180+
|db| {
181+
let span = expr.span.to(op.span);
182+
db.span_suggestion_with_applicability(
183+
span,
184+
"try this",
185+
format!("{}.{}", snippet(cx, op.span, "_"), good_method),
186+
Applicability::MachineApplicable, // snippet
187+
);
188+
},
189+
);
101190
}
191+
} else {
192+
return;
193+
}
194+
}
195+
196+
fn find_good_method_for_match<'a>(
197+
arms: &HirVec<Arm>,
198+
path_left: &QPath,
199+
path_right: &QPath,
200+
expected_left: &[&str],
201+
expected_right: &[&str],
202+
should_be_left: &'a str,
203+
should_be_right: &'a str
204+
) -> Option<&'a str> {
205+
let body_node_pair = if match_qpath(path_left, expected_left) && match_qpath(path_right, expected_right) {
206+
(&(*arms[0].body).node, &(*arms[1].body).node)
207+
} else if match_qpath(path_right, expected_left) && match_qpath(path_left, expected_right) {
208+
(&(*arms[1].body).node, &(*arms[0].body).node)
209+
} else {
210+
return None;
211+
};
212+
213+
match body_node_pair {
214+
(ExprKind::Lit(ref lit_left), ExprKind::Lit(ref lit_right)) => {
215+
match (&lit_left.node, &lit_right.node) {
216+
(LitKind::Bool(true), LitKind::Bool(false)) => Some(should_be_left),
217+
(LitKind::Bool(false), LitKind::Bool(true)) => Some(should_be_right),
218+
_ => None,
219+
}
220+
},
221+
_ => None,
102222
}
103223
}

‎tests/ui/if_let_redundant_pattern_matching.rs

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,4 +42,34 @@ fn main() {
4242
if let Ok(x) = Ok::<i32,i32>(42) {
4343
println!("{}", x);
4444
}
45+
46+
match Ok::<i32, i32>(42) {
47+
Ok(_) => true,
48+
Err(_) => false,
49+
};
50+
51+
match Ok::<i32, i32>(42) {
52+
Ok(_) => false,
53+
Err(_) => true,
54+
};
55+
56+
match Err::<i32, i32>(42) {
57+
Ok(_) => false,
58+
Err(_) => true,
59+
};
60+
61+
match Err::<i32, i32>(42) {
62+
Ok(_) => true,
63+
Err(_) => false,
64+
};
65+
66+
match Some(42) {
67+
Some(_) => true,
68+
None => false,
69+
};
70+
71+
match None::<()> {
72+
Some(_) => false,
73+
None => true,
74+
};
4575
}

‎tests/ui/if_let_redundant_pattern_matching.stderr

Lines changed: 55 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,5 +30,59 @@ error: redundant pattern matching, consider using `is_some()`
3030
28 | | }
3131
| |_____- help: try this: `if Some(42).is_some()`
3232

33-
error: aborting due to 4 previous errors
33+
error: redundant pattern matching, consider using `is_ok()`
34+
--> $DIR/if_let_redundant_pattern_matching.rs:46:5
35+
|
36+
46 | / match Ok::<i32, i32>(42) {
37+
47 | | Ok(_) => true,
38+
48 | | Err(_) => false,
39+
49 | | };
40+
| |_____^ help: try this: `Ok::<i32, i32>(42).is_ok()`
41+
42+
error: redundant pattern matching, consider using `is_err()`
43+
--> $DIR/if_let_redundant_pattern_matching.rs:51:5
44+
|
45+
51 | / match Ok::<i32, i32>(42) {
46+
52 | | Ok(_) => false,
47+
53 | | Err(_) => true,
48+
54 | | };
49+
| |_____^ help: try this: `Ok::<i32, i32>(42).is_err()`
50+
51+
error: redundant pattern matching, consider using `is_err()`
52+
--> $DIR/if_let_redundant_pattern_matching.rs:56:5
53+
|
54+
56 | / match Err::<i32, i32>(42) {
55+
57 | | Ok(_) => false,
56+
58 | | Err(_) => true,
57+
59 | | };
58+
| |_____^ help: try this: `Err::<i32, i32>(42).is_err()`
59+
60+
error: redundant pattern matching, consider using `is_ok()`
61+
--> $DIR/if_let_redundant_pattern_matching.rs:61:5
62+
|
63+
61 | / match Err::<i32, i32>(42) {
64+
62 | | Ok(_) => true,
65+
63 | | Err(_) => false,
66+
64 | | };
67+
| |_____^ help: try this: `Err::<i32, i32>(42).is_ok()`
68+
69+
error: redundant pattern matching, consider using `is_some()`
70+
--> $DIR/if_let_redundant_pattern_matching.rs:66:5
71+
|
72+
66 | / match Some(42) {
73+
67 | | Some(_) => true,
74+
68 | | None => false,
75+
69 | | };
76+
| |_____^ help: try this: `Some(42).is_some()`
77+
78+
error: redundant pattern matching, consider using `is_none()`
79+
--> $DIR/if_let_redundant_pattern_matching.rs:71:5
80+
|
81+
71 | / match None::<()> {
82+
72 | | Some(_) => false,
83+
73 | | None => true,
84+
74 | | };
85+
| |_____^ help: try this: `None::<()>.is_none()`
86+
87+
error: aborting due to 10 previous errors
3488

0 commit comments

Comments
 (0)