Skip to content

Commit 9f49773

Browse files
dgulottadgulotta
committed
fix option_if_let_else when Err variant is ignored
1 parent 714c64c commit 9f49773

File tree

4 files changed

+59
-15
lines changed

4 files changed

+59
-15
lines changed

Diff for: ‎clippy_lints/src/option_if_let_else.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -187,7 +187,7 @@ fn try_get_option_occurrence<'tcx>(
187187
let mut app = Applicability::Unspecified;
188188

189189
let (none_body, is_argless_call) = match none_body.kind {
190-
ExprKind::Call(call_expr, []) if !none_body.span.from_expansion() => (call_expr, true),
190+
ExprKind::Call(call_expr, []) if !none_body.span.from_expansion() && !is_result => (call_expr, true),
191191
_ => (none_body, false),
192192
};
193193

Diff for: ‎tests/ui/option_if_let_else.fixed

+10
Original file line numberDiff line numberDiff line change
@@ -125,6 +125,16 @@ fn complex_subpat() -> DummyEnum {
125125
DummyEnum::Two
126126
}
127127

128+
// #10335
129+
pub fn test_result_err_ignored_1(r: Result<&[u8], &[u8]>) -> Vec<u8> {
130+
r.map_or_else(|_| Vec::new(), |s| s.to_owned())
131+
}
132+
133+
// #10335
134+
pub fn test_result_err_ignored_2(r: Result<&[u8], &[u8]>) -> Vec<u8> {
135+
r.map_or_else(|_| Vec::new(), |s| s.to_owned())
136+
}
137+
128138
fn main() {
129139
let optional = Some(5);
130140
let _ = optional.map_or(5, |x| x + 2);

Diff for: ‎tests/ui/option_if_let_else.rs

+16
Original file line numberDiff line numberDiff line change
@@ -152,6 +152,22 @@ fn complex_subpat() -> DummyEnum {
152152
DummyEnum::Two
153153
}
154154

155+
// #10335
156+
pub fn test_result_err_ignored_1(r: Result<&[u8], &[u8]>) -> Vec<u8> {
157+
match r {
158+
//~^ option_if_let_else
159+
Ok(s) => s.to_owned(),
160+
Err(_) => Vec::new(),
161+
}
162+
}
163+
164+
// #10335
165+
pub fn test_result_err_ignored_2(r: Result<&[u8], &[u8]>) -> Vec<u8> {
166+
if let Ok(s) = r { s.to_owned() }
167+
//~^ option_if_let_else
168+
else { Vec::new() }
169+
}
170+
155171
fn main() {
156172
let optional = Some(5);
157173
let _ = if let Some(x) = optional { x + 2 } else { 5 };

Diff for: ‎tests/ui/option_if_let_else.stderr

+32-14
Original file line numberDiff line numberDiff line change
@@ -188,14 +188,32 @@ LL + true
188188
LL + })
189189
|
190190

191+
error: use Option::map_or_else instead of an if let/else
192+
--> tests/ui/option_if_let_else.rs:157:5
193+
|
194+
LL | / match r {
195+
LL | |
196+
LL | | Ok(s) => s.to_owned(),
197+
LL | | Err(_) => Vec::new(),
198+
LL | | }
199+
| |_____^ help: try: `r.map_or_else(|_| Vec::new(), |s| s.to_owned())`
200+
201+
error: use Option::map_or_else instead of an if let/else
202+
--> tests/ui/option_if_let_else.rs:166:5
203+
|
204+
LL | / if let Ok(s) = r { s.to_owned() }
205+
LL | |
206+
LL | | else { Vec::new() }
207+
| |_______________________^ help: try: `r.map_or_else(|_| Vec::new(), |s| s.to_owned())`
208+
191209
error: use Option::map_or instead of an if let/else
192-
--> tests/ui/option_if_let_else.rs:157:13
210+
--> tests/ui/option_if_let_else.rs:173:13
193211
|
194212
LL | let _ = if let Some(x) = optional { x + 2 } else { 5 };
195213
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `optional.map_or(5, |x| x + 2)`
196214

197215
error: use Option::map_or instead of an if let/else
198-
--> tests/ui/option_if_let_else.rs:168:13
216+
--> tests/ui/option_if_let_else.rs:184:13
199217
|
200218
LL | let _ = if let Some(x) = Some(0) {
201219
| _____________^
@@ -217,13 +235,13 @@ LL ~ });
217235
|
218236

219237
error: use Option::map_or instead of an if let/else
220-
--> tests/ui/option_if_let_else.rs:197:13
238+
--> tests/ui/option_if_let_else.rs:213:13
221239
|
222240
LL | let _ = if let Some(x) = Some(0) { s.len() + x } else { s.len() };
223241
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `Some(0).map_or(s.len(), |x| s.len() + x)`
224242

225243
error: use Option::map_or instead of an if let/else
226-
--> tests/ui/option_if_let_else.rs:202:13
244+
--> tests/ui/option_if_let_else.rs:218:13
227245
|
228246
LL | let _ = if let Some(x) = Some(0) {
229247
| _____________^
@@ -245,7 +263,7 @@ LL ~ });
245263
|
246264

247265
error: use Option::map_or instead of an if let/else
248-
--> tests/ui/option_if_let_else.rs:242:13
266+
--> tests/ui/option_if_let_else.rs:258:13
249267
|
250268
LL | let _ = match s {
251269
| _____________^
@@ -256,7 +274,7 @@ LL | | };
256274
| |_____^ help: try: `s.map_or(1, |string| string.len())`
257275

258276
error: use Option::map_or instead of an if let/else
259-
--> tests/ui/option_if_let_else.rs:247:13
277+
--> tests/ui/option_if_let_else.rs:263:13
260278
|
261279
LL | let _ = match Some(10) {
262280
| _____________^
@@ -267,7 +285,7 @@ LL | | };
267285
| |_____^ help: try: `Some(10).map_or(5, |a| a + 1)`
268286

269287
error: use Option::map_or instead of an if let/else
270-
--> tests/ui/option_if_let_else.rs:254:13
288+
--> tests/ui/option_if_let_else.rs:270:13
271289
|
272290
LL | let _ = match res {
273291
| _____________^
@@ -278,7 +296,7 @@ LL | | };
278296
| |_____^ help: try: `res.map_or(1, |a| a + 1)`
279297

280298
error: use Option::map_or instead of an if let/else
281-
--> tests/ui/option_if_let_else.rs:259:13
299+
--> tests/ui/option_if_let_else.rs:275:13
282300
|
283301
LL | let _ = match res {
284302
| _____________^
@@ -289,13 +307,13 @@ LL | | };
289307
| |_____^ help: try: `res.map_or(1, |a| a + 1)`
290308

291309
error: use Option::map_or instead of an if let/else
292-
--> tests/ui/option_if_let_else.rs:264:13
310+
--> tests/ui/option_if_let_else.rs:280:13
293311
|
294312
LL | let _ = if let Ok(a) = res { a + 1 } else { 5 };
295313
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `res.map_or(5, |a| a + 1)`
296314

297315
error: use Option::map_or instead of an if let/else
298-
--> tests/ui/option_if_let_else.rs:282:17
316+
--> tests/ui/option_if_let_else.rs:298:17
299317
|
300318
LL | let _ = match initial {
301319
| _________________^
@@ -306,7 +324,7 @@ LL | | };
306324
| |_________^ help: try: `initial.as_ref().map_or(42, |value| do_something(value))`
307325

308326
error: use Option::map_or instead of an if let/else
309-
--> tests/ui/option_if_let_else.rs:290:17
327+
--> tests/ui/option_if_let_else.rs:306:17
310328
|
311329
LL | let _ = match initial {
312330
| _________________^
@@ -317,7 +335,7 @@ LL | | };
317335
| |_________^ help: try: `initial.as_mut().map_or(42, |value| do_something2(value))`
318336

319337
error: use Option::map_or_else instead of an if let/else
320-
--> tests/ui/option_if_let_else.rs:314:24
338+
--> tests/ui/option_if_let_else.rs:330:24
321339
|
322340
LL | let mut _hashmap = if let Some(hm) = &opt {
323341
| ________________________^
@@ -329,10 +347,10 @@ LL | | };
329347
| |_____^ help: try: `opt.as_ref().map_or_else(HashMap::new, |hm| hm.clone())`
330348

331349
error: use Option::map_or_else instead of an if let/else
332-
--> tests/ui/option_if_let_else.rs:321:19
350+
--> tests/ui/option_if_let_else.rs:337:19
333351
|
334352
LL | let mut _hm = if let Some(hm) = &opt { hm.clone() } else { new_map!() };
335353
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `opt.as_ref().map_or_else(|| new_map!(), |hm| hm.clone())`
336354

337-
error: aborting due to 25 previous errors
355+
error: aborting due to 27 previous errors
338356

0 commit comments

Comments
 (0)