Rollup merge of #89251 - estebank:negative-index-literals, r=davidtwco

Detect when negative literal indices are used and suggest appropriate code

Author: Manishearth

compiler/rustc_typeck/src/check/expr.rs

@@ -2137,7 +2137,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
             idx_t
         } else {
             let base_t = self.structurally_resolved_type(base.span, base_t);
-            match self.lookup_indexing(expr, base, base_t, idx_t) {
+            match self.lookup_indexing(expr, base, base_t, idx, idx_t) {
                 Some((index_ty, element_ty)) => {
                     // two-phase not needed because index_ty is never mutable
                     self.demand_coerce(idx, idx_t, index_ty, None, AllowTwoPhase::No);

compiler/rustc_typeck/src/check/place_op.rs

@@ -1,5 +1,7 @@
 use crate::check::method::MethodCallee;
 use crate::check::{has_expected_num_generic_args, FnCtxt, PlaceOp};
+use rustc_ast as ast;
+use rustc_errors::Applicability;
 use rustc_hir as hir;
 use rustc_infer::infer::type_variable::{TypeVariableOrigin, TypeVariableOriginKind};
 use rustc_infer::infer::InferOk;
@@ -47,6 +49,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
         expr: &hir::Expr<'_>,
         base_expr: &'tcx hir::Expr<'tcx>,
         base_ty: Ty<'tcx>,
+        index_expr: &'tcx hir::Expr<'tcx>,
         idx_ty: Ty<'tcx>,
     ) -> Option<(/*index type*/ Ty<'tcx>, /*element type*/ Ty<'tcx>)> {
         // FIXME(#18741) -- this is almost but not quite the same as the
@@ -56,12 +59,42 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
         let mut autoderef = self.autoderef(base_expr.span, base_ty);
         let mut result = None;
         while result.is_none() && autoderef.next().is_some() {
-            result = self.try_index_step(expr, base_expr, &autoderef, idx_ty);
+            result = self.try_index_step(expr, base_expr, &autoderef, idx_ty, index_expr);
         }
         self.register_predicates(autoderef.into_obligations());
         result
     }
 
+    fn negative_index(
+        &self,
+        ty: Ty<'tcx>,
+        span: Span,
+        base_expr: &hir::Expr<'_>,
+    ) -> Option<(Ty<'tcx>, Ty<'tcx>)> {
+        let ty = self.resolve_vars_if_possible(ty);
+        let mut err = self.tcx.sess.struct_span_err(
+            span,
+            &format!("negative integers cannot be used to index on a `{}`", ty),
+        );
+        err.span_label(span, &format!("cannot use a negative integer for indexing on `{}`", ty));
+        if let (hir::ExprKind::Path(..), Ok(snippet)) =
+            (&base_expr.kind, self.tcx.sess.source_map().span_to_snippet(base_expr.span))
+        {
+            // `foo[-1]` to `foo[foo.len() - 1]`
+            err.span_suggestion_verbose(
+                span.shrink_to_lo(),
+                &format!(
+                    "to access an element starting from the end of the `{}`, compute the index",
+                    ty,
+                ),
+                format!("{}.len() ", snippet),
+                Applicability::MachineApplicable,
+            );
+        }
+        err.emit();
+        Some((self.tcx.ty_error(), self.tcx.ty_error()))
+    }
+
     /// To type-check `base_expr[index_expr]`, we progressively autoderef
     /// (and otherwise adjust) `base_expr`, looking for a type which either
     /// supports builtin indexing or overloaded indexing.
@@ -73,6 +106,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
         base_expr: &hir::Expr<'_>,
         autoderef: &Autoderef<'a, 'tcx>,
         index_ty: Ty<'tcx>,
+        index_expr: &hir::Expr<'_>,
     ) -> Option<(/*index type*/ Ty<'tcx>, /*element type*/ Ty<'tcx>)> {
         let adjusted_ty =
             self.structurally_resolved_type(autoderef.span(), autoderef.final_ty(false));
@@ -82,6 +116,27 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
             expr, base_expr, adjusted_ty, index_ty
         );
 
+        if let hir::ExprKind::Unary(
+            hir::UnOp::Neg,
+            hir::Expr {
+                kind: hir::ExprKind::Lit(hir::Lit { node: ast::LitKind::Int(..), .. }),
+                ..
+            },
+        ) = index_expr.kind
+        {
+            match adjusted_ty.kind() {
+                ty::Adt(ty::AdtDef { did, .. }, _)
+                    if self.tcx.is_diagnostic_item(sym::vec_type, *did) =>
+                {
+                    return self.negative_index(adjusted_ty, index_expr.span, base_expr);
+                }
+                ty::Slice(_) | ty::Array(_, _) => {
+                    return self.negative_index(adjusted_ty, index_expr.span, base_expr);
+                }
+                _ => {}
+            }
+        }
+
         for unsize in [false, true] {
             let mut self_ty = adjusted_ty;
             if unsize {

src/test/ui/suggestions/negative-literal-index.fixed

@@ -0,0 +1,22 @@
+// run-rustfix
+
+use std::ops::Index;
+struct X;
+impl Index<i32> for X {
+    type Output = ();
+
+    fn index(&self, _: i32) -> &() {
+        &()
+    }
+}
+
+fn main() {
+    let x = vec![1, 2, 3];
+    x[x.len() -1]; //~ ERROR negative integers cannot be used to index on a
+    let x = [1, 2, 3];
+    x[x.len() -1]; //~ ERROR negative integers cannot be used to index on a
+    let x = &[1, 2, 3];
+    x[x.len() -1]; //~ ERROR negative integers cannot be used to index on a
+    let _ = x;
+    X[-1];
+}

src/test/ui/suggestions/negative-literal-index.rs

@@ -0,0 +1,22 @@
+// run-rustfix
+
+use std::ops::Index;
+struct X;
+impl Index<i32> for X {
+    type Output = ();
+
+    fn index(&self, _: i32) -> &() {
+        &()
+    }
+}
+
+fn main() {
+    let x = vec![1, 2, 3];
+    x[-1]; //~ ERROR negative integers cannot be used to index on a
+    let x = [1, 2, 3];
+    x[-1]; //~ ERROR negative integers cannot be used to index on a
+    let x = &[1, 2, 3];
+    x[-1]; //~ ERROR negative integers cannot be used to index on a
+    let _ = x;
+    X[-1];
+}

src/test/ui/suggestions/negative-literal-index.stderr

@@ -0,0 +1,35 @@
+error: negative integers cannot be used to index on a `Vec<{integer}>`
+  --> $DIR/negative-literal-index.rs:15:7
+   |
+LL |     x[-1];
+   |       ^^ cannot use a negative integer for indexing on `Vec<{integer}>`
+   |
+help: to access an element starting from the end of the `Vec<{integer}>`, compute the index
+   |
+LL |     x[x.len() -1];
+   |       +++++++
+
+error: negative integers cannot be used to index on a `[{integer}; 3]`
+  --> $DIR/negative-literal-index.rs:17:7
+   |
+LL |     x[-1];
+   |       ^^ cannot use a negative integer for indexing on `[{integer}; 3]`
+   |
+help: to access an element starting from the end of the `[{integer}; 3]`, compute the index
+   |
+LL |     x[x.len() -1];
+   |       +++++++
+
+error: negative integers cannot be used to index on a `[{integer}; 3]`
+  --> $DIR/negative-literal-index.rs:19:7
+   |
+LL |     x[-1];
+   |       ^^ cannot use a negative integer for indexing on `[{integer}; 3]`
+   |
+help: to access an element starting from the end of the `[{integer}; 3]`, compute the index
+   |
+LL |     x[x.len() -1];
+   |       +++++++
+
+error: aborting due to 3 previous errors
+