Skip to content

Commit 11072b5

Browse files
committed
lint vecs, version bump, more tests
1 parent 86b6644 commit 11072b5

8 files changed

+119
-46
lines changed

clippy_lints/src/methods/iter_out_of_bounds.rs

+40-25
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
use clippy_utils::diagnostics::span_lint_and_note;
2-
use clippy_utils::{is_trait_method, match_def_path, paths};
2+
use clippy_utils::higher::VecArgs;
3+
use clippy_utils::{expr_or_init, is_trait_method, match_def_path, paths};
34
use rustc_ast::LitKind;
45
use rustc_hir::{Expr, ExprKind};
56
use rustc_lint::LateContext;
@@ -8,34 +9,49 @@ use rustc_span::sym;
89

910
use super::ITER_OUT_OF_BOUNDS;
1011

12+
fn expr_as_u128(cx: &LateContext<'_>, e: &Expr<'_>) -> Option<u128> {
13+
if let ExprKind::Lit(lit) = expr_or_init(cx, e).kind
14+
&& let LitKind::Int(n, _) = lit.node
15+
{
16+
Some(n)
17+
} else {
18+
None
19+
}
20+
}
21+
1122
/// Attempts to extract the length out of an iterator expression.
1223
fn get_iterator_length<'tcx>(cx: &LateContext<'tcx>, iter: &'tcx Expr<'tcx>) -> Option<u128> {
13-
let iter_ty = cx.typeck_results().expr_ty(iter);
14-
15-
if let ty::Adt(adt, substs) = iter_ty.kind() {
16-
let did = adt.did();
24+
let ty::Adt(adt, substs) = cx.typeck_results().expr_ty(iter).kind() else {
25+
return None;
26+
};
27+
let did = adt.did();
1728

18-
if match_def_path(cx, did, &paths::ARRAY_INTO_ITER) {
19-
// For array::IntoIter<T, const N: usize>, the length is the second generic
20-
// parameter.
21-
substs
22-
.const_at(1)
23-
.try_eval_target_usize(cx.tcx, cx.param_env)
24-
.map(u128::from)
25-
} else if match_def_path(cx, did, &paths::SLICE_ITER)
26-
&& let ExprKind::MethodCall(_, recv, ..) = iter.kind
27-
&& let ExprKind::Array(array) = recv.peel_borrows().kind
28-
{
29+
if match_def_path(cx, did, &paths::ARRAY_INTO_ITER) {
30+
// For array::IntoIter<T, const N: usize>, the length is the second generic
31+
// parameter.
32+
substs
33+
.const_at(1)
34+
.try_eval_target_usize(cx.tcx, cx.param_env)
35+
.map(u128::from)
36+
} else if match_def_path(cx, did, &paths::SLICE_ITER)
37+
&& let ExprKind::MethodCall(_, recv, ..) = iter.kind
38+
{
39+
if let ty::Array(_, len) = cx.typeck_results().expr_ty(recv).peel_refs().kind() {
2940
// For slice::Iter<'_, T>, the receiver might be an array literal: [1,2,3].iter().skip(..)
30-
array.len().try_into().ok()
31-
} else if match_def_path(cx, did, &paths::ITER_EMPTY) {
32-
Some(0)
33-
} else if match_def_path(cx, did, &paths::ITER_ONCE) {
34-
Some(1)
41+
len.try_eval_target_usize(cx.tcx, cx.param_env).map(u128::from)
42+
} else if let Some(args) = VecArgs::hir(cx, expr_or_init(cx, recv)) {
43+
match args {
44+
VecArgs::Vec(vec) => vec.len().try_into().ok(),
45+
VecArgs::Repeat(_, len) => expr_as_u128(cx, len),
46+
}
3547
} else {
3648
None
3749
}
38-
} else {
50+
} else if match_def_path(cx, did, &paths::ITER_EMPTY) {
51+
Some(0)
52+
} else if match_def_path(cx, did, &paths::ITER_ONCE) {
53+
Some(1)
54+
} else {
3955
None
4056
}
4157
}
@@ -50,9 +66,8 @@ fn check<'tcx>(
5066
) {
5167
if is_trait_method(cx, expr, sym::Iterator)
5268
&& let Some(len) = get_iterator_length(cx, recv)
53-
&& let ExprKind::Lit(lit) = arg.kind
54-
&& let LitKind::Int(skip, _) = lit.node
55-
&& skip > len
69+
&& let Some(skipped) = expr_as_u128(cx, arg)
70+
&& skipped > len
5671
{
5772
span_lint_and_note(cx, ITER_OUT_OF_BOUNDS, expr.span, message, None, note);
5873
}

clippy_lints/src/methods/mod.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -3557,7 +3557,7 @@ declare_clippy_lint! {
35573557
/// ```rust
35583558
/// for _ in [1, 2, 3].iter() {}
35593559
/// ```
3560-
#[clippy::version = "1.73.0"]
3560+
#[clippy::version = "1.74.0"]
35613561
pub ITER_OUT_OF_BOUNDS,
35623562
suspicious,
35633563
"calls to `.take()` or `.skip()` that are out of bounds"

tests/ui/iter_out_of_bounds.rs

+19-3
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,7 @@
1+
//@no-rustfix
2+
13
#![deny(clippy::iter_out_of_bounds)]
4+
#![allow(clippy::useless_vec)]
25

36
fn opaque_empty_iter() -> impl Iterator<Item = ()> {
47
std::iter::empty()
@@ -21,12 +24,25 @@ fn main() {
2124
for _ in [1, 2, 3].iter().skip(4) {}
2225
//~^ ERROR: this `.skip()` call skips more items than the iterator will produce
2326

27+
for _ in [1; 3].iter().skip(4) {}
28+
//~^ ERROR: this `.skip()` call skips more items than the iterator will produce
29+
2430
// Should not lint
2531
for _ in opaque_empty_iter().skip(1) {}
2632

27-
// Should not lint
28-
let empty: [i8; 0] = [];
29-
for _ in empty.iter().skip(1) {}
33+
for _ in vec![1, 2, 3].iter().skip(4) {}
34+
//~^ ERROR: this `.skip()` call skips more items than the iterator will produce
35+
36+
for _ in vec![1; 3].iter().skip(4) {}
37+
//~^ ERROR: this `.skip()` call skips more items than the iterator will produce
38+
39+
let x = [1, 2, 3];
40+
for _ in x.iter().skip(4) {}
41+
//~^ ERROR: this `.skip()` call skips more items than the iterator will produce
42+
43+
let n = 4;
44+
for _ in x.iter().skip(n) {}
45+
//~^ ERROR: this `.skip()` call skips more items than the iterator will produce
3046

3147
let empty = std::iter::empty::<i8>;
3248

tests/ui/iter_out_of_bounds.stderr

+49-9
Original file line numberDiff line numberDiff line change
@@ -1,71 +1,111 @@
11
error: this `.skip()` call skips more items than the iterator will produce
2-
--> $DIR/iter_out_of_bounds.rs:8:14
2+
--> $DIR/iter_out_of_bounds.rs:11:14
33
|
44
LL | for _ in [1, 2, 3].iter().skip(4) {
55
| ^^^^^^^^^^^^^^^^^^^^^^^^
66
|
77
= note: this operation is useless and will create an empty iterator
88
note: the lint level is defined here
9-
--> $DIR/iter_out_of_bounds.rs:1:9
9+
--> $DIR/iter_out_of_bounds.rs:3:9
1010
|
1111
LL | #![deny(clippy::iter_out_of_bounds)]
1212
| ^^^^^^^^^^^^^^^^^^^^^^^^^^
1313

1414
error: this `.take()` call takes more items than the iterator will produce
15-
--> $DIR/iter_out_of_bounds.rs:12:19
15+
--> $DIR/iter_out_of_bounds.rs:15:19
1616
|
1717
LL | for (i, _) in [1, 2, 3].iter().take(4).enumerate() {
1818
| ^^^^^^^^^^^^^^^^^^^^^^^^
1919
|
2020
= note: this operation is useless and the returned iterator will simply yield the same items
2121

2222
error: this `.take()` call takes more items than the iterator will produce
23-
--> $DIR/iter_out_of_bounds.rs:18:14
23+
--> $DIR/iter_out_of_bounds.rs:21:14
2424
|
2525
LL | for _ in (&&&&&&[1, 2, 3]).iter().take(4) {}
2626
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
2727
|
2828
= note: this operation is useless and the returned iterator will simply yield the same items
2929

3030
error: this `.skip()` call skips more items than the iterator will produce
31-
--> $DIR/iter_out_of_bounds.rs:21:14
31+
--> $DIR/iter_out_of_bounds.rs:24:14
3232
|
3333
LL | for _ in [1, 2, 3].iter().skip(4) {}
3434
| ^^^^^^^^^^^^^^^^^^^^^^^^
3535
|
3636
= note: this operation is useless and will create an empty iterator
3737

38+
error: this `.skip()` call skips more items than the iterator will produce
39+
--> $DIR/iter_out_of_bounds.rs:27:14
40+
|
41+
LL | for _ in [1; 3].iter().skip(4) {}
42+
| ^^^^^^^^^^^^^^^^^^^^^
43+
|
44+
= note: this operation is useless and will create an empty iterator
45+
3846
error: this `.skip()` call skips more items than the iterator will produce
3947
--> $DIR/iter_out_of_bounds.rs:33:14
4048
|
49+
LL | for _ in vec![1, 2, 3].iter().skip(4) {}
50+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
51+
|
52+
= note: this operation is useless and will create an empty iterator
53+
54+
error: this `.skip()` call skips more items than the iterator will produce
55+
--> $DIR/iter_out_of_bounds.rs:36:14
56+
|
57+
LL | for _ in vec![1; 3].iter().skip(4) {}
58+
| ^^^^^^^^^^^^^^^^^^^^^^^^^
59+
|
60+
= note: this operation is useless and will create an empty iterator
61+
62+
error: this `.skip()` call skips more items than the iterator will produce
63+
--> $DIR/iter_out_of_bounds.rs:40:14
64+
|
65+
LL | for _ in x.iter().skip(4) {}
66+
| ^^^^^^^^^^^^^^^^
67+
|
68+
= note: this operation is useless and will create an empty iterator
69+
70+
error: this `.skip()` call skips more items than the iterator will produce
71+
--> $DIR/iter_out_of_bounds.rs:44:14
72+
|
73+
LL | for _ in x.iter().skip(n) {}
74+
| ^^^^^^^^^^^^^^^^
75+
|
76+
= note: this operation is useless and will create an empty iterator
77+
78+
error: this `.skip()` call skips more items than the iterator will produce
79+
--> $DIR/iter_out_of_bounds.rs:49:14
80+
|
4181
LL | for _ in empty().skip(1) {}
4282
| ^^^^^^^^^^^^^^^
4383
|
4484
= note: this operation is useless and will create an empty iterator
4585

4686
error: this `.take()` call takes more items than the iterator will produce
47-
--> $DIR/iter_out_of_bounds.rs:36:14
87+
--> $DIR/iter_out_of_bounds.rs:52:14
4888
|
4989
LL | for _ in empty().take(1) {}
5090
| ^^^^^^^^^^^^^^^
5191
|
5292
= note: this operation is useless and the returned iterator will simply yield the same items
5393

5494
error: this `.skip()` call skips more items than the iterator will produce
55-
--> $DIR/iter_out_of_bounds.rs:39:14
95+
--> $DIR/iter_out_of_bounds.rs:55:14
5696
|
5797
LL | for _ in std::iter::once(1).skip(2) {}
5898
| ^^^^^^^^^^^^^^^^^^^^^^^^^^
5999
|
60100
= note: this operation is useless and will create an empty iterator
61101

62102
error: this `.take()` call takes more items than the iterator will produce
63-
--> $DIR/iter_out_of_bounds.rs:42:14
103+
--> $DIR/iter_out_of_bounds.rs:58:14
64104
|
65105
LL | for _ in std::iter::once(1).take(2) {}
66106
| ^^^^^^^^^^^^^^^^^^^^^^^^^^
67107
|
68108
= note: this operation is useless and the returned iterator will simply yield the same items
69109

70-
error: aborting due to 8 previous errors
110+
error: aborting due to 13 previous errors
71111

tests/ui/iter_skip_next.fixed

+1
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
#![allow(clippy::disallowed_names)]
55
#![allow(clippy::iter_nth)]
66
#![allow(clippy::useless_vec)]
7+
#![allow(clippy::iter_out_of_bounds)]
78
#![allow(unused_mut, dead_code)]
89

910
extern crate option_helpers;

tests/ui/iter_skip_next.rs

+1
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
#![allow(clippy::disallowed_names)]
55
#![allow(clippy::iter_nth)]
66
#![allow(clippy::useless_vec)]
7+
#![allow(clippy::iter_out_of_bounds)]
78
#![allow(unused_mut, dead_code)]
89

910
extern crate option_helpers;

tests/ui/iter_skip_next.stderr

+7-7
Original file line numberDiff line numberDiff line change
@@ -1,43 +1,43 @@
11
error: called `skip(..).next()` on an iterator
2-
--> $DIR/iter_skip_next.rs:16:28
2+
--> $DIR/iter_skip_next.rs:17:28
33
|
44
LL | let _ = some_vec.iter().skip(42).next();
55
| ^^^^^^^^^^^^^^^^ help: use `nth` instead: `.nth(42)`
66
|
77
= note: `-D clippy::iter-skip-next` implied by `-D warnings`
88

99
error: called `skip(..).next()` on an iterator
10-
--> $DIR/iter_skip_next.rs:17:36
10+
--> $DIR/iter_skip_next.rs:18:36
1111
|
1212
LL | let _ = some_vec.iter().cycle().skip(42).next();
1313
| ^^^^^^^^^^^^^^^^ help: use `nth` instead: `.nth(42)`
1414

1515
error: called `skip(..).next()` on an iterator
16-
--> $DIR/iter_skip_next.rs:18:20
16+
--> $DIR/iter_skip_next.rs:19:20
1717
|
1818
LL | let _ = (1..10).skip(10).next();
1919
| ^^^^^^^^^^^^^^^^ help: use `nth` instead: `.nth(10)`
2020

2121
error: called `skip(..).next()` on an iterator
22-
--> $DIR/iter_skip_next.rs:19:33
22+
--> $DIR/iter_skip_next.rs:20:33
2323
|
2424
LL | let _ = &some_vec[..].iter().skip(3).next();
2525
| ^^^^^^^^^^^^^^^ help: use `nth` instead: `.nth(3)`
2626

2727
error: called `skip(..).next()` on an iterator
28-
--> $DIR/iter_skip_next.rs:27:26
28+
--> $DIR/iter_skip_next.rs:28:26
2929
|
3030
LL | let _: Vec<&str> = sp.skip(1).next().unwrap().split(' ').collect();
3131
| ^^^^^^^^^^^^^^^ help: use `nth` instead: `.nth(1)`
3232

3333
error: called `skip(..).next()` on an iterator
34-
--> $DIR/iter_skip_next.rs:29:29
34+
--> $DIR/iter_skip_next.rs:30:29
3535
|
3636
LL | let _: Vec<&str> = s.skip(1).next().unwrap().split(' ').collect();
3737
| ^^^^^^^^^^^^^^^ help: use `nth` instead: `.nth(1)`
3838

3939
error: called `skip(..).next()` on an iterator
40-
--> $DIR/iter_skip_next.rs:35:29
40+
--> $DIR/iter_skip_next.rs:36:29
4141
|
4242
LL | let _: Vec<&str> = s.skip(1).next().unwrap().split(' ').collect();
4343
| ^^^^^^^^^^^^^^^ help: use `nth` instead: `.nth(1)`

tests/ui/iter_skip_next_unfixable.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
#![warn(clippy::iter_skip_next)]
2-
#![allow(dead_code)]
2+
#![allow(dead_code, clippy::iter_out_of_bounds)]
33
//@no-rustfix
44
/// Checks implementation of `ITER_SKIP_NEXT` lint
55
fn main() {

0 commit comments

Comments
 (0)