diff --git a/clippy_lints/src/loops.rs b/clippy_lints/src/loops.rs index 9e45757f3f0..0d1b960cc1f 100644 --- a/clippy_lints/src/loops.rs +++ b/clippy_lints/src/loops.rs @@ -1068,7 +1068,7 @@ fn check_for_loop_range<'a, 'tcx>( // linting condition: we only indexed one variable, and indexed it directly if visitor.indexed_indirectly.is_empty() && visitor.indexed_directly.len() == 1 { - let (indexed, indexed_extent) = visitor + let (indexed, (indexed_extent, indexed_ty)) = visitor .indexed_directly .into_iter() .next() @@ -1119,7 +1119,7 @@ fn check_for_loop_range<'a, 'tcx>( } } - if is_len_call(end, indexed) { + if is_len_call(end, indexed) || is_end_eq_array_len(cx, end, limits, indexed_ty) { String::new() } else { match limits { @@ -1207,6 +1207,28 @@ fn is_len_call(expr: &Expr, var: Name) -> bool { false } +fn is_end_eq_array_len( + cx: &LateContext<'_, '_>, + end: &Expr, + limits: ast::RangeLimits, + indexed_ty: Ty<'_>, +) -> bool { + if_chain! { + if let ExprKind::Lit(ref lit) = end.node; + if let ast::LitKind::Int(end_int, _) = lit.node; + if let ty::TyKind::Array(_, arr_len_const) = indexed_ty.sty; + if let Some(arr_len) = arr_len_const.assert_usize(cx.tcx); + then { + return match limits { + ast::RangeLimits::Closed => end_int + 1 >= arr_len.into(), + ast::RangeLimits::HalfOpen => end_int >= arr_len.into(), + }; + } + } + + false +} + fn check_for_loop_reverse_range<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, arg: &'tcx Expr, expr: &'tcx Expr) { // if this for loop is iterating over a two-sided range... if let Some(higher::Range { @@ -1678,7 +1700,7 @@ struct VarVisitor<'a, 'tcx: 'a> { indexed_indirectly: FxHashMap>, /// subset of `indexed` of vars that are indexed directly: `v[i]` /// this will not contain cases like `v[calc_index(i)]` or `v[(i + 4) % N]` - indexed_directly: FxHashMap>, + indexed_directly: FxHashMap, Ty<'tcx>)>, /// Any names that are used outside an index operation. /// Used to detect things like `&mut vec` used together with `vec[i]` referenced: FxHashSet, @@ -1725,7 +1747,10 @@ impl<'a, 'tcx> VarVisitor<'a, 'tcx> { self.indexed_indirectly.insert(seqvar.segments[0].ident.name, Some(extent)); } if index_used_directly { - self.indexed_directly.insert(seqvar.segments[0].ident.name, Some(extent)); + self.indexed_directly.insert( + seqvar.segments[0].ident.name, + (Some(extent), self.cx.tables.node_id_to_type(seqexpr.hir_id)), + ); } return false; // no need to walk further *on the variable* } @@ -1734,7 +1759,10 @@ impl<'a, 'tcx> VarVisitor<'a, 'tcx> { self.indexed_indirectly.insert(seqvar.segments[0].ident.name, None); } if index_used_directly { - self.indexed_directly.insert(seqvar.segments[0].ident.name, None); + self.indexed_directly.insert( + seqvar.segments[0].ident.name, + (None, self.cx.tables.node_id_to_type(seqexpr.hir_id)), + ); } return false; // no need to walk further *on the variable* } diff --git a/tests/ui/for_loop.stderr b/tests/ui/for_loop.stderr index 33176335783..695209de53f 100644 --- a/tests/ui/for_loop.stderr +++ b/tests/ui/for_loop.stderr @@ -97,8 +97,8 @@ error: the loop variable `j` is only used to index `STATIC`. | ^^^^ help: consider using an iterator | -110 | for in STATIC.iter().take(4) { - | ^^^^^^ ^^^^^^^^^^^^^^^^^^^^^ +110 | for in &STATIC { + | ^^^^^^ ^^^^^^^ error: the loop variable `j` is only used to index `CONST`. --> $DIR/for_loop.rs:114:14 @@ -107,8 +107,8 @@ error: the loop variable `j` is only used to index `CONST`. | ^^^^ help: consider using an iterator | -114 | for in CONST.iter().take(4) { - | ^^^^^^ ^^^^^^^^^^^^^^^^^^^^ +114 | for in &CONST { + | ^^^^^^ ^^^^^^ error: the loop variable `i` is used to index `vec` --> $DIR/for_loop.rs:118:14 diff --git a/tests/ui/needless_range_loop.rs b/tests/ui/needless_range_loop.rs index 3da9267d38b..c1992bba548 100644 --- a/tests/ui/needless_range_loop.rs +++ b/tests/ui/needless_range_loop.rs @@ -13,7 +13,7 @@ fn calc_idx(i: usize) -> usize { } fn main() { - let ns = [2, 3, 5, 7]; + let ns = vec![2, 3, 5, 7]; for i in 3..10 { println!("{}", ns[i]); @@ -76,4 +76,18 @@ fn main() { for i in x..=x + 4 { vec[i] += 1; } + + let arr = [1,2,3]; + + for i in 0..3 { + println!("{}", arr[i]); + } + + for i in 0..2 { + println!("{}", arr[i]); + } + + for i in 1..3 { + println!("{}", arr[i]); + } } diff --git a/tests/ui/needless_range_loop.stderr b/tests/ui/needless_range_loop.stderr index d62a0434d0b..688e9fc3a2c 100644 --- a/tests/ui/needless_range_loop.stderr +++ b/tests/ui/needless_range_loop.stderr @@ -50,5 +50,35 @@ help: consider using an iterator 76 | for in vec.iter_mut().skip(x).take(4 + 1) { | ^^^^^^ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ -error: aborting due to 5 previous errors +error: the loop variable `i` is only used to index `arr`. + --> $DIR/needless_range_loop.rs:82:14 + | +82 | for i in 0..3 { + | ^^^^ +help: consider using an iterator + | +82 | for in &arr { + | ^^^^^^ ^^^^ + +error: the loop variable `i` is only used to index `arr`. + --> $DIR/needless_range_loop.rs:86:14 + | +86 | for i in 0..2 { + | ^^^^ +help: consider using an iterator + | +86 | for in arr.iter().take(2) { + | ^^^^^^ ^^^^^^^^^^^^^^^^^^ + +error: the loop variable `i` is only used to index `arr`. + --> $DIR/needless_range_loop.rs:90:14 + | +90 | for i in 1..3 { + | ^^^^ +help: consider using an iterator + | +90 | for in arr.iter().skip(1) { + | ^^^^^^ ^^^^^^^^^^^^^^^^^^ + +error: aborting due to 8 previous errors