Skip to content

Commit 6b256aa

Browse files
alex-700alex-700
committed
fix redundant_pattern_matching lint
- now it gives correct suggestion in case of macros - better tests - remove a couple of non-relevant tests
1 parent d01a498 commit 6b256aa

File tree

4 files changed

+142
-47
lines changed

4 files changed

+142
-47
lines changed

‎clippy_lints/src/redundant_pattern_matching.rs‎

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for RedundantPatternMatching {
5959

6060
fn find_sugg_for_if_let<'a, 'tcx>(
6161
cx: &LateContext<'a, 'tcx>,
62-
expr: &'tcx Expr<'_>,
62+
_expr: &'tcx Expr<'_>,
6363
op: &Expr<'_>,
6464
arms: &[Arm<'_>],
6565
keyword: &'static str,
@@ -103,14 +103,21 @@ fn find_sugg_for_if_let<'a, 'tcx>(
103103
arms[0].pat.span,
104104
&format!("redundant pattern matching, consider using `{}`", good_method),
105105
|diag| {
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());
106+
// while let ... = ... { ... }
107+
// ^^^^^^^^^^^^^^^^^^^^^^^^^^^
108+
let expr_span = arms[1].pat.span;
109+
110+
// while let ... = ... { ... }
111+
// ^^^
112+
let op_span = op.span.source_callsite();
113+
114+
// while let ... = ... { ... }
115+
// ^^^^^^^^^^^^^^^^^^^
116+
let span = expr_span.until(op_span.shrink_to_hi());
110117
diag.span_suggestion(
111118
span,
112119
"try this",
113-
format!("{} {}.{}", keyword, snippet(cx, op.span, "_"), good_method),
120+
format!("{} {}.{}", keyword, snippet(cx, op_span, "_"), good_method),
114121
Applicability::MachineApplicable, // snippet
115122
);
116123
},

‎tests/ui/redundant_pattern_matching.fixed‎

Lines changed: 42 additions & 16 deletions
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, clippy::needless_bool)]
5+
#![allow(clippy::unit_arg, unused_must_use, clippy::needless_bool, deprecated)]
66

77
fn main() {
88
if Ok::<i32, i32>(42).is_ok() {}
@@ -62,12 +62,31 @@ fn main() {
6262

6363
let _ = if Ok::<usize, ()>(4).is_ok() { true } else { false };
6464

65-
let _ = does_something();
66-
let _ = returns_unit();
67-
6865
let opt = Some(false);
6966
let x = if opt.is_some() { true } else { false };
7067
takes_bool(x);
68+
69+
issue5504();
70+
71+
let _ = if gen_opt().is_some() {
72+
1
73+
} else if gen_opt().is_none() {
74+
2
75+
} else if gen_res().is_ok() {
76+
3
77+
} else if gen_res().is_err() {
78+
4
79+
} else {
80+
5
81+
};
82+
}
83+
84+
fn gen_opt() -> Option<()> {
85+
None
86+
}
87+
88+
fn gen_res() -> Result<(), ()> {
89+
Ok(())
7190
}
7291

7392
fn takes_bool(_: bool) {}
@@ -76,18 +95,25 @@ fn foo() {}
7695

7796
fn bar() {}
7897

79-
fn does_something() -> bool {
80-
if Ok::<i32, i32>(4).is_ok() {
81-
true
82-
} else {
83-
false
84-
}
98+
macro_rules! m {
99+
() => {
100+
Some(42u32)
101+
};
85102
}
86103

87-
fn returns_unit() {
88-
if Ok::<i32, i32>(4).is_ok() {
89-
true
90-
} else {
91-
false
92-
};
104+
fn issue5504() {
105+
fn result_opt() -> Result<Option<i32>, i32> {
106+
Err(42)
107+
}
108+
109+
fn try_result_opt() -> Result<i32, i32> {
110+
while r#try!(result_opt()).is_some() {}
111+
if r#try!(result_opt()).is_some() {}
112+
Ok(42)
113+
}
114+
115+
try_result_opt();
116+
117+
if m!().is_some() {}
118+
while m!().is_some() {}
93119
}

‎tests/ui/redundant_pattern_matching.rs‎

Lines changed: 42 additions & 16 deletions
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, clippy::needless_bool)]
5+
#![allow(clippy::unit_arg, unused_must_use, clippy::needless_bool, deprecated)]
66

77
fn main() {
88
if let Ok(_) = Ok::<i32, i32>(42) {}
@@ -83,12 +83,31 @@ fn main() {
8383

8484
let _ = if let Ok(_) = Ok::<usize, ()>(4) { true } else { false };
8585

86-
let _ = does_something();
87-
let _ = returns_unit();
88-
8986
let opt = Some(false);
9087
let x = if let Some(_) = opt { true } else { false };
9188
takes_bool(x);
89+
90+
issue5504();
91+
92+
let _ = if let Some(_) = gen_opt() {
93+
1
94+
} else if let None = gen_opt() {
95+
2
96+
} else if let Ok(_) = gen_res() {
97+
3
98+
} else if let Err(_) = gen_res() {
99+
4
100+
} else {
101+
5
102+
};
103+
}
104+
105+
fn gen_opt() -> Option<()> {
106+
None
107+
}
108+
109+
fn gen_res() -> Result<(), ()> {
110+
Ok(())
92111
}
93112

94113
fn takes_bool(_: bool) {}
@@ -97,18 +116,25 @@ fn foo() {}
97116

98117
fn bar() {}
99118

100-
fn does_something() -> bool {
101-
if let Ok(_) = Ok::<i32, i32>(4) {
102-
true
103-
} else {
104-
false
105-
}
119+
macro_rules! m {
120+
() => {
121+
Some(42u32)
122+
};
106123
}
107124

108-
fn returns_unit() {
109-
if let Ok(_) = Ok::<i32, i32>(4) {
110-
true
111-
} else {
112-
false
113-
};
125+
fn issue5504() {
126+
fn result_opt() -> Result<Option<i32>, i32> {
127+
Err(42)
128+
}
129+
130+
fn try_result_opt() -> Result<i32, i32> {
131+
while let Some(_) = r#try!(result_opt()) {}
132+
if let Some(_) = r#try!(result_opt()) {}
133+
Ok(42)
134+
}
135+
136+
try_result_opt();
137+
138+
if let Some(_) = m!() {}
139+
while let Some(_) = m!() {}
114140
}

‎tests/ui/redundant_pattern_matching.stderr‎

Lines changed: 45 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -137,22 +137,58 @@ LL | let _ = if let Ok(_) = Ok::<usize, ()>(4) { true } else { false };
137137
| -------^^^^^--------------------- help: try this: `if Ok::<usize, ()>(4).is_ok()`
138138

139139
error: redundant pattern matching, consider using `is_some()`
140-
--> $DIR/redundant_pattern_matching.rs:90:20
140+
--> $DIR/redundant_pattern_matching.rs:87:20
141141
|
142142
LL | let x = if let Some(_) = opt { true } else { false };
143143
| -------^^^^^^^------ help: try this: `if opt.is_some()`
144144

145-
error: redundant pattern matching, consider using `is_ok()`
146-
--> $DIR/redundant_pattern_matching.rs:101:12
145+
error: redundant pattern matching, consider using `is_some()`
146+
--> $DIR/redundant_pattern_matching.rs:92:20
147+
|
148+
LL | let _ = if let Some(_) = gen_opt() {
149+
| -------^^^^^^^------------ help: try this: `if gen_opt().is_some()`
150+
151+
error: redundant pattern matching, consider using `is_none()`
152+
--> $DIR/redundant_pattern_matching.rs:94:19
147153
|
148-
LL | if let Ok(_) = Ok::<i32, i32>(4) {
149-
| -------^^^^^-------------------- help: try this: `if Ok::<i32, i32>(4).is_ok()`
154+
LL | } else if let None = gen_opt() {
155+
| -------^^^^------------ help: try this: `if gen_opt().is_none()`
150156

151157
error: redundant pattern matching, consider using `is_ok()`
152-
--> $DIR/redundant_pattern_matching.rs:109:12
158+
--> $DIR/redundant_pattern_matching.rs:96:19
159+
|
160+
LL | } else if let Ok(_) = gen_res() {
161+
| -------^^^^^------------ help: try this: `if gen_res().is_ok()`
162+
163+
error: redundant pattern matching, consider using `is_err()`
164+
--> $DIR/redundant_pattern_matching.rs:98:19
165+
|
166+
LL | } else if let Err(_) = gen_res() {
167+
| -------^^^^^^------------ help: try this: `if gen_res().is_err()`
168+
169+
error: redundant pattern matching, consider using `is_some()`
170+
--> $DIR/redundant_pattern_matching.rs:131:19
171+
|
172+
LL | while let Some(_) = r#try!(result_opt()) {}
173+
| ----------^^^^^^^----------------------- help: try this: `while r#try!(result_opt()).is_some()`
174+
175+
error: redundant pattern matching, consider using `is_some()`
176+
--> $DIR/redundant_pattern_matching.rs:132:16
177+
|
178+
LL | if let Some(_) = r#try!(result_opt()) {}
179+
| -------^^^^^^^----------------------- help: try this: `if r#try!(result_opt()).is_some()`
180+
181+
error: redundant pattern matching, consider using `is_some()`
182+
--> $DIR/redundant_pattern_matching.rs:138:12
183+
|
184+
LL | if let Some(_) = m!() {}
185+
| -------^^^^^^^------- help: try this: `if m!().is_some()`
186+
187+
error: redundant pattern matching, consider using `is_some()`
188+
--> $DIR/redundant_pattern_matching.rs:139:15
153189
|
154-
LL | if let Ok(_) = Ok::<i32, i32>(4) {
155-
| -------^^^^^-------------------- help: try this: `if Ok::<i32, i32>(4).is_ok()`
190+
LL | while let Some(_) = m!() {}
191+
| ----------^^^^^^^------- help: try this: `while m!().is_some()`
156192

157-
error: aborting due to 22 previous errors
193+
error: aborting due to 28 previous errors
158194

0 commit comments

Comments
 (0)