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_.

diff --git a/clippy_lints/src/indexing_slicing.rs b/clippy_lints/src/indexing_slicing.rs
index 1f2c7755eee898c7640a11a495caa2b11c57938a..01f9f45c589119c65df02ed56c2d2fd93618fe50 100644 (file)
--- a/clippy_lints/src/indexing_slicing.rs
+++ b/clippy_lints/src/indexing_slicing.rs
@@ -34,7 +34,7 @@
 }
 
 /// **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 @@
 ///
 /// **Example:**
 /// ```rust
+/// // Vector
 /// let x = vec![0; 5];
+///
 /// // Bad
 /// x[2];
 /// &x[2..100];
@@ -52,10 +54,29 @@
 /// &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 @@ fn get_lints(&self) -> LintArray {
 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 @@ fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr) {
                 // 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 @@ fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr) {
                                         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 @@ fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr) {
                 }
                 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.

diff --git a/tests/ui/indexing_slicing.rs b/tests/ui/indexing_slicing.rs
index 913063b8dd548f81cce6435855f53427f345c2e7..a22c9034346d612660a3241b393fc05b974d717e 100644 (file)
--- a/tests/ui/indexing_slicing.rs
+++ b/tests/ui/indexing_slicing.rs
@@ -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];

diff --git a/tests/ui/indexing_slicing.stderr b/tests/ui/indexing_slicing.stderr
index 642817d9e94033e57cc55caf6fa0c1818eac97f2..605a96e8b8c2dd99a25557c1fa3a9eaedc78690f 100644 (file)
--- a/tests/ui/indexing_slicing.stderr
+++ b/tests/ui/indexing_slicing.stderr
@@ -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
-   |
-50 |     &empty[..=0];
-   |      ^^^^^^^^^^^
-
-error: slicing may panic.
-  --> $DIR/indexing_slicing.rs:50:6
+  --> $DIR/indexing_slicing.rs:47:6
    |
-50 |     &empty[..=0];
+47 |     &empty[..=4];
    |      ^^^^^^^^^^^
-   |
-   = 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