Skip to content

Commit 8fa9d63

Browse files
alex-700alex-700
committed
fix redundant_pattern_matching lint
- now it gives correct suggestion in case of macros - better tests
1 parent d01a498 commit 8fa9d63

File tree

4 files changed

+156
-11
lines changed

4 files changed

+156
-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: 46 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,28 @@ fn main() {
6868
let opt = Some(false);
6969
let x = if opt.is_some() { true } else { false };
7070
takes_bool(x);
71+
72+
issue5504();
73+
74+
let _ = if gen_opt().is_some() {
75+
1
76+
} else if gen_opt().is_none() {
77+
2
78+
} else if gen_res().is_ok() {
79+
3
80+
} else if gen_res().is_err() {
81+
4
82+
} else {
83+
5
84+
};
85+
}
86+
87+
fn gen_opt() -> Option<()> {
88+
None
89+
}
90+
91+
fn gen_res() -> Result<(), ()> {
92+
Ok(())
7193
}
7294

7395
fn takes_bool(_: bool) {}
@@ -91,3 +113,26 @@ fn returns_unit() {
91113
false
92114
};
93115
}
116+
117+
macro_rules! m {
118+
() => {
119+
Some(42u32)
120+
};
121+
}
122+
123+
fn issue5504() {
124+
fn result_opt() -> Result<Option<i32>, i32> {
125+
Err(42)
126+
}
127+
128+
fn try_result_opt() -> Result<i32, i32> {
129+
while r#try!(result_opt()).is_some() {}
130+
if r#try!(result_opt()).is_some() {}
131+
Ok(42)
132+
}
133+
134+
try_result_opt();
135+
136+
if m!().is_some() {}
137+
while m!().is_some() {}
138+
}

‎tests/ui/redundant_pattern_matching.rs

Lines changed: 46 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,28 @@ fn main() {
8989
let opt = Some(false);
9090
let x = if let Some(_) = opt { true } else { false };
9191
takes_bool(x);
92+
93+
issue5504();
94+
95+
let _ = if let Some(_) = gen_opt() {
96+
1
97+
} else if let None = gen_opt() {
98+
2
99+
} else if let Ok(_) = gen_res() {
100+
3
101+
} else if let Err(_) = gen_res() {
102+
4
103+
} else {
104+
5
105+
};
106+
}
107+
108+
fn gen_opt() -> Option<()> {
109+
None
110+
}
111+
112+
fn gen_res() -> Result<(), ()> {
113+
Ok(())
92114
}
93115

94116
fn takes_bool(_: bool) {}
@@ -112,3 +134,26 @@ fn returns_unit() {
112134
false
113135
};
114136
}
137+
138+
macro_rules! m {
139+
() => {
140+
Some(42u32)
141+
};
142+
}
143+
144+
fn issue5504() {
145+
fn result_opt() -> Result<Option<i32>, i32> {
146+
Err(42)
147+
}
148+
149+
fn try_result_opt() -> Result<i32, i32> {
150+
while let Some(_) = r#try!(result_opt()) {}
151+
if let Some(_) = r#try!(result_opt()) {}
152+
Ok(42)
153+
}
154+
155+
try_result_opt();
156+
157+
if let Some(_) = m!() {}
158+
while let Some(_) = m!() {}
159+
}

‎tests/ui/redundant_pattern_matching.stderr

Lines changed: 51 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -142,17 +142,65 @@ error: redundant pattern matching, consider using `is_some()`
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_some()`
146+
--> $DIR/redundant_pattern_matching.rs:95: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:97:19
153+
|
154+
LL | } else if let None = gen_opt() {
155+
| -------^^^^------------ help: try this: `if gen_opt().is_none()`
156+
157+
error: redundant pattern matching, consider using `is_ok()`
158+
--> $DIR/redundant_pattern_matching.rs:99: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:101:19
165+
|
166+
LL | } else if let Err(_) = gen_res() {
167+
| -------^^^^^^------------ help: try this: `if gen_res().is_err()`
168+
145169
error: redundant pattern matching, consider using `is_ok()`
146-
--> $DIR/redundant_pattern_matching.rs:101:12
170+
--> $DIR/redundant_pattern_matching.rs:123:12
147171
|
148172
LL | if let Ok(_) = Ok::<i32, i32>(4) {
149173
| -------^^^^^-------------------- help: try this: `if Ok::<i32, i32>(4).is_ok()`
150174

151175
error: redundant pattern matching, consider using `is_ok()`
152-
--> $DIR/redundant_pattern_matching.rs:109:12
176+
--> $DIR/redundant_pattern_matching.rs:131:12
153177
|
154178
LL | if let Ok(_) = Ok::<i32, i32>(4) {
155179
| -------^^^^^-------------------- help: try this: `if Ok::<i32, i32>(4).is_ok()`
156180

157-
error: aborting due to 22 previous errors
181+
error: redundant pattern matching, consider using `is_some()`
182+
--> $DIR/redundant_pattern_matching.rs:150:19
183+
|
184+
LL | while let Some(_) = r#try!(result_opt()) {}
185+
| ----------^^^^^^^----------------------- help: try this: `while r#try!(result_opt()).is_some()`
186+
187+
error: redundant pattern matching, consider using `is_some()`
188+
--> $DIR/redundant_pattern_matching.rs:151:16
189+
|
190+
LL | if let Some(_) = r#try!(result_opt()) {}
191+
| -------^^^^^^^----------------------- help: try this: `if r#try!(result_opt()).is_some()`
192+
193+
error: redundant pattern matching, consider using `is_some()`
194+
--> $DIR/redundant_pattern_matching.rs:157:12
195+
|
196+
LL | if let Some(_) = m!() {}
197+
| -------^^^^^^^------- help: try this: `if m!().is_some()`
198+
199+
error: redundant pattern matching, consider using `is_some()`
200+
--> $DIR/redundant_pattern_matching.rs:158:15
201+
|
202+
LL | while let Some(_) = m!() {}
203+
| ----------^^^^^^^------- help: try this: `while m!().is_some()`
204+
205+
error: aborting due to 30 previous errors
158206

0 commit comments

Comments
 (0)