Fix: Some suggestions generated by the option_if_let_else lint did not compile

This commit is contained in:
Elias Holzmann 2023-02-12 20:00:19 +01:00
parent 371120bdbf
commit d80ca09f5e
4 changed files with 56 additions and 15 deletions

View file

@ -122,7 +122,7 @@ fn try_get_option_occurrence<'tcx>(
ExprKind::Unary(UnOp::Deref, inner_expr) | ExprKind::AddrOf(_, _, inner_expr) => inner_expr, ExprKind::Unary(UnOp::Deref, inner_expr) | ExprKind::AddrOf(_, _, inner_expr) => inner_expr,
_ => expr, _ => expr,
}; };
let inner_pat = try_get_inner_pat(cx, pat)?; let (inner_pat, is_result) = try_get_inner_pat_and_is_result(cx, pat)?;
if_chain! { if_chain! {
if let PatKind::Binding(bind_annotation, _, id, None) = inner_pat.kind; if let PatKind::Binding(bind_annotation, _, id, None) = inner_pat.kind;
if let Some(some_captures) = can_move_expr_to_closure(cx, if_then); if let Some(some_captures) = can_move_expr_to_closure(cx, if_then);
@ -176,7 +176,7 @@ fn try_get_option_occurrence<'tcx>(
), ),
none_expr: format!( none_expr: format!(
"{}{}", "{}{}",
if method_sugg == "map_or" { "" } else { "|| " }, if method_sugg == "map_or" { "" } else if is_result { "|_| " } else { "|| "},
Sugg::hir_with_context(cx, none_body, ctxt, "..", &mut app), Sugg::hir_with_context(cx, none_body, ctxt, "..", &mut app),
), ),
}); });
@ -186,11 +186,13 @@ fn try_get_option_occurrence<'tcx>(
None None
} }
fn try_get_inner_pat<'tcx>(cx: &LateContext<'tcx>, pat: &Pat<'tcx>) -> Option<&'tcx Pat<'tcx>> { fn try_get_inner_pat_and_is_result<'tcx>(cx: &LateContext<'tcx>, pat: &Pat<'tcx>) -> Option<(&'tcx Pat<'tcx>, bool)> {
if let PatKind::TupleStruct(ref qpath, [inner_pat], ..) = pat.kind { if let PatKind::TupleStruct(ref qpath, [inner_pat], ..) = pat.kind {
let res = cx.qpath_res(qpath, pat.hir_id); let res = cx.qpath_res(qpath, pat.hir_id);
if is_res_lang_ctor(cx, res, OptionSome) || is_res_lang_ctor(cx, res, ResultOk) { if is_res_lang_ctor(cx, res, OptionSome) {
return Some(inner_pat); return Some((inner_pat, false));
} else if is_res_lang_ctor(cx, res, ResultOk) {
return Some((inner_pat, true));
} }
} }
None None

View file

@ -92,6 +92,15 @@ fn pattern_to_vec(pattern: &str) -> Vec<String> {
.collect::<Vec<_>>() .collect::<Vec<_>>()
} }
// #10335
fn test_result_impure_else(variable: Result<u32, &str>) {
variable.map_or_else(|_| {
println!("Err");
}, |binding| {
println!("Ok {binding}");
})
}
enum DummyEnum { enum DummyEnum {
One(u8), One(u8),
Two, Two,
@ -113,6 +122,7 @@ fn main() {
unop_bad(&None, None); unop_bad(&None, None);
let _ = longer_body(None); let _ = longer_body(None);
test_map_or_else(None); test_map_or_else(None);
test_result_impure_else(Ok(42));
let _ = negative_tests(None); let _ = negative_tests(None);
let _ = impure_else(None); let _ = impure_else(None);

View file

@ -115,6 +115,15 @@ fn pattern_to_vec(pattern: &str) -> Vec<String> {
.collect::<Vec<_>>() .collect::<Vec<_>>()
} }
// #10335
fn test_result_impure_else(variable: Result<u32, &str>) {
if let Ok(binding) = variable {
println!("Ok {binding}");
} else {
println!("Err");
}
}
enum DummyEnum { enum DummyEnum {
One(u8), One(u8),
Two, Two,
@ -136,6 +145,7 @@ fn main() {
unop_bad(&None, None); unop_bad(&None, None);
let _ = longer_body(None); let _ = longer_body(None);
test_map_or_else(None); test_map_or_else(None);
test_result_impure_else(Ok(42));
let _ = negative_tests(None); let _ = negative_tests(None);
let _ = impure_else(None); let _ = impure_else(None);

View file

@ -152,14 +152,33 @@ LL | | vec![s.to_string()]
LL | | } LL | | }
| |_____________^ help: try: `s.find('.').map_or_else(|| vec![s.to_string()], |idx| vec![s[..idx].to_string(), s[idx..].to_string()])` | |_____________^ help: try: `s.find('.').map_or_else(|| vec![s.to_string()], |idx| vec![s[..idx].to_string(), s[idx..].to_string()])`
error: use Option::map_or_else instead of an if let/else
--> $DIR/option_if_let_else.rs:120:5
|
LL | / if let Ok(binding) = variable {
LL | | println!("Ok {binding}");
LL | | } else {
LL | | println!("Err");
LL | | }
| |_____^
|
help: try
|
LL ~ variable.map_or_else(|_| {
LL + println!("Err");
LL + }, |binding| {
LL + println!("Ok {binding}");
LL + })
|
error: use Option::map_or instead of an if let/else error: use Option::map_or instead of an if let/else
--> $DIR/option_if_let_else.rs:133:13 --> $DIR/option_if_let_else.rs:142:13
| |
LL | let _ = if let Some(x) = optional { x + 2 } else { 5 }; LL | let _ = if let Some(x) = optional { x + 2 } else { 5 };
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `optional.map_or(5, |x| x + 2)` | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `optional.map_or(5, |x| x + 2)`
error: use Option::map_or instead of an if let/else error: use Option::map_or instead of an if let/else
--> $DIR/option_if_let_else.rs:142:13 --> $DIR/option_if_let_else.rs:152:13
| |
LL | let _ = if let Some(x) = Some(0) { LL | let _ = if let Some(x) = Some(0) {
| _____________^ | _____________^
@ -181,13 +200,13 @@ LL ~ });
| |
error: use Option::map_or instead of an if let/else error: use Option::map_or instead of an if let/else
--> $DIR/option_if_let_else.rs:170:13 --> $DIR/option_if_let_else.rs:180:13
| |
LL | let _ = if let Some(x) = Some(0) { s.len() + x } else { s.len() }; LL | let _ = if let Some(x) = Some(0) { s.len() + x } else { s.len() };
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `Some(0).map_or(s.len(), |x| s.len() + x)` | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `Some(0).map_or(s.len(), |x| s.len() + x)`
error: use Option::map_or instead of an if let/else error: use Option::map_or instead of an if let/else
--> $DIR/option_if_let_else.rs:174:13 --> $DIR/option_if_let_else.rs:184:13
| |
LL | let _ = if let Some(x) = Some(0) { LL | let _ = if let Some(x) = Some(0) {
| _____________^ | _____________^
@ -207,7 +226,7 @@ LL ~ });
| |
error: use Option::map_or instead of an if let/else error: use Option::map_or instead of an if let/else
--> $DIR/option_if_let_else.rs:213:13 --> $DIR/option_if_let_else.rs:223:13
| |
LL | let _ = match s { LL | let _ = match s {
| _____________^ | _____________^
@ -217,7 +236,7 @@ LL | | };
| |_____^ help: try: `s.map_or(1, |string| string.len())` | |_____^ help: try: `s.map_or(1, |string| string.len())`
error: use Option::map_or instead of an if let/else error: use Option::map_or instead of an if let/else
--> $DIR/option_if_let_else.rs:217:13 --> $DIR/option_if_let_else.rs:227:13
| |
LL | let _ = match Some(10) { LL | let _ = match Some(10) {
| _____________^ | _____________^
@ -227,7 +246,7 @@ LL | | };
| |_____^ help: try: `Some(10).map_or(5, |a| a + 1)` | |_____^ help: try: `Some(10).map_or(5, |a| a + 1)`
error: use Option::map_or instead of an if let/else error: use Option::map_or instead of an if let/else
--> $DIR/option_if_let_else.rs:223:13 --> $DIR/option_if_let_else.rs:233:13
| |
LL | let _ = match res { LL | let _ = match res {
| _____________^ | _____________^
@ -237,7 +256,7 @@ LL | | };
| |_____^ help: try: `res.map_or(1, |a| a + 1)` | |_____^ help: try: `res.map_or(1, |a| a + 1)`
error: use Option::map_or instead of an if let/else error: use Option::map_or instead of an if let/else
--> $DIR/option_if_let_else.rs:227:13 --> $DIR/option_if_let_else.rs:237:13
| |
LL | let _ = match res { LL | let _ = match res {
| _____________^ | _____________^
@ -247,10 +266,10 @@ LL | | };
| |_____^ help: try: `res.map_or(1, |a| a + 1)` | |_____^ help: try: `res.map_or(1, |a| a + 1)`
error: use Option::map_or instead of an if let/else error: use Option::map_or instead of an if let/else
--> $DIR/option_if_let_else.rs:231:13 --> $DIR/option_if_let_else.rs:241:13
| |
LL | let _ = if let Ok(a) = res { a + 1 } else { 5 }; LL | let _ = if let Ok(a) = res { a + 1 } else { 5 };
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `res.map_or(5, |a| a + 1)` | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `res.map_or(5, |a| a + 1)`
error: aborting due to 20 previous errors error: aborting due to 21 previous errors