fix: collapsible_match suggests ref/derefs when needed

Author: vishruth-thimmaiah

clippy_lints/src/matches/collapsible_match.rs

@@ -4,20 +4,22 @@ use clippy_utils::msrvs::Msrv;
 use clippy_utils::source::snippet;
 use clippy_utils::visitors::is_local_used;
 use clippy_utils::{
-    SpanlessEq, is_res_lang_ctor, is_unit_expr, path_to_local, peel_blocks_with_stmt, peel_ref_operators,
+    SpanlessEq, get_ref_operators, is_res_lang_ctor, is_unit_expr, path_to_local, peel_blocks_with_stmt,
+    peel_ref_operators,
 };
+use rustc_ast::BorrowKind;
 use rustc_errors::MultiSpan;
 use rustc_hir::LangItem::OptionNone;
-use rustc_hir::{Arm, Expr, HirId, Pat, PatExpr, PatExprKind, PatKind};
+use rustc_hir::{Arm, Expr, ExprKind, HirId, Pat, PatExpr, PatExprKind, PatKind};
 use rustc_lint::LateContext;
 use rustc_span::Span;
 
 use super::{COLLAPSIBLE_MATCH, pat_contains_disallowed_or};
 
-pub(super) fn check_match<'tcx>(cx: &LateContext<'tcx>, arms: &'tcx [Arm<'_>], msrv: Msrv) {
+pub(super) fn check_match<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, arms: &'tcx [Arm<'_>], msrv: Msrv) {
     if let Some(els_arm) = arms.iter().rfind(|arm| arm_is_wild_like(cx, arm)) {
         for arm in arms {
-            check_arm(cx, true, arm.pat, arm.body, arm.guard, Some(els_arm.body), msrv);
+            check_arm(cx, true, arm.pat, expr, arm.body, arm.guard, Some(els_arm.body), msrv);
         }
     }
 }
@@ -27,15 +29,18 @@ pub(super) fn check_if_let<'tcx>(
     pat: &'tcx Pat<'_>,
     body: &'tcx Expr<'_>,
     else_expr: Option<&'tcx Expr<'_>>,
+    let_expr: &'tcx Expr<'_>,
     msrv: Msrv,
 ) {
-    check_arm(cx, false, pat, body, None, else_expr, msrv);
+    check_arm(cx, false, pat, let_expr, body, None, else_expr, msrv);
 }
 
+#[allow(clippy::too_many_arguments)]
 fn check_arm<'tcx>(
     cx: &LateContext<'tcx>,
     outer_is_match: bool,
     outer_pat: &'tcx Pat<'tcx>,
+    outer_cond: &'tcx Expr<'tcx>,
     outer_then_body: &'tcx Expr<'tcx>,
     outer_guard: Option<&'tcx Expr<'tcx>>,
     outer_else_body: Option<&'tcx Expr<'tcx>>,
@@ -82,6 +87,9 @@ fn check_arm<'tcx>(
             },
             IfLetOrMatch::Match(_, arms, ..) => !arms.iter().any(|arm| is_local_used(cx, arm, binding_id)),
         }
