fix: `manual_ok_err` suggests wrongly with references

4 files changed, 108 insertions, 4 deletions

diff --git a/clippy_lints/src/matches/manual_ok_err.rs b/clippy_lints/src/matches/manual_ok_err.rs
index 4959908dad6..edbb556fd97 100644
--- a/clippy_lints/src/matches/manual_ok_err.rs
+++ b/clippy_lints/src/matches/manual_ok_err.rs
@@ -1,9 +1,9 @@
 use clippy_utils::diagnostics::span_lint_and_sugg;
 use clippy_utils::source::{indent_of, reindent_multiline};
 use clippy_utils::sugg::Sugg;
-use clippy_utils::ty::option_arg_ty;
+use clippy_utils::ty::{option_arg_ty, peel_mid_ty_refs_is_mutable};
 use clippy_utils::{get_parent_expr, is_res_lang_ctor, path_res, peel_blocks, span_contains_comment};
-use rustc_ast::BindingMode;
+use rustc_ast::{BindingMode, Mutability};
 use rustc_errors::Applicability;
 use rustc_hir::LangItem::{OptionNone, OptionSome, ResultErr};
 use rustc_hir::def::{DefKind, Res};
@@ -133,7 +133,21 @@ fn apply_lint(cx: &LateContext<'_>, expr: &Expr<'_>, scrutinee: &Expr<'_>, is_ok
         Applicability::MachineApplicable
     };
     let scrut = Sugg::hir_with_applicability(cx, scrutinee, "..", &mut app).maybe_paren();
-    let sugg = format!("{scrut}.{method}()");
+
+    let scrutinee_ty = cx.typeck_results().expr_ty(scrutinee);
+    let (_, n_ref, mutability) = peel_mid_ty_refs_is_mutable(scrutinee_ty);
+    let prefix = if n_ref > 0 {
+        if mutability == Mutability::Mut {
+            ".as_mut()"
+        } else {
+            ".as_ref()"
+        }
+    } else {
+        ""
+    };
+
+    let sugg = format!("{scrut}{prefix}.{method}()");
+
     // If the expression being expanded is the `if …` part of an `else if …`, it must be blockified.
     let sugg = if let Some(parent_expr) = get_parent_expr(cx, expr)
         && let ExprKind::If(_, _, Some(else_part)) = parent_expr.kind
diff --git a/tests/ui/manual_ok_err.fixed b/tests/ui/manual_ok_err.fixed
index e6f799aa58d..9b70ce0df43 100644
--- a/tests/ui/manual_ok_err.fixed
+++ b/tests/ui/manual_ok_err.fixed
@@ -103,3 +103,27 @@ fn issue14239() {
     };
     //~^^^^^ manual_ok_err
 }
+
+mod issue15051 {
+    struct Container {
+        field: Result<bool, ()>,
+    }
+
+    #[allow(clippy::needless_borrow)]
+    fn with_addr_of(x: &Container) -> Option<&bool> {
+        (&x.field).as_ref().ok()
+    }
+
+    fn from_fn(x: &Container) -> Option<&bool> {
+        let result_with_ref = || &x.field;
+        result_with_ref().as_ref().ok()
+    }
+
+    fn result_with_ref_mut(x: &mut Container) -> &mut Result<bool, ()> {
+        &mut x.field
+    }
+
+    fn from_fn_mut(x: &mut Container) -> Option<&mut bool> {
+        result_with_ref_mut(x).as_mut().ok()
+    }
+}
diff --git a/tests/ui/manual_ok_err.rs b/tests/ui/manual_ok_err.rs
index 972b2c41ee7..dee90463824 100644
--- a/tests/ui/manual_ok_err.rs
+++ b/tests/ui/manual_ok_err.rs
@@ -141,3 +141,39 @@ fn issue14239() {
     };
     //~^^^^^ manual_ok_err
 }
+
+mod issue15051 {
+    struct Container {
+        field: Result<bool, ()>,
+    }
+
+    #[allow(clippy::needless_borrow)]
+    fn with_addr_of(x: &Container) -> Option<&bool> {
+        match &x.field {
+            //~^ manual_ok_err
+            Ok(panel) => Some(panel),
+            Err(_) => None,
+        }
+    }
+
+    fn from_fn(x: &Container) -> Option<&bool> {
+        let result_with_ref = || &x.field;
+        match result_with_ref() {
+            //~^ manual_ok_err
+            Ok(panel) => Some(panel),
+            Err(_) => None,
+        }
+    }
+
+    fn result_with_ref_mut(x: &mut Container) -> &mut Result<bool, ()> {
+        &mut x.field
+    }
+
+    fn from_fn_mut(x: &mut Container) -> Option<&mut bool> {
+        match result_with_ref_mut(x) {
+            //~^ manual_ok_err
+            Ok(panel) => Some(panel),
+            Err(_) => None,
+        }
+    }
+}
diff --git a/tests/ui/manual_ok_err.stderr b/tests/ui/manual_ok_err.stderr
index 040e170f397..448fbffc050 100644
--- a/tests/ui/manual_ok_err.stderr
+++ b/tests/ui/manual_ok_err.stderr
@@ -111,5 +111,35 @@ LL +         "1".parse::<u8>().ok()
 LL ~     };
    |
 
-error: aborting due to 9 previous errors
+error: manual implementation of `ok`
+  --> tests/ui/manual_ok_err.rs:152:9
+   |
+LL | /         match &x.field {
+LL | |
+LL | |             Ok(panel) => Some(panel),
+LL | |             Err(_) => None,
+LL | |         }
+   | |_________^ help: replace with: `(&x.field).as_ref().ok()`
+
+error: manual implementation of `ok`
+  --> tests/ui/manual_ok_err.rs:161:9
+   |
+LL | /         match result_with_ref() {
+LL | |
+LL | |             Ok(panel) => Some(panel),
+LL | |             Err(_) => None,
+LL | |         }
+   | |_________^ help: replace with: `result_with_ref().as_ref().ok()`
+
+error: manual implementation of `ok`
+  --> tests/ui/manual_ok_err.rs:173:9
+   |
+LL | /         match result_with_ref_mut(x) {
+LL | |
+LL | |             Ok(panel) => Some(panel),
+LL | |             Err(_) => None,
+LL | |         }
+   | |_________^ help: replace with: `result_with_ref_mut(x).as_mut().ok()`
+
+error: aborting due to 12 previous errors