Handle ref, mut, &, and &mut on the option

4 files changed, 158 insertions, 25 deletions

diff --git a/clippy_lints/src/option_if_let_else.rs b/clippy_lints/src/option_if_let_else.rs
index 66971ee0262..4e501f4ca02 100644
--- a/clippy_lints/src/option_if_let_else.rs
+++ b/clippy_lints/src/option_if_let_else.rs
@@ -140,18 +140,24 @@ fn contains_return_break_continue<'tcx>(expression: &'tcx Expr<'tcx>) -> bool {
 /// this function returns an OptionIfLetElseOccurence struct with details if
 /// this construct is found, or None if this construct is not found.
 fn detect_option_if_let_else<'a>(cx: &LateContext<'_, 'a>, expr: &'a Expr<'a>) -> Option<OptionIfLetElseOccurence> {
-    //(String, String, String, String)> {
     if_chain! {
-        // if !utils::in_macro(expr.span); // Don't lint macros, because it behaves weirdly
+        if !utils::in_macro(expr.span); // Don't lint macros, because it behaves weirdly
         if let ExprKind::Match(let_body, arms, MatchSource::IfLetDesugar{contains_else_clause: true}) = &expr.kind;
         if arms.len() == 2;
-        if match_type(cx, &cx.tables.expr_ty(let_body), &paths::OPTION);
+        // if type_is_option(cx, &cx.tables.expr_ty(let_body).kind);
         if !is_result_ok(cx, let_body); // Don't lint on Result::ok because a different lint does it already
-        if let PatKind::TupleStruct(_, &[inner_pat], _) = &arms[0].pat.kind;
-        if let PatKind::Binding(_, _, id, _) = &inner_pat.kind;
+        if let PatKind::TupleStruct(struct_qpath, &[inner_pat], _) = &arms[0].pat.kind;
+        if utils::match_qpath(struct_qpath, &paths::OPTION_SOME);
+        if let PatKind::Binding(bind_annotation, _, id, _) = &inner_pat.kind;
         if !contains_return_break_continue(arms[0].body);
         if !contains_return_break_continue(arms[1].body);
         then {
+            let (capture_mut, capture_ref, capture_ref_mut) = match bind_annotation {
+                BindingAnnotation::Unannotated => (false, false, false),
+                BindingAnnotation::Mutable => (true, false, false),
+                BindingAnnotation::Ref => (false, true, false),
+                BindingAnnotation::RefMut => (false, false, true),
+            };
             let some_body = if let ExprKind::Block(Block { stmts: statements, expr: Some(expr), .. }, _)
                 = &arms[0].body.kind {
                 if let &[] = &statements {
@@ -189,7 +195,7 @@ fn detect_option_if_let_else<'a>(cx: &LateContext<'_, 'a>, expr: &'a Expr<'a>) -
                     }
                 }
             });
-            let parens_around_option = match &let_body.kind {
+            let (parens_around_option, as_ref, as_mut, let_body) = match &let_body.kind {
                 ExprKind::Call(..)
                         | ExprKind::MethodCall(..)
                         | ExprKind::Loop(..)
@@ -197,13 +203,15 @@ fn detect_option_if_let_else<'a>(cx: &LateContext<'_, 'a>, expr: &'a Expr<'a>) -
                         | ExprKind::Block(..)
                         | ExprKind::Field(..)
                         | ExprKind::Path(_)
-                    => false,
-                _ => true,
+                    => (false, capture_ref, capture_ref_mut, let_body),
+                ExprKind::Unary(UnOp::UnDeref, expr) => (false, capture_ref, capture_ref_mut, expr),
+                ExprKind::AddrOf(_, mutability, expr) => (false, mutability == &Mutability::Not, mutability == &Mutability::Mut, expr),
+                _ => (true, capture_ref, capture_ref_mut, let_body),
             };
             Some(OptionIfLetElseOccurence {
-                option: format!("{}{}{}", if parens_around_option { "(" } else { "" }, Sugg::hir(cx, let_body, ".."), if parens_around_option { ")" } else { "" }),
+                option: format!("{}{}{}{}", if parens_around_option { "(" } else { "" }, Sugg::hir(cx, let_body, ".."), if parens_around_option { ")" } else { "" }, if as_mut { ".as_mut()" } else if as_ref { ".as_ref()" } else { "" }),
                 method_sugg: format!("{}", method_sugg),
-                some_expr: format!("|{}| {}", capture_name, Sugg::hir(cx, some_body, "..")),
+                some_expr: format!("|{}{}{}| {}", if false { "ref " } else { "" }, if capture_mut { "mut " } else { "" }, capture_name, Sugg::hir(cx, some_body, "..")),
                 none_expr: format!("{}{}", if method_sugg == "map_or" { "" } else { "|| " }, Sugg::hir(cx, none_body, "..")),
                 wrap_braces,
             })
diff --git a/tests/ui/option_if_let_else.fixed b/tests/ui/option_if_let_else.fixed
index 80b162714ac..695a460cc4e 100644
--- a/tests/ui/option_if_let_else.fixed
+++ b/tests/ui/option_if_let_else.fixed
@@ -11,8 +11,22 @@ fn else_if_option(string: Option<&str>) -> Option<(bool, &str)> {
     } else { string.map_or(Some((false, "")), |x| Some((true, x))) }
 }
 
