mirror of
https://github.com/rust-lang/rust.git
synced 2024-11-24 15:54:15 +00:00
Second pass at addressing changes requested
The changes reflected in this commit (requested in PR #2790) are as follows: - Extended `INDEXING_SLICING` documentation to include the array type so that it is clearer when indexing operations are allowed. - Variable `ty` defined identically in multiple scopes was moved to an outer scope so it's only defined once. - Added a missing return statement to ensure only one lint is triggered by a scenario. - Prettified match statement with a `let` clause. (I learned something new!) - Added `&x[5..].iter().map(|x| 2 * x).collect::<Vec<i32>>()` and `&x[2..].iter().map(|x| 2 * x).collect::<Vec<i32>>()` to the test cases. The first _should trigger the lint/stderr_ and the second _should not_.
This commit is contained in:
Shea Newton
parent
a7c0ff3fa6
commit
8b59542acc
@ -34,7 +34,7 @@ declare_clippy_lint! {
|
||||
}
|
||||
|
||||
/// **What it does:** Checks for usage of indexing or slicing. Does not report
|
||||
/// if we can tell that the indexing or slicing operations on an array are in
|
||||
/// on arrays if we can tell that the indexing or slicing operations are in
|
||||
/// bounds.
|
||||
///
|
||||
/// **Why is this bad?** Indexing and slicing can panic at runtime and there are
|
||||
@ -44,7 +44,9 @@ declare_clippy_lint! {
|
||||
///
|
||||
/// **Example:**
|
||||
/// ```rust
|
||||
/// // Vector
|
||||
/// let x = vec![0; 5];
|
||||
///
|
||||
/// // Bad
|
||||
/// x[2];
|
||||
/// &x[2..100];
|
||||
@ -52,10 +54,29 @@ declare_clippy_lint! {
|
||||
/// &x[..100];
|
||||
///
|
||||
/// // Good
|
||||
/// x.get(2)
|
||||
/// x.get(2..100)
|
||||
/// x.get(2..)
|
||||
/// x.get(..100)
|
||||
/// x.get(2);
|
||||
/// x.get(2..100);
|
||||
/// x.get(2..);
|
||||
/// x.get(..100);
|
||||
///
|
||||
/// // Array
|
||||
/// let y = [0, 1, 2, 3];
|
||||
///
|
||||
/// // Bad
|
||||
/// y[10];
|
||||
/// &y[10..100];
|
||||
/// &y[10..];
|
||||
/// &y[..100];
|
||||
///
|
||||
/// // Good
|
||||
/// y[2];
|
||||
/// &y[2..];
|
||||
/// &y[..2];
|
||||
/// &y[0..3];
|
||||
/// y.get(10);
|
||||
/// y.get(10..100);
|
||||
/// y.get(10..);
|
||||
/// y.get(..100);
|
||||
/// ```
|
||||
declare_clippy_lint! {
|
||||
pub INDEXING_SLICING,
|
||||
@ -75,6 +96,7 @@ impl LintPass for IndexingSlicing {
|
||||
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)`.
|
||||
@ -82,7 +104,6 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for IndexingSlicing {
|
||||
// ExprPath handles &x[..] and x[var]
|
||||
ExprStruct(..) | ExprPath(..) => {
|
||||
if let Some(range) = higher::range(cx, index) {
|
||||
let ty = cx.tables.expr_ty(array);
|
||||
if let ty::TyArray(_, s) = ty.sty {
|
||||
let size: u128 = s.assert_usize(cx.tcx).unwrap().into();
|
||||
// Index is a constant range.
|
||||
@ -94,27 +115,24 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for IndexingSlicing {
|
||||
expr.span,
|
||||
"range is out of bounds",
|
||||
);
|
||||
} else {
|
||||
// Range is in bounds, ok.
|
||||
return;
|
||||
}
|
||||
} // Else range is in bounds, ok.
|
||||
|
||||
return;
|
||||
}
|
||||
}
|
||||
|
||||
let help_msg;
|
||||
match (range.start, range.end) {
|
||||
let help_msg = match (range.start, range.end) {
|
||||
(None, Some(_)) => {
|
||||
help_msg = "Consider using `.get(..n)`or `.get_mut(..n)` instead";
|
||||
"Consider using `.get(..n)`or `.get_mut(..n)` instead"
|
||||
}
|
||||
(Some(_), None) => {
|
||||
help_msg = "Consider using `.get(n..)` or .get_mut(n..)` instead";
|
||||
"Consider using `.get(n..)` or .get_mut(n..)` instead"
|
||||
}
|
||||
(Some(_), Some(_)) => {
|
||||
help_msg =
|
||||
"Consider using `.get(n..m)` or `.get_mut(n..m)` instead";
|
||||
"Consider using `.get(n..m)` or `.get_mut(n..m)` instead"
|
||||
}
|
||||
(None, None) => return, // [..] is ok
|
||||
}
|
||||
(None, None) => return, // [..] is ok.
|
||||
};
|
||||
|
||||
utils::span_help_and_lint(
|
||||
cx,
|
||||
@ -135,7 +153,6 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for IndexingSlicing {
|
||||
}
|
||||
ExprLit(..) => {
|
||||
// [n]
|
||||
let ty = cx.tables.expr_ty(array);
|
||||
if let ty::TyArray(_, s) = ty.sty {
|
||||
let size: u128 = s.assert_usize(cx.tcx).unwrap().into();
|
||||
// Index is a constant uint.
|
||||
|
||||
@ -30,6 +30,8 @@ fn main() {
|
||||
&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
|
||||
|
||||
let y = &x;
|
||||
y[0];
|
||||
|
||||
@ -61,14 +61,6 @@ error: range is out of bounds
|
||||
20 | &x[1..5];
|
||||
| ^^^^^^^
|
||||
|
||||
error: slicing may panic.
|
||||
--> $DIR/indexing_slicing.rs:20:6
|
||||
|
|
||||
20 | &x[1..5];
|
||||
| ^^^^^^^
|
||||
|
|
||||
= help: Consider using `.get(n..m)` or `.get_mut(n..m)` instead
|
||||
|
||||
error: slicing may panic.
|
||||
--> $DIR/indexing_slicing.rs:21:6
|
||||
|
|
||||
@ -91,181 +83,123 @@ error: range is out of bounds
|
||||
26 | &x[..=4];
|
||||
| ^^^^^^^
|
||||
|
||||
error: slicing may panic.
|
||||
--> $DIR/indexing_slicing.rs:26:6
|
||||
|
|
||||
26 | &x[..=4];
|
||||
| ^^^^^^^
|
||||
|
|
||||
= help: Consider using `.get(..n)`or `.get_mut(..n)` instead
|
||||
|
||||
error: range is out of bounds
|
||||
--> $DIR/indexing_slicing.rs:30:6
|
||||
|
|
||||
30 | &x[5..];
|
||||
| ^^^^^^
|
||||
|
||||
error: slicing may panic.
|
||||
--> $DIR/indexing_slicing.rs:30:6
|
||||
|
|
||||
30 | &x[5..];
|
||||
| ^^^^^^
|
||||
|
|
||||
= help: Consider using `.get(n..)` or .get_mut(n..)` instead
|
||||
|
||||
error: range is out of bounds
|
||||
--> $DIR/indexing_slicing.rs:32:6
|
||||
|
|
||||
32 | &x[..5];
|
||||
| ^^^^^^
|
||||
|
||||
error: slicing may panic.
|
||||
--> $DIR/indexing_slicing.rs:32:6
|
||||
error: range is out of bounds
|
||||
--> $DIR/indexing_slicing.rs:33:6
|
||||
|
|
||||
32 | &x[..5];
|
||||
33 | &x[5..].iter().map(|x| 2 * x).collect::<Vec<i32>>();
|
||||
| ^^^^^^
|
||||
|
|
||||
= help: Consider using `.get(..n)`or `.get_mut(..n)` instead
|
||||
|
||||
error: indexing may panic.
|
||||
--> $DIR/indexing_slicing.rs:35:5
|
||||
--> $DIR/indexing_slicing.rs:37:5
|
||||
|
|
||||
35 | y[0];
|
||||
37 | y[0];
|
||||
| ^^^^
|
||||
|
|
||||
= help: Consider using `.get(n)` or `.get_mut(n)` instead
|
||||
|
||||
error: slicing may panic.
|
||||
--> $DIR/indexing_slicing.rs:36:6
|
||||
--> $DIR/indexing_slicing.rs:38:6
|
||||
|
|
||||
36 | &y[1..2];
|
||||
38 | &y[1..2];
|
||||
| ^^^^^^^
|
||||
|
|
||||
= help: Consider using `.get(n..m)` or `.get_mut(n..m)` instead
|
||||
|
||||
error: slicing may panic.
|
||||
--> $DIR/indexing_slicing.rs:39:6
|
||||
--> $DIR/indexing_slicing.rs:41:6
|
||||
|
|
||||
39 | &y[..=4];
|
||||
41 | &y[..=4];
|
||||
| ^^^^^^^
|
||||
|
|
||||
= help: Consider using `.get(..n)`or `.get_mut(..n)` instead
|
||||
|
||||
error: const index is out of bounds
|
||||
--> $DIR/indexing_slicing.rs:42:5
|
||||
--> $DIR/indexing_slicing.rs:44:5
|
||||
|
|
||||
42 | empty[0];
|
||||
44 | empty[0];
|
||||
| ^^^^^^^^
|
||||
|
||||
error: range is out of bounds
|
||||
--> $DIR/indexing_slicing.rs:43:6
|
||||
|
|
||||
43 | &empty[1..5];
|
||||
| ^^^^^^^^^^^
|
||||
|
||||
error: slicing may panic.
|
||||
--> $DIR/indexing_slicing.rs:43:6
|
||||
|
|
||||
43 | &empty[1..5];
|
||||
| ^^^^^^^^^^^
|
||||
|
|
||||
= help: Consider using `.get(n..m)` or `.get_mut(n..m)` instead
|
||||
|
||||
error: range is out of bounds
|
||||
--> $DIR/indexing_slicing.rs:45:6
|
||||
|
|
||||
45 | &empty[..=4];
|
||||
45 | &empty[1..5];
|
||||
| ^^^^^^^^^^^
|
||||
|
||||
error: slicing may panic.
|
||||
--> $DIR/indexing_slicing.rs:45:6
|
||||
|
|
||||
45 | &empty[..=4];
|
||||
| ^^^^^^^^^^^
|
||||
|
|
||||
= help: Consider using `.get(..n)`or `.get_mut(..n)` instead
|
||||
|
||||
error: range is out of bounds
|
||||
--> $DIR/indexing_slicing.rs:50:6
|
||||
--> $DIR/indexing_slicing.rs:47:6
|
||||
|
|
||||
50 | &empty[..=0];
|
||||
47 | &empty[..=4];
|
||||
| ^^^^^^^^^^^
|
||||
|
||||
error: slicing may panic.
|
||||
--> $DIR/indexing_slicing.rs:50:6
|
||||
|
|
||||
50 | &empty[..=0];
|
||||
| ^^^^^^^^^^^
|
||||
|
|
||||
= help: Consider using `.get(..n)`or `.get_mut(..n)` instead
|
||||
|
||||
error: range is out of bounds
|
||||
--> $DIR/indexing_slicing.rs:52:6
|
||||
|
|
||||
52 | &empty[1..];
|
||||
| ^^^^^^^^^^
|
||||
|
||||
error: slicing may panic.
|
||||
--> $DIR/indexing_slicing.rs:52:6
|
||||
|
|
||||
52 | &empty[1..];
|
||||
| ^^^^^^^^^^
|
||||
|
|
||||
= help: Consider using `.get(n..)` or .get_mut(n..)` instead
|
||||
52 | &empty[..=0];
|
||||
| ^^^^^^^^^^^
|
||||
|
||||
error: range is out of bounds
|
||||
--> $DIR/indexing_slicing.rs:53:6
|
||||
--> $DIR/indexing_slicing.rs:54:6
|
||||
|
|
||||
53 | &empty[..4];
|
||||
54 | &empty[1..];
|
||||
| ^^^^^^^^^^
|
||||
|
||||
error: slicing may panic.
|
||||
--> $DIR/indexing_slicing.rs:53:6
|
||||
error: range is out of bounds
|
||||
--> $DIR/indexing_slicing.rs:55:6
|
||||
|
|
||||
53 | &empty[..4];
|
||||
55 | &empty[..4];
|
||||
| ^^^^^^^^^^
|
||||
|
|
||||
= help: Consider using `.get(..n)`or `.get_mut(..n)` instead
|
||||
|
||||
error: indexing may panic.
|
||||
--> $DIR/indexing_slicing.rs:56:5
|
||||
--> $DIR/indexing_slicing.rs:58:5
|
||||
|
|
||||
56 | v[0];
|
||||
58 | v[0];
|
||||
| ^^^^
|
||||
|
|
||||
= help: Consider using `.get(n)` or `.get_mut(n)` instead
|
||||
|
||||
error: indexing may panic.
|
||||
--> $DIR/indexing_slicing.rs:57:5
|
||||
--> $DIR/indexing_slicing.rs:59:5
|
||||
|
|
||||
57 | v[10];
|
||||
59 | v[10];
|
||||
| ^^^^^
|
||||
|
|
||||
= help: Consider using `.get(n)` or `.get_mut(n)` instead
|
||||
|
||||
error: slicing may panic.
|
||||
--> $DIR/indexing_slicing.rs:58:6
|
||||
--> $DIR/indexing_slicing.rs:60:6
|
||||
|
|
||||
58 | &v[10..100];
|
||||
60 | &v[10..100];
|
||||
| ^^^^^^^^^^
|
||||
|
|
||||
= help: Consider using `.get(n..m)` or `.get_mut(n..m)` instead
|
||||
|
||||
error: slicing may panic.
|
||||
--> $DIR/indexing_slicing.rs:59:6
|
||||
--> $DIR/indexing_slicing.rs:61:6
|
||||
|
|
||||
59 | &v[10..];
|
||||
61 | &v[10..];
|
||||
| ^^^^^^^
|
||||
|
|
||||
= help: Consider using `.get(n..)` or .get_mut(n..)` instead
|
||||
|
||||
error: slicing may panic.
|
||||
--> $DIR/indexing_slicing.rs:60:6
|
||||
--> $DIR/indexing_slicing.rs:62:6
|
||||
|
|
||||
60 | &v[..100];
|
||||
62 | &v[..100];
|
||||
| ^^^^^^^^
|
||||
|
|
||||
= help: Consider using `.get(..n)`or `.get_mut(..n)` instead
|
||||
|
||||
error: aborting due to 36 previous errors
|
||||
error: aborting due to 28 previous errors
|
||||
|
||||
|
||||
Loading…
Reference in New Issue
Block a user