+        // Check if the inner expression contains any borrows/dereferences
+        && let ref_types = get_ref_operators(cx, inner_scrutinee)
+        && let Some(method) = build_ref_method_chain(ref_types)
     {
         let msg = format!(
             "this `{}` can be collapsed into the outer `{}`",
@@ -103,6 +111,10 @@ fn check_arm<'tcx>(
             let mut help_span = MultiSpan::from_spans(vec![binding_span, inner_then_pat.span]);
             help_span.push_span_label(binding_span, "replace this binding");
             help_span.push_span_label(inner_then_pat.span, format!("with this pattern{replace_msg}"));
+            if !method.is_empty() {
+                let outer_cond_msg = format!("use: `{}{}`", snippet(cx, outer_cond.span, ".."), method);
+                help_span.push_span_label(outer_cond.span, outer_cond_msg);
+            }
             diag.span_help(
                 help_span,
                 "the outer pattern can be modified to include the inner pattern",
@@ -148,3 +160,30 @@ fn find_pat_binding_and_is_innermost_parent_pat_struct(pat: &Pat<'_>, hir_id: Hi
     });
     (span, is_innermost_parent_pat_struct)
 }
+
+/// Builds a chain of reference-manipulation method calls (e.g., `.as_ref()`, `.as_mut()`,
+/// `.copied()`) based on reference operators
+fn build_ref_method_chain(expr: Vec<&Expr<'_>>) -> Option<String> {
+    let mut req_method_calls = String::new();
+
+    for ref_operator in expr {
+        match ref_operator.kind {
+            ExprKind::AddrOf(BorrowKind::Raw, _, _) => {
+                return None;
+            },
+            ExprKind::AddrOf(_, m, _) if m.is_mut() => {
+                req_method_calls.push_str(".as_mut()");
+            },
+            ExprKind::AddrOf(_, _, _) => {
+                req_method_calls.push_str(".as_ref()");
+            },
+            // Deref operator is the only operator that this function should have received
+            ExprKind::Unary(_, _) => {
+                req_method_calls.push_str(".copied()");
+            },
+            _ => (),
+        }
+    }
+
+    Some(req_method_calls)
+}

clippy_lints/src/matches/mod.rs

@@ -1073,7 +1073,7 @@ impl<'tcx> LateLintPass<'tcx> for Matches {
                 significant_drop_in_scrutinee::check_match(cx, expr, ex, arms, source);
             }
 
-            collapsible_match::check_match(cx, arms, self.msrv);
+            collapsible_match::check_match(cx, ex, arms, self.msrv);
             if !from_expansion {
                 // These don't depend on a relationship between multiple arms
                 match_wild_err_arm::check(cx, ex, arms);
@@ -1137,7 +1137,14 @@ impl<'tcx> LateLintPass<'tcx> for Matches {
                 match_ref_pats::check(cx, ex, arms.iter().map(|el| el.pat), expr);
             }
         } else if let Some(if_let) = higher::IfLet::hir(cx, expr) {
-            collapsible_match::check_if_let(cx, if_let.let_pat, if_let.if_then, if_let.if_else, self.msrv);
+            collapsible_match::check_if_let(
+                cx,
+                if_let.let_pat,
+                if_let.if_then,
+                if_let.if_else,
+                if_let.let_expr,
+                self.msrv,
+            );
             significant_drop_in_scrutinee::check_if_let(cx, expr, if_let.let_expr, if_let.if_then, if_let.if_else);
             if !from_expansion {
                 if let Some(else_expr) = if_let.if_else {

clippy_utils/src/lib.rs

@@ -2404,6 +2404,24 @@ pub fn peel_ref_operators<'hir>(cx: &LateContext<'_>, mut expr: &'hir Expr<'hir>
     expr
 }
 
+/// Returns a `Vec` of `Expr`s containing `AddrOf` operators (`&`) or deref operators (`*`) of a
+/// given expression.
+pub fn get_ref_operators<'hir>(cx: &LateContext<'_>, expr: &'hir Expr<'hir>) -> Vec<&'hir Expr<'hir>> {
+    let mut operators = Vec::new();
+    peel_hir_expr_while(expr, |expr| match expr.kind {
+        ExprKind::AddrOf(_, _, e) => {
+            operators.push(expr);
+            Some(e)
+        },
+        ExprKind::Unary(UnOp::Deref, e) if cx.typeck_results().expr_ty(e).is_ref() => {
+            operators.push(expr);
+            Some(e)
+        },
+        _ => None,
+    });
+    operators
+}
+
 pub fn is_hir_ty_cfg_dependant(cx: &LateContext<'_>, ty: &hir::Ty<'_>) -> bool {
     if let TyKind::Path(QPath::Resolved(_, path)) = ty.kind
         && let Res::Def(_, def_id) = path.res

tests/ui/collapsible_match.rs

@@ -316,6 +316,47 @@ fn lint_emitted_at_right_node(opt: Option<Result<u64, String>>) {
     };
 }
 
+pub fn issue_14155() {
+    let mut arr = ["a", "b", "c"];
+    if let Some(last) = arr.last() {
+        match *last {
+            //~^ collapsible_match
+            "a" | "b" => {
+                unimplemented!()
+            },
+            _ => (),
+        }
+    }
+
+    if let Some(last) = arr.last() {
+        match &last {
+            //~^ collapsible_match
+            &&"a" | &&"b" => {
+                unimplemented!()
+            },
+            _ => (),
+        }
+    }
+
+    if let Some(mut last) = arr.last_mut() {
+        match &mut last {
+            //~^ collapsible_match
+            &mut &mut "a" | &mut &mut "b" => {
+                unimplemented!()
+            },
+            _ => (),
+        }
+    }
+
+    const NULL_PTR: *const &'static str = std::ptr::null();
+    if let Some(last) = arr.last() {
+        match &raw const *last {
+            NULL_PTR => unimplemented!(),
+            _ => (),
+        }
+    }
+}
+
 fn make<T>() -> T {
     unimplemented!()
 }

tests/ui/collapsible_match.stderr

@@ -250,5 +250,74 @@ LL |     if let Issue9647::A { a: Some(a), .. } = x {
 LL |         if let Some(u) = a {
    |                ^^^^^^^ with this pattern
 
-error: aborting due to 13 previous errors
+error: this `match` can be collapsed into the outer `if let`
+  --> tests/ui/collapsible_match.rs:322:9
+   |
+LL | /         match *last {
+LL | |
+LL | |             "a" | "b" => {
+LL | |                 unimplemented!()
+LL | |             },
+LL | |             _ => (),
+LL | |         }
+   | |_________^
+   |
+help: the outer pattern can be modified to include the inner pattern
+  --> tests/ui/collapsible_match.rs:321:17
+   |
+LL |     if let Some(last) = arr.last() {
+   |                 ^^^^    ---------- use: `arr.last().copied()`
+   |                 |
+   |                 replace this binding
+...
+LL |             "a" | "b" => {
+   |             ^^^^^^^^^ with this pattern
+
+error: this `match` can be collapsed into the outer `if let`
+  --> tests/ui/collapsible_match.rs:332:9
+   |
+LL | /         match &last {
+LL | |
+LL | |             &&"a" | &&"b" => {
+LL | |                 unimplemented!()
+LL | |             },
+LL | |             _ => (),
+LL | |         }
+   | |_________^
+   |
+help: the outer pattern can be modified to include the inner pattern
+  --> tests/ui/collapsible_match.rs:331:17
+   |
+LL |     if let Some(last) = arr.last() {
+   |                 ^^^^    ---------- use: `arr.last().as_ref()`
+   |                 |
+   |                 replace this binding
+...
+LL |             &&"a" | &&"b" => {
+   |             ^^^^^^^^^^^^^ with this pattern
+
+error: this `match` can be collapsed into the outer `if let`
+  --> tests/ui/collapsible_match.rs:342:9
+   |
+LL | /         match &mut last {
+LL | |
+LL | |             &mut &mut "a" | &mut &mut "b" => {
+LL | |                 unimplemented!()
+LL | |             },
+LL | |             _ => (),
+LL | |         }
+   | |_________^
+   |
+help: the outer pattern can be modified to include the inner pattern
+  --> tests/ui/collapsible_match.rs:341:17
+   |
+LL |     if let Some(mut last) = arr.last_mut() {
+   |                 ^^^^^^^^    -------------- use: `arr.last_mut().as_mut()`
+   |                 |
+   |                 replace this binding
+...
+LL |             &mut &mut "a" | &mut &mut "b" => {
+   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ with this pattern
+
+error: aborting due to 16 previous errors
 

tests/ui/collapsible_match2.stderr

@@ -77,6 +77,8 @@ LL | |         },
 help: the outer pattern can be modified to include the inner pattern
   --> tests/ui/collapsible_match2.rs:54:14
    |
+LL |     match Some(&[1]) {
+   |           ---------- use: `Some(&[1]).copied()`
 LL |         Some(s) => match *s {
    |              ^ replace this binding
 LL |