-fn unop_bad(string: &Option<&str>) -> usize {
-    (*string).map_or(0, |s| s.len())
+fn unop_bad(string: &Option<&str>, mut num: Option<i32>) {
+    let _ = string.map_or(0, |s| s.len());
+    let _ = num.as_ref().map_or(&0, |s| s);
+    let _ = num.as_mut().map_or(&mut 0, |s| {
+        *s += 1;
+        s
+    });
+    let _ = num.as_ref().map_or(&0, |s| s);
+    let _ = num.map_or(0, |mut s| {
+        s += 1;
+        s
+    });
+    let _ = num.as_mut().map_or(&mut 0, |s| {
+        *s += 1;
+        s
+    });
 }
 
 fn longer_body(arg: Option<u32>) -> u32 {
@@ -53,7 +67,7 @@ fn main() {
     let _ = optional.map_or(5, |x| x + 2);
     let _ = bad1(None);
     let _ = else_if_option(None);
-    let _ = unop_bad(&None);
+    unop_bad(&None, None);
     let _ = longer_body(None);
     test_map_or_else(None);
     let _ = negative_tests(None);
diff --git a/tests/ui/option_if_let_else.rs b/tests/ui/option_if_let_else.rs
index 7c43fbea373..6f9d506d347 100644
--- a/tests/ui/option_if_let_else.rs
+++ b/tests/ui/option_if_let_else.rs
@@ -19,12 +19,40 @@ fn else_if_option(string: Option<&str>) -> Option<(bool, &str)> {
     }
 }
 
-fn unop_bad(string: &Option<&str>) -> usize {
-    if let Some(s) = *string {
+fn unop_bad(string: &Option<&str>, mut num: Option<i32>) {
+    let _ = if let Some(s) = *string {
         s.len()
     } else {
         0
-    }
+    };
+    let _ = if let Some(s) = &num {
+        s
+    } else {
+        &0
+    };
+    let _ = if let Some(s) = &mut num {
+        *s += 1;
+        s
+    } else {
+        &mut 0
+    };
+    let _ = if let Some(ref s) = num {
+        s
+    } else {
+        &0
+    };
+    let _ = if let Some(mut s) = num {
+        s += 1;
+        s
+    } else {
+        0
+    };
+    let _ = if let Some(ref mut s) = num {
+        *s += 1;
+        s
+    } else {
+        &mut 0
+    };
 }
 
 fn longer_body(arg: Option<u32>) -> u32 {
@@ -69,7 +97,7 @@ fn main() {
     let _ = if let Some(x) = optional { x + 2 } else { 5 };
     let _ = bad1(None);
     let _ = else_if_option(None);
-    let _ = unop_bad(&None);
+    unop_bad(&None, None);
     let _ = longer_body(None);
     test_map_or_else(None);
     let _ = negative_tests(None);
diff --git a/tests/ui/option_if_let_else.stderr b/tests/ui/option_if_let_else.stderr
index b932fe59759..9240d3edb27 100644
--- a/tests/ui/option_if_let_else.stderr
+++ b/tests/ui/option_if_let_else.stderr
@@ -22,17 +22,100 @@ LL | |     }
    | |_____^ help: try: `{ string.map_or(Some((false, "")), |x| Some((true, x))) }`
 
 error: use Option::map_or instead of an if let/else
-  --> $DIR/option_if_let_else.rs:23:5
+  --> $DIR/option_if_let_else.rs:23:13
    |
-LL | /     if let Some(s) = *string {
+LL |       let _ = if let Some(s) = *string {
+   |  _____________^
 LL | |         s.len()
 LL | |     } else {
 LL | |         0
-LL | |     }
-   | |_____^ help: try: `(*string).map_or(0, |s| s.len())`
+LL | |     };
+   | |_____^ help: try: `string.map_or(0, |s| s.len())`
+
+error: use Option::map_or instead of an if let/else
+  --> $DIR/option_if_let_else.rs:28:13
+   |
+LL |       let _ = if let Some(s) = &num {
+   |  _____________^
+LL | |         s
+LL | |     } else {
+LL | |         &0
+LL | |     };
+   | |_____^ help: try: `num.as_ref().map_or(&0, |s| s)`
+
+error: use Option::map_or instead of an if let/else
+  --> $DIR/option_if_let_else.rs:33:13
+   |
+LL |       let _ = if let Some(s) = &mut num {
+   |  _____________^
+LL | |         *s += 1;
+LL | |         s
+LL | |     } else {
+LL | |         &mut 0
+LL | |     };
+   | |_____^
+   |
+help: try
+   |
+LL |     let _ = num.as_mut().map_or(&mut 0, |s| {
+LL |         *s += 1;
+LL |         s
+LL |     });
+   |
+
+error: use Option::map_or instead of an if let/else
+  --> $DIR/option_if_let_else.rs:39:13
+   |
+LL |       let _ = if let Some(ref s) = num {
+   |  _____________^
+LL | |         s
+LL | |     } else {
+LL | |         &0
+LL | |     };
+   | |_____^ help: try: `num.as_ref().map_or(&0, |s| s)`
+
+error: use Option::map_or instead of an if let/else
+  --> $DIR/option_if_let_else.rs:44:13
+   |
+LL |       let _ = if let Some(mut s) = num {
+   |  _____________^
+LL | |         s += 1;
+LL | |         s
+LL | |     } else {
+LL | |         0
+LL | |     };
+   | |_____^
+   |
+help: try
+   |
+LL |     let _ = num.map_or(0, |mut s| {
+LL |         s += 1;
+LL |         s
+LL |     });
+   |
+
+error: use Option::map_or instead of an if let/else
+  --> $DIR/option_if_let_else.rs:50:13
+   |
+LL |       let _ = if let Some(ref mut s) = num {
+   |  _____________^
+LL | |         *s += 1;
+LL | |         s
+LL | |     } else {
+LL | |         &mut 0
+LL | |     };
+   | |_____^
+   |
+help: try
+   |
+LL |     let _ = num.as_mut().map_or(&mut 0, |s| {
+LL |         *s += 1;
+LL |         s
+LL |     });
+   |
 
 error: use Option::map_or instead of an if let/else
-  --> $DIR/option_if_let_else.rs:31:5
+  --> $DIR/option_if_let_else.rs:59:5
    |
 LL | /     if let Some(x) = arg {
 LL | |         let y = x * x;
@@ -51,7 +134,7 @@ LL |     })
    |
 
 error: use Option::map_or_else instead of an if let/else
-  --> $DIR/option_if_let_else.rs:40:13
+  --> $DIR/option_if_let_else.rs:68:13
    |
 LL |       let _ = if let Some(x) = arg {
    |  _____________^
@@ -74,10 +157,10 @@ LL |     }, |x| x * x * x * x);
    |
 
 error: use Option::map_or instead of an if let/else
-  --> $DIR/option_if_let_else.rs:69:13
+  --> $DIR/option_if_let_else.rs:97:13
    |
 LL |     let _ = if let Some(x) = optional { x + 2 } else { 5 };
    |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `optional.map_or(5, |x| x + 2)`
 
-error: aborting due to 6 previous errors
+error: aborting due to 11 previous errors