Skip to content

Commit 88bcf1c

Browse files
authored
fix option-if-let-else lint (#15394)
some simple twists Fixes #15002 Fixes #15379 changelog: [`option_if_let_else`]: Don't remove raw pointer derefs in suggestions changelog: [`option_if_let_else`]: Don't suggest passing argless functions to `Result::map_or_else`
2 parents fa09b86 + 8f6b43d commit 88bcf1c

File tree

4 files changed

+51
-5
lines changed

4 files changed

+51
-5
lines changed

clippy_lints/src/option_if_let_else.rs

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -127,7 +127,8 @@ fn try_get_option_occurrence<'tcx>(
127127
if_else: &'tcx Expr<'_>,
128128
) -> Option<OptionOccurrence> {
129129
let cond_expr = match expr.kind {
130-
ExprKind::Unary(UnOp::Deref, inner_expr) | ExprKind::AddrOf(_, _, inner_expr) => inner_expr,
130+
ExprKind::AddrOf(_, _, inner_expr) => inner_expr,
131+
ExprKind::Unary(UnOp::Deref, inner_expr) if !cx.typeck_results().expr_ty(inner_expr).is_raw_ptr() => inner_expr,
131132
_ => expr,
132133
};
133134
let (inner_pat, is_result) = try_get_inner_pat_and_is_result(cx, pat)?;
@@ -223,8 +224,8 @@ fn try_get_option_occurrence<'tcx>(
223224

224225
let mut app = Applicability::Unspecified;
225226

226-
let (none_body, is_argless_call) = match none_body.kind {
227-
ExprKind::Call(call_expr, []) if !none_body.span.from_expansion() => (call_expr, true),
227+
let (none_body, can_omit_arg) = match none_body.kind {
228+
ExprKind::Call(call_expr, []) if !none_body.span.from_expansion() && !is_result => (call_expr, true),
228229
_ => (none_body, false),
229230
};
230231

@@ -241,7 +242,7 @@ fn try_get_option_occurrence<'tcx>(
241242
),
242243
none_expr: format!(
243244
"{}{}",
244-
if method_sugg == "map_or" || is_argless_call {
245+
if method_sugg == "map_or" || can_omit_arg {
245246
""
246247
} else if is_result {
247248
"|_| "

tests/ui/option_if_let_else.fixed

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -302,3 +302,15 @@ mod issue11059 {
302302
if let Some(o) = o { o } else { &S }
303303
}
304304
}
305+
306+
fn issue15379() {
307+
let opt = Some(0usize);
308+
let opt_raw_ptr = &opt as *const Option<usize>;
309+
let _ = unsafe { (*opt_raw_ptr).map_or(1, |o| o) };
310+
//~^ option_if_let_else
311+
}
312+
313+
fn issue15002() {
314+
let res: Result<String, ()> = Ok("_".to_string());
315+
let _ = res.map_or_else(|_| String::new(), |s| s.clone());
316+
}

tests/ui/option_if_let_else.rs

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -365,3 +365,19 @@ mod issue11059 {
365365
if let Some(o) = o { o } else { &S }
366366
}
367367
}
368+
369+
fn issue15379() {
370+
let opt = Some(0usize);
371+
let opt_raw_ptr = &opt as *const Option<usize>;
372+
let _ = unsafe { if let Some(o) = *opt_raw_ptr { o } else { 1 } };
373+
//~^ option_if_let_else
374+
}
375+
376+
fn issue15002() {
377+
let res: Result<String, ()> = Ok("_".to_string());
378+
let _ = match res {
379+
//~^ option_if_let_else
380+
Ok(s) => s.clone(),
381+
Err(_) => String::new(),
382+
};
383+
}

tests/ui/option_if_let_else.stderr

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -334,5 +334,22 @@ error: use Option::map_or_else instead of an if let/else
334334
LL | let mut _hm = if let Some(hm) = &opt { hm.clone() } else { new_map!() };
335335
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `opt.as_ref().map_or_else(|| new_map!(), |hm| hm.clone())`
336336

337-
error: aborting due to 25 previous errors
337+
error: use Option::map_or instead of an if let/else
338+
--> tests/ui/option_if_let_else.rs:372:22
339+
|
340+
LL | let _ = unsafe { if let Some(o) = *opt_raw_ptr { o } else { 1 } };
341+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `(*opt_raw_ptr).map_or(1, |o| o)`
342+
343+
error: use Option::map_or_else instead of an if let/else
344+
--> tests/ui/option_if_let_else.rs:378:13
345+
|
346+
LL | let _ = match res {
347+
| _____________^
348+
LL | |
349+
LL | | Ok(s) => s.clone(),
350+
LL | | Err(_) => String::new(),
351+
LL | | };
352+
| |_____^ help: try: `res.map_or_else(|_| String::new(), |s| s.clone())`
353+
354+
error: aborting due to 27 previous errors
338355

0 commit comments

Comments
 (0)