Auto merge of #10337 - EliasHolzmann:fix/10335, r=dswij
Fix: Some suggestions generated by the option_if_let_else lint did not compile This addresses a bug in Clippy where the fix suggestend by the `option_if_let_else` lint would not compile for `Result`s which have an impure expression in the `else` branch. --- changelog: [`option_if_let_else`]: Fixed incorrect suggestion for `Result`s [#10337](https://github.com/rust-lang/rust-clippy/pull/10337) <!-- changelog_checked --> Fixes #10335.
This commit is contained in:
bors
commit
407c352688
|
|
@ -122,7 +122,7 @@ fn try_get_option_occurrence<'tcx>(
|
|||
ExprKind::Unary(UnOp::Deref, inner_expr) | ExprKind::AddrOf(_, _, inner_expr) => inner_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 let PatKind::Binding(bind_annotation, _, id, None) = inner_pat.kind;
|
||||
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!(
|
||||
"{}{}",
|
||||
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),
|
||||
),
|
||||
});
|
||||
|
|
@ -186,11 +186,13 @@ fn try_get_option_occurrence<'tcx>(
|
|||
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 {
|
||||
let res = cx.qpath_res(qpath, pat.hir_id);
|
||||
if is_res_lang_ctor(cx, res, OptionSome) || is_res_lang_ctor(cx, res, ResultOk) {
|
||||
return Some(inner_pat);
|
||||
if is_res_lang_ctor(cx, res, OptionSome) {
|
||||
return Some((inner_pat, false));
|
||||
} else if is_res_lang_ctor(cx, res, ResultOk) {
|
||||
return Some((inner_pat, true));
|
||||
}
|
||||
}
|
||||
None
|
||||
|
|
|
|||
|
|
@ -92,6 +92,15 @@ fn pattern_to_vec(pattern: &str) -> Vec<String> {
|
|||
.collect::<Vec<_>>()
|
||||
}
|
||||
|
||||
// #10335
|
||||
fn test_result_impure_else(variable: Result<u32, &str>) {
|
||||
variable.map_or_else(|_| {
|
||||
println!("Err");
|
||||
}, |binding| {
|
||||
println!("Ok {binding}");
|
||||
})
|
||||
}
|
||||
|
||||
enum DummyEnum {
|
||||
One(u8),
|
||||
Two,
|
||||
|
|
@ -113,6 +122,7 @@ fn main() {
|
|||
unop_bad(&None, None);
|
||||
let _ = longer_body(None);
|
||||
test_map_or_else(None);
|
||||
test_result_impure_else(Ok(42));
|
||||
let _ = negative_tests(None);
|
||||
let _ = impure_else(None);
|
||||
|
||||
|
|
|
|||
|
|
@ -115,6 +115,15 @@ fn pattern_to_vec(pattern: &str) -> Vec<String> {
|
|||
.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 {
|
||||
One(u8),
|
||||
Two,
|
||||
|
|
@ -136,6 +145,7 @@ fn main() {
|
|||
unop_bad(&None, None);
|
||||
let _ = longer_body(None);
|
||||
test_map_or_else(None);
|
||||
test_result_impure_else(Ok(42));
|
||||
let _ = negative_tests(None);
|
||||
let _ = impure_else(None);
|
||||
|
||||
|
|
|
|||
|
|
@ -152,14 +152,33 @@ LL | | vec![s.to_string()]
|
|||
LL | | }
|
||||
| |_____________^ 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
|
||||
--> $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 };
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `optional.map_or(5, |x| x + 2)`
|
||||
|
||||
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) {
|
||||
| _____________^
|
||||
|
|
@ -181,13 +200,13 @@ LL ~ });
|
|||
|
|
||||
|
||||
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() };
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `Some(0).map_or(s.len(), |x| s.len() + x)`
|
||||
|
||||
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) {
|
||||
| _____________^
|
||||
|
|
@ -207,7 +226,7 @@ LL ~ });
|
|||
|
|
||||
|
||||
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 {
|
||||
| _____________^
|
||||
|
|
@ -217,7 +236,7 @@ LL | | };
|
|||
| |_____^ help: try: `s.map_or(1, |string| string.len())`
|
||||
|
||||
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) {
|
||||
| _____________^
|
||||
|
|
@ -227,7 +246,7 @@ LL | | };
|
|||
| |_____^ help: try: `Some(10).map_or(5, |a| a + 1)`
|
||||
|
||||
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 {
|
||||
| _____________^
|
||||
|
|
@ -237,7 +256,7 @@ LL | | };
|
|||
| |_____^ help: try: `res.map_or(1, |a| a + 1)`
|
||||
|
||||
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 {
|
||||
| _____________^
|
||||
|
|
@ -247,10 +266,10 @@ LL | | };
|
|||
| |_____^ help: try: `res.map_or(1, |a| a + 1)`
|
||||
|
||||
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 };
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `res.map_or(5, |a| a + 1)`
|
||||
|
||||
error: aborting due to 20 previous errors
|
||||
error: aborting due to 21 previous errors
|
||||
|
||||
|
|
|
|||
Loading…
Reference in New Issue