Skip to content

Commit 22b3196

Browse files
borsbors
committed
Auto merge of rust-lang#10753 - disco07:master, r=xFrednet
redundant_pattern_matching This PR try to solve this issue rust-lang/rust-clippy#10726, but it enter in conflict with another test. changelog: none Try to test this: ``` let _w = match x { Some(_) => true, _ => false, }; ``` this happen: ``` error: match expression looks like `matches!` macro --> $DIR/match_expr_like_matches_macro.rs:21:14 | LL | let _w = match x { | ______________^ LL | | Some(_) => true, LL | | _ => false, LL | | }; | |_____^ help: try this: `matches!(x, Some(_))` +error: redundant pattern matching, consider using `is_some()` + --> $DIR/match_expr_like_matches_macro.rs:21:14 + | +LL | let _w = match x { + | ______________^ +LL | | Some(_) => true, +LL | | _ => false, +LL | | }; + | |_____^ help: try this: `x.is_some()` + | + = note: `-D clippy::redundant-pattern-matching` implied by `-D warnings` + ``` I need some help to fix this. Thanks
2 parents 270cbeb + 79a8867 commit 22b3196

10 files changed

+385
-98
lines changed

‎clippy_lints/src/matches/match_like_matches.rs‎

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,12 @@
1+
use super::REDUNDANT_PATTERN_MATCHING;
12
use clippy_utils::diagnostics::span_lint_and_sugg;
3+
use clippy_utils::is_lint_allowed;
24
use clippy_utils::is_wild;
35
use clippy_utils::source::snippet_with_applicability;
46
use clippy_utils::span_contains_comment;
57
use rustc_ast::{Attribute, LitKind};
68
use rustc_errors::Applicability;
7-
use rustc_hir::{Arm, BorrowKind, Expr, ExprKind, Guard, Pat};
9+
use rustc_hir::{Arm, BorrowKind, Expr, ExprKind, Guard, Pat, PatKind, QPath};
810
use rustc_lint::{LateContext, LintContext};
911
use rustc_middle::ty;
1012
use rustc_span::source_map::Spanned;
@@ -99,6 +101,14 @@ where
99101
}
100102
}
101103

104+
for arm in iter_without_last.clone() {
105+
if let Some(pat) = arm.1 {
106+
if !is_lint_allowed(cx, REDUNDANT_PATTERN_MATCHING, pat.hir_id) && is_some(pat.kind) {
107+
return false;
108+
}
109+
}
110+
}
111+
102112
// The suggestion may be incorrect, because some arms can have `cfg` attributes
103113
// evaluated into `false` and so such arms will be stripped before.
104114
let mut applicability = Applicability::MaybeIncorrect;
@@ -170,3 +180,13 @@ fn find_bool_lit(ex: &ExprKind<'_>) -> Option<bool> {
170180
_ => None,
171181
}
172182
}
183+
184+
fn is_some(path_kind: PatKind<'_>) -> bool {
185+
match path_kind {
186+
PatKind::TupleStruct(QPath::Resolved(_, path), [first, ..], _) if is_wild(first) => {
187+
let name = path.segments[0].ident;
188+
name.name == rustc_span::sym::Some
189+
},
190+
_ => false,
191+
}
192+
}

‎clippy_lints/src/matches/redundant_pattern_match.rs‎

