From 39bde6d46a5c7dbebdc66afb8ac166a4e9087231 Mon Sep 17 00:00:00 2001
From: Samuel Tardieu <sam@rfc1149.net>
Date: Wed, 12 Feb 2025 17:15:27 +0100
Subject: [PATCH] Reorganize code in `mem_replace.rs`

Make each lint be more self-contained, and trigger only the next lint
when the previous one didn't already rewrite the expression.
---
 clippy_lints/src/mem_replace.rs | 86 +++++++++++++++++++--------------
 1 file changed, 50 insertions(+), 36 deletions(-)

diff --git a/clippy_lints/src/mem_replace.rs b/clippy_lints/src/mem_replace.rs
index 41528c5dee3a..4f3279d292d1 100644
--- a/clippy_lints/src/mem_replace.rs
+++ b/clippy_lints/src/mem_replace.rs
@@ -103,26 +103,31 @@ declare_clippy_lint! {
 impl_lint_pass!(MemReplace =>
     [MEM_REPLACE_OPTION_WITH_NONE, MEM_REPLACE_WITH_UNINIT, MEM_REPLACE_WITH_DEFAULT]);
 
-fn check_replace_option_with_none(cx: &LateContext<'_>, dest: &Expr<'_>, expr_span: Span) {
-    // Since this is a late pass (already type-checked),
-    // and we already know that the second argument is an
-    // `Option`, we do not need to check the first
-    // argument's type. All that's left is to get
-    // the replacee's expr after peeling off the `&mut`
-    let sugg_expr = peel_ref_operators(cx, dest);
-    let mut applicability = Applicability::MachineApplicable;
-    span_lint_and_sugg(
-        cx,
-        MEM_REPLACE_OPTION_WITH_NONE,
-        expr_span,
-        "replacing an `Option` with `None`",
-        "consider `Option::take()` instead",
-        format!(
-            "{}.take()",
-            Sugg::hir_with_context(cx, sugg_expr, expr_span.ctxt(), "", &mut applicability).maybe_par()
-        ),
-        applicability,
-    );
+fn check_replace_option_with_none(cx: &LateContext<'_>, src: &Expr<'_>, dest: &Expr<'_>, expr_span: Span) -> bool {
+    if is_res_lang_ctor(cx, path_res(cx, src), OptionNone) {
+        // Since this is a late pass (already type-checked),
+        // and we already know that the second argument is an
+        // `Option`, we do not need to check the first
+        // argument's type. All that's left is to get
+        // the replacee's expr after peeling off the `&mut`
+        let sugg_expr = peel_ref_operators(cx, dest);
+        let mut applicability = Applicability::MachineApplicable;
+        span_lint_and_sugg(
+            cx,
+            MEM_REPLACE_OPTION_WITH_NONE,
+            expr_span,
+            "replacing an `Option` with `None`",
+            "consider `Option::take()` instead",
+            format!(
+                "{}.take()",
+                Sugg::hir_with_context(cx, sugg_expr, expr_span.ctxt(), "", &mut applicability).maybe_par()
+            ),
+            applicability,
+        );
+        true
+    } else {
+        false
+    }
 }
 
 fn check_replace_with_uninit(cx: &LateContext<'_>, src: &Expr<'_>, dest: &Expr<'_>, expr_span: Span) {
@@ -181,27 +186,34 @@ fn check_replace_with_uninit(cx: &LateContext<'_>, src: &Expr<'_>, dest: &Expr<'
     }
 }
 
-fn check_replace_with_default(cx: &LateContext<'_>, src: &Expr<'_>, dest: &Expr<'_>, expr_span: Span) {
-    // disable lint for primitives
-    let expr_type = cx.typeck_results().expr_ty_adjusted(src);
-    if is_non_aggregate_primitive_type(expr_type) {
-        return;
-    }
-    if is_default_equivalent(cx, src) && !expr_span.in_external_macro(cx.tcx.sess.source_map()) {
-        let Some(top_crate) = std_or_core(cx) else { return };
+fn check_replace_with_default(
+    cx: &LateContext<'_>,
+    src: &Expr<'_>,
+    dest: &Expr<'_>,
+    expr: &Expr<'_>,
+    msrv: &Msrv,
+) -> bool {
+    if msrv.meets(msrvs::MEM_TAKE) && is_expr_used_or_unified(cx.tcx, expr)
+        // disable lint for primitives
+        && let expr_type = cx.typeck_results().expr_ty_adjusted(src)
+        && !is_non_aggregate_primitive_type(expr_type)
+        && is_default_equivalent(cx, src)
+        && !expr.span.in_external_macro(cx.tcx.sess.source_map())
+        && let Some(top_crate) = std_or_core(cx)
+    {
         span_lint_and_then(
             cx,
             MEM_REPLACE_WITH_DEFAULT,
-            expr_span,
+            expr.span,
             format!(
                 "replacing a value of type `T` with `T::default()` is better expressed using `{top_crate}::mem::take`"
             ),
             |diag| {
-                if !expr_span.from_expansion() {
+                if !expr.span.from_expansion() {
                     let suggestion = format!("{top_crate}::mem::take({})", snippet(cx, dest.span, ""));
 
                     diag.span_suggestion(
-                        expr_span,
+                        expr.span,
                         "consider using",
                         suggestion,
                         Applicability::MachineApplicable,
@@ -209,6 +221,9 @@ fn check_replace_with_default(cx: &LateContext<'_>, src: &Expr<'_>, dest: &Expr<
                 }
             },
         );
+        true
+    } else {
+        false
     }
 }
 
@@ -233,12 +248,11 @@ impl<'tcx> LateLintPass<'tcx> for MemReplace {
             && cx.tcx.is_diagnostic_item(sym::mem_replace, def_id)
         {
             // Check that second argument is `Option::None`
-            if is_res_lang_ctor(cx, path_res(cx, src), OptionNone) {
-                check_replace_option_with_none(cx, dest, expr.span);
-            } else if self.msrv.meets(msrvs::MEM_TAKE) && is_expr_used_or_unified(cx.tcx, expr) {
-                check_replace_with_default(cx, src, dest, expr.span);
+            if !check_replace_option_with_none(cx, src, dest, expr.span)
+                && !check_replace_with_default(cx, src, dest, expr, &self.msrv)
+            {
+                check_replace_with_uninit(cx, src, dest, expr.span);
             }
-            check_replace_with_uninit(cx, src, dest, expr.span);
         }
     }
     extract_msrv_attr!(LateContext);