This commit represents an attempt to address changes requested in the process of...

The changes reflected in this commit are as follows:

- Revised `IndexingSlicingPass` struct name to IndexingSlicing for consistency with the rest of the code base.
- Revised match arm condition to use `(..)` shorthand in favor of `(_, _, _)`.
- Restored a couple telling variable names.
- Calls to `cx.span_lint` were revised to use `utils::span_help_and_lint`.
- Took a stab at refactoring some generalizable calls to `utils::span_help_and_lint` to minimize duplicate code.
- Revised INDEXING_SLICING declaration to pedantic rather than restriction.
- Added `&x[0..].get(..3)` to the test cases.

diff --git a/clippy_lints/src/indexing_slicing.rs b/clippy_lints/src/indexing_slicing.rs
index d7f2a37c5fec4a4d8003f4d3d1f2a0751502315d..1f2c7755eee898c7640a11a495caa2b11c57938a 100644 (file)
--- a/clippy_lints/src/indexing_slicing.rs
+++ b/clippy_lints/src/indexing_slicing.rs
@@ -59,30 +59,30 @@
 /// ```
 declare_clippy_lint! {
     pub INDEXING_SLICING,
-    restriction,
+    pedantic,
     "indexing/slicing usage"
 }
 
 #[derive(Copy, Clone)]
-pub struct IndexingSlicingPass;
+pub struct IndexingSlicing;
 
