Preserve panic behavior for inclusive length bounds

Author: Aadi11z

clippy_lints/src/loops/needless_range_loop.rs

@@ -117,7 +117,8 @@ pub(super) fn check<'tcx>(
                     end_is_start_plus_val = start_equal_left | start_equal_right;
                 }
 
-                if is_len_call(end, indexed) || is_end_eq_array_len(cx, end, limits, indexed_ty) {
+                let end_is_indexed_len = matches!(limits, ast::RangeLimits::HalfOpen) && is_len_call(end, indexed);
+                if end_is_indexed_len || is_end_eq_array_len(cx, end, limits, indexed_ty) {
                     String::new()
                 } else if visitor.indexed_mut.contains(&indexed) && contains_name(indexed, take_expr, cx) {
                     return;
@@ -150,7 +151,27 @@ pub(super) fn check<'tcx>(
                 mem::swap(&mut method_1, &mut method_2);
             }
 
+            let panic_behavior_note = "this suggestion preserves panic behavior, but the panic will occur \
+                before iteration if the range end is out of bounds";
+
             if visitor.nonindex {
+                let mut applicability = Applicability::HasPlaceholders;
+                let (repl, note) = if !take_is_empty {
+                    // Use a slice bound to preserve panic behavior.
+                    let end_snippet = snippet_with_applicability(cx, end.unwrap().span, "..", &mut applicability);
+                    let range = match limits {
+                        ast::RangeLimits::Closed => format!("..={end_snippet}"),
+                        ast::RangeLimits::HalfOpen => format!("..{end_snippet}"),
+                    };
+
+                    (
+                        format!("{indexed}[{range}].{method}().enumerate(){slice_skip}"),
+                        Some(panic_behavior_note),
+                    )
+                } else {
+                    (format!("{indexed}.{method}().enumerate(){method_1}{method_2}"), None)
+                };
+
                 span_lint_and_then(
                     cx,
                     NEEDLESS_RANGE_LOOP,
@@ -159,20 +180,20 @@ pub(super) fn check<'tcx>(
                     |diag| {
                         diag.multipart_suggestion(
                             "consider using an iterator and enumerate()",
-                            vec![
-                                (pat.span, format!("({}, <item>)", ident.name)),
-                                (span, format!("{indexed}.{method}().enumerate(){method_1}{method_2}")),
-                            ],
-                            Applicability::HasPlaceholders,
+                            vec![(pat.span, format!("({}, <item>)", ident.name)), (span, repl)],
+                            applicability,
                         );
+                        if let Some(note) = note {
+                            diag.note(note);
+                        }
                     },
                 );
             } else {
                 let mut applicability = Applicability::HasPlaceholders;
                 let (repl, note) = if starts_at_zero && take_is_empty {
                     (format!("&{ref_mut}{indexed}"), None)
                 } else if !take_is_empty {
-                    // Use a slice bound to preserve panic behaviour
+                    // Use a slice bound to preserve panic behavior
                     let end_snippet = snippet_with_applicability(cx, end.unwrap().span, "..", &mut applicability);
                     let range = match limits {
                         ast::RangeLimits::Closed => format!("..={end_snippet}"),
@@ -181,10 +202,7 @@ pub(super) fn check<'tcx>(
 
                     (
                         format!("{indexed}[{range}].{method}(){slice_skip}"),
-                        Some(
-                            "this suggestion preserves panic behavior, but the panic will occur \
-                            before iteration if the upper bound exceeds the collection length",
-                        ),
+                        Some(panic_behavior_note),
                     )
                 } else {
                     (format!("{indexed}.{method}(){method_1}{method_2}"), None)
@@ -236,8 +254,8 @@ fn is_end_eq_array_len<'tcx>(
         && let Some(arr_len) = arr_len_const.try_to_target_usize(cx.tcx)
     {
         return match limits {
-            ast::RangeLimits::Closed => end_int.get() + 1 >= arr_len.into(),
-            ast::RangeLimits::HalfOpen => end_int.get() >= arr_len.into(),
+            ast::RangeLimits::Closed => end_int.get().checked_add(1) == Some(arr_len.into()),
+            ast::RangeLimits::HalfOpen => end_int.get() == arr_len.into(),
         };
     }
 

tests/ui/needless_range_loop.rs

@@ -242,3 +242,29 @@ fn issue_15068() {
         let _ = a[0][i];
     }
 }
+
+fn issue_16003() {
+    let vec = vec![1, 2, 3];
+
+    for i in 0..=vec.len() {
+        //~^ needless_range_loop
+        let _ = vec[i];
+    }
+
+    for i in 0..=vec.len() {
+        //~^ needless_range_loop
+        let _ = (i, vec[i]);
+    }
+
+    let arr = [1, 2, 3];
+
+    for i in 0..4 {
+        //~^ needless_range_loop
+        let _ = arr[i];
+    }
+
+    for i in 0..=3 {
+        //~^ needless_range_loop
+        let _ = arr[i];
+    }
+}

tests/ui/needless_range_loop.stderr

@@ -66,7 +66,7 @@ error: the loop variable `i` is only used to index `vec2`
 LL |     for i in 0..vec.len() {
    |              ^^^^^^^^^^^^
    |
-   = note: this suggestion preserves panic behavior, but the panic will occur before iteration if the upper bound exceeds the collection length
+   = note: this suggestion preserves panic behavior, but the panic will occur before iteration if the range end is out of bounds
 help: consider using an iterator
    |
 LL -     for i in 0..vec.len() {
@@ -91,7 +91,7 @@ error: the loop variable `i` is only used to index `vec`
 LL |     for i in 0..MAX_LEN {
    |              ^^^^^^^^^^
    |
-   = note: this suggestion preserves panic behavior, but the panic will occur before iteration if the upper bound exceeds the collection length
+   = note: this suggestion preserves panic behavior, but the panic will occur before iteration if the range end is out of bounds
 help: consider using an iterator
    |
 LL -     for i in 0..MAX_LEN {
@@ -104,7 +104,7 @@ error: the loop variable `i` is only used to index `vec`
 LL |     for i in 0..=MAX_LEN {
    |              ^^^^^^^^^^^
    |
-   = note: this suggestion preserves panic behavior, but the panic will occur before iteration if the upper bound exceeds the collection length
+   = note: this suggestion preserves panic behavior, but the panic will occur before iteration if the range end is out of bounds
 help: consider using an iterator
    |
 LL -     for i in 0..=MAX_LEN {
@@ -117,7 +117,7 @@ error: the loop variable `i` is only used to index `vec`
 LL |     for i in 5..10 {
    |              ^^^^^
    |
-   = note: this suggestion preserves panic behavior, but the panic will occur before iteration if the upper bound exceeds the collection length
+   = note: this suggestion preserves panic behavior, but the panic will occur before iteration if the range end is out of bounds
 help: consider using an iterator
    |
 LL -     for i in 5..10 {
@@ -130,7 +130,7 @@ error: the loop variable `i` is only used to index `vec`
 LL |     for i in 5..=10 {
    |              ^^^^^^
    |
-   = note: this suggestion preserves panic behavior, but the panic will occur before iteration if the upper bound exceeds the collection length
+   = note: this suggestion preserves panic behavior, but the panic will occur before iteration if the range end is out of bounds
 help: consider using an iterator
    |
 LL -     for i in 5..=10 {
@@ -155,10 +155,11 @@ error: the loop variable `i` is used to index `vec`
 LL |     for i in 5..10 {
    |              ^^^^^
    |
+   = note: this suggestion preserves panic behavior, but the panic will occur before iteration if the range end is out of bounds
 help: consider using an iterator and enumerate()
    |
 LL -     for i in 5..10 {
-LL +     for (i, <item>) in vec.iter().enumerate().take(10).skip(5) {
+LL +     for (i, <item>) in vec[..10].iter().enumerate().skip(5) {
    |
 
 error: the loop variable `i` is used to index `vec`
@@ -179,10 +180,11 @@ error: the loop variable `i` is used to index `a`
 LL |     for i in 0..MAX_LEN {
    |              ^^^^^^^^^^
    |
+   = note: this suggestion preserves panic behavior, but the panic will occur before iteration if the range end is out of bounds
 help: consider using an iterator and enumerate()
    |
 LL -     for i in 0..MAX_LEN {
-LL +     for (i, <item>) in a.iter().enumerate().take(MAX_LEN) {
+LL +     for (i, <item>) in a[..MAX_LEN].iter().enumerate() {
    |
 
 error: the loop variable `i` is only used to index `a`
@@ -191,12 +193,64 @@ error: the loop variable `i` is only used to index `a`
 LL |     for i in 0..MAX_LEN {
    |              ^^^^^^^^^^
    |
-   = note: this suggestion preserves panic behavior, but the panic will occur before iteration if the upper bound exceeds the collection length
+   = note: this suggestion preserves panic behavior, but the panic will occur before iteration if the range end is out of bounds
 help: consider using an iterator
    |
 LL -     for i in 0..MAX_LEN {
 LL +     for <item> in a[..MAX_LEN].iter() {
    |
 
-error: aborting due to 16 previous errors
+error: the loop variable `i` is only used to index `vec`
+  --> tests/ui/needless_range_loop.rs:249:14
+   |
+LL |     for i in 0..=vec.len() {
+   |              ^^^^^^^^^^^^^
+   |
+   = note: this suggestion preserves panic behavior, but the panic will occur before iteration if the range end is out of bounds
+help: consider using an iterator
+   |
+LL -     for i in 0..=vec.len() {
+LL +     for <item> in vec[..=vec.len()].iter() {
+   |
+
+error: the loop variable `i` is used to index `vec`
+  --> tests/ui/needless_range_loop.rs:254:14
+   |
+LL |     for i in 0..=vec.len() {
+   |              ^^^^^^^^^^^^^
+   |
+   = note: this suggestion preserves panic behavior, but the panic will occur before iteration if the range end is out of bounds
+help: consider using an iterator and enumerate()
+   |
+LL -     for i in 0..=vec.len() {
+LL +     for (i, <item>) in vec[..=vec.len()].iter().enumerate() {
+   |
+
+error: the loop variable `i` is only used to index `arr`
+  --> tests/ui/needless_range_loop.rs:261:14
+   |
+LL |     for i in 0..4 {
+   |              ^^^^
+   |
+   = note: this suggestion preserves panic behavior, but the panic will occur before iteration if the range end is out of bounds
+help: consider using an iterator
+   |
+LL -     for i in 0..4 {
+LL +     for <item> in arr[..4].iter() {
+   |
+
+error: the loop variable `i` is only used to index `arr`
+  --> tests/ui/needless_range_loop.rs:266:14
+   |
+LL |     for i in 0..=3 {
+   |              ^^^^^
+   |
+   = note: this suggestion preserves panic behavior, but the panic will occur before iteration if the range end is out of bounds
+help: consider using an iterator
+   |
+LL -     for i in 0..=3 {
+LL +     for <item> in arr[..=3].iter() {
+   |
+
+error: aborting due to 20 previous errors
 

tests/ui/needless_range_loop2.stderr

@@ -4,7 +4,7 @@ error: the loop variable `i` is only used to index `ns`
 LL |     for i in 3..10 {
    |              ^^^^^
    |
-   = note: this suggestion preserves panic behavior, but the panic will occur before iteration if the upper bound exceeds the collection length
+   = note: this suggestion preserves panic behavior, but the panic will occur before iteration if the range end is out of bounds
    = note: `-D clippy::needless-range-loop` implied by `-D warnings`
    = help: to override `-D warnings` add `#[allow(clippy::needless_range_loop)]`
 help: consider using an iterator
@@ -43,7 +43,7 @@ error: the loop variable `i` is only used to index `vec`
 LL |     for i in x..x + 4 {
    |              ^^^^^^^^
    |
-   = note: this suggestion preserves panic behavior, but the panic will occur before iteration if the upper bound exceeds the collection length
+   = note: this suggestion preserves panic behavior, but the panic will occur before iteration if the range end is out of bounds
 help: consider using an iterator
    |
 LL -     for i in x..x + 4 {
@@ -56,7 +56,7 @@ error: the loop variable `i` is only used to index `vec`
 LL |     for i in x..=x + 4 {
    |              ^^^^^^^^^
    |
-   = note: this suggestion preserves panic behavior, but the panic will occur before iteration if the upper bound exceeds the collection length
+   = note: this suggestion preserves panic behavior, but the panic will occur before iteration if the range end is out of bounds
 help: consider using an iterator
    |
 LL -     for i in x..=x + 4 {
@@ -81,7 +81,7 @@ error: the loop variable `i` is only used to index `arr`
 LL |     for i in 0..2 {
    |              ^^^^
    |
-   = note: this suggestion preserves panic behavior, but the panic will occur before iteration if the upper bound exceeds the collection length
+   = note: this suggestion preserves panic behavior, but the panic will occur before iteration if the range end is out of bounds
 help: consider using an iterator
    |
 LL -     for i in 0..2 {