Skip to content

Commit ef9c38c

Browse files
committed
fix redundant_pattern_matching lint
- now it gives correct suggestion in case of macros
1 parent d01a498 commit ef9c38c

4 files changed

+92
-11
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: 26 additions & 1 deletion
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() {}
@@ -68,6 +68,8 @@ fn main() {
6868
let opt = Some(false);
6969
let x = if opt.is_some() { true } else { false };
7070
takes_bool(x);
71+
72+
issue5504();
7173
}
7274

7375
fn takes_bool(_: bool) {}
@@ -91,3 +93,26 @@ fn returns_unit() {
9193
false
9294
};
9395
}
96+
97+
macro_rules! m {
98+
() => {
99+
Some(42u32)
100+
};
101+
}
102+
103+
fn issue5504() {
104+
fn result_opt() -> Result<Option<i32>, i32> {
105+
Err(42)
106+
}
107+
108+
fn try_result_opt() -> Result<i32, i32> {
109+
while r#try!(result_opt()).is_some() {}
110+
if r#try!(result_opt()).is_some() {}
111+
Ok(42)
112+
}
113+
114+
try_result_opt();
115+
116+
if m!().is_some() {}
117+
while m!().is_some() {}
118+
}

tests/ui/redundant_pattern_matching.rs

Lines changed: 26 additions & 1 deletion
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) {}
@@ -89,6 +89,8 @@ fn main() {
8989
let opt = Some(false);
9090
let x = if let Some(_) = opt { true } else { false };
9191
takes_bool(x);
92+
93+
issue5504();
9294
}
9395

9496
fn takes_bool(_: bool) {}
@@ -112,3 +114,26 @@ fn returns_unit() {
112114
false
113115
};
114116
}
117+
118+
macro_rules! m {
119+
() => {
120+
Some(42u32)
121+
};
122+
}
123+
124+
fn issue5504() {
125+
fn result_opt() -> Result<Option<i32>, i32> {
126+
Err(42)
127+
}
128+
129+
fn try_result_opt() -> Result<i32, i32> {
130+
while let Some(_) = r#try!(result_opt()) {}
131+
if let Some(_) = r#try!(result_opt()) {}
132+
Ok(42)
133+
}
134+
135+
try_result_opt();
136+
137+
if let Some(_) = m!() {}
138+
while let Some(_) = m!() {}
139+
}

tests/ui/redundant_pattern_matching.stderr

Lines changed: 27 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -143,16 +143,40 @@ LL | let x = if let Some(_) = opt { true } else { false };
143143
| -------^^^^^^^------ help: try this: `if opt.is_some()`
144144

145145
error: redundant pattern matching, consider using `is_ok()`
146-
--> $DIR/redundant_pattern_matching.rs:101:12
146+
--> $DIR/redundant_pattern_matching.rs:103:12
147147
|
148148
LL | if let Ok(_) = Ok::<i32, i32>(4) {
149149
| -------^^^^^-------------------- help: try this: `if Ok::<i32, i32>(4).is_ok()`
150150

151151
error: redundant pattern matching, consider using `is_ok()`
152-
--> $DIR/redundant_pattern_matching.rs:109:12
152+
--> $DIR/redundant_pattern_matching.rs:111:12
153153
|
154154
LL | if let Ok(_) = Ok::<i32, i32>(4) {
155155
| -------^^^^^-------------------- help: try this: `if Ok::<i32, i32>(4).is_ok()`
156156

157-
error: aborting due to 22 previous errors
157+
error: redundant pattern matching, consider using `is_some()`
158+
--> $DIR/redundant_pattern_matching.rs:130:19
159+
|
160+
LL | while let Some(_) = r#try!(result_opt()) {}
161+
| ----------^^^^^^^----------------------- help: try this: `while r#try!(result_opt()).is_some()`
162+
163+
error: redundant pattern matching, consider using `is_some()`
164+
--> $DIR/redundant_pattern_matching.rs:131:16
165+
|
166+
LL | if let Some(_) = r#try!(result_opt()) {}
167+
| -------^^^^^^^----------------------- help: try this: `if r#try!(result_opt()).is_some()`
168+
169+
error: redundant pattern matching, consider using `is_some()`
170+
--> $DIR/redundant_pattern_matching.rs:137:12
171+
|
172+
LL | if let Some(_) = m!() {}
173+
| -------^^^^^^^------- help: try this: `if m!().is_some()`
174+
175+
error: redundant pattern matching, consider using `is_some()`
176+
--> $DIR/redundant_pattern_matching.rs:138:15
177+
|
178+
LL | while let Some(_) = m!() {}
179+
| ----------^^^^^^^------- help: try this: `while m!().is_some()`
180+
181+
error: aborting due to 26 previous errors
158182

0 commit comments

Comments
 (0)