-impl LintPass for IndexingSlicingPass {
+impl LintPass for IndexingSlicing {
     fn get_lints(&self) -> LintArray {
         lint_array!(INDEXING_SLICING, OUT_OF_BOUNDS_INDEXING)
     }
 }
 
-impl<'a, 'tcx> LateLintPass<'a, 'tcx> for IndexingSlicingPass {
+impl<'a, 'tcx> LateLintPass<'a, 'tcx> for IndexingSlicing {
     fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr) {
-        if let ExprIndex(ref a, ref b) = &expr.node {
-            match &b.node {
+        if let ExprIndex(ref array, ref index) = &expr.node {
+            match &index.node {
                 // Both ExprStruct and ExprPath require this approach's checks
-                // on the `range` returned by `higher::range(cx, b)`.
+                // 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, b) {
-                        let ty = cx.tables.expr_ty(a);
+                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.
@@ -100,49 +100,48 @@ fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr) {
                                 }
                             }
                         }
+
+                        let help_msg;
                         match (range.start, range.end) {
                             (None, Some(_)) => {
-                                cx.span_lint(
-                                    INDEXING_SLICING,
-                                    expr.span,
-                                    "slicing may panic. Consider using \
-                                     `.get(..n)`or `.get_mut(..n)` instead",
-                                );
+                                help_msg = "Consider using `.get(..n)`or `.get_mut(..n)` instead";
                             }
                             (Some(_), None) => {
-                                cx.span_lint(
-                                    INDEXING_SLICING,
-                                    expr.span,
-                                    "slicing may panic. Consider using \
-                                     `.get(n..)` or .get_mut(n..)` instead",
-                                );
+                                help_msg = "Consider using `.get(n..)` or .get_mut(n..)` instead";
                             }
                             (Some(_), Some(_)) => {
-                                cx.span_lint(
-                                    INDEXING_SLICING,
-                                    expr.span,
-                                    "slicing may panic. Consider using \
-                                     `.get(n..m)` or `.get_mut(n..m)` instead",
-                                );
+                                help_msg =
+                                    "Consider using `.get(n..m)` or `.get_mut(n..m)` instead";
                             }
-                            (None, None) => (),
+                            (None, None) => return, // [..] is ok
                         }
+
+                        utils::span_help_and_lint(
+                            cx,
+                            INDEXING_SLICING,
+                            expr.span,
+                            "slicing may panic.",
+                            help_msg,
+                        );
                     } else {
-                        cx.span_lint(
+                        utils::span_help_and_lint(
+                            cx,
                             INDEXING_SLICING,
                             expr.span,
-                            "indexing may panic. Consider using `.get(n)` or \
-                             `.get_mut(n)` instead",
+                            "indexing may panic.",
+                            "Consider using `.get(n)` or `.get_mut(n)` instead",
                         );
                     }
                 }
-                ExprLit(_) => {
+                ExprLit(..) => {
                     // [n]
-                    let ty = cx.tables.expr_ty(a);
+                    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.
-                        if let Some((Constant::Int(const_index), _)) = constant(cx, cx.tables, b) {
+                        if let Some((Constant::Int(const_index), _)) =
+                            constant(cx, cx.tables, index)
+                        {
                             if size <= const_index {
                                 utils::span_lint(
                                     cx,
@@ -154,11 +153,12 @@ fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr) {
                             // Else index is in bounds, ok.
                         }
                     } else {
-                        cx.span_lint(
+                        utils::span_help_and_lint(
+                            cx,
                             INDEXING_SLICING,
                             expr.span,
-                            "indexing may panic. Consider using `.get(n)` or \
-                             `.get_mut(n)` instead",
+                            "indexing may panic.",
+                            "Consider using `.get(n)` or `.get_mut(n)` instead",
                         );
                     }
                 }

diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs
index e261fe417f7e95add41dd8ce59ebd35d15768b57..621b21429a9861fb93130da282809def2ccc63b3 100644 (file)
--- a/clippy_lints/src/lib.rs
+++ b/clippy_lints/src/lib.rs
@@ -431,7 +431,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry) {
     reg.register_late_lint_pass(box unwrap::Pass);
     reg.register_late_lint_pass(box duration_subsec::DurationSubsec);
     reg.register_late_lint_pass(box default_trait_access::DefaultTraitAccess);
-    reg.register_late_lint_pass(box indexing_slicing::IndexingSlicingPass);
+    reg.register_late_lint_pass(box indexing_slicing::IndexingSlicing);
 
     reg.register_lint_group("clippy_restriction", vec![
         arithmetic::FLOAT_ARITHMETIC,

diff --git a/tests/ui/indexing_slicing.rs b/tests/ui/indexing_slicing.rs
index 2437df96bd10c9993fc312c0dc3dc6a6aa0063c4..913063b8dd548f81cce6435855f53427f345c2e7 100644 (file)
--- a/tests/ui/indexing_slicing.rs
+++ b/tests/ui/indexing_slicing.rs
@@ -21,6 +21,7 @@ fn main() {
     &x[1..][..5];
     &x[0..3];
     &x[0..][..3];
+    &x[0..].get(..3); // Ok
     &x[0..=4];
     &x[..=4];
     &x[..];

diff --git a/tests/ui/indexing_slicing.stderr b/tests/ui/indexing_slicing.stderr
index 30231a31d191c3fb56eb15622b12b0b2a7ec98a7..642817d9e94033e57cc55caf6fa0c1818eac97f2 100644 (file)
--- a/tests/ui/indexing_slicing.stderr
+++ b/tests/ui/indexing_slicing.stderr
@@ -1,40 +1,51 @@
-error: indexing may panic. Consider using `.get(n)` or `.get_mut(n)` instead
+error: indexing may panic.
   --> $DIR/indexing_slicing.rs:11:5
    |
 11 |     x[index];
    |     ^^^^^^^^
    |
    = note: `-D indexing-slicing` implied by `-D warnings`
+   = help: Consider using `.get(n)` or `.get_mut(n)` instead
 
-error: slicing may panic. Consider using `.get(n..m)` or `.get_mut(n..m)` instead
+error: slicing may panic.
   --> $DIR/indexing_slicing.rs:12:6
    |
 12 |     &x[index_from..index_to];
    |      ^^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = help: Consider using `.get(n..m)` or `.get_mut(n..m)` instead
 
-error: slicing may panic. Consider using `.get(..n)`or `.get_mut(..n)` instead
+error: slicing may panic.
   --> $DIR/indexing_slicing.rs:13:6
    |
 13 |     &x[index_from..][..index_to];
    |      ^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = help: Consider using `.get(..n)`or `.get_mut(..n)` instead
 
-error: slicing may panic. Consider using `.get(n..)` or .get_mut(n..)` instead
+error: slicing may panic.
   --> $DIR/indexing_slicing.rs:13:6
    |
 13 |     &x[index_from..][..index_to];
    |      ^^^^^^^^^^^^^^^
+   |
+   = help: Consider using `.get(n..)` or .get_mut(n..)` instead
 
-error: slicing may panic. 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. 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
@@ -50,173 +61,211 @@ error: range is out of bounds
 20 |     &x[1..5];
    |      ^^^^^^^
 
-error: slicing may panic. Consider using `.get(n..m)` or `.get_mut(n..m)` instead
+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. Consider using `.get(..n)`or `.get_mut(..n)` instead
+error: slicing may panic.
   --> $DIR/indexing_slicing.rs:21:6
    |
 21 |     &x[1..][..5];
    |      ^^^^^^^^^^^
+   |
+   = help: Consider using `.get(..n)`or `.get_mut(..n)` instead
 
-error: slicing may panic. Consider using `.get(..n)`or `.get_mut(..n)` instead
+error: slicing may panic.
   --> $DIR/indexing_slicing.rs:23:6
    |
 23 |     &x[0..][..3];
    |      ^^^^^^^^^^^
+   |
+   = help: Consider using `.get(..n)`or `.get_mut(..n)` instead
 
 error: range is out of bounds
-  --> $DIR/indexing_slicing.rs:25:6
+  --> $DIR/indexing_slicing.rs:26:6
    |
-25 |     &x[..=4];
+26 |     &x[..=4];
    |      ^^^^^^^
 
-error: slicing may panic. Consider using `.get(..n)`or `.get_mut(..n)` instead
-  --> $DIR/indexing_slicing.rs:25:6
+error: slicing may panic.
+  --> $DIR/indexing_slicing.rs:26:6
    |
-25 |     &x[..=4];
+26 |     &x[..=4];
    |      ^^^^^^^
+   |
+   = help: Consider using `.get(..n)`or `.get_mut(..n)` instead
 
 error: range is out of bounds
-  --> $DIR/indexing_slicing.rs:29:6
+  --> $DIR/indexing_slicing.rs:30:6
    |
-29 |     &x[5..];
+30 |     &x[5..];
    |      ^^^^^^
 
-error: slicing may panic. Consider using `.get(n..)` or .get_mut(n..)` instead
-  --> $DIR/indexing_slicing.rs:29:6
+error: slicing may panic.
+  --> $DIR/indexing_slicing.rs:30:6
    |
-29 |     &x[5..];
+30 |     &x[5..];
    |      ^^^^^^
+   |
+   = help: Consider using `.get(n..)` or .get_mut(n..)` instead
 
 error: range is out of bounds
-  --> $DIR/indexing_slicing.rs:31:6
+  --> $DIR/indexing_slicing.rs:32:6
    |
-31 |     &x[..5];
+32 |     &x[..5];
    |      ^^^^^^
 
-error: slicing may panic. Consider using `.get(..n)`or `.get_mut(..n)` instead
-  --> $DIR/indexing_slicing.rs:31:6
+error: slicing may panic.
+  --> $DIR/indexing_slicing.rs:32:6
    |
-31 |     &x[..5];
+32 |     &x[..5];
    |      ^^^^^^
+   |
+   = help: Consider using `.get(..n)`or `.get_mut(..n)` instead
 
-error: indexing may panic. Consider using `.get(n)` or `.get_mut(n)` instead
-  --> $DIR/indexing_slicing.rs:34:5
+error: indexing may panic.
+  --> $DIR/indexing_slicing.rs:35:5
    |
-34 |     y[0];
+35 |     y[0];
    |     ^^^^
+   |
+   = help: Consider using `.get(n)` or `.get_mut(n)` instead
 
-error: slicing may panic. Consider using `.get(n..m)` or `.get_mut(n..m)` instead
-  --> $DIR/indexing_slicing.rs:35:6
+error: slicing may panic.
+  --> $DIR/indexing_slicing.rs:36:6
    |
-35 |     &y[1..2];
+36 |     &y[1..2];
    |      ^^^^^^^
+   |
+   = help: Consider using `.get(n..m)` or `.get_mut(n..m)` instead
 
-error: slicing may panic. Consider using `.get(..n)`or `.get_mut(..n)` instead
-  --> $DIR/indexing_slicing.rs:38:6
+error: slicing may panic.
+  --> $DIR/indexing_slicing.rs:39:6
    |
-38 |     &y[..=4];
+39 |     &y[..=4];
    |      ^^^^^^^
+   |
+   = help: Consider using `.get(..n)`or `.get_mut(..n)` instead
 
 error: const index is out of bounds
-  --> $DIR/indexing_slicing.rs:41:5
+  --> $DIR/indexing_slicing.rs:42:5
    |
-41 |     empty[0];
+42 |     empty[0];
    |     ^^^^^^^^
 
 error: range is out of bounds
-  --> $DIR/indexing_slicing.rs:42:6
+  --> $DIR/indexing_slicing.rs:43:6
    |
-42 |     &empty[1..5];
+43 |     &empty[1..5];
    |      ^^^^^^^^^^^
 
-error: slicing may panic. Consider using `.get(n..m)` or `.get_mut(n..m)` instead
-  --> $DIR/indexing_slicing.rs:42:6
+error: slicing may panic.
+  --> $DIR/indexing_slicing.rs:43:6
    |
-42 |     &empty[1..5];
+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:44:6
+  --> $DIR/indexing_slicing.rs:45:6
    |
-44 |     &empty[..=4];
+45 |     &empty[..=4];
    |      ^^^^^^^^^^^
 
-error: slicing may panic. Consider using `.get(..n)`or `.get_mut(..n)` instead
-  --> $DIR/indexing_slicing.rs:44:6
+error: slicing may panic.
+  --> $DIR/indexing_slicing.rs:45:6
    |
-44 |     &empty[..=4];
+45 |     &empty[..=4];
    |      ^^^^^^^^^^^
+   |
+   = help: Consider using `.get(..n)`or `.get_mut(..n)` instead
 
 error: range is out of bounds
-  --> $DIR/indexing_slicing.rs:49:6
+  --> $DIR/indexing_slicing.rs:50:6
    |
-49 |     &empty[..=0];
+50 |     &empty[..=0];
    |      ^^^^^^^^^^^
 
-error: slicing may panic. Consider using `.get(..n)`or `.get_mut(..n)` instead
-  --> $DIR/indexing_slicing.rs:49:6
+error: slicing may panic.
+  --> $DIR/indexing_slicing.rs:50:6
    |
-49 |     &empty[..=0];
+50 |     &empty[..=0];
    |      ^^^^^^^^^^^
+   |
+   = help: Consider using `.get(..n)`or `.get_mut(..n)` instead
 
 error: range is out of bounds
-  --> $DIR/indexing_slicing.rs:51:6
+  --> $DIR/indexing_slicing.rs:52:6
    |
-51 |     &empty[1..];
+52 |     &empty[1..];
    |      ^^^^^^^^^^
 
-error: slicing may panic. Consider using `.get(n..)` or .get_mut(n..)` instead
-  --> $DIR/indexing_slicing.rs:51:6
+error: slicing may panic.
+  --> $DIR/indexing_slicing.rs:52:6
    |
-51 |     &empty[1..];
+52 |     &empty[1..];
    |      ^^^^^^^^^^
+   |
+   = help: Consider using `.get(n..)` or .get_mut(n..)` instead
 
 error: range is out of bounds
-  --> $DIR/indexing_slicing.rs:52:6
+  --> $DIR/indexing_slicing.rs:53:6
    |
-52 |     &empty[..4];
+53 |     &empty[..4];
    |      ^^^^^^^^^^
 
-error: slicing may panic. Consider using `.get(..n)`or `.get_mut(..n)` instead
-  --> $DIR/indexing_slicing.rs:52:6
+error: slicing may panic.
+  --> $DIR/indexing_slicing.rs:53:6
    |
-52 |     &empty[..4];
+53 |     &empty[..4];
    |      ^^^^^^^^^^
+   |
+   = help: Consider using `.get(..n)`or `.get_mut(..n)` instead
 
-error: indexing may panic. Consider using `.get(n)` or `.get_mut(n)` instead
-  --> $DIR/indexing_slicing.rs:55:5
+error: indexing may panic.
+  --> $DIR/indexing_slicing.rs:56:5
    |
-55 |     v[0];
+56 |     v[0];
    |     ^^^^
+   |
+   = help: Consider using `.get(n)` or `.get_mut(n)` instead
 
-error: indexing may panic. Consider using `.get(n)` or `.get_mut(n)` instead
-  --> $DIR/indexing_slicing.rs:56:5
+error: indexing may panic.
+  --> $DIR/indexing_slicing.rs:57:5
    |
-56 |     v[10];
+57 |     v[10];
    |     ^^^^^
+   |
+   = help: Consider using `.get(n)` or `.get_mut(n)` instead
 
-error: slicing may panic. Consider using `.get(n..m)` or `.get_mut(n..m)` instead
-  --> $DIR/indexing_slicing.rs:57:6
+error: slicing may panic.
+  --> $DIR/indexing_slicing.rs:58:6
    |
-57 |     &v[10..100];
+58 |     &v[10..100];
    |      ^^^^^^^^^^
+   |
+   = help: Consider using `.get(n..m)` or `.get_mut(n..m)` instead
 
-error: slicing may panic. Consider using `.get(n..)` or .get_mut(n..)` instead
-  --> $DIR/indexing_slicing.rs:58:6
+error: slicing may panic.
+  --> $DIR/indexing_slicing.rs:59:6
    |
-58 |     &v[10..];
+59 |     &v[10..];
    |      ^^^^^^^
+   |
+   = help: Consider using `.get(n..)` or .get_mut(n..)` instead
 
-error: slicing may panic. Consider using `.get(..n)`or `.get_mut(..n)` instead
-  --> $DIR/indexing_slicing.rs:59:6
+error: slicing may panic.
+  --> $DIR/indexing_slicing.rs:60:6
    |
-59 |     &v[..100];
+60 |     &v[..100];
    |      ^^^^^^^^
+   |
+   = help: Consider using `.get(..n)`or `.get_mut(..n)` instead
 
 error: aborting due to 36 previous errors