diff --git a/clippy_lints/src/option_if_let_else.rs b/clippy_lints/src/option_if_let_else.rs
index bbbcda069c5..aa6d4004268 100644
--- a/clippy_lints/src/option_if_let_else.rs
+++ b/clippy_lints/src/option_if_let_else.rs
@@ -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
diff --git a/tests/ui/option_if_let_else.fixed b/tests/ui/option_if_let_else.fixed
index 57f341e0276..2b8ce5477cc 100644
--- a/tests/ui/option_if_let_else.fixed
+++ b/tests/ui/option_if_let_else.fixed
@@ -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);
diff --git a/tests/ui/option_if_let_else.rs b/tests/ui/option_if_let_else.rs
index 19f9f704517..cfbec8cb27d 100644
--- a/tests/ui/option_if_let_else.rs
+++ b/tests/ui/option_if_let_else.rs
@@ -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);
diff --git a/tests/ui/option_if_let_else.stderr b/tests/ui/option_if_let_else.stderr
index f5e4affb672..91d52fc79b8 100644
--- a/tests/ui/option_if_let_else.stderr
+++ b/tests/ui/option_if_let_else.stderr
@@ -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