Lines changed: 148 additions & 67 deletions
Original file line numberDiff line numberDiff line change
@@ -189,73 +189,7 @@ pub(super) fn check_match<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, op
189189
if arms.len() == 2 {
190190
let node_pair = (&arms[0].pat.kind, &arms[1].pat.kind);
191191

192-
let found_good_method = match node_pair {
193-
(
194-
PatKind::TupleStruct(ref path_left, patterns_left, _),
195-
PatKind::TupleStruct(ref path_right, patterns_right, _),
196-
) if patterns_left.len() == 1 && patterns_right.len() == 1 => {
197-
if let (PatKind::Wild, PatKind::Wild) = (&patterns_left[0].kind, &patterns_right[0].kind) {
198-
find_good_method_for_match(
199-
cx,
200-
arms,
201-
path_left,
202-
path_right,
203-
Item::Lang(ResultOk),
204-
Item::Lang(ResultErr),
205-
"is_ok()",
206-
"is_err()",
207-
)
208-
.or_else(|| {
209-
find_good_method_for_match(
210-
cx,
211-
arms,
212-
path_left,
213-
path_right,
214-
Item::Diag(sym::IpAddr, sym!(V4)),
215-
Item::Diag(sym::IpAddr, sym!(V6)),
216-
"is_ipv4()",
217-
"is_ipv6()",
218-
)
219-
})
220-
} else {
221-
None
222-
}
223-
},
224-
(PatKind::TupleStruct(ref path_left, patterns, _), PatKind::Path(ref path_right))
225-
| (PatKind::Path(ref path_left), PatKind::TupleStruct(ref path_right, patterns, _))
226-
if patterns.len() == 1 =>
227-
{
228-
if let PatKind::Wild = patterns[0].kind {
229-
find_good_method_for_match(
230-
cx,
231-
arms,
232-
path_left,
233-
path_right,
234-
Item::Lang(OptionSome),
235-
Item::Lang(OptionNone),
236-
"is_some()",
237-
"is_none()",
238-
)
239-
.or_else(|| {
240-
find_good_method_for_match(
241-
cx,
242-
arms,
243-
path_left,
244-
path_right,
245-
Item::Lang(PollReady),
246-
Item::Lang(PollPending),
247-
"is_ready()",
248-
"is_pending()",
249-
)
250-
})
251-
} else {
252-
None
253-
}
254-
},
255-
_ => None,
256-
};
257-
258-
if let Some(good_method) = found_good_method {
192+
if let Some(good_method) = found_good_method(cx, arms, node_pair) {
259193
let span = expr.span.to(op.span);
260194
let result_expr = match &op.kind {
261195
ExprKind::AddrOf(_, _, borrowed) => borrowed,
@@ -279,6 +213,127 @@ pub(super) fn check_match<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, op
279213
}
280214
}
281215

216+
fn found_good_method<'a>(
217+
cx: &LateContext<'_>,
218+
arms: &[Arm<'_>],
219+
node: (&PatKind<'_>, &PatKind<'_>),
220+
) -> Option<&'a str> {
221+
match node {
222+
(
223+
PatKind::TupleStruct(ref path_left, patterns_left, _),
224+
PatKind::TupleStruct(ref path_right, patterns_right, _),
225+
) if patterns_left.len() == 1 && patterns_right.len() == 1 => {
226+
if let (PatKind::Wild, PatKind::Wild) = (&patterns_left[0].kind, &patterns_right[0].kind) {
227+
find_good_method_for_match(
228+
cx,
229+
arms,
230+
path_left,
231+
path_right,
232+
Item::Lang(ResultOk),
233+
Item::Lang(ResultErr),
234+
"is_ok()",
235+
"is_err()",
236+
)
237+
.or_else(|| {
238+
find_good_method_for_match(
239+
cx,
240+
arms,
241+
path_left,
242+
path_right,
243+
Item::Diag(sym::IpAddr, sym!(V4)),
244+
Item::Diag(sym::IpAddr, sym!(V6)),
245+
"is_ipv4()",
246+
"is_ipv6()",
247+
)
248+
})
249+
} else {
250+
None
251+
}
252+
},
253+
(PatKind::TupleStruct(ref path_left, patterns, _), PatKind::Path(ref path_right))
254+
| (PatKind::Path(ref path_left), PatKind::TupleStruct(ref path_right, patterns, _))
255+
if patterns.len() == 1 =>
256+
{
257+
if let PatKind::Wild = patterns[0].kind {
258+
find_good_method_for_match(
259+
cx,
260+
arms,
261+
path_left,
262+
path_right,
263+
Item::Lang(OptionSome),
264+
Item::Lang(OptionNone),
265+
"is_some()",
266+
"is_none()",
267+
)
268+
.or_else(|| {
269+
find_good_method_for_match(
270+
cx,
271+
arms,
272+
path_left,
273+
path_right,
274+
Item::Lang(PollReady),
275+
Item::Lang(PollPending),
276+
"is_ready()",
277+
"is_pending()",
278+
)
279+
})
280+
} else {
281+
None
282+
}
283+
},
284+
(PatKind::TupleStruct(ref path_left, patterns, _), PatKind::Wild) if patterns.len() == 1 => {
285+
if let PatKind::Wild = patterns[0].kind {
286+
get_good_method(cx, arms, path_left)
287+
} else {
288+
None
289+
}
290+
},
291+
(PatKind::Path(ref path_left), PatKind::Wild) => get_good_method(cx, arms, path_left),
292+
_ => None,
293+
}
294+
}
295+
296+
fn get_ident(path: &QPath<'_>) -> Option<rustc_span::symbol::Ident> {
297+
match path {
298+
QPath::Resolved(_, path) => {
299+
let name = path.segments[0].ident;
300+
Some(name)
301+
},
302+
_ => None,
303+
}
304+
}
305+
306+
fn get_good_method<'a>(cx: &LateContext<'_>, arms: &[Arm<'_>], path_left: &QPath<'_>) -> Option<&'a str> {
307+
if let Some(name) = get_ident(path_left) {
308+
return match name.as_str() {
309+
"Ok" => {
310+
find_good_method_for_matches_macro(cx, arms, path_left, Item::Lang(ResultOk), "is_ok()", "is_err()")
311+
},
312+
"Err" => {
313+
find_good_method_for_matches_macro(cx, arms, path_left, Item::Lang(ResultErr), "is_err()", "is_ok()")
314+
},
315+
"Some" => find_good_method_for_matches_macro(
316+
cx,
317+
arms,
318+
path_left,
319+
Item::Lang(OptionSome),
320+
"is_some()",
321+
"is_none()",
322+
),
323+
"None" => find_good_method_for_matches_macro(
324+
cx,
325+
arms,
326+
path_left,
327+
Item::Lang(OptionNone),
328+
"is_none()",
329+
"is_some()",
330+
),
331+
_ => None,
332+
};
333+
}
334+
None
335+
}
336+
282337
#[derive(Clone, Copy)]
283338
enum Item {
284339
Lang(LangItem),
@@ -345,3 +400,29 @@ fn find_good_method_for_match<'a>(
345400
_ => None,
346401
}
347402
}
403+
404+
fn find_good_method_for_matches_macro<'a>(
405+
cx: &LateContext<'_>,
406+
arms: &[Arm<'_>],
407+
path_left: &QPath<'_>,
408+
expected_item_left: Item,
409+
should_be_left: &'a str,
410+
should_be_right: &'a str,
411+
) -> Option<&'a str> {
412+
let first_pat = arms[0].pat;
413+
414+
let body_node_pair = if is_pat_variant(cx, first_pat, path_left, expected_item_left) {
415+
(&arms[0].body.kind, &arms[1].body.kind)
416+
} else {
417+
return None;
418+
};
419+
420+
match body_node_pair {
421+
(ExprKind::Lit(lit_left), ExprKind::Lit(lit_right)) => match (&lit_left.node, &lit_right.node) {
422+
(LitKind::Bool(true), LitKind::Bool(false)) => Some(should_be_left),
423+
(LitKind::Bool(false), LitKind::Bool(true)) => Some(should_be_right),
424+
_ => None,
425+
},
426+
_ => None,
427+
}
428+
}

