diff --git a/clippy_lints/src/methods/iter_out_of_bounds.rs b/clippy_lints/src/methods/iter_out_of_bounds.rs index 0a94700ae8e..79c6d63254b 100644 --- a/clippy_lints/src/methods/iter_out_of_bounds.rs +++ b/clippy_lints/src/methods/iter_out_of_bounds.rs @@ -1,5 +1,6 @@ use clippy_utils::diagnostics::span_lint_and_note; -use clippy_utils::{is_trait_method, match_def_path, paths}; +use clippy_utils::higher::VecArgs; +use clippy_utils::{expr_or_init, is_trait_method, match_def_path, paths}; use rustc_ast::LitKind; use rustc_hir::{Expr, ExprKind}; use rustc_lint::LateContext; @@ -8,34 +9,49 @@ use super::ITER_OUT_OF_BOUNDS; +fn expr_as_u128(cx: &LateContext<'_>, e: &Expr<'_>) -> Option { + if let ExprKind::Lit(lit) = expr_or_init(cx, e).kind + && let LitKind::Int(n, _) = lit.node + { + Some(n) + } else { + None + } +} + /// Attempts to extract the length out of an iterator expression. fn get_iterator_length<'tcx>(cx: &LateContext<'tcx>, iter: &'tcx Expr<'tcx>) -> Option { - let iter_ty = cx.typeck_results().expr_ty(iter); + let ty::Adt(adt, substs) = cx.typeck_results().expr_ty(iter).kind() else { + return None; + }; + let did = adt.did(); - if let ty::Adt(adt, substs) = iter_ty.kind() { - let did = adt.did(); - - if match_def_path(cx, did, &paths::ARRAY_INTO_ITER) { - // For array::IntoIter, the length is the second generic - // parameter. - substs - .const_at(1) - .try_eval_target_usize(cx.tcx, cx.param_env) - .map(u128::from) - } else if match_def_path(cx, did, &paths::SLICE_ITER) - && let ExprKind::MethodCall(_, recv, ..) = iter.kind - && let ExprKind::Array(array) = recv.peel_borrows().kind - { + if match_def_path(cx, did, &paths::ARRAY_INTO_ITER) { + // For array::IntoIter, the length is the second generic + // parameter. + substs + .const_at(1) + .try_eval_target_usize(cx.tcx, cx.param_env) + .map(u128::from) + } else if match_def_path(cx, did, &paths::SLICE_ITER) + && let ExprKind::MethodCall(_, recv, ..) = iter.kind + { + if let ty::Array(_, len) = cx.typeck_results().expr_ty(recv).peel_refs().kind() { // For slice::Iter<'_, T>, the receiver might be an array literal: [1,2,3].iter().skip(..) - array.len().try_into().ok() - } else if match_def_path(cx, did, &paths::ITER_EMPTY) { - Some(0) - } else if match_def_path(cx, did, &paths::ITER_ONCE) { - Some(1) + len.try_eval_target_usize(cx.tcx, cx.param_env).map(u128::from) + } else if let Some(args) = VecArgs::hir(cx, expr_or_init(cx, recv)) { + match args { + VecArgs::Vec(vec) => vec.len().try_into().ok(), + VecArgs::Repeat(_, len) => expr_as_u128(cx, len), + } } else { None } - } else { + } else if match_def_path(cx, did, &paths::ITER_EMPTY) { + Some(0) + } else if match_def_path(cx, did, &paths::ITER_ONCE) { + Some(1) + } else { None } } @@ -50,9 +66,8 @@ fn check<'tcx>( ) { if is_trait_method(cx, expr, sym::Iterator) && let Some(len) = get_iterator_length(cx, recv) - && let ExprKind::Lit(lit) = arg.kind - && let LitKind::Int(skip, _) = lit.node - && skip > len + && let Some(skipped) = expr_as_u128(cx, arg) + && skipped > len { span_lint_and_note(cx, ITER_OUT_OF_BOUNDS, expr.span, message, None, note); } diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs index 5dbbd7f9618..76beb9380a9 100644 --- a/clippy_lints/src/methods/mod.rs +++ b/clippy_lints/src/methods/mod.rs @@ -3557,7 +3557,7 @@ /// ```rust /// for _ in [1, 2, 3].iter() {} /// ``` - #[clippy::version = "1.73.0"] + #[clippy::version = "1.74.0"] pub ITER_OUT_OF_BOUNDS, suspicious, "calls to `.take()` or `.skip()` that are out of bounds" diff --git a/tests/ui/iter_out_of_bounds.rs b/tests/ui/iter_out_of_bounds.rs index 07908f929cd..a2c17b452d6 100644 --- a/tests/ui/iter_out_of_bounds.rs +++ b/tests/ui/iter_out_of_bounds.rs @@ -1,4 +1,7 @@ +//@no-rustfix + #![deny(clippy::iter_out_of_bounds)] +#![allow(clippy::useless_vec)] fn opaque_empty_iter() -> impl Iterator { std::iter::empty() @@ -21,12 +24,25 @@ fn main() { for _ in [1, 2, 3].iter().skip(4) {} //~^ ERROR: this `.skip()` call skips more items than the iterator will produce + for _ in [1; 3].iter().skip(4) {} + //~^ ERROR: this `.skip()` call skips more items than the iterator will produce + // Should not lint for _ in opaque_empty_iter().skip(1) {} - // Should not lint - let empty: [i8; 0] = []; - for _ in empty.iter().skip(1) {} + for _ in vec![1, 2, 3].iter().skip(4) {} + //~^ ERROR: this `.skip()` call skips more items than the iterator will produce + + for _ in vec![1; 3].iter().skip(4) {} + //~^ ERROR: this `.skip()` call skips more items than the iterator will produce + + let x = [1, 2, 3]; + for _ in x.iter().skip(4) {} + //~^ ERROR: this `.skip()` call skips more items than the iterator will produce + + let n = 4; + for _ in x.iter().skip(n) {} + //~^ ERROR: this `.skip()` call skips more items than the iterator will produce let empty = std::iter::empty::; diff --git a/tests/ui/iter_out_of_bounds.stderr b/tests/ui/iter_out_of_bounds.stderr index c819ca3771f..d4b7b4355d0 100644 --- a/tests/ui/iter_out_of_bounds.stderr +++ b/tests/ui/iter_out_of_bounds.stderr @@ -1,18 +1,18 @@ error: this `.skip()` call skips more items than the iterator will produce - --> $DIR/iter_out_of_bounds.rs:8:14 + --> $DIR/iter_out_of_bounds.rs:11:14 | LL | for _ in [1, 2, 3].iter().skip(4) { | ^^^^^^^^^^^^^^^^^^^^^^^^ | = note: this operation is useless and will create an empty iterator note: the lint level is defined here - --> $DIR/iter_out_of_bounds.rs:1:9 + --> $DIR/iter_out_of_bounds.rs:3:9 | LL | #![deny(clippy::iter_out_of_bounds)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^ error: this `.take()` call takes more items than the iterator will produce - --> $DIR/iter_out_of_bounds.rs:12:19 + --> $DIR/iter_out_of_bounds.rs:15:19 | LL | for (i, _) in [1, 2, 3].iter().take(4).enumerate() { | ^^^^^^^^^^^^^^^^^^^^^^^^ @@ -20,7 +20,7 @@ LL | for (i, _) in [1, 2, 3].iter().take(4).enumerate() { = note: this operation is useless and the returned iterator will simply yield the same items error: this `.take()` call takes more items than the iterator will produce - --> $DIR/iter_out_of_bounds.rs:18:14 + --> $DIR/iter_out_of_bounds.rs:21:14 | LL | for _ in (&&&&&&[1, 2, 3]).iter().take(4) {} | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -28,23 +28,63 @@ LL | for _ in (&&&&&&[1, 2, 3]).iter().take(4) {} = note: this operation is useless and the returned iterator will simply yield the same items error: this `.skip()` call skips more items than the iterator will produce - --> $DIR/iter_out_of_bounds.rs:21:14 + --> $DIR/iter_out_of_bounds.rs:24:14 | LL | for _ in [1, 2, 3].iter().skip(4) {} | ^^^^^^^^^^^^^^^^^^^^^^^^ | = note: this operation is useless and will create an empty iterator +error: this `.skip()` call skips more items than the iterator will produce + --> $DIR/iter_out_of_bounds.rs:27:14 + | +LL | for _ in [1; 3].iter().skip(4) {} + | ^^^^^^^^^^^^^^^^^^^^^ + | + = note: this operation is useless and will create an empty iterator + error: this `.skip()` call skips more items than the iterator will produce --> $DIR/iter_out_of_bounds.rs:33:14 | +LL | for _ in vec![1, 2, 3].iter().skip(4) {} + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: this operation is useless and will create an empty iterator + +error: this `.skip()` call skips more items than the iterator will produce + --> $DIR/iter_out_of_bounds.rs:36:14 + | +LL | for _ in vec![1; 3].iter().skip(4) {} + | ^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: this operation is useless and will create an empty iterator + +error: this `.skip()` call skips more items than the iterator will produce + --> $DIR/iter_out_of_bounds.rs:40:14 + | +LL | for _ in x.iter().skip(4) {} + | ^^^^^^^^^^^^^^^^ + | + = note: this operation is useless and will create an empty iterator + +error: this `.skip()` call skips more items than the iterator will produce + --> $DIR/iter_out_of_bounds.rs:44:14 + | +LL | for _ in x.iter().skip(n) {} + | ^^^^^^^^^^^^^^^^ + | + = note: this operation is useless and will create an empty iterator + +error: this `.skip()` call skips more items than the iterator will produce + --> $DIR/iter_out_of_bounds.rs:49:14 + | LL | for _ in empty().skip(1) {} | ^^^^^^^^^^^^^^^ | = note: this operation is useless and will create an empty iterator error: this `.take()` call takes more items than the iterator will produce - --> $DIR/iter_out_of_bounds.rs:36:14 + --> $DIR/iter_out_of_bounds.rs:52:14 | LL | for _ in empty().take(1) {} | ^^^^^^^^^^^^^^^ @@ -52,7 +92,7 @@ LL | for _ in empty().take(1) {} = note: this operation is useless and the returned iterator will simply yield the same items error: this `.skip()` call skips more items than the iterator will produce - --> $DIR/iter_out_of_bounds.rs:39:14 + --> $DIR/iter_out_of_bounds.rs:55:14 | LL | for _ in std::iter::once(1).skip(2) {} | ^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -60,12 +100,12 @@ LL | for _ in std::iter::once(1).skip(2) {} = note: this operation is useless and will create an empty iterator error: this `.take()` call takes more items than the iterator will produce - --> $DIR/iter_out_of_bounds.rs:42:14 + --> $DIR/iter_out_of_bounds.rs:58:14 | LL | for _ in std::iter::once(1).take(2) {} | ^^^^^^^^^^^^^^^^^^^^^^^^^^ | = note: this operation is useless and the returned iterator will simply yield the same items -error: aborting due to 8 previous errors +error: aborting due to 13 previous errors diff --git a/tests/ui/iter_skip_next.fixed b/tests/ui/iter_skip_next.fixed index 310b24a9cde..3e41b363249 100644 --- a/tests/ui/iter_skip_next.fixed +++ b/tests/ui/iter_skip_next.fixed @@ -4,6 +4,7 @@ #![allow(clippy::disallowed_names)] #![allow(clippy::iter_nth)] #![allow(clippy::useless_vec)] +#![allow(clippy::iter_out_of_bounds)] #![allow(unused_mut, dead_code)] extern crate option_helpers; diff --git a/tests/ui/iter_skip_next.rs b/tests/ui/iter_skip_next.rs index 222d6a2a184..6d96441ca96 100644 --- a/tests/ui/iter_skip_next.rs +++ b/tests/ui/iter_skip_next.rs @@ -4,6 +4,7 @@ #![allow(clippy::disallowed_names)] #![allow(clippy::iter_nth)] #![allow(clippy::useless_vec)] +#![allow(clippy::iter_out_of_bounds)] #![allow(unused_mut, dead_code)] extern crate option_helpers; diff --git a/tests/ui/iter_skip_next.stderr b/tests/ui/iter_skip_next.stderr index ca6970b27f1..4ee26e088ce 100644 --- a/tests/ui/iter_skip_next.stderr +++ b/tests/ui/iter_skip_next.stderr @@ -1,5 +1,5 @@ error: called `skip(..).next()` on an iterator - --> $DIR/iter_skip_next.rs:16:28 + --> $DIR/iter_skip_next.rs:17:28 | LL | let _ = some_vec.iter().skip(42).next(); | ^^^^^^^^^^^^^^^^ help: use `nth` instead: `.nth(42)` @@ -7,37 +7,37 @@ LL | let _ = some_vec.iter().skip(42).next(); = note: `-D clippy::iter-skip-next` implied by `-D warnings` error: called `skip(..).next()` on an iterator - --> $DIR/iter_skip_next.rs:17:36 + --> $DIR/iter_skip_next.rs:18:36 | LL | let _ = some_vec.iter().cycle().skip(42).next(); | ^^^^^^^^^^^^^^^^ help: use `nth` instead: `.nth(42)` error: called `skip(..).next()` on an iterator - --> $DIR/iter_skip_next.rs:18:20 + --> $DIR/iter_skip_next.rs:19:20 | LL | let _ = (1..10).skip(10).next(); | ^^^^^^^^^^^^^^^^ help: use `nth` instead: `.nth(10)` error: called `skip(..).next()` on an iterator - --> $DIR/iter_skip_next.rs:19:33 + --> $DIR/iter_skip_next.rs:20:33 | LL | let _ = &some_vec[..].iter().skip(3).next(); | ^^^^^^^^^^^^^^^ help: use `nth` instead: `.nth(3)` error: called `skip(..).next()` on an iterator - --> $DIR/iter_skip_next.rs:27:26 + --> $DIR/iter_skip_next.rs:28:26 | LL | let _: Vec<&str> = sp.skip(1).next().unwrap().split(' ').collect(); | ^^^^^^^^^^^^^^^ help: use `nth` instead: `.nth(1)` error: called `skip(..).next()` on an iterator - --> $DIR/iter_skip_next.rs:29:29 + --> $DIR/iter_skip_next.rs:30:29 | LL | let _: Vec<&str> = s.skip(1).next().unwrap().split(' ').collect(); | ^^^^^^^^^^^^^^^ help: use `nth` instead: `.nth(1)` error: called `skip(..).next()` on an iterator - --> $DIR/iter_skip_next.rs:35:29 + --> $DIR/iter_skip_next.rs:36:29 | LL | let _: Vec<&str> = s.skip(1).next().unwrap().split(' ').collect(); | ^^^^^^^^^^^^^^^ help: use `nth` instead: `.nth(1)` diff --git a/tests/ui/iter_skip_next_unfixable.rs b/tests/ui/iter_skip_next_unfixable.rs index 9c224f4117d..6c98bdc8c88 100644 --- a/tests/ui/iter_skip_next_unfixable.rs +++ b/tests/ui/iter_skip_next_unfixable.rs @@ -1,5 +1,5 @@ #![warn(clippy::iter_skip_next)] -#![allow(dead_code)] +#![allow(dead_code, clippy::iter_out_of_bounds)] //@no-rustfix /// Checks implementation of `ITER_SKIP_NEXT` lint fn main() {