[red-knot] Minor follow-up on slice expression inference (#13982)

## Summary

Minor follow-up to #13917 — thanks @AlexWaygood for the post-merge
review.

- Add
SliceLiteralType::as_tuple
- Use .expect() instead of SAFETY
comment
- Match on ::try_from
result
- Add TODO comment regarding raising a diagnostic for `"foo"["bar":"baz"]`

crates/red_knot_python_semantic/resources/mdtest/subscript/string.md

@@ -86,3 +86,14 @@ substring2 = str_instance()[0:5]
 # TODO: Support overloads... Should be `str`
 reveal_type(substring2)  # revealed: @Todo
 ```
+
+## Unsupported slice types
+
+```py
+# TODO: It would be great if we raised an error here. This can be done once
+# we have support for overloads and generics, and once typeshed has a more
+# precise annotation for `str.__getitem__`, that makes use of the generic
+# `slice[..]` type. We could then infer `slice[str, str]` here and see that
+# it doesn't match the signature of `str.__getitem__`.
+"foo"["bar":"baz"]
+```

crates/red_knot_python_semantic/src/types.rs

@@ -1944,6 +1944,11 @@ pub struct SliceLiteralType<'db> {
     step: Option<i32>,
 }
 
+impl<'db> SliceLiteralType<'db> {
+    fn as_tuple(self, db: &dyn Db) -> (Option<i32>, Option<i32>, Option<i32>) {
+        (self.start(db), self.stop(db), self.step(db))
+    }
+}
 #[salsa::interned]
 pub struct TupleType<'db> {
     #[return_ref]

crates/red_knot_python_semantic/src/types/infer.rs

@@ -3226,9 +3226,7 @@ impl<'db> TypeInferenceBuilder<'db> {
             // Ex) Given `("a", 1, Null)[0:2]`, return `("a", 1)`
             (Type::Tuple(tuple_ty), Type::SliceLiteral(slice_ty)) => {
                 let elements = tuple_ty.elements(self.db);
-                let start = slice_ty.start(self.db);
+                let (start, stop, step) = slice_ty.as_tuple(self.db);
-                let stop = slice_ty.stop(self.db);
-                let step = slice_ty.step(self.db);
 
                 if let Ok(new_elements) = elements.as_ref().py_slice(start, stop, step) {
                     let new_elements: Vec<_> = new_elements.copied().collect();
@@ -3266,9 +3264,7 @@ impl<'db> TypeInferenceBuilder<'db> {
             // Ex) Given `"value"[1:3]`, return `"al"`
             (Type::StringLiteral(literal_ty), Type::SliceLiteral(slice_ty)) => {
                 let literal_value = literal_ty.value(self.db);
-                let start = slice_ty.start(self.db);
+                let (start, stop, step) = slice_ty.as_tuple(self.db);
-                let stop = slice_ty.stop(self.db);
-                let step = slice_ty.step(self.db);
 
                 let chars: Vec<_> = literal_value.chars().collect();
                 let result = if let Ok(new_chars) = chars.as_slice().py_slice(start, stop, step) {
@@ -3305,9 +3301,7 @@ impl<'db> TypeInferenceBuilder<'db> {
             // Ex) Given `b"value"[1:3]`, return `b"al"`
             (Type::BytesLiteral(literal_ty), Type::SliceLiteral(slice_ty)) => {
                 let literal_value = literal_ty.value(self.db);
-                let start = slice_ty.start(self.db);
+                let (start, stop, step) = slice_ty.as_tuple(self.db);
-                let stop = slice_ty.stop(self.db);
-                let step = slice_ty.step(self.db);
 
                 if let Ok(new_bytes) = literal_value.as_ref().py_slice(start, stop, step) {
                     let new_bytes: Vec<u8> = new_bytes.copied().collect();
@@ -3418,9 +3412,10 @@ impl<'db> TypeInferenceBuilder<'db> {
         let ty_step = self.infer_optional_expression(step.as_deref());
 
         let type_to_slice_argument = |ty: Option<Type<'db>>| match ty {
-            Some(Type::IntLiteral(n)) if i32::try_from(n).is_ok() => {
+            Some(Type::IntLiteral(n)) => match i32::try_from(n) {
-                SliceArg::Arg(Some(i32::try_from(n).expect("checked in branch arm")))
+                Ok(n) => SliceArg::Arg(Some(n)),
-            }
+                Err(_) => SliceArg::Unsupported,
+            },
             Some(Type::BooleanLiteral(b)) => SliceArg::Arg(Some(i32::from(b))),
             Some(Type::None) => SliceArg::Arg(None),
             Some(Type::Instance(class)) if class.is_known(self.db, KnownClass::NoneType) => {

crates/red_knot_python_semantic/src/util/subscript.rs

@@ -17,8 +17,8 @@ fn from_nonnegative_i32(index: i32) -> usize {
     static_assertions::const_assert!(usize::BITS >= 32);
     debug_assert!(index >= 0);
 
-    // SAFETY: `index` is non-negative, and `usize` is at least 32 bits.
+    usize::try_from(index)
-    usize::try_from(index).unwrap()
+        .expect("Should only ever pass a positive integer to `from_nonnegative_i32`")
 }
 
 fn from_negative_i32(index: i32) -> usize {