‎tests/ui/match_expr_like_matches_macro.fixed‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ fn main() {
1515
let _y = matches!(x, Some(0));
1616

1717
// Lint
18-
let _w = matches!(x, Some(_));
18+
let _w = x.is_some();
1919

2020
// Turn into is_none
2121
let _z = x.is_none();

‎tests/ui/match_expr_like_matches_macro.stderr‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10,15 +10,17 @@ LL | | };
1010
|
1111
= note: `-D clippy::match-like-matches-macro` implied by `-D warnings`
1212

13-
error: match expression looks like `matches!` macro
13+
error: redundant pattern matching, consider using `is_some()`
1414
--> $DIR/match_expr_like_matches_macro.rs:21:14
1515
|
1616
LL | let _w = match x {
1717
| ______________^
1818
LL | | Some(_) => true,
1919
LL | | _ => false,
2020
LL | | };
21-
| |_____^ help: try this: `matches!(x, Some(_))`
21+
| |_____^ help: try this: `x.is_some()`
22+
|
23+
= note: `-D clippy::redundant-pattern-matching` implied by `-D warnings`
2224

2325
error: redundant pattern matching, consider using `is_none()`
2426
--> $DIR/match_expr_like_matches_macro.rs:27:14
@@ -29,8 +31,6 @@ LL | | Some(_) => false,
2931
LL | | None => true,
3032
LL | | };
3133
| |_____^ help: try this: `x.is_none()`
32-
|
33-
= note: `-D clippy::redundant-pattern-matching` implied by `-D warnings`
3434

3535
error: match expression looks like `matches!` macro
3636
--> $DIR/match_expr_like_matches_macro.rs:33:15

‎tests/ui/redundant_pattern_matching_option.fixed‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@ fn main() {
4646
let _ = if opt.is_some() { true } else { false };
4747

4848
issue6067();
49+
issue10726();
4950

5051
let _ = if gen_opt().is_some() {
5152
1
@@ -88,3 +89,21 @@ fn issue7921() {
8889
if (&None::<()>).is_none() {}
8990
if (&None::<()>).is_none() {}
9091
}
92+
93+
fn issue10726() {
94+
let x = Some(42);
95+
96+
x.is_some();
97+
98+
x.is_none();
99+
100+
x.is_none();
101+
102+
x.is_some();
103+
104+
// Don't lint
105+
match x {
106+
Some(21) => true,
107+
_ => false,
108+
};
109+
}

‎tests/ui/redundant_pattern_matching_option.rs‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,7 @@ fn main() {
5555
let _ = if let Some(_) = opt { true } else { false };
5656

5757
issue6067();
58+
issue10726();
5859

5960
let _ = if let Some(_) = gen_opt() {
6061
1
@@ -103,3 +104,33 @@ fn issue7921() {
103104
if let None = *(&None::<()>) {}
104105
if let None = *&None::<()> {}
105106
}
107+
108+
fn issue10726() {
109+
let x = Some(42);
110+
111+
match x {
112+
Some(_) => true,
113+
_ => false,
114+
};
115+
116+
match x {
117+
None => true,
118+
_ => false,
119+
};
120+
121+
match x {
122+
Some(_) => false,
123+
_ => true,
124+
};
125+
126+
match x {
127+
None => false,
128+
_ => true,
129+
};
130+
131+
// Don't lint
132+
match x {
133+
Some(21) => true,
134+
_ => false,
135+
};
136+
}

0 commit comments

Comments
 (0)