fix(match_as_ref): suggest as_ref when the reference needs to be cast (#15934)

I first implemented the naive version, which just talks about, and
suggests using, `Option::as_ref`. But the resulting error message
sounded a bit misleading -- after all, the manual implementation was
factually that of `Option::as_mut`, and we only suggest `as_ref` as a
means of downcasting. So then I added another help message to mention
the need for reference downcasting, which.. still looks awkward.

Honestly it might be the easiest to hide the reference downcasting into
the `.map` after all, so that it encapsulates _all_ the casting needed
following the switch to `as_ref`/`as_mut`.

changelog: [`match_as_ref`]: suggest `as_ref` when the reference needs
to be cast

r? @samueltardieu

Author: samueltardieu

clippy_lints/src/matches/match_as_ref.rs

@@ -21,24 +21,30 @@ pub(crate) fn check(cx: &LateContext<'_>, ex: &Expr<'_>, arms: &[Arm<'_>], expr:
         } else {
             None
         }
+        && let output_ty = cx.typeck_results().expr_ty(expr)
+        && let input_ty = cx.typeck_results().expr_ty(ex)
+        && let Some(input_ty) = option_arg_ty(cx, input_ty)
+        && let Some(output_ty) = option_arg_ty(cx, output_ty)
+        && let ty::Ref(_, output_ty, output_mutbl) = *output_ty.kind()
     {
         let method = match arm_ref_mutbl {
             Mutability::Not => "as_ref",
             Mutability::Mut => "as_mut",
         };
 
-        let output_ty = cx.typeck_results().expr_ty(expr);
-        let input_ty = cx.typeck_results().expr_ty(ex);
+        // ```
+        // let _: Option<&T> = match opt {
+        //     Some(ref mut t) => Some(t),
+        //     None => None,
+        // };
+        // ```
+        // We need to suggest `t.as_ref()` in order downcast the reference from `&mut` to `&`.
+        // We may or may not need to cast the type as well, for which we'd need `.map()`, and that could
+        // theoretically take care of the reference downcasting as well, but we chose to keep these two
+        // operations separate
+        let need_as_ref = arm_ref_mutbl == Mutability::Mut && output_mutbl == Mutability::Not;
 
-        let cast = if let Some(input_ty) = option_arg_ty(cx, input_ty)
-            && let Some(output_ty) = option_arg_ty(cx, output_ty)
-            && let ty::Ref(_, output_ty, _) = *output_ty.kind()
-            && input_ty != output_ty
-        {
-            ".map(|x| x as _)"
-        } else {
-            ""
-        };
+        let cast = if input_ty == output_ty { "" } else { ".map(|x| x as _)" };
 
         let mut applicability = Applicability::MachineApplicable;
         span_lint_and_then(
@@ -47,15 +53,28 @@ pub(crate) fn check(cx: &LateContext<'_>, ex: &Expr<'_>, arms: &[Arm<'_>], expr:
             expr.span,
             format!("manual implementation of `Option::{method}`"),
             |diag| {
-                diag.span_suggestion_verbose(
-                    expr.span,
-                    format!("use `Option::{method}()` directly"),
-                    format!(
-                        "{}.{method}(){cast}",
-                        Sugg::hir_with_applicability(cx, ex, "_", &mut applicability).maybe_paren(),
-                    ),
-                    applicability,
-                );
+                if need_as_ref {
+                    diag.note("but the type is coerced to a non-mutable reference, and so `as_ref` can used instead");
+                    diag.span_suggestion_verbose(
+                        expr.span,
+                        "use `Option::as_ref()`",
+                        format!(
+                            "{}.as_ref(){cast}",
+                            Sugg::hir_with_applicability(cx, ex, "_", &mut applicability).maybe_paren(),
+                        ),
+                        applicability,
+                    );
+                } else {
+                    diag.span_suggestion_verbose(
+                        expr.span,
+                        format!("use `Option::{method}()` directly"),
+                        format!(
+                            "{}.{method}(){cast}",
+                            Sugg::hir_with_applicability(cx, ex, "_", &mut applicability).maybe_paren(),
+                        ),
+                        applicability,
+                    );
+                }
             },
         );
     }

tests/ui/match_as_ref.fixed

@@ -84,3 +84,9 @@ fn recv_requiring_parens() {
 
     let _ = (!S).as_ref();
 }
+
+fn issue15932() {
+    let _: Option<&u32> = Some(0).as_ref();
+
+    let _: Option<&dyn std::fmt::Debug> = Some(0).as_ref().map(|x| x as _);
+}

tests/ui/match_as_ref.rs

@@ -100,3 +100,17 @@ fn recv_requiring_parens() {
         Some(ref v) => Some(v),
     };
 }
+
+fn issue15932() {
+    let _: Option<&u32> = match Some(0) {
+        //~^ match_as_ref
+        None => None,
+        Some(ref mut v) => Some(v),
+    };
+
+    let _: Option<&dyn std::fmt::Debug> = match Some(0) {
+        //~^ match_as_ref
+        None => None,
+        Some(ref mut v) => Some(v),
+    };
+}

tests/ui/match_as_ref.stderr

@@ -83,5 +83,49 @@ LL -     };
 LL +     let _ = (!S).as_ref();
    |
 
-error: aborting due to 4 previous errors
+error: manual implementation of `Option::as_mut`
+  --> tests/ui/match_as_ref.rs:105:27
+   |
+LL |       let _: Option<&u32> = match Some(0) {
+   |  ___________________________^
+LL | |
+LL | |         None => None,
+LL | |         Some(ref mut v) => Some(v),
+LL | |     };
+   | |_____^
+   |
+   = note: but the type is coerced to a non-mutable reference, and so `as_ref` can used instead
+help: use `Option::as_ref()`
+   |
+LL -     let _: Option<&u32> = match Some(0) {
+LL -
+LL -         None => None,
+LL -         Some(ref mut v) => Some(v),
+LL -     };
+LL +     let _: Option<&u32> = Some(0).as_ref();
+   |
+
+error: manual implementation of `Option::as_mut`
+  --> tests/ui/match_as_ref.rs:111:43
+   |
+LL |       let _: Option<&dyn std::fmt::Debug> = match Some(0) {
+   |  ___________________________________________^
+LL | |
+LL | |         None => None,
+LL | |         Some(ref mut v) => Some(v),
+LL | |     };
+   | |_____^
+   |
+   = note: but the type is coerced to a non-mutable reference, and so `as_ref` can used instead
+help: use `Option::as_ref()`
+   |
+LL -     let _: Option<&dyn std::fmt::Debug> = match Some(0) {
+LL -
+LL -         None => None,
+LL -         Some(ref mut v) => Some(v),
+LL -     };
+LL +     let _: Option<&dyn std::fmt::Debug> = Some(0).as_ref().map(|x| x as _);
+   |
+
+error: aborting due to 6 previous errors