diff --git a/clippy_lints/src/loops.rs b/clippy_lints/src/loops.rs index f2df53aee4f..7c8b6f483bc 100644 --- a/clippy_lints/src/loops.rs +++ b/clippy_lints/src/loops.rs @@ -779,6 +779,17 @@ fn check_for_loop<'tcx>( detect_same_item_push(cx, pat, arg, body, expr); } +// this function assumes the given expression is a `for` loop. +fn get_span_of_entire_for_loop(expr: &Expr<'_>) -> Span { + // for some reason this is the only way to get the `Span` + // of the entire `for` loop + if let ExprKind::Match(_, arms, _) = &expr.kind { + arms[0].body.span + } else { + unreachable!() + } +} + fn same_var<'tcx>(cx: &LateContext<'tcx>, expr: &Expr<'_>, var: HirId) -> bool { if_chain! { if let ExprKind::Path(qpath) = &expr.kind; @@ -1138,7 +1149,7 @@ fn build_manual_memcpy_suggestion<'tcx>( }; format!( - "{}.clone_from_slice(&{}[{}..{}])", + "{}.clone_from_slice(&{}[{}..{}]);", dst, src_base_str, src_offset.maybe_par(), @@ -1218,7 +1229,7 @@ fn detect_manual_memcpy<'tcx>( span_lint_and_sugg( cx, MANUAL_MEMCPY, - expr.span, + get_span_of_entire_for_loop(expr), "it looks like you're manually copying between slices", "try replacing the loop by", big_sugg, @@ -1734,13 +1745,7 @@ fn check_for_loop_explicit_counter<'tcx>( then { let mut applicability = Applicability::MachineApplicable; - // for some reason this is the only way to get the `Span` - // of the entire `for` loop - let for_span = if let ExprKind::Match(_, arms, _) = &expr.kind { - arms[0].body.span - } else { - unreachable!() - }; + let for_span = get_span_of_entire_for_loop(expr); span_lint_and_sugg( cx, diff --git a/tests/ui/manual_memcpy.stderr b/tests/ui/manual_memcpy.stderr index 464b18984fb..db62ed90d97 100644 --- a/tests/ui/manual_memcpy.stderr +++ b/tests/ui/manual_memcpy.stderr @@ -1,148 +1,204 @@ error: it looks like you're manually copying between slices - --> $DIR/manual_memcpy.rs:7:14 + --> $DIR/manual_memcpy.rs:7:5 | -LL | for i in 0..src.len() { - | ^^^^^^^^^^^^ help: try replacing the loop by: `dst[..src.len()].clone_from_slice(&src[..])` +LL | / for i in 0..src.len() { +LL | | dst[i] = src[i]; +LL | | } + | |_____^ help: try replacing the loop by: `dst[..src.len()].clone_from_slice(&src[..]);` | = note: `-D clippy::manual-memcpy` implied by `-D warnings` error: it looks like you're manually copying between slices - --> $DIR/manual_memcpy.rs:12:14 + --> $DIR/manual_memcpy.rs:12:5 | -LL | for i in 0..src.len() { - | ^^^^^^^^^^^^ help: try replacing the loop by: `dst[10..(src.len() + 10)].clone_from_slice(&src[..])` +LL | / for i in 0..src.len() { +LL | | dst[i + 10] = src[i]; +LL | | } + | |_____^ help: try replacing the loop by: `dst[10..(src.len() + 10)].clone_from_slice(&src[..]);` error: it looks like you're manually copying between slices - --> $DIR/manual_memcpy.rs:17:14 + --> $DIR/manual_memcpy.rs:17:5 | -LL | for i in 0..src.len() { - | ^^^^^^^^^^^^ help: try replacing the loop by: `dst[..src.len()].clone_from_slice(&src[10..(src.len() + 10)])` +LL | / for i in 0..src.len() { +LL | | dst[i] = src[i + 10]; +LL | | } + | |_____^ help: try replacing the loop by: `dst[..src.len()].clone_from_slice(&src[10..(src.len() + 10)]);` error: it looks like you're manually copying between slices - --> $DIR/manual_memcpy.rs:22:14 + --> $DIR/manual_memcpy.rs:22:5 | -LL | for i in 11..src.len() { - | ^^^^^^^^^^^^^ help: try replacing the loop by: `dst[11..src.len()].clone_from_slice(&src[(11 - 10)..(src.len() - 10)])` +LL | / for i in 11..src.len() { +LL | | dst[i] = src[i - 10]; +LL | | } + | |_____^ help: try replacing the loop by: `dst[11..src.len()].clone_from_slice(&src[(11 - 10)..(src.len() - 10)]);` error: it looks like you're manually copying between slices - --> $DIR/manual_memcpy.rs:27:14 + --> $DIR/manual_memcpy.rs:27:5 | -LL | for i in 0..dst.len() { - | ^^^^^^^^^^^^ help: try replacing the loop by: `dst.clone_from_slice(&src[..dst.len()])` +LL | / for i in 0..dst.len() { +LL | | dst[i] = src[i]; +LL | | } + | |_____^ help: try replacing the loop by: `dst.clone_from_slice(&src[..dst.len()]);` error: it looks like you're manually copying between slices - --> $DIR/manual_memcpy.rs:40:14 + --> $DIR/manual_memcpy.rs:40:5 | -LL | for i in 10..256 { - | ^^^^^^^ +LL | / for i in 10..256 { +LL | | dst[i] = src[i - 5]; +LL | | dst2[i + 500] = src[i] +LL | | } + | |_____^ | help: try replacing the loop by | -LL | for i in dst[10..256].clone_from_slice(&src[(10 - 5)..(256 - 5)]) -LL | dst2[(10 + 500)..(256 + 500)].clone_from_slice(&src[10..256]) { +LL | dst[10..256].clone_from_slice(&src[(10 - 5)..(256 - 5)]); +LL | dst2[(10 + 500)..(256 + 500)].clone_from_slice(&src[10..256]); | error: it looks like you're manually copying between slices - --> $DIR/manual_memcpy.rs:52:14 + --> $DIR/manual_memcpy.rs:52:5 | -LL | for i in 10..LOOP_OFFSET { - | ^^^^^^^^^^^^^^^ help: try replacing the loop by: `dst[(10 + LOOP_OFFSET)..(LOOP_OFFSET + LOOP_OFFSET)].clone_from_slice(&src[(10 - some_var)..(LOOP_OFFSET - some_var)])` +LL | / for i in 10..LOOP_OFFSET { +LL | | dst[i + LOOP_OFFSET] = src[i - some_var]; +LL | | } + | |_____^ help: try replacing the loop by: `dst[(10 + LOOP_OFFSET)..(LOOP_OFFSET + LOOP_OFFSET)].clone_from_slice(&src[(10 - some_var)..(LOOP_OFFSET - some_var)]);` error: it looks like you're manually copying between slices - --> $DIR/manual_memcpy.rs:65:14 + --> $DIR/manual_memcpy.rs:65:5 | -LL | for i in 0..src_vec.len() { - | ^^^^^^^^^^^^^^^^ help: try replacing the loop by: `dst_vec[..src_vec.len()].clone_from_slice(&src_vec[..])` +LL | / for i in 0..src_vec.len() { +LL | | dst_vec[i] = src_vec[i]; +LL | | } + | |_____^ 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/manual_memcpy.rs:94:14 + --> $DIR/manual_memcpy.rs:94:5 | -LL | for i in from..from + src.len() { - | ^^^^^^^^^^^^^^^^^^^^^^ help: try replacing the loop by: `dst[from..(from + src.len())].clone_from_slice(&src[..(from + src.len() - from)])` +LL | / for i in from..from + src.len() { +LL | | dst[i] = src[i - from]; +LL | | } + | |_____^ help: try replacing the loop by: `dst[from..(from + src.len())].clone_from_slice(&src[..(from + src.len() - from)]);` error: it looks like you're manually copying between slices - --> $DIR/manual_memcpy.rs:98:14 + --> $DIR/manual_memcpy.rs:98:5 | -LL | for i in from..from + 3 { - | ^^^^^^^^^^^^^^ help: try replacing the loop by: `dst[from..(from + 3)].clone_from_slice(&src[..(from + 3 - from)])` +LL | / for i in from..from + 3 { +LL | | dst[i] = src[i - from]; +LL | | } + | |_____^ help: try replacing the loop by: `dst[from..(from + 3)].clone_from_slice(&src[..(from + 3 - from)]);` error: it looks like you're manually copying between slices - --> $DIR/manual_memcpy.rs:103:14 + --> $DIR/manual_memcpy.rs:103:5 | -LL | for i in 0..5 { - | ^^^^ help: try replacing the loop by: `dst[..5].clone_from_slice(&src[..5])` +LL | / for i in 0..5 { +LL | | dst[i - 0] = src[i]; +LL | | } + | |_____^ help: try replacing the loop by: `dst[..5].clone_from_slice(&src[..5]);` error: it looks like you're manually copying between slices - --> $DIR/manual_memcpy.rs:108:14 + --> $DIR/manual_memcpy.rs:108:5 | -LL | for i in 0..0 { - | ^^^^ help: try replacing the loop by: `dst[..0].clone_from_slice(&src[..0])` +LL | / for i in 0..0 { +LL | | dst[i] = src[i]; +LL | | } + | |_____^ help: try replacing the loop by: `dst[..0].clone_from_slice(&src[..0]);` error: it looks like you're manually copying between slices - --> $DIR/manual_memcpy.rs:120:14 + --> $DIR/manual_memcpy.rs:120:5 | -LL | for i in 3..src.len() { - | ^^^^^^^^^^^^ help: try replacing the loop by: `dst[3..src.len()].clone_from_slice(&src[..(src.len() - 3)])` +LL | / for i in 3..src.len() { +LL | | dst[i] = src[count]; +LL | | count += 1; +LL | | } + | |_____^ help: try replacing the loop by: `dst[3..src.len()].clone_from_slice(&src[..(src.len() - 3)]);` error: it looks like you're manually copying between slices - --> $DIR/manual_memcpy.rs:126:14 + --> $DIR/manual_memcpy.rs:126:5 | -LL | for i in 3..src.len() { - | ^^^^^^^^^^^^ help: try replacing the loop by: `dst[..(src.len() - 3)].clone_from_slice(&src[3..])` +LL | / for i in 3..src.len() { +LL | | dst[count] = src[i]; +LL | | count += 1; +LL | | } + | |_____^ help: try replacing the loop by: `dst[..(src.len() - 3)].clone_from_slice(&src[3..]);` error: it looks like you're manually copying between slices - --> $DIR/manual_memcpy.rs:132:14 + --> $DIR/manual_memcpy.rs:132:5 | -LL | for i in 0..src.len() { - | ^^^^^^^^^^^^ help: try replacing the loop by: `dst[3..(src.len() + 3)].clone_from_slice(&src[..])` +LL | / for i in 0..src.len() { +LL | | dst[count] = src[i]; +LL | | count += 1; +LL | | } + | |_____^ help: try replacing the loop by: `dst[3..(src.len() + 3)].clone_from_slice(&src[..]);` error: it looks like you're manually copying between slices - --> $DIR/manual_memcpy.rs:138:14 + --> $DIR/manual_memcpy.rs:138:5 | -LL | for i in 0..src.len() { - | ^^^^^^^^^^^^ help: try replacing the loop by: `dst[..src.len()].clone_from_slice(&src[3..(src.len() + 3)])` +LL | / for i in 0..src.len() { +LL | | dst[i] = src[count]; +LL | | count += 1; +LL | | } + | |_____^ help: try replacing the loop by: `dst[..src.len()].clone_from_slice(&src[3..(src.len() + 3)]);` error: it looks like you're manually copying between slices - --> $DIR/manual_memcpy.rs:144:14 + --> $DIR/manual_memcpy.rs:144:5 | -LL | for i in 3..(3 + src.len()) { - | ^^^^^^^^^^^^^^^^^^ help: try replacing the loop by: `dst[3..((3 + src.len()))].clone_from_slice(&src[..((3 + src.len()) - 3)])` +LL | / for i in 3..(3 + src.len()) { +LL | | dst[i] = src[count]; +LL | | count += 1; +LL | | } + | |_____^ help: try replacing the loop by: `dst[3..((3 + src.len()))].clone_from_slice(&src[..((3 + src.len()) - 3)]);` error: it looks like you're manually copying between slices - --> $DIR/manual_memcpy.rs:150:14 + --> $DIR/manual_memcpy.rs:150:5 | -LL | for i in 5..src.len() { - | ^^^^^^^^^^^^ help: try replacing the loop by: `dst[5..src.len()].clone_from_slice(&src[(3 - 2)..((src.len() - 2) + 3 - 5)])` +LL | / for i in 5..src.len() { +LL | | dst[i] = src[count - 2]; +LL | | count += 1; +LL | | } + | |_____^ help: try replacing the loop by: `dst[5..src.len()].clone_from_slice(&src[(3 - 2)..((src.len() - 2) + 3 - 5)]);` error: it looks like you're manually copying between slices - --> $DIR/manual_memcpy.rs:156:14 + --> $DIR/manual_memcpy.rs:156:5 | -LL | for i in 3..10 { - | ^^^^^ help: try replacing the loop by: `dst[3..10].clone_from_slice(&src[5..(10 + 5 - 3)])` +LL | / for i in 3..10 { +LL | | dst[i] = src[count]; +LL | | count += 1; +LL | | } + | |_____^ help: try replacing the loop by: `dst[3..10].clone_from_slice(&src[5..(10 + 5 - 3)]);` error: it looks like you're manually copying between slices - --> $DIR/manual_memcpy.rs:163:14 + --> $DIR/manual_memcpy.rs:163:5 | -LL | for i in 0..src.len() { - | ^^^^^^^^^^^^ +LL | / for i in 0..src.len() { +LL | | dst[count] = src[i]; +LL | | dst2[count2] = src[i]; +LL | | count += 1; +LL | | count2 += 1; +LL | | } + | |_____^ | help: try replacing the loop by | -LL | for i in dst[3..(src.len() + 3)].clone_from_slice(&src[..]) -LL | dst2[30..(src.len() + 30)].clone_from_slice(&src[..]) { +LL | dst[3..(src.len() + 3)].clone_from_slice(&src[..]); +LL | dst2[30..(src.len() + 30)].clone_from_slice(&src[..]); | error: it looks like you're manually copying between slices - --> $DIR/manual_memcpy.rs:173:14 + --> $DIR/manual_memcpy.rs:173:5 | -LL | for i in 0..1 << 1 { - | ^^^^^^^^^ help: try replacing the loop by: `dst[(0 << 1)..((1 << 1) + (0 << 1))].clone_from_slice(&src[2..((1 << 1) + 2)])` +LL | / for i in 0..1 << 1 { +LL | | dst[count] = src[i + 2]; +LL | | count += 1; +LL | | } + | |_____^ help: try replacing the loop by: `dst[(0 << 1)..((1 << 1) + (0 << 1))].clone_from_slice(&src[2..((1 << 1) + 2)]);` error: it looks like you're manually copying between slices - --> $DIR/manual_memcpy.rs:181:14 + --> $DIR/manual_memcpy.rs:181:5 | -LL | for i in 0..src.len() { - | ^^^^^^^^^^^^ help: try replacing the loop by: `dst[..src.len()].clone_from_slice(&src[..])` +LL | / for i in 0..src.len() { +LL | | dst[i] = src[i].clone(); +LL | | } + | |_____^ help: try replacing the loop by: `dst[..src.len()].clone_from_slice(&src[..]);` error: aborting due to 22 previous errors