Skip to content

Commit 1c0e4e5

Browse files
borsbors
committed
Auto merge of rust-lang#5483 - alex-700:fix-redundant-pattern-matching, r=flip1995
fix redundant_pattern_matching lint - now it handles `while let` case (related to rust-lang#5462) - better suggestions in `if let` case changelog: Fix suggestion in `redundant_pattern_matching` and also apply this lint to the `while let` case
2 parents f1fb815 + 092c459 commit 1c0e4e5

File tree

4 files changed

+153
-51
lines changed

4 files changed

+153
-51
lines changed

‎clippy_lints/src/redundant_pattern_matching.rs

+21-9
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
1-
use crate::utils::{match_qpath, paths, snippet, span_lint_and_then};
1+
use crate::utils::{match_qpath, match_trait_method, paths, snippet, span_lint_and_then};
2+
use if_chain::if_chain;
23
use rustc_ast::ast::LitKind;
34
use rustc_errors::Applicability;
45
use rustc_hir::{Arm, Expr, ExprKind, MatchSource, PatKind, QPath};
@@ -48,9 +49,8 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for RedundantPatternMatching {
4849
if let ExprKind::Match(op, arms, ref match_source) = &expr.kind {
4950
match match_source {
5051
MatchSource::Normal => find_sugg_for_match(cx, expr, op, arms),
51-
MatchSource::IfLetDesugar { contains_else_clause } => {
52-
find_sugg_for_if_let(cx, expr, op, arms, *contains_else_clause)
53-
},
52+
MatchSource::IfLetDesugar { .. } => find_sugg_for_if_let(cx, expr, op, arms, "if"),
53+
MatchSource::WhileLetDesugar => find_sugg_for_if_let(cx, expr, op, arms, "while"),
5454
_ => return,
5555
}
5656
}
@@ -62,7 +62,7 @@ fn find_sugg_for_if_let<'a, 'tcx>(
6262
expr: &'tcx Expr<'_>,
6363
op: &Expr<'_>,
6464
arms: &[Arm<'_>],
65-
has_else: bool,
65+
keyword: &'static str,
6666
) {
6767
let good_method = match arms[0].pat.kind {
6868
PatKind::TupleStruct(ref path, ref patterns, _) if patterns.len() == 1 => {
@@ -86,20 +86,32 @@ fn find_sugg_for_if_let<'a, 'tcx>(
8686
_ => return,
8787
};
8888

89-
let maybe_semi = if has_else { "" } else { ";" };
89+
// check that `while_let_on_iterator` lint does not trigger
90+
if_chain! {
91+
if keyword == "while";
92+
if let ExprKind::MethodCall(method_path, _, _) = op.kind;
93+
if method_path.ident.name == sym!(next);
94+
if match_trait_method(cx, op, &paths::ITERATOR);
95+
then {
96+
return;
97+
}
98+
}
9099

91100
span_lint_and_then(
92101
cx,
93102
REDUNDANT_PATTERN_MATCHING,
94103
arms[0].pat.span,
95104
&format!("redundant pattern matching, consider using `{}`", good_method),
96105
|diag| {
97-
let span = expr.span.to(op.span);
106+
// in the case of WhileLetDesugar expr.span == op.span incorrectly.
107+
// this is a workaround to restore true value of expr.span
108+
let expr_span = expr.span.to(arms[1].span);
109+
let span = expr_span.until(op.span.shrink_to_hi());
98110
diag.span_suggestion(
99111
span,
100112
"try this",
101-
format!("{}.{}{}", snippet(cx, op.span, "_"), good_method, maybe_semi),
102-
Applicability::MaybeIncorrect, // snippet
113+
format!("{} {}.{}", keyword, snippet(cx, op.span, "_"), good_method),
114+
Applicability::MachineApplicable, // snippet
103115
);
104116
},
105117
);

‎tests/ui/redundant_pattern_matching.fixed

+42-9
Original file line numberDiff line numberDiff line change
@@ -2,16 +2,37 @@
22

33
#![warn(clippy::all)]
44
#![warn(clippy::redundant_pattern_matching)]
5-
#![allow(clippy::unit_arg, unused_must_use)]
5+
#![allow(clippy::unit_arg, unused_must_use, clippy::needless_bool)]
66

77
fn main() {
8-
Ok::<i32, i32>(42).is_ok();
8+
if Ok::<i32, i32>(42).is_ok() {}
99

10-
Err::<i32, i32>(42).is_err();
10+
if Err::<i32, i32>(42).is_err() {}
1111

12-
None::<()>.is_none();
12+
if None::<()>.is_none() {}
1313

14-
Some(42).is_some();
14+
if Some(42).is_some() {}
15+
16+
if Some(42).is_some() {
17+
foo();
18+
} else {
19+
bar();
20+
}
21+
22+
while Some(42).is_some() {}
23+
24+
while Some(42).is_none() {}
25+
26+
while None::<()>.is_none() {}
27+
28+
while Ok::<i32, i32>(10).is_ok() {}
29+
30+
while Ok::<i32, i32>(10).is_err() {}
31+
32+
let mut v = vec![1, 2, 3];
33+
while v.pop().is_some() {
34+
foo();
35+
}
1536

1637
if Ok::<i32, i32>(42).is_ok() {}
1738

@@ -39,22 +60,34 @@ fn main() {
3960

4061
let _ = None::<()>.is_none();
4162

42-
let _ = Ok::<usize, ()>(4).is_ok();
63+
let _ = if Ok::<usize, ()>(4).is_ok() { true } else { false };
4364

4465
let _ = does_something();
4566
let _ = returns_unit();
4667

4768
let opt = Some(false);
48-
let x = opt.is_some();
69+
let x = if opt.is_some() { true } else { false };
4970
takes_bool(x);
5071
}
5172

5273
fn takes_bool(_: bool) {}
5374

75+
fn foo() {}
76+
77+
fn bar() {}
78+
5479
fn does_something() -> bool {
55-
Ok::<i32, i32>(4).is_ok()
80+
if Ok::<i32, i32>(4).is_ok() {
81+
true
82+
} else {
83+
false
84+
}
5685
}
5786

5887
fn returns_unit() {
59-
Ok::<i32, i32>(4).is_ok();
88+
if Ok::<i32, i32>(4).is_ok() {
89+
true
90+
} else {
91+
false
92+
};
6093
}

‎tests/ui/redundant_pattern_matching.rs

+26-1
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22

33
#![warn(clippy::all)]
44
#![warn(clippy::redundant_pattern_matching)]
5-
#![allow(clippy::unit_arg, unused_must_use)]
5+
#![allow(clippy::unit_arg, unused_must_use, clippy::needless_bool)]
66

77
fn main() {
88
if let Ok(_) = Ok::<i32, i32>(42) {}
@@ -13,6 +13,27 @@ fn main() {
1313

1414
if let Some(_) = Some(42) {}
1515

16+
if let Some(_) = Some(42) {
17+
foo();
18+
} else {
19+
bar();
20+
}
21+
22+
while let Some(_) = Some(42) {}
23+
24+
while let None = Some(42) {}
25+
26+
while let None = None::<()> {}
27+
28+
while let Ok(_) = Ok::<i32, i32>(10) {}
29+
30+
while let Err(_) = Ok::<i32, i32>(10) {}
31+
32+
let mut v = vec![1, 2, 3];
33+
while let Some(_) = v.pop() {
34+
foo();
35+
}
36+
1637
if Ok::<i32, i32>(42).is_ok() {}
1738

1839
if Err::<i32, i32>(42).is_err() {}
@@ -72,6 +93,10 @@ fn main() {
7293

7394
fn takes_bool(_: bool) {}
7495

96+
fn foo() {}
97+
98+
fn bar() {}
99+
75100
fn does_something() -> bool {
76101
if let Ok(_) = Ok::<i32, i32>(4) {
77102
true

‎tests/ui/redundant_pattern_matching.stderr

+64-32
Original file line numberDiff line numberDiff line change
@@ -2,30 +2,72 @@ error: redundant pattern matching, consider using `is_ok()`
22
--> $DIR/redundant_pattern_matching.rs:8:12
33
|
44
LL | if let Ok(_) = Ok::<i32, i32>(42) {}
5-
| -------^^^^^------------------------ help: try this: `Ok::<i32, i32>(42).is_ok();`
5+
| -------^^^^^--------------------- help: try this: `if Ok::<i32, i32>(42).is_ok()`
66
|
77
= note: `-D clippy::redundant-pattern-matching` implied by `-D warnings`
88

99
error: redundant pattern matching, consider using `is_err()`
1010
--> $DIR/redundant_pattern_matching.rs:10:12
1111
|
1212
LL | if let Err(_) = Err::<i32, i32>(42) {}
13-
| -------^^^^^^------------------------- help: try this: `Err::<i32, i32>(42).is_err();`
13+
| -------^^^^^^---------------------- help: try this: `if Err::<i32, i32>(42).is_err()`
1414

1515
error: redundant pattern matching, consider using `is_none()`
1616
--> $DIR/redundant_pattern_matching.rs:12:12
1717
|
1818
LL | if let None = None::<()> {}
19-
| -------^^^^---------------- help: try this: `None::<()>.is_none();`
19+
| -------^^^^------------- help: try this: `if None::<()>.is_none()`
2020

2121
error: redundant pattern matching, consider using `is_some()`
2222
--> $DIR/redundant_pattern_matching.rs:14:12
2323
|
2424
LL | if let Some(_) = Some(42) {}
25-
| -------^^^^^^^-------------- help: try this: `Some(42).is_some();`
25+
| -------^^^^^^^----------- help: try this: `if Some(42).is_some()`
26+
27+
error: redundant pattern matching, consider using `is_some()`
28+
--> $DIR/redundant_pattern_matching.rs:16:12
29+
|
30+
LL | if let Some(_) = Some(42) {
31+
| -------^^^^^^^----------- help: try this: `if Some(42).is_some()`
32+
33+
error: redundant pattern matching, consider using `is_some()`
34+
--> $DIR/redundant_pattern_matching.rs:22:15
35+
|
36+
LL | while let Some(_) = Some(42) {}
37+
| ----------^^^^^^^----------- help: try this: `while Some(42).is_some()`
38+
39+
error: redundant pattern matching, consider using `is_none()`
40+
--> $DIR/redundant_pattern_matching.rs:24:15
41+
|
42+
LL | while let None = Some(42) {}
43+
| ----------^^^^----------- help: try this: `while Some(42).is_none()`
44+
45+
error: redundant pattern matching, consider using `is_none()`
46+
--> $DIR/redundant_pattern_matching.rs:26:15
47+
|
48+
LL | while let None = None::<()> {}
49+
| ----------^^^^------------- help: try this: `while None::<()>.is_none()`
2650

2751
error: redundant pattern matching, consider using `is_ok()`
28-
--> $DIR/redundant_pattern_matching.rs:28:5
52+
--> $DIR/redundant_pattern_matching.rs:28:15
53+
|
54+
LL | while let Ok(_) = Ok::<i32, i32>(10) {}
55+
| ----------^^^^^--------------------- help: try this: `while Ok::<i32, i32>(10).is_ok()`
56+
57+
error: redundant pattern matching, consider using `is_err()`
58+
--> $DIR/redundant_pattern_matching.rs:30:15
59+
|
60+
LL | while let Err(_) = Ok::<i32, i32>(10) {}
61+
| ----------^^^^^^--------------------- help: try this: `while Ok::<i32, i32>(10).is_err()`
62+
63+
error: redundant pattern matching, consider using `is_some()`
64+
--> $DIR/redundant_pattern_matching.rs:33:15
65+
|
66+
LL | while let Some(_) = v.pop() {
67+
| ----------^^^^^^^---------- help: try this: `while v.pop().is_some()`
68+
69+
error: redundant pattern matching, consider using `is_ok()`
70+
--> $DIR/redundant_pattern_matching.rs:49:5
2971
|
3072
LL | / match Ok::<i32, i32>(42) {
3173
LL | | Ok(_) => true,
@@ -34,7 +76,7 @@ LL | | };
3476
| |_____^ help: try this: `Ok::<i32, i32>(42).is_ok()`
3577

3678
error: redundant pattern matching, consider using `is_err()`
37-
--> $DIR/redundant_pattern_matching.rs:33:5
79+
--> $DIR/redundant_pattern_matching.rs:54:5
3880
|
3981
LL | / match Ok::<i32, i32>(42) {
4082
LL | | Ok(_) => false,
@@ -43,7 +85,7 @@ LL | | };
4385
| |_____^ help: try this: `Ok::<i32, i32>(42).is_err()`
4486

4587
error: redundant pattern matching, consider using `is_err()`
46-
--> $DIR/redundant_pattern_matching.rs:38:5
88+
--> $DIR/redundant_pattern_matching.rs:59:5
4789
|
4890
LL | / match Err::<i32, i32>(42) {
4991
LL | | Ok(_) => false,
@@ -52,7 +94,7 @@ LL | | };
5294
| |_____^ help: try this: `Err::<i32, i32>(42).is_err()`
5395

5496
error: redundant pattern matching, consider using `is_ok()`
55-
--> $DIR/redundant_pattern_matching.rs:43:5
97+
--> $DIR/redundant_pattern_matching.rs:64:5
5698
|
5799
LL | / match Err::<i32, i32>(42) {
58100
LL | | Ok(_) => true,
@@ -61,7 +103,7 @@ LL | | };
61103
| |_____^ help: try this: `Err::<i32, i32>(42).is_ok()`
62104

63105
error: redundant pattern matching, consider using `is_some()`
64-
--> $DIR/redundant_pattern_matching.rs:48:5
106+
--> $DIR/redundant_pattern_matching.rs:69:5
65107
|
66108
LL | / match Some(42) {
67109
LL | | Some(_) => true,
@@ -70,7 +112,7 @@ LL | | };
70112
| |_____^ help: try this: `Some(42).is_some()`
71113

72114
error: redundant pattern matching, consider using `is_none()`
73-
--> $DIR/redundant_pattern_matching.rs:53:5
115+
--> $DIR/redundant_pattern_matching.rs:74:5
74116
|
75117
LL | / match None::<()> {
76118
LL | | Some(_) => false,
@@ -79,7 +121,7 @@ LL | | };
79121
| |_____^ help: try this: `None::<()>.is_none()`
80122

81123
error: redundant pattern matching, consider using `is_none()`
82-
--> $DIR/redundant_pattern_matching.rs:58:13
124+
--> $DIR/redundant_pattern_matching.rs:79:13
83125
|
84126
LL | let _ = match None::<()> {
85127
| _____________^
@@ -89,38 +131,28 @@ LL | | };
89131
| |_____^ help: try this: `None::<()>.is_none()`
90132

91133
error: redundant pattern matching, consider using `is_ok()`
92-
--> $DIR/redundant_pattern_matching.rs:63:20
134+
--> $DIR/redundant_pattern_matching.rs:84:20
93135
|
94136
LL | let _ = if let Ok(_) = Ok::<usize, ()>(4) { true } else { false };
95-
| -------^^^^^--------------------------------------------- help: try this: `Ok::<usize, ()>(4).is_ok()`
137+
| -------^^^^^--------------------- help: try this: `if Ok::<usize, ()>(4).is_ok()`
96138

97139
error: redundant pattern matching, consider using `is_some()`
98-
--> $DIR/redundant_pattern_matching.rs:69:20
140+
--> $DIR/redundant_pattern_matching.rs:90:20
99141
|
100142
LL | let x = if let Some(_) = opt { true } else { false };
101-
| -------^^^^^^^------------------------------ help: try this: `opt.is_some()`
143+
| -------^^^^^^^------ help: try this: `if opt.is_some()`
102144

103145
error: redundant pattern matching, consider using `is_ok()`
104-
--> $DIR/redundant_pattern_matching.rs:76:12
146+
--> $DIR/redundant_pattern_matching.rs:101:12
105147
|
106-
LL | if let Ok(_) = Ok::<i32, i32>(4) {
107-
| _____- ^^^^^
108-
LL | | true
109-
LL | | } else {
110-
LL | | false
111-
LL | | }
112-
| |_____- help: try this: `Ok::<i32, i32>(4).is_ok()`
148+
LL | if let Ok(_) = Ok::<i32, i32>(4) {
149+
| -------^^^^^-------------------- help: try this: `if Ok::<i32, i32>(4).is_ok()`
113150

114151
error: redundant pattern matching, consider using `is_ok()`
115-
--> $DIR/redundant_pattern_matching.rs:84:12
152+
--> $DIR/redundant_pattern_matching.rs:109:12
116153
|
117-
LL | if let Ok(_) = Ok::<i32, i32>(4) {
118-
| _____- ^^^^^
119-
LL | | true
120-
LL | | } else {
121-
LL | | false
122-
LL | | };
123-
| |_____- help: try this: `Ok::<i32, i32>(4).is_ok()`
154+
LL | if let Ok(_) = Ok::<i32, i32>(4) {
155+
| -------^^^^^-------------------- help: try this: `if Ok::<i32, i32>(4).is_ok()`
124156

125-
error: aborting due to 15 previous errors
157+
error: aborting due to 22 previous errors
126158

0 commit comments

Comments
 (0)