Skip to content

Commit 83d7f69

Browse files
committed
Fix: Preserve panic behavior in needless_range_loop suggestions
Preserve panic semantics in `needless_range_loop` iterator suggestions by using slicing instead of `.take(n)` where the original range would panic on out-of-bounds access. Also: - use `snippet_with_applicability` for generated slice bounds - emit the panic-behavior note only for affected suggestions - update UI tests accordingly
1 parent 840a6fa commit 83d7f69

3 files changed

Lines changed: 42 additions & 15 deletions

File tree

clippy_lints/src/loops/needless_range_loop.rs

Lines changed: 22 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
use super::NEEDLESS_RANGE_LOOP;
22
use clippy_utils::diagnostics::span_lint_and_then;
3-
use clippy_utils::source::snippet;
3+
use clippy_utils::source::{snippet, snippet_with_applicability};
44
use clippy_utils::ty::has_iter_method;
55
use clippy_utils::visitors::is_local_used;
66
use clippy_utils::{SpanlessEq, contains_name, higher, is_integer_const, peel_hir_expr_while, sugg};
@@ -165,10 +165,24 @@ pub(super) fn check<'tcx>(
165165
},
166166
);
167167
} else {
168-
let repl = if starts_at_zero && take_is_empty {
169-
format!("&{ref_mut}{indexed}")
168+
let mut applicability = Applicability::HasPlaceholders;
169+
let (repl, note) = if starts_at_zero && take_is_empty {
170+
(format!("&{ref_mut}{indexed}"), None)
171+
} else if !take_is_empty {
172+
// Adding condition for when 'take' is not empty Fixing `.take(n)` with slicing to preserve panic
173+
// semantics
174+
(
175+
format!(
176+
"{indexed}[..{}].{method}(){method_2}",
177+
snippet_with_applicability(cx, end.unwrap().span, "..", &mut applicability)
178+
),
179+
Some(
180+
"this suggestion preserves panic behavior, but the panic will occur \
181+
before iteration if the upper bound exceeds the collection length",
182+
),
183+
)
170184
} else {
171-
format!("{indexed}.{method}(){method_1}{method_2}")
185+
(format!("{indexed}.{method}(){method_1}{method_2}"), None)
172186
};
173187

174188
span_lint_and_then(
@@ -180,8 +194,11 @@ pub(super) fn check<'tcx>(
180194
diag.multipart_suggestion(
181195
"consider using an iterator",
182196
vec![(pat.span, "<item>".to_string()), (span, repl)],
183-
Applicability::HasPlaceholders,
197+
applicability,
184198
);
199+
if let Some(note) = note {
200+
diag.note(note);
201+
}
185202
},
186203
);
187204
}

tests/ui/needless_range_loop.stderr

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -66,10 +66,11 @@ error: the loop variable `i` is only used to index `vec2`
6666
LL | for i in 0..vec.len() {
6767
| ^^^^^^^^^^^^
6868
|
69+
= note: this suggestion preserves panic behavior, but the panic will occur before iteration if the upper bound exceeds the collection length
6970
help: consider using an iterator
7071
|
7172
LL - for i in 0..vec.len() {
72-
LL + for <item> in vec2.iter().take(vec.len()) {
73+
LL + for <item> in vec2[..vec.len()].iter() {
7374
|
7475

7576
error: the loop variable `i` is only used to index `vec`
@@ -90,10 +91,11 @@ error: the loop variable `i` is only used to index `vec`
9091
LL | for i in 0..MAX_LEN {
9192
| ^^^^^^^^^^
9293
|
94+
= note: this suggestion preserves panic behavior, but the panic will occur before iteration if the upper bound exceeds the collection length
9395
help: consider using an iterator
9496
|
9597
LL - for i in 0..MAX_LEN {
96-
LL + for <item> in vec.iter().take(MAX_LEN) {
98+
LL + for <item> in vec[..MAX_LEN].iter() {
9799
|
98100

99101
error: the loop variable `i` is only used to index `vec`
@@ -102,10 +104,11 @@ error: the loop variable `i` is only used to index `vec`
102104
LL | for i in 0..=MAX_LEN {
103105
| ^^^^^^^^^^^
104106
|
107+
= note: this suggestion preserves panic behavior, but the panic will occur before iteration if the upper bound exceeds the collection length
105108
help: consider using an iterator
106109
|
107110
LL - for i in 0..=MAX_LEN {
108-
LL + for <item> in vec.iter().take(MAX_LEN + 1) {
111+
LL + for <item> in vec[..MAX_LEN].iter() {
109112
|
110113

111114
error: the loop variable `i` is only used to index `vec`
@@ -114,10 +117,11 @@ error: the loop variable `i` is only used to index `vec`
114117
LL | for i in 5..10 {
115118
| ^^^^^
116119
|
120+
= note: this suggestion preserves panic behavior, but the panic will occur before iteration if the upper bound exceeds the collection length
117121
help: consider using an iterator
118122
|
119123
LL - for i in 5..10 {
120-
LL + for <item> in vec.iter().take(10).skip(5) {
124+
LL + for <item> in vec[..10].iter().skip(5) {
121125
|
122126

123127
error: the loop variable `i` is only used to index `vec`
@@ -126,10 +130,11 @@ error: the loop variable `i` is only used to index `vec`
126130
LL | for i in 5..=10 {
127131
| ^^^^^^
128132
|
133+
= note: this suggestion preserves panic behavior, but the panic will occur before iteration if the upper bound exceeds the collection length
129134
help: consider using an iterator
130135
|
131136
LL - for i in 5..=10 {
132-
LL + for <item> in vec.iter().take(10 + 1).skip(5) {
137+
LL + for <item> in vec[..10].iter().skip(5) {
133138
|
134139

135140
error: the loop variable `i` is used to index `vec`
@@ -186,10 +191,11 @@ error: the loop variable `i` is only used to index `a`
186191
LL | for i in 0..MAX_LEN {
187192
| ^^^^^^^^^^
188193
|
194+
= note: this suggestion preserves panic behavior, but the panic will occur before iteration if the upper bound exceeds the collection length
189195
help: consider using an iterator
190196
|
191197
LL - for i in 0..MAX_LEN {
192-
LL + for <item> in a.iter().take(MAX_LEN) {
198+
LL + for <item> in a[..MAX_LEN].iter() {
193199
|
194200

195201
error: aborting due to 16 previous errors

tests/ui/needless_range_loop2.stderr

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,12 +4,13 @@ error: the loop variable `i` is only used to index `ns`
44
LL | for i in 3..10 {
55
| ^^^^^
66
|
7+
= note: this suggestion preserves panic behavior, but the panic will occur before iteration if the upper bound exceeds the collection length
78
= note: `-D clippy::needless-range-loop` implied by `-D warnings`
89
= help: to override `-D warnings` add `#[allow(clippy::needless_range_loop)]`
910
help: consider using an iterator
1011
|
1112
LL - for i in 3..10 {
12-
LL + for <item> in ns.iter().take(10).skip(3) {
13+
LL + for <item> in ns[..10].iter().skip(3) {
1314
|
1415

1516
error: the loop variable `i` is only used to index `ms`
@@ -42,10 +43,11 @@ error: the loop variable `i` is only used to index `vec`
4243
LL | for i in x..x + 4 {
4344
| ^^^^^^^^
4445
|
46+
= note: this suggestion preserves panic behavior, but the panic will occur before iteration if the upper bound exceeds the collection length
4547
help: consider using an iterator
4648
|
4749
LL - for i in x..x + 4 {
48-
LL + for <item> in vec.iter_mut().skip(x).take(4) {
50+
LL + for <item> in vec[..x + 4].iter_mut().take(4) {
4951
|
5052

5153
error: the loop variable `i` is only used to index `vec`
@@ -54,10 +56,11 @@ error: the loop variable `i` is only used to index `vec`
5456
LL | for i in x..=x + 4 {
5557
| ^^^^^^^^^
5658
|
59+
= note: this suggestion preserves panic behavior, but the panic will occur before iteration if the upper bound exceeds the collection length
5760
help: consider using an iterator
5861
|
5962
LL - for i in x..=x + 4 {
60-
LL + for <item> in vec.iter_mut().skip(x).take(4 + 1) {
63+
LL + for <item> in vec[..x + 4].iter_mut().take(4 + 1) {
6164
|
6265

6366
error: the loop variable `i` is only used to index `arr`
@@ -78,10 +81,11 @@ error: the loop variable `i` is only used to index `arr`
7881
LL | for i in 0..2 {
7982
| ^^^^
8083
|
84+
= note: this suggestion preserves panic behavior, but the panic will occur before iteration if the upper bound exceeds the collection length
8185
help: consider using an iterator
8286
|
8387
LL - for i in 0..2 {
84-
LL + for <item> in arr.iter().take(2) {
88+
LL + for <item> in arr[..2].iter() {
8589
|
8690

8791
error: the loop variable `i` is only used to index `arr`

0 commit comments

Comments
 (0)