From 86b66443797bfa89dbc099722c20c9c2c5943217 Mon Sep 17 00:00:00 2001 From: y21 <30553356+y21@users.noreply.github.com> Date: Thu, 24 Aug 2023 20:12:03 +0200 Subject: [PATCH 1/3] new lint: `iter_out_of_bounds` --- CHANGELOG.md | 1 + clippy_lints/src/declared_lints.rs | 1 + .../src/methods/iter_out_of_bounds.rs | 91 +++++++++++++++++++ clippy_lints/src/methods/mod.rs | 32 ++++++- clippy_utils/src/paths.rs | 2 + tests/ui/iter_out_of_bounds.rs | 44 +++++++++ tests/ui/iter_out_of_bounds.stderr | 71 +++++++++++++++ tests/ui/iter_skip_zero.fixed | 2 +- tests/ui/iter_skip_zero.rs | 2 +- 9 files changed, 242 insertions(+), 4 deletions(-) create mode 100644 clippy_lints/src/methods/iter_out_of_bounds.rs create mode 100644 tests/ui/iter_out_of_bounds.rs create mode 100644 tests/ui/iter_out_of_bounds.stderr diff --git a/CHANGELOG.md b/CHANGELOG.md index 4e33cb7b4570..43633b36925c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5030,6 +5030,7 @@ Released 2018-09-13 [`iter_nth_zero`]: https://rust-lang.github.io/rust-clippy/master/index.html#iter_nth_zero [`iter_on_empty_collections`]: https://rust-lang.github.io/rust-clippy/master/index.html#iter_on_empty_collections [`iter_on_single_items`]: https://rust-lang.github.io/rust-clippy/master/index.html#iter_on_single_items +[`iter_out_of_bounds`]: https://rust-lang.github.io/rust-clippy/master/index.html#iter_out_of_bounds [`iter_overeager_cloned`]: https://rust-lang.github.io/rust-clippy/master/index.html#iter_overeager_cloned [`iter_skip_next`]: https://rust-lang.github.io/rust-clippy/master/index.html#iter_skip_next [`iter_skip_zero`]: https://rust-lang.github.io/rust-clippy/master/index.html#iter_skip_zero diff --git a/clippy_lints/src/declared_lints.rs b/clippy_lints/src/declared_lints.rs index 1be9720fbbf8..10cb82013061 100644 --- a/clippy_lints/src/declared_lints.rs +++ b/clippy_lints/src/declared_lints.rs @@ -364,6 +364,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[ crate::methods::ITER_NTH_ZERO_INFO, crate::methods::ITER_ON_EMPTY_COLLECTIONS_INFO, crate::methods::ITER_ON_SINGLE_ITEMS_INFO, + crate::methods::ITER_OUT_OF_BOUNDS_INFO, crate::methods::ITER_OVEREAGER_CLONED_INFO, crate::methods::ITER_SKIP_NEXT_INFO, crate::methods::ITER_SKIP_ZERO_INFO, diff --git a/clippy_lints/src/methods/iter_out_of_bounds.rs b/clippy_lints/src/methods/iter_out_of_bounds.rs new file mode 100644 index 000000000000..0a94700ae8ed --- /dev/null +++ b/clippy_lints/src/methods/iter_out_of_bounds.rs @@ -0,0 +1,91 @@ +use clippy_utils::diagnostics::span_lint_and_note; +use clippy_utils::{is_trait_method, match_def_path, paths}; +use rustc_ast::LitKind; +use rustc_hir::{Expr, ExprKind}; +use rustc_lint::LateContext; +use rustc_middle::ty::{self}; +use rustc_span::sym; + +use super::ITER_OUT_OF_BOUNDS; + +/// 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); + + 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 + { + // 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) + } else { + None + } + } else { + None + } +} + +fn check<'tcx>( + cx: &LateContext<'tcx>, + expr: &'tcx Expr<'tcx>, + recv: &'tcx Expr<'tcx>, + arg: &'tcx Expr<'tcx>, + message: &'static str, + note: &'static str, +) { + 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 + { + span_lint_and_note(cx, ITER_OUT_OF_BOUNDS, expr.span, message, None, note); + } +} + +pub(super) fn check_skip<'tcx>( + cx: &LateContext<'tcx>, + expr: &'tcx Expr<'tcx>, + recv: &'tcx Expr<'tcx>, + arg: &'tcx Expr<'tcx>, +) { + check( + cx, + expr, + recv, + arg, + "this `.skip()` call skips more items than the iterator will produce", + "this operation is useless and will create an empty iterator", + ); +} + +pub(super) fn check_take<'tcx>( + cx: &LateContext<'tcx>, + expr: &'tcx Expr<'tcx>, + recv: &'tcx Expr<'tcx>, + arg: &'tcx Expr<'tcx>, +) { + check( + cx, + expr, + recv, + arg, + "this `.take()` call takes more items than the iterator will produce", + "this operation is useless and the returned iterator will simply yield the same items", + ); +} diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs index 5075137caa0c..5dbbd7f96182 100644 --- a/clippy_lints/src/methods/mod.rs +++ b/clippy_lints/src/methods/mod.rs @@ -43,6 +43,7 @@ mod iter_next_slice; mod iter_nth; mod iter_nth_zero; mod iter_on_single_or_empty_collections; +mod iter_out_of_bounds; mod iter_overeager_cloned; mod iter_skip_next; mod iter_skip_zero; @@ -3538,6 +3539,30 @@ declare_clippy_lint! { "acquiring a write lock when a read lock would work" } +declare_clippy_lint! { + /// ### What it does + /// Looks for iterator combinator calls such as `.take(x)` or `.skip(x)` + /// where `x` is greater than the amount of items that an iterator will produce. + /// + /// ### Why is this bad? + /// Taking or skipping more items than there are in an iterator either creates an iterator + /// with all items from the original iterator or an iterator with no items at all. + /// This is most likely not what the user intended to do. + /// + /// ### Example + /// ```rust + /// for _ in [1, 2, 3].iter().take(4) {} + /// ``` + /// Use instead: + /// ```rust + /// for _ in [1, 2, 3].iter() {} + /// ``` + #[clippy::version = "1.73.0"] + pub ITER_OUT_OF_BOUNDS, + suspicious, + "calls to `.take()` or `.skip()` that are out of bounds" +} + pub struct Methods { avoid_breaking_exported_api: bool, msrv: Msrv, @@ -3676,7 +3701,8 @@ impl_lint_pass!(Methods => [ STRING_LIT_CHARS_ANY, ITER_SKIP_ZERO, FILTER_MAP_BOOL_THEN, - READONLY_WRITE_LOCK + READONLY_WRITE_LOCK, + ITER_OUT_OF_BOUNDS, ]); /// Extracts a method call name, args, and `Span` of the method name. @@ -4136,6 +4162,7 @@ impl Methods { }, ("skip", [arg]) => { iter_skip_zero::check(cx, expr, arg); + iter_out_of_bounds::check_skip(cx, expr, recv, arg); if let Some(("cloned", recv2, [], _span2, _)) = method_call(recv) { iter_overeager_cloned::check(cx, expr, recv, recv2, @@ -4163,7 +4190,8 @@ impl Methods { } }, ("step_by", [arg]) => iterator_step_by_zero::check(cx, expr, arg), - ("take", [_arg]) => { + ("take", [arg]) => { + iter_out_of_bounds::check_take(cx, expr, recv, arg); if let Some(("cloned", recv2, [], _span2, _)) = method_call(recv) { iter_overeager_cloned::check(cx, expr, recv, recv2, iter_overeager_cloned::Op::LaterCloned, false); diff --git a/clippy_utils/src/paths.rs b/clippy_utils/src/paths.rs index e72d063cfd9b..61022ea0651e 100644 --- a/clippy_utils/src/paths.rs +++ b/clippy_utils/src/paths.rs @@ -49,6 +49,7 @@ pub const IDENT: [&str; 3] = ["rustc_span", "symbol", "Ident"]; pub const IDENT_AS_STR: [&str; 4] = ["rustc_span", "symbol", "Ident", "as_str"]; pub const INSERT_STR: [&str; 4] = ["alloc", "string", "String", "insert_str"]; pub const ITER_EMPTY: [&str; 5] = ["core", "iter", "sources", "empty", "Empty"]; +pub const ITER_ONCE: [&str; 5] = ["core", "iter", "sources", "once", "Once"]; pub const ITERTOOLS_NEXT_TUPLE: [&str; 3] = ["itertools", "Itertools", "next_tuple"]; #[cfg(feature = "internal")] pub const KW_MODULE: [&str; 3] = ["rustc_span", "symbol", "kw"]; @@ -166,3 +167,4 @@ pub const DEBUG_STRUCT: [&str; 4] = ["core", "fmt", "builders", "DebugStruct"]; pub const ORD_CMP: [&str; 4] = ["core", "cmp", "Ord", "cmp"]; #[expect(clippy::invalid_paths)] // not sure why it thinks this, it works so pub const BOOL_THEN: [&str; 4] = ["core", "bool", "", "then"]; +pub const ARRAY_INTO_ITER: [&str; 4] = ["core", "array", "iter", "IntoIter"]; diff --git a/tests/ui/iter_out_of_bounds.rs b/tests/ui/iter_out_of_bounds.rs new file mode 100644 index 000000000000..07908f929cde --- /dev/null +++ b/tests/ui/iter_out_of_bounds.rs @@ -0,0 +1,44 @@ +#![deny(clippy::iter_out_of_bounds)] + +fn opaque_empty_iter() -> impl Iterator { + std::iter::empty() +} + +fn main() { + for _ in [1, 2, 3].iter().skip(4) { + //~^ ERROR: this `.skip()` call skips more items than the iterator will produce + unreachable!(); + } + for (i, _) in [1, 2, 3].iter().take(4).enumerate() { + //~^ ERROR: this `.take()` call takes more items than the iterator will produce + assert!(i <= 2); + } + + #[allow(clippy::needless_borrow)] + for _ in (&&&&&&[1, 2, 3]).iter().take(4) {} + //~^ ERROR: this `.take()` call takes more items than the iterator will produce + + for _ in [1, 2, 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) {} + + let empty = std::iter::empty::; + + for _ in empty().skip(1) {} + //~^ ERROR: this `.skip()` call skips more items than the iterator will produce + + for _ in empty().take(1) {} + //~^ ERROR: this `.take()` call takes more items than the iterator will produce + + for _ in std::iter::once(1).skip(2) {} + //~^ ERROR: this `.skip()` call skips more items than the iterator will produce + + for _ in std::iter::once(1).take(2) {} + //~^ ERROR: this `.take()` call takes more items than the iterator will produce +} diff --git a/tests/ui/iter_out_of_bounds.stderr b/tests/ui/iter_out_of_bounds.stderr new file mode 100644 index 000000000000..c819ca3771f3 --- /dev/null +++ b/tests/ui/iter_out_of_bounds.stderr @@ -0,0 +1,71 @@ +error: this `.skip()` call skips more items than the iterator will produce + --> $DIR/iter_out_of_bounds.rs:8: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 + | +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 + | +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 + | +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 + | +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:33: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 + | +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 + | +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 + | +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 + diff --git a/tests/ui/iter_skip_zero.fixed b/tests/ui/iter_skip_zero.fixed index 62a83d5905b4..447d07100e95 100644 --- a/tests/ui/iter_skip_zero.fixed +++ b/tests/ui/iter_skip_zero.fixed @@ -1,5 +1,5 @@ //@aux-build:proc_macros.rs -#![allow(clippy::useless_vec, unused)] +#![allow(clippy::useless_vec, clippy::iter_out_of_bounds, unused)] #![warn(clippy::iter_skip_zero)] #[macro_use] diff --git a/tests/ui/iter_skip_zero.rs b/tests/ui/iter_skip_zero.rs index c96696dde65d..ba63c3981808 100644 --- a/tests/ui/iter_skip_zero.rs +++ b/tests/ui/iter_skip_zero.rs @@ -1,5 +1,5 @@ //@aux-build:proc_macros.rs -#![allow(clippy::useless_vec, unused)] +#![allow(clippy::useless_vec, clippy::iter_out_of_bounds, unused)] #![warn(clippy::iter_skip_zero)] #[macro_use] From 11072b51fa2b8fbcf68f8cf6a5f4140b19c16241 Mon Sep 17 00:00:00 2001 From: y21 <30553356+y21@users.noreply.github.com> Date: Fri, 25 Aug 2023 01:13:35 +0200 Subject: [PATCH 2/3] lint vecs, version bump, more tests --- .../src/methods/iter_out_of_bounds.rs | 65 ++++++++++++------- clippy_lints/src/methods/mod.rs | 2 +- tests/ui/iter_out_of_bounds.rs | 22 ++++++- tests/ui/iter_out_of_bounds.stderr | 58 ++++++++++++++--- tests/ui/iter_skip_next.fixed | 1 + tests/ui/iter_skip_next.rs | 1 + tests/ui/iter_skip_next.stderr | 14 ++-- tests/ui/iter_skip_next_unfixable.rs | 2 +- 8 files changed, 119 insertions(+), 46 deletions(-) diff --git a/clippy_lints/src/methods/iter_out_of_bounds.rs b/clippy_lints/src/methods/iter_out_of_bounds.rs index 0a94700ae8ed..79c6d63254b2 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 rustc_span::sym; 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 5dbbd7f96182..76beb9380a95 100644 --- a/clippy_lints/src/methods/mod.rs +++ b/clippy_lints/src/methods/mod.rs @@ -3557,7 +3557,7 @@ declare_clippy_lint! { /// ```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 07908f929cde..a2c17b452d66 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 c819ca3771f3..d4b7b4355d05 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 310b24a9cde1..3e41b3632490 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 222d6a2a1848..6d96441ca96f 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 ca6970b27f16..4ee26e088ce3 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 9c224f4117d3..6c98bdc8c880 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() { From 33cc140f55e254c7c88e81b059afcc430bba3e5e Mon Sep 17 00:00:00 2001 From: y21 <30553356+y21@users.noreply.github.com> Date: Sun, 27 Aug 2023 23:38:43 +0200 Subject: [PATCH 3/3] add more negative tests --- tests/ui/iter_out_of_bounds.rs | 10 ++++++++++ tests/ui/iter_out_of_bounds.stderr | 10 +++++++++- 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/tests/ui/iter_out_of_bounds.rs b/tests/ui/iter_out_of_bounds.rs index a2c17b452d66..c34eb1be0c55 100644 --- a/tests/ui/iter_out_of_bounds.rs +++ b/tests/ui/iter_out_of_bounds.rs @@ -57,4 +57,14 @@ fn main() { for _ in std::iter::once(1).take(2) {} //~^ ERROR: this `.take()` call takes more items than the iterator will produce + + for x in [].iter().take(1) { + //~^ ERROR: this `.take()` call takes more items than the iterator will produce + let _: &i32 = x; + } + + // ok, not out of bounds + for _ in [1].iter().take(1) {} + for _ in [1, 2, 3].iter().take(2) {} + for _ in [1, 2, 3].iter().skip(2) {} } diff --git a/tests/ui/iter_out_of_bounds.stderr b/tests/ui/iter_out_of_bounds.stderr index d4b7b4355d05..98ee28fcaf6e 100644 --- a/tests/ui/iter_out_of_bounds.stderr +++ b/tests/ui/iter_out_of_bounds.stderr @@ -107,5 +107,13 @@ 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 13 previous errors +error: this `.take()` call takes more items than the iterator will produce + --> $DIR/iter_out_of_bounds.rs:61:14 + | +LL | for x in [].iter().take(1) { + | ^^^^^^^^^^^^^^^^^ + | + = note: this operation is useless and the returned iterator will simply yield the same items + +error: aborting due to 14 previous errors