Skip to content

Commit d118a19

Browse files
committed
fix: option_if_let_else suggests wrongly when coersion requires explicit cast
1 parent a25cbd5 commit d118a19

File tree

4 files changed

+58
-3
lines changed

4 files changed

+58
-3
lines changed

clippy_lints/src/option_if_let_else.rs

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,7 @@ declare_lint_pass!(OptionIfLetElse => [OPTION_IF_LET_ELSE]);
9292
struct OptionOccurrence {
9393
option: String,
9494
method_sugg: String,
95+
method_generics: Option<String>,
9596
some_expr: String,
9697
none_expr: String,
9798
}
@@ -184,6 +185,11 @@ fn try_get_option_occurrence<'tcx>(
184185
}
185186
}
186187

188+
let none_ty_adj = cx.typeck_results().expr_ty_adjusted(none_body);
189+
let method_generics = (method_sugg == "map_or_else"
190+
&& !cx.typeck_results().expr_adjustments(none_body).is_empty())
191+
.then(|| format!("::<{none_ty_adj}, _, _>"));
192+
187193
let mut app = Applicability::Unspecified;
188194

189195
let (none_body, is_argless_call) = match none_body.kind {
@@ -198,6 +204,7 @@ fn try_get_option_occurrence<'tcx>(
198204
as_mut,
199205
),
200206
method_sugg: method_sugg.to_string(),
207+
method_generics,
201208
some_expr: format!(
202209
"|{capture_mut}{capture_name}| {}",
203210
Sugg::hir_with_context(cx, some_body, ctxt, "..", &mut app),
@@ -313,8 +320,12 @@ impl<'tcx> LateLintPass<'tcx> for OptionIfLetElse {
313320
format!("use Option::{} instead of an if let/else", det.method_sugg),
314321
"try",
315322
format!(
316-
"{}.{}({}, {})",
317-
det.option, det.method_sugg, det.none_expr, det.some_expr
323+
"{}.{}{}({}, {})",
324+
det.option,
325+
det.method_sugg,
326+
det.method_generics.unwrap_or_default(),
327+
det.none_expr,
328+
det.some_expr
318329
),
319330
Applicability::MaybeIncorrect,
320331
);

tests/ui/option_if_let_else.fixed

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -268,3 +268,19 @@ fn issue11893() {
268268
panic!("Haven't thought about this condition.");
269269
}
270270
}
271+
272+
mod issue11059 {
273+
use std::fmt::Debug;
274+
275+
fn box_coercion_unsize(o: Option<i32>) -> Box<dyn Debug> {
276+
o.map_or_else::<std::boxed::Box<dyn std::fmt::Debug>, _, _>(|| Box::new("foo"), |o| Box::new(o))
277+
//~^ option_if_let_else
278+
}
279+
280+
static S: String = String::new();
281+
282+
fn deref_with_overload(o: Option<&str>) -> &str {
283+
o.map_or_else::<&str, _, _>(|| &S, |o| o)
284+
//~^ option_if_let_else
285+
}
286+
}

tests/ui/option_if_let_else.rs

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -331,3 +331,19 @@ fn issue11893() {
331331
panic!("Haven't thought about this condition.");
332332
}
333333
}
334+
335+
mod issue11059 {
336+
use std::fmt::Debug;
337+
338+
fn box_coercion_unsize(o: Option<i32>) -> Box<dyn Debug> {
339+
if let Some(o) = o { Box::new(o) } else { Box::new("foo") }
340+
//~^ option_if_let_else
341+
}
342+
343+
static S: String = String::new();
344+
345+
fn deref_with_overload(o: Option<&str>) -> &str {
346+
if let Some(o) = o { o } else { &S }
347+
//~^ option_if_let_else
348+
}
349+
}

tests/ui/option_if_let_else.stderr

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -334,5 +334,17 @@ 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_else instead of an if let/else
338+
--> tests/ui/option_if_let_else.rs:339:9
339+
|
340+
LL | if let Some(o) = o { Box::new(o) } else { Box::new("foo") }
341+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `o.map_or_else::<std::boxed::Box<dyn std::fmt::Debug>, _, _>(|| Box::new("foo"), |o| Box::new(o))`
342+
343+
error: use Option::map_or_else instead of an if let/else
344+
--> tests/ui/option_if_let_else.rs:346:9
345+
|
346+
LL | if let Some(o) = o { o } else { &S }
347+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `o.map_or_else::<&str, _, _>(|| &S, |o| o)`
348+
349+
error: aborting due to 27 previous errors
338350

0 commit comments

Comments
 (0)