From a9d42e6d6d8ce6fe9789c8d06223b7b2e28ff253 Mon Sep 17 00:00:00 2001
From: Yudai Fukushima <49578068+granddaifuku@users.noreply.github.com>
Date: Wed, 8 Nov 2023 17:03:07 +0000
Subject: [PATCH] fix: reduce [`manual_memcpy`] indexing when array length is
 same to loop range

Format

refactor: extract function to shrink function length

fix: remove cmp to calculate range

fix: replace if_chain with let chains
---
 clippy_lints/src/loops/manual_memcpy.rs       | 47 ++++++++++++++++---
 .../ui/manual_memcpy/without_loop_counters.rs | 20 ++++++++
 .../without_loop_counters.stderr              | 31 +++++++++++-
 3 files changed, 90 insertions(+), 8 deletions(-)

diff --git a/clippy_lints/src/loops/manual_memcpy.rs b/clippy_lints/src/loops/manual_memcpy.rs
index 89f70519c..40d56240b 100644
--- a/clippy_lints/src/loops/manual_memcpy.rs
+++ b/clippy_lints/src/loops/manual_memcpy.rs
@@ -178,7 +178,9 @@ fn build_manual_memcpy_suggestion<'tcx>(
     let dst_base_str = snippet(cx, dst.base.span, "???");
     let src_base_str = snippet(cx, src.base.span, "???");
 
-    let dst = if dst_offset == sugg::EMPTY && dst_limit == sugg::EMPTY {
+    let dst = if (dst_offset == sugg::EMPTY && dst_limit == sugg::EMPTY)
+        || is_array_length_equal_to_range(cx, start, end, dst.base)
+    {
         dst_base_str
     } else {
         format!("{dst_base_str}[{}..{}]", dst_offset.maybe_par(), dst_limit.maybe_par()).into()
@@ -190,11 +192,13 @@ fn build_manual_memcpy_suggestion<'tcx>(
         "clone_from_slice"
     };
 
-    format!(
-        "{dst}.{method_str}(&{src_base_str}[{}..{}]);",
-        src_offset.maybe_par(),
-        src_limit.maybe_par()
-    )
+    let src = if is_array_length_equal_to_range(cx, start, end, src.base) {
+        src_base_str
+    } else {
+        format!("{src_base_str}[{}..{}]", src_offset.maybe_par(), src_limit.maybe_par()).into()
+    };
+
+    format!("{dst}.{method_str}(&{src});")
 }
 
 /// a wrapper of `Sugg`. Besides what `Sugg` do, this removes unnecessary `0`;
@@ -452,3 +456,34 @@ fn get_loop_counters<'a, 'tcx>(
             .into()
     })
 }
+
+fn is_array_length_equal_to_range(cx: &LateContext<'_>, start: &Expr<'_>, end: &Expr<'_>, arr: &Expr<'_>) -> bool {
+    fn extract_lit_value(expr: &Expr<'_>) -> Option<u128> {
+        if let ExprKind::Lit(lit) = expr.kind
+            && let ast::LitKind::Int(value, _) = lit.node
+        {
+            Some(value)
+        } else {
+            None
+        }
+    }
+
+    let arr_ty = cx.typeck_results().expr_ty(arr).peel_refs();
+
+    if let ty::Array(_, s) = arr_ty.kind() {
+        let size: u128 = if let Some(size) = s.try_eval_target_usize(cx.tcx, cx.param_env) {
+            size.into()
+        } else {
+            return false;
+        };
+
+        let range = match (extract_lit_value(start), extract_lit_value(end)) {
+            (Some(start_value), Some(end_value)) => end_value - start_value,
+            _ => return false,
+        };
+
+        size == range
+    } else {
+        false
+    }
+}
diff --git a/tests/ui/manual_memcpy/without_loop_counters.rs b/tests/ui/manual_memcpy/without_loop_counters.rs
index a224001a3..8146091a2 100644
--- a/tests/ui/manual_memcpy/without_loop_counters.rs
+++ b/tests/ui/manual_memcpy/without_loop_counters.rs
@@ -138,6 +138,26 @@ pub fn manual_copy(src: &[i32], dst: &mut [i32], dst2: &mut [i32]) {
     for i in 0..dst.len() {
         dst[i] = src[i];
     }
+
+    // Range is equal to array length
+    let src = [0, 1, 2, 3, 4];
+    let mut dst = [0; 4];
+    for i in 0..4 {
+        //~^ ERROR: it looks like you're manually copying between slices
+        dst[i] = src[i];
+    }
+
+    let mut dst = [0; 6];
+    for i in 0..5 {
+        //~^ ERROR: it looks like you're manually copying between slices
+        dst[i] = src[i];
+    }
+
+    let mut dst = [0; 5];
+    for i in 0..5 {
+        //~^ ERROR: it looks like you're manually copying between slices
+        dst[i] = src[i];
+    }
 }
 
 #[warn(clippy::needless_range_loop, clippy::manual_memcpy)]
diff --git a/tests/ui/manual_memcpy/without_loop_counters.stderr b/tests/ui/manual_memcpy/without_loop_counters.stderr
index b9dbda6ed..4b5cd274d 100644
--- a/tests/ui/manual_memcpy/without_loop_counters.stderr
+++ b/tests/ui/manual_memcpy/without_loop_counters.stderr
@@ -106,7 +106,7 @@ LL | /     for i in 0..5 {
 LL | |
 LL | |         dst[i - 0] = src[i];
 LL | |     }
-   | |_____^ help: try replacing the loop by: `dst[..5].copy_from_slice(&src[..5]);`
+   | |_____^ help: try replacing the loop by: `dst[..5].copy_from_slice(&src);`
 
 error: it looks like you're manually copying between slices
   --> $DIR/without_loop_counters.rs:121:5
@@ -120,11 +120,38 @@ LL | |     }
 error: it looks like you're manually copying between slices
   --> $DIR/without_loop_counters.rs:145:5
    |
+LL | /     for i in 0..4 {
+LL | |
+LL | |         dst[i] = src[i];
+LL | |     }
+   | |_____^ help: try replacing the loop by: `dst.copy_from_slice(&src[..4]);`
+
+error: it looks like you're manually copying between slices
+  --> $DIR/without_loop_counters.rs:151:5
+   |
+LL | /     for i in 0..5 {
+LL | |
+LL | |         dst[i] = src[i];
+LL | |     }
+   | |_____^ help: try replacing the loop by: `dst[..5].copy_from_slice(&src);`
+
+error: it looks like you're manually copying between slices
+  --> $DIR/without_loop_counters.rs:157:5
+   |
+LL | /     for i in 0..5 {
+LL | |
+LL | |         dst[i] = src[i];
+LL | |     }
+   | |_____^ help: try replacing the loop by: `dst.copy_from_slice(&src);`
+
+error: it looks like you're manually copying between slices
+  --> $DIR/without_loop_counters.rs:165:5
+   |
 LL | /     for i in 0..src.len() {
 LL | |
 LL | |         dst[i] = src[i].clone();
 LL | |     }
    | |_____^ help: try replacing the loop by: `dst[..src.len()].clone_from_slice(&src[..]);`
 
-error: aborting due to 13 previous errors
+error: aborting due to 16 previous errors