Simplify manual_memcpy suggestion in some cases

Author: pengowen123

clippy_lints/src/loops.rs

@@ -950,8 +950,20 @@ fn detect_manual_memcpy<'a, 'tcx>(
                     ("0", _, x, false) | (x, false, "0", false) => x.into(),
                     ("0", _, x, true) | (x, false, "0", true) => format!("-{}", x),
                     (x, false, y, false) => format!("({} + {})", x, y),
-                    (x, false, y, true) => format!("({} - {})", x, y),
-                    (x, true, y, false) => format!("({} - {})", y, x),
+                    (x, false, y, true) => {
+                        if x == y {
+                            "0".into()
+                        } else {
+                            format!("({} - {})", x, y)
+                        }
+                    },
+                    (x, true, y, false) => {
+                        if x == y {
+                            "0".into()
+                        } else {
+                            format!("({} - {})", y, x)
+                        }
+                    },
                     (x, true, y, true) => format!("-({} + {})", x, y),
                 }
             };
@@ -972,15 +984,41 @@ fn detect_manual_memcpy<'a, 'tcx>(
                     }
                 }
 
+                let mut end_expr = end;
+                let mut reduced_end_expr = false;
+
+                if_chain! {
+                    if let ExprKind::Binary(ref op, ref left, ref right) = end.node;
+                    if let BinOpKind::Add = op.node;
+                    then {
+                        if let Some(left) = sugg::Sugg::hir_opt(cx, left) {
+                            if left.to_string() == offset.value {
+                                end_expr = right;
+                                reduced_end_expr = true;
+                            }
+                        }
+                        if let Some(right) = sugg::Sugg::hir_opt(cx, right) {
+                            if right.to_string() == offset.value {
+                                end_expr = left;
+                                reduced_end_expr = true;
+                            }
+                        }
+                    }
+                }
+
                 let end_str = match limits {
                     ast::RangeLimits::Closed => {
-                        let end = sugg::Sugg::hir(cx, end, "<count>");
+                        let end = sugg::Sugg::hir(cx, end_expr, "<count>");
                         format!("{}", end + sugg::ONE)
                     },
-                    ast::RangeLimits::HalfOpen => format!("{}", snippet(cx, end.span, "..")),
+                    ast::RangeLimits::HalfOpen => format!("{}", snippet(cx, end_expr.span, "..")),
                 };
 
-                print_sum(&Offset::positive(end_str), &offset)
+                if reduced_end_expr {
+                    end_str
+                } else {
+                    print_sum(&Offset::positive(end_str), &offset)
+                }
             } else {
                 "..".into()
             };
@@ -1003,7 +1041,12 @@ fn detect_manual_memcpy<'a, 'tcx>(
                         format!("{}[{}..{}]", dst_var.var_name, dst_offset, dst_limit)
                     };
 
-                    format!("{}.clone_from_slice(&{}[{}..{}])", dst, src_var.var_name, src_offset, src_limit)
+                    // If the src range is simply `0..src.len()`, suggest copying the entire slice
+                    if src_offset == "0" && src_limit == format!("{}.len()", src_var.var_name) {
+                        format!("{}.clone_from_slice(&{})", dst, src_var.var_name)
+                    } else {
+                        format!("{}.clone_from_slice(&{}[{}..{}])", dst, src_var.var_name, src_offset, src_limit)
+                    }
                 })
                 .join("\n    ");
 

tests/ui/for_loop.rs

@@ -550,6 +550,15 @@ pub fn manual_copy(src: &[i32], dst: &mut [i32], dst2: &mut [i32]) {
     for i in 0..10 {
         dst_vec[i] = src[i];
     }
+
+    // Copying entire source slice - simplify suggestion (issue #3004)
+    let src = [0, 1, 2, 3, 4];
+    let mut dst = [0, 0, 0, 0, 0, 0];
+    let from = 1;
+
+    for i in from..from + src.len() {
+        dst[i] = src[i - from];
+    }
 }
 
 #[warn(clippy::needless_range_loop)]

tests/ui/for_loop.stderr

@@ -482,22 +482,28 @@ error: it looks like you're manually copying between slices
     |              ^^^^^^^^^^^^^^^^ help: try replacing the loop by: `dst_vec[..src_vec.len()].clone_from_slice(&src_vec[..])`
 
 error: it looks like you're manually copying between slices
-   --> $DIR/for_loop.rs:557:14
+   --> $DIR/for_loop.rs:559:14
     |
-557 |     for i in 0..src.len() {
+559 |     for i in from..from + src.len() {
+    |              ^^^^^^^^^^^^^^^^^^^^^^ help: try replacing the loop by: `dst[from..from + src.len()].clone_from_slice(&src)`
+
+error: it looks like you're manually copying between slices
+   --> $DIR/for_loop.rs:566:14
+    |
+566 |     for i in 0..src.len() {
     |              ^^^^^^^^^^^^ help: try replacing the loop by: `dst[..src.len()].clone_from_slice(&src[..])`
 
 error: the variable `count` is used as a loop counter. Consider using `for (count, item) in text.chars().enumerate()` or similar iterators
-   --> $DIR/for_loop.rs:618:19
+   --> $DIR/for_loop.rs:627:19
     |
-618 |         for ch in text.chars() {
+627 |         for ch in text.chars() {
     |                   ^^^^^^^^^^^^
 
 error: the variable `count` is used as a loop counter. Consider using `for (count, item) in text.chars().enumerate()` or similar iterators
-   --> $DIR/for_loop.rs:629:19
+   --> $DIR/for_loop.rs:638:19
     |
-629 |         for ch in text.chars() {
+638 |         for ch in text.chars() {
     |                   ^^^^^^^^^^^^
 
-error: aborting due to 61 previous errors
+error: aborting due to 62 previous errors