match_as_ref: improve diagnostics

clippy_lints/src/matches/match_as_ref.rs

@@ -1,6 +1,7 @@
-use clippy_utils::diagnostics::span_lint_and_sugg;
+use clippy_utils::diagnostics::span_lint_and_then;
 use clippy_utils::res::{MaybeDef, MaybeQPath};
-use clippy_utils::source::snippet_with_applicability;
+use clippy_utils::sugg::Sugg;
+use clippy_utils::ty::option_arg_ty;
 use clippy_utils::{is_none_arm, peel_blocks};
 use rustc_errors::Applicability;
 use rustc_hir::{Arm, BindingMode, ByRef, Expr, ExprKind, LangItem, Mutability, PatKind, QPath};
@@ -10,54 +11,58 @@ use rustc_middle::ty;
 use super::MATCH_AS_REF;
 
 pub(crate) fn check(cx: &LateContext<'_>, ex: &Expr<'_>, arms: &[Arm<'_>], expr: &Expr<'_>) {
-    if arms.len() == 2 && arms[0].guard.is_none() && arms[1].guard.is_none() {
-        let arm_ref_mut = if is_none_arm(cx, &arms[0]) {
-            is_ref_some_arm(cx, &arms[1])
-        } else if is_none_arm(cx, &arms[1]) {
-            is_ref_some_arm(cx, &arms[0])
+    if let [arm1, arm2] = arms
+        && arm1.guard.is_none()
+        && arm2.guard.is_none()
+        && let Some(arm_ref_mutbl) = if is_none_arm(cx, arm1) {
+            as_ref_some_arm(cx, arm2)
+        } else if is_none_arm(cx, arm2) {
+            as_ref_some_arm(cx, arm1)
         } else {
             None
+        }
+    {
+        let method = match arm_ref_mutbl {
+            Mutability::Not => "as_ref",
+            Mutability::Mut => "as_mut",
         };
-        if let Some(rb) = arm_ref_mut {
-            let suggestion = match rb {
-                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 output_ty = cx.typeck_results().expr_ty(expr);
+        let input_ty = cx.typeck_results().expr_ty(ex);
 
-            let cast = if let ty::Adt(_, args) = input_ty.kind()
-                && let input_ty = args.type_at(0)
-                && let ty::Adt(_, args) = output_ty.kind()
-                && let output_ty = args.type_at(0)
-                && let ty::Ref(_, output_ty, _) = *output_ty.kind()
-                && input_ty != output_ty
-            {
-                ".map(|x| x as _)"
-            } else {
-                ""
-            };
+        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 mut applicability = Applicability::MachineApplicable;
-            span_lint_and_sugg(
-                cx,
-                MATCH_AS_REF,
-                expr.span,
-                format!("use `{suggestion}()` instead"),
-                "try",
-                format!(
-                    "{}.{suggestion}(){cast}",
-                    snippet_with_applicability(cx, ex.span, "_", &mut applicability),
-                ),
-                applicability,
-            );
-        }
+        let mut applicability = Applicability::MachineApplicable;
+        span_lint_and_then(
+            cx,
+            MATCH_AS_REF,
+            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,
+                );
+            },
+        );
     }
 }
 
 // Checks if arm has the form `Some(ref v) => Some(v)` (checks for `ref` and `ref mut`)
-fn is_ref_some_arm(cx: &LateContext<'_>, arm: &Arm<'_>) -> Option<Mutability> {
+fn as_ref_some_arm(cx: &LateContext<'_>, arm: &Arm<'_>) -> Option<Mutability> {
     if let PatKind::TupleStruct(ref qpath, [first_pat, ..], _) = arm.pat.kind
         && cx
             .qpath_res(qpath, arm.pat.hir_id)

tests/ui/match_as_ref.fixed

@@ -71,3 +71,16 @@ mod issue15691 {
         };
     }
 }
+
+fn recv_requiring_parens() {
+    struct S;
+
+    impl std::ops::Not for S {
+        type Output = Option<u64>;
+        fn not(self) -> Self::Output {
+            None
+        }
+    }
+
+    let _ = (!S).as_ref();
+}

tests/ui/match_as_ref.rs

@@ -83,3 +83,20 @@ mod issue15691 {
         };
     }
 }
+
+fn recv_requiring_parens() {
+    struct S;
+
+    impl std::ops::Not for S {
+        type Output = Option<u64>;
+        fn not(self) -> Self::Output {
+            None
+        }
+    }
+
+    let _ = match !S {
+        //~^ match_as_ref
+        None => None,
+        Some(ref v) => Some(v),
+    };
+}

tests/ui/match_as_ref.stderr

@@ -1,4 +1,4 @@
-error: use `as_ref()` instead
+error: manual implementation of `Option::as_ref`
   --> tests/ui/match_as_ref.rs:6:33
    |
 LL |       let borrowed: Option<&()> = match owned {
@@ -7,12 +7,21 @@ LL | |
 LL | |         None => None,
 LL | |         Some(ref v) => Some(v),
 LL | |     };
-   | |_____^ help: try: `owned.as_ref()`
+   | |_____^
    |
    = note: `-D clippy::match-as-ref` implied by `-D warnings`
    = help: to override `-D warnings` add `#[allow(clippy::match_as_ref)]`
+help: use `Option::as_ref()` directly
+   |
+LL -     let borrowed: Option<&()> = match owned {
+LL -
+LL -         None => None,
+LL -         Some(ref v) => Some(v),
+LL -     };
+LL +     let borrowed: Option<&()> = owned.as_ref();
+   |
 
-error: use `as_mut()` instead
+error: manual implementation of `Option::as_mut`
   --> tests/ui/match_as_ref.rs:13:39
    |
 LL |       let borrow_mut: Option<&mut ()> = match mut_owned {
@@ -21,17 +30,58 @@ LL | |
 LL | |         None => None,
 LL | |         Some(ref mut v) => Some(v),
 LL | |     };
-   | |_____^ help: try: `mut_owned.as_mut()`
+   | |_____^
+   |
+help: use `Option::as_mut()` directly
+   |
+LL -     let borrow_mut: Option<&mut ()> = match mut_owned {
+LL -
+LL -         None => None,
+LL -         Some(ref mut v) => Some(v),
+LL -     };
+LL +     let borrow_mut: Option<&mut ()> = mut_owned.as_mut();
+   |
 
-error: use `as_ref()` instead
+error: manual implementation of `Option::as_ref`
   --> tests/ui/match_as_ref.rs:32:13
    |
 LL | /             match self.source {
 LL | |
 LL | |                 Some(ref s) => Some(s),
 LL | |                 None => None,
 LL | |             }
-   | |_____________^ help: try: `self.source.as_ref().map(|x| x as _)`
+   | |_____________^
+   |
+help: use `Option::as_ref()` directly
+   |
+LL -             match self.source {
+LL -
+LL -                 Some(ref s) => Some(s),
+LL -                 None => None,
+LL -             }
+LL +             self.source.as_ref().map(|x| x as _)
+   |
+
+error: manual implementation of `Option::as_ref`
+  --> tests/ui/match_as_ref.rs:97:13
+   |
+LL |       let _ = match !S {
+   |  _____________^
+LL | |
+LL | |         None => None,
+LL | |         Some(ref v) => Some(v),
+LL | |     };
+   | |_____^
+   |
+help: use `Option::as_ref()` directly
+   |
+LL -     let _ = match !S {
+LL -
+LL -         None => None,
+LL -         Some(ref v) => Some(v),
+LL -     };
+LL +     let _ = (!S).as_ref();
+   |
 
-error: aborting due to 3 previous errors
+error: aborting due to 4 previous errors