diff --git a/clippy_lints/src/indexing_slicing.rs b/clippy_lints/src/indexing_slicing.rs index 01f9f45c589..b4e6414195e 100644 --- a/clippy_lints/src/indexing_slicing.rs +++ b/clippy_lints/src/indexing_slicing.rs @@ -1,8 +1,9 @@ //! lint on indexing and slicing operations use crate::consts::{constant, Constant}; +use crate::utils; +use crate::utils::higher; use crate::utils::higher::Range; -use crate::utils::{self, higher}; use rustc::hir::*; use rustc::lint::*; use rustc::ty; @@ -97,89 +98,65 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for IndexingSlicing { fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr) { if let ExprIndex(ref array, ref index) = &expr.node { let ty = cx.tables.expr_ty(array); - match &index.node { - // Both ExprStruct and ExprPath require this approach's checks - // on the `range` returned by `higher::range(cx, index)`. - // ExprStruct handles &x[n..m], &x[n..] and &x[..n]. - // ExprPath handles &x[..] and x[var] - ExprStruct(..) | ExprPath(..) => { - if let Some(range) = higher::range(cx, index) { - if let ty::TyArray(_, s) = ty.sty { - let size: u128 = s.assert_usize(cx.tcx).unwrap().into(); - // Index is a constant range. - if let Some((start, end)) = to_const_range(cx, range, size) { - if start > size || end > size { - utils::span_lint( - cx, - OUT_OF_BOUNDS_INDEXING, - expr.span, - "range is out of bounds", - ); - } // Else range is in bounds, ok. - - return; - } + if let Some(range) = higher::range(cx, index) { + // Ranged indexes, i.e. &x[n..m], &x[n..], &x[..n] and &x[..] + if let ty::TyArray(_, s) = ty.sty { + let size: u128 = s.assert_usize(cx.tcx).unwrap().into(); + // Index is a constant range. + if let Some((start, end)) = to_const_range(cx, range, size) { + if start > size || end > size { + utils::span_lint( + cx, + OUT_OF_BOUNDS_INDEXING, + expr.span, + "range is out of bounds", + ); } - - let help_msg = match (range.start, range.end) { - (None, Some(_)) => { - "Consider using `.get(..n)`or `.get_mut(..n)` instead" - } - (Some(_), None) => { - "Consider using `.get(n..)` or .get_mut(n..)` instead" - } - (Some(_), Some(_)) => { - "Consider using `.get(n..m)` or `.get_mut(n..m)` instead" - } - (None, None) => return, // [..] is ok. - }; - - utils::span_help_and_lint( - cx, - INDEXING_SLICING, - expr.span, - "slicing may panic.", - help_msg, - ); - } else { - utils::span_help_and_lint( - cx, - INDEXING_SLICING, - expr.span, - "indexing may panic.", - "Consider using `.get(n)` or `.get_mut(n)` instead", - ); + return; } } - ExprLit(..) => { - // [n] - if let ty::TyArray(_, s) = ty.sty { - let size: u128 = s.assert_usize(cx.tcx).unwrap().into(); - // Index is a constant uint. - if let Some((Constant::Int(const_index), _)) = - constant(cx, cx.tables, index) - { - if size <= const_index { - utils::span_lint( - cx, - OUT_OF_BOUNDS_INDEXING, - expr.span, - "const index is out of bounds", - ); - } - // Else index is in bounds, ok. + + let help_msg = match (range.start, range.end) { + (None, Some(_)) => "Consider using `.get(..n)`or `.get_mut(..n)` instead", + (Some(_), None) => "Consider using `.get(n..)` or .get_mut(n..)` instead", + (Some(_), Some(_)) => "Consider using `.get(n..m)` or `.get_mut(n..m)` instead", + (None, None) => return, // [..] is ok. + }; + + utils::span_help_and_lint( + cx, + INDEXING_SLICING, + expr.span, + "slicing may panic.", + help_msg, + ); + } else { + // Catchall non-range index, i.e. [n] or [n << m] + if let ty::TyArray(_, s) = ty.sty { + let size: u128 = s.assert_usize(cx.tcx).unwrap().into(); + // Index is a constant uint. + if let Some((Constant::Int(const_index), _)) = constant(cx, cx.tables, index) { + if size <= const_index { + utils::span_lint( + cx, + OUT_OF_BOUNDS_INDEXING, + expr.span, + "const index is out of bounds", + ); } - } else { - utils::span_help_and_lint( - cx, - INDEXING_SLICING, - expr.span, - "indexing may panic.", - "Consider using `.get(n)` or `.get_mut(n)` instead", - ); + // Else index is in bounds, ok. + + return; } } - _ => (), + + utils::span_help_and_lint( + cx, + INDEXING_SLICING, + expr.span, + "indexing may panic.", + "Consider using `.get(n)` or `.get_mut(n)` instead", + ); } } } diff --git a/tests/ui/indexing_slicing.rs b/tests/ui/indexing_slicing.rs index a22c9034346..16174afb106 100644 --- a/tests/ui/indexing_slicing.rs +++ b/tests/ui/indexing_slicing.rs @@ -9,55 +9,63 @@ fn main() { let index_from: usize = 2; let index_to: usize = 3; x[index]; - &x[index_from..index_to]; - &x[index_from..][..index_to]; &x[index..]; &x[..index]; - x[0]; - x[3]; + &x[index_from..index_to]; + &x[index_from..][..index_to]; // Two lint reports, one for [index_from..] and another for [..index_to]. x[4]; x[1 << 3]; - &x[1..5]; - &x[1..][..5]; - &x[0..3]; - &x[0..][..3]; - &x[0..].get(..3); // Ok - &x[0..=4]; &x[..=4]; - &x[..]; - &x[1..]; - &x[4..]; + &x[1..5]; + &x[5..][..10]; // Two lint reports, one for [5..] and another for [..10]. &x[5..]; - &x[..4]; &x[..5]; &x[5..].iter().map(|x| 2 * x).collect::<Vec<i32>>(); - &x[2..].iter().map(|x| 2 * x).collect::<Vec<i32>>(); // Ok + &x[0..=4]; + &x[0..][..3]; + &x[1..][..5]; + + &x[4..]; // Ok, should not produce stderr. + &x[..4]; // Ok, should not produce stderr. + &x[..]; // Ok, should not produce stderr. + &x[1..]; // Ok, should not produce stderr. + &x[2..].iter().map(|x| 2 * x).collect::<Vec<i32>>(); // Ok, should not produce stderr. + &x[0..].get(..3); // Ok, should not produce stderr. + x[0]; // Ok, should not produce stderr. + x[3]; // Ok, should not produce stderr. + &x[0..3]; // Ok, should not produce stderr. let y = &x; y[0]; &y[1..2]; - &y[..]; &y[0..=4]; &y[..=4]; + &y[..]; // Ok, should not produce stderr. + let empty: [i8; 0] = []; empty[0]; &empty[1..5]; &empty[0..=4]; &empty[..=4]; - &empty[..]; - &empty[0..]; - &empty[0..0]; - &empty[0..=0]; - &empty[..=0]; - &empty[..0]; &empty[1..]; &empty[..4]; + &empty[0..=0]; + &empty[..=0]; + + &empty[0..]; // Ok, should not produce stderr. + &empty[0..0]; // Ok, should not produce stderr. + &empty[..0]; // Ok, should not produce stderr. + &empty[..]; // Ok, should not produce stderr. let v = vec![0; 5]; v[0]; v[10]; + v[1 << 3]; &v[10..100]; + &x[10..][..100]; // Two lint reports, one for [10..] and another for [..100]. &v[10..]; &v[..100]; + + &v[..]; // Ok, should not produce stderr. } diff --git a/tests/ui/indexing_slicing.stderr b/tests/ui/indexing_slicing.stderr index 605a96e8b8c..c9aefe0349a 100644 --- a/tests/ui/indexing_slicing.stderr +++ b/tests/ui/indexing_slicing.stderr @@ -10,109 +10,135 @@ error: indexing may panic. error: slicing may panic. --> $DIR/indexing_slicing.rs:12:6 | -12 | &x[index_from..index_to]; +12 | &x[index..]; + | ^^^^^^^^^^ + | + = help: Consider using `.get(n..)` or .get_mut(n..)` instead + +error: slicing may panic. + --> $DIR/indexing_slicing.rs:13:6 + | +13 | &x[..index]; + | ^^^^^^^^^^ + | + = help: Consider using `.get(..n)`or `.get_mut(..n)` instead + +error: slicing may panic. + --> $DIR/indexing_slicing.rs:14:6 + | +14 | &x[index_from..index_to]; | ^^^^^^^^^^^^^^^^^^^^^^^ | = help: Consider using `.get(n..m)` or `.get_mut(n..m)` instead error: slicing may panic. - --> $DIR/indexing_slicing.rs:13:6 + --> $DIR/indexing_slicing.rs:15:6 | -13 | &x[index_from..][..index_to]; +15 | &x[index_from..][..index_to]; // Two lint reports, one for [index_from..] and another for [..index_to]. | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ | = help: Consider using `.get(..n)`or `.get_mut(..n)` instead error: slicing may panic. - --> $DIR/indexing_slicing.rs:13:6 + --> $DIR/indexing_slicing.rs:15:6 | -13 | &x[index_from..][..index_to]; +15 | &x[index_from..][..index_to]; // Two lint reports, one for [index_from..] and another for [..index_to]. | ^^^^^^^^^^^^^^^ | = help: Consider using `.get(n..)` or .get_mut(n..)` instead -error: slicing may panic. - --> $DIR/indexing_slicing.rs:14:6 - | -14 | &x[index..]; - | ^^^^^^^^^^ - | - = help: Consider using `.get(n..)` or .get_mut(n..)` instead - -error: slicing may panic. - --> $DIR/indexing_slicing.rs:15:6 - | -15 | &x[..index]; - | ^^^^^^^^^^ - | - = help: Consider using `.get(..n)`or `.get_mut(..n)` instead - error: const index is out of bounds - --> $DIR/indexing_slicing.rs:18:5 + --> $DIR/indexing_slicing.rs:16:5 | -18 | x[4]; +16 | x[4]; | ^^^^ | = note: `-D out-of-bounds-indexing` implied by `-D warnings` +error: const index is out of bounds + --> $DIR/indexing_slicing.rs:17:5 + | +17 | x[1 << 3]; + | ^^^^^^^^^ + +error: range is out of bounds + --> $DIR/indexing_slicing.rs:18:6 + | +18 | &x[..=4]; + | ^^^^^^^ + +error: range is out of bounds + --> $DIR/indexing_slicing.rs:19:6 + | +19 | &x[1..5]; + | ^^^^^^^ + +error: slicing may panic. + --> $DIR/indexing_slicing.rs:20:6 + | +20 | &x[5..][..10]; // Two lint reports, one for [5..] and another for [..10]. + | ^^^^^^^^^^^^ + | + = help: Consider using `.get(..n)`or `.get_mut(..n)` instead + error: range is out of bounds --> $DIR/indexing_slicing.rs:20:6 | -20 | &x[1..5]; - | ^^^^^^^ +20 | &x[5..][..10]; // Two lint reports, one for [5..] and another for [..10]. + | ^^^^^^ -error: slicing may panic. +error: range is out of bounds --> $DIR/indexing_slicing.rs:21:6 | -21 | &x[1..][..5]; +21 | &x[5..]; + | ^^^^^^ + +error: range is out of bounds + --> $DIR/indexing_slicing.rs:22:6 + | +22 | &x[..5]; + | ^^^^^^ + +error: range is out of bounds + --> $DIR/indexing_slicing.rs:23:6 + | +23 | &x[5..].iter().map(|x| 2 * x).collect::<Vec<i32>>(); + | ^^^^^^ + +error: range is out of bounds + --> $DIR/indexing_slicing.rs:24:6 + | +24 | &x[0..=4]; + | ^^^^^^^^ + +error: slicing may panic. + --> $DIR/indexing_slicing.rs:25:6 + | +25 | &x[0..][..3]; | ^^^^^^^^^^^ | = help: Consider using `.get(..n)`or `.get_mut(..n)` instead error: slicing may panic. - --> $DIR/indexing_slicing.rs:23:6 + --> $DIR/indexing_slicing.rs:26:6 | -23 | &x[0..][..3]; +26 | &x[1..][..5]; | ^^^^^^^^^^^ | = help: Consider using `.get(..n)`or `.get_mut(..n)` instead -error: range is out of bounds - --> $DIR/indexing_slicing.rs:26:6 - | -26 | &x[..=4]; - | ^^^^^^^ - -error: range is out of bounds - --> $DIR/indexing_slicing.rs:30:6 - | -30 | &x[5..]; - | ^^^^^^ - -error: range is out of bounds - --> $DIR/indexing_slicing.rs:32:6 - | -32 | &x[..5]; - | ^^^^^^ - -error: range is out of bounds - --> $DIR/indexing_slicing.rs:33:6 - | -33 | &x[5..].iter().map(|x| 2 * x).collect::<Vec<i32>>(); - | ^^^^^^ - error: indexing may panic. - --> $DIR/indexing_slicing.rs:37:5 + --> $DIR/indexing_slicing.rs:39:5 | -37 | y[0]; +39 | y[0]; | ^^^^ | = help: Consider using `.get(n)` or `.get_mut(n)` instead error: slicing may panic. - --> $DIR/indexing_slicing.rs:38:6 + --> $DIR/indexing_slicing.rs:40:6 | -38 | &y[1..2]; +40 | &y[1..2]; | ^^^^^^^ | = help: Consider using `.get(n..m)` or `.get_mut(n..m)` instead @@ -120,86 +146,128 @@ error: slicing may panic. error: slicing may panic. --> $DIR/indexing_slicing.rs:41:6 | -41 | &y[..=4]; +41 | &y[0..=4]; + | ^^^^^^^^ + | + = help: Consider using `.get(n..m)` or `.get_mut(n..m)` instead + +error: slicing may panic. + --> $DIR/indexing_slicing.rs:42:6 + | +42 | &y[..=4]; | ^^^^^^^ | = help: Consider using `.get(..n)`or `.get_mut(..n)` instead error: const index is out of bounds - --> $DIR/indexing_slicing.rs:44:5 + --> $DIR/indexing_slicing.rs:47:5 | -44 | empty[0]; +47 | empty[0]; | ^^^^^^^^ error: range is out of bounds - --> $DIR/indexing_slicing.rs:45:6 + --> $DIR/indexing_slicing.rs:48:6 | -45 | &empty[1..5]; +48 | &empty[1..5]; | ^^^^^^^^^^^ error: range is out of bounds - --> $DIR/indexing_slicing.rs:47:6 + --> $DIR/indexing_slicing.rs:49:6 | -47 | &empty[..=4]; +49 | &empty[0..=4]; + | ^^^^^^^^^^^^ + +error: range is out of bounds + --> $DIR/indexing_slicing.rs:50:6 + | +50 | &empty[..=4]; | ^^^^^^^^^^^ +error: range is out of bounds + --> $DIR/indexing_slicing.rs:51:6 + | +51 | &empty[1..]; + | ^^^^^^^^^^ + error: range is out of bounds --> $DIR/indexing_slicing.rs:52:6 | -52 | &empty[..=0]; - | ^^^^^^^^^^^ +52 | &empty[..4]; + | ^^^^^^^^^^ + +error: range is out of bounds + --> $DIR/indexing_slicing.rs:53:6 + | +53 | &empty[0..=0]; + | ^^^^^^^^^^^^ error: range is out of bounds --> $DIR/indexing_slicing.rs:54:6 | -54 | &empty[1..]; - | ^^^^^^^^^^ - -error: range is out of bounds - --> $DIR/indexing_slicing.rs:55:6 - | -55 | &empty[..4]; - | ^^^^^^^^^^ +54 | &empty[..=0]; + | ^^^^^^^^^^^ error: indexing may panic. - --> $DIR/indexing_slicing.rs:58:5 + --> $DIR/indexing_slicing.rs:62:5 | -58 | v[0]; +62 | v[0]; | ^^^^ | = help: Consider using `.get(n)` or `.get_mut(n)` instead error: indexing may panic. - --> $DIR/indexing_slicing.rs:59:5 + --> $DIR/indexing_slicing.rs:63:5 | -59 | v[10]; +63 | v[10]; | ^^^^^ | = help: Consider using `.get(n)` or `.get_mut(n)` instead -error: slicing may panic. - --> $DIR/indexing_slicing.rs:60:6 +error: indexing may panic. + --> $DIR/indexing_slicing.rs:64:5 | -60 | &v[10..100]; +64 | v[1 << 3]; + | ^^^^^^^^^ + | + = help: Consider using `.get(n)` or `.get_mut(n)` instead + +error: slicing may panic. + --> $DIR/indexing_slicing.rs:65:6 + | +65 | &v[10..100]; | ^^^^^^^^^^ | = help: Consider using `.get(n..m)` or `.get_mut(n..m)` instead error: slicing may panic. - --> $DIR/indexing_slicing.rs:61:6 + --> $DIR/indexing_slicing.rs:66:6 | -61 | &v[10..]; +66 | &x[10..][..100]; // Two lint reports, one for [10..] and another for [..100]. + | ^^^^^^^^^^^^^^ + | + = help: Consider using `.get(..n)`or `.get_mut(..n)` instead + +error: range is out of bounds + --> $DIR/indexing_slicing.rs:66:6 + | +66 | &x[10..][..100]; // Two lint reports, one for [10..] and another for [..100]. + | ^^^^^^^ + +error: slicing may panic. + --> $DIR/indexing_slicing.rs:67:6 + | +67 | &v[10..]; | ^^^^^^^ | = help: Consider using `.get(n..)` or .get_mut(n..)` instead error: slicing may panic. - --> $DIR/indexing_slicing.rs:62:6 + --> $DIR/indexing_slicing.rs:68:6 | -62 | &v[..100]; +68 | &v[..100]; | ^^^^^^^^ | = help: Consider using `.get(..n)`or `.get_mut(..n)` instead -error: aborting due to 28 previous errors +error: aborting due to 38 previous errors