Repository: rhaiscript/rhai (language: rust)

Issue:
Feature: syntax `str[range]` support like rust string indexing by range
Expect:

```rust
let s = "abcd";
let n = s[1..3];
```

Now it reports:

```text
2: let n = s[1..3];
             ^ Data type incorrect: core::ops::range::Range<i64> (expecting i64)
```

Would this feature could be supported? Maybe I can contribute if you don't mind to add this, and give me some help to start.



Discussion / hints:
Well, there is `sub_string` that does something similar...

The actual implementation may be quite involved, since indexer for strings cannot be overridden.  It is built-in.  Internally it uses mutable references to do its magic.  For example, if we implement this correctly for strings, you can do this:

```rust
let s = "abcdefg";
s[1..3] = "xyz";    // you can even modify a sub-range inside a string!
print(s);    // prints `axyzdefg`
```

When Rhai was first developed, ranges were not in, so this feature was left uncoded.

It should be similar to how indexing into a string to get at a single character would work:

```rust
let s = "abcdefg";
s[1] = 'x';
print(s);    // prints `axcdefg`
```

You may look into the `Target` type to see how character indexing is done, and then adapt it to ranges if you want a shot at it?


Fix patch:
diff --git a/src/ast/expr.rs b/src/ast/expr.rs
index 8870b1b29..71bb924d1 100644
--- a/src/ast/expr.rs
+++ b/src/ast/expr.rs
@@ -530,6 +530,9 @@ impl Expr {
                             Self::IntegerConstant(ref start, ..),
                             Self::IntegerConstant(ref end, ..),
                         ) => (*start..*end).into(),
+                        (Self::IntegerConstant(ref start, ..), Self::Unit(..)) => {
+                            (*start..INT::MAX).into()
+                        }
                         _ => return None,
                     },
                     // x..=y
@@ -538,6 +541,9 @@ impl Expr {
                             Self::IntegerConstant(ref start, ..),
                             Self::IntegerConstant(ref end, ..),
                         ) => (*start..=*end).into(),
+                        (Self::IntegerConstant(ref start, ..), Self::Unit(..)) => {
+                            (*start..=INT::MAX).into()
+                        }
                         _ => return None,
                     },
                     _ => return None,
diff --git a/src/eval/chaining.rs b/src/eval/chaining.rs
index 7e1815cb7..3d72c9ace 100644
--- a/src/eval/chaining.rs
+++ b/src/eval/chaining.rs
@@ -4,6 +4,7 @@
 use super::{Caches, GlobalRuntimeState, Target};
 use crate::ast::{ASTFlags, BinaryExpr, Expr, OpAssignment};
 use crate::engine::{FN_IDX_GET, FN_IDX_SET};
+use crate::tokenizer::Token;
 use crate::types::dynamic::Union;
 use crate::{
     calc_fn_hash, Dynamic, Engine, FnArgsVec, OnceCell, Position, RhaiResult, RhaiResultOf, Scope,
@@ -295,53 +296,127 @@ impl Engine {
 
             #[cfg(not(feature = "no_index"))]
             Dynamic(Union::Str(s, ..)) => {
-                // val_string[idx]
-                let index = idx
-                    .as_int()
-                    .map_err(|typ| self.make_type_mismatch_err::<crate::INT>(typ, idx_pos))?;
+                match idx.as_int() {
+                    Ok(index) => {
+                        let (ch, offset) = if index >= 0 {
+                            #[allow(clippy::absurd_extreme_comparisons)]
+                            if index >= crate::MAX_USIZE_INT {
+                                return Err(ERR::ErrorStringBounds(
+                                    s.chars().count(),
+                                    index,
+                                    idx_pos,
+                                )
+                                .into());
+                            }
 
-                let (ch, offset) = if index >= 0 {
-                    #[allow(clippy::absurd_extreme_comparisons)]
-                    if index >= crate::MAX_USIZE_INT {
-                        return Err(
-                            ERR::ErrorStringBounds(s.chars().count(), index, idx_pos).into()
-                        );
-                    }
+                            #[allow(clippy::cast_sign_loss, clippy::cast_possible_truncation)]
+                            let offset = index as usize;
+                            (
+                                s.chars().nth(offset).ok_or_else(|| {
+                                    ERR::ErrorStringBounds(s.chars().count(), index, idx_pos)
+                                })?,
+                                offset,
+                            )
+                        } else {
+                            let abs_index = index.unsigned_abs();
+
+                            #[allow(clippy::unnecessary_cast)]
+                            if abs_index as u64 > usize::MAX as u64 {
+                                return Err(ERR::ErrorStringBounds(
+                                    s.chars().count(),
+                                    index,
+                                    idx_pos,
+                                )
+                                .into());
+                            }
 
-                    #[allow(clippy::cast_sign_loss, clippy::cast_possible_truncation)]
-                    let offset = index as usize;
-                    (
-                        s.chars().nth(offset).ok_or_else(|| {
-                            ERR::ErrorStringBounds(s.chars().count(), index, idx_pos)
-                        })?,
-                        offset,
-                    )
-                } else {
-                    let abs_index = index.unsigned_abs();
+                            #[allow(clippy::cast_possible_truncation)]
+                            let offset = abs_index as usize;
+                            (
+                                // Count from end if negative
+                                s.chars().rev().nth(offset - 1).ok_or_else(|| {
+                                    ERR::ErrorStringBounds(s.chars().count(), index, idx_pos)
+                                })?,
+                                offset,
+                            )
+                        };
 
-                    #[allow(clippy::unnecessary_cast)]
-                    if abs_index as u64 > usize::MAX as u64 {
-                        return Err(
-                            ERR::ErrorStringBounds(s.chars().count(), index, idx_pos).into()
-                        );
+                        Ok(Target::StringChar {
+                            source: target,
+                            value: ch.into(),
+                            index: offset,
+                        })
                     }
+                    Err(typ) => {
+                        if typ == "core::ops::range::Range<i64>" {
+                            // val_str[range]
+                            let idx = idx.read_lock::<core::ops::Range<i64>>().unwrap();
+                            let range = &*idx;
+                            let chars_count = s.chars().count();
+                            let start = if range.start >= 0 {
+                                range.start as usize
+                            } else {
+                                super::calc_index(chars_count, range.start, true, || {
+                                    ERR::ErrorStringBounds(chars_count, range.start, idx_pos).into()
+                                })?
+                            };
+                            let end = if range.end >= 0 {
+                                range.end as usize
+                            } else {
+                                super::calc_index(chars_count, range.end, true, || {
+                                    ERR::ErrorStringBounds(chars_count, range.end, idx_pos).into()
+                                })
+                                .unwrap_or(0)
+                            };
 
-                    #[allow(clippy::cast_possible_truncation)]
-                    let offset = abs_index as usize;
-                    (
-                        // Count from end if negative
-                        s.chars().rev().nth(offset - 1).ok_or_else(|| {
-                            ERR::ErrorStringBounds(s.chars().count(), index, idx_pos)
-                        })?,
-                        offset,
-                    )
-                };
+                            let take = if end > start { end - start } else { 0 };
+
+                            let value = s.chars().skip(start).take(take).collect::<String>();
+                            return Ok(Target::StringSlice {
+                                source: target,
+                                value: value.into(),
+                                start: start,
+                                end: end,
+                                exclusive: true,
+                            });
+                        } else if typ == "core::ops::range::RangeInclusive<i64>" {
+                            // val_str[range]
+                            let idx = idx.read_lock::<core::ops::RangeInclusive<i64>>().unwrap();
+                            let range = &*idx;
+                            let chars_count = s.chars().count();
+                            let start = if *range.start() >= 0 {
+                                *range.start() as usize
+                            } else {
+                                super::calc_index(chars_count, *range.start(), true, || {
+                                    ERR::ErrorStringBounds(chars_count, *range.start(), idx_pos)
+                                        .into()
+                                })?
+                            };
+                            let end = if *range.end() >= 0 {
+                                *range.end() as usize
+                            } else {
+                                super::calc_index(chars_count, *range.end(), true, || {
+                                    ERR::ErrorStringBounds(chars_count, *range.end(), idx_pos)
+                                        .into()
+                                })
+                                .unwrap_or(0)
+                            };
 
-                Ok(Target::StringChar {
-                    source: target,
-                    value: ch.into(),
-                    index: offset,
-                })
+                            let take = if end > start { end - start + 1 } else { 0 };
+
+                            let value = s.chars().skip(start).take(take).collect::<String>();
+                            return Ok(Target::StringSlice {
+                                source: target,
+                                value: value.into(),
+                                start,
+                                end,
+                                exclusive: false,
+                            });
+                        } else {
+                            return Err(self.make_type_mismatch_err::<crate::INT>(typ, idx_pos));
+                        }
+                    }
+                }
             }
 
             #[cfg(not(feature = "no_closure"))]
@@ -399,6 +474,13 @@ impl Engine {
             (_, ChainType::Indexing) if rhs.is_constant() => {
                 idx_values.push(rhs.get_literal_value().unwrap())
             }
+            #[cfg(not(feature = "no_index"))]
+            (Expr::FnCall(fnc, _), ChainType::Indexing)
+                if fnc.op_token == Some(Token::InclusiveRange)
+                    || fnc.op_token == Some(Token::ExclusiveRange) =>
+            {
+                idx_values.push(rhs.get_literal_value().unwrap())
+            }
             // Short-circuit for simple method call: {expr}.func()
             #[cfg(not(feature = "no_object"))]
             (Expr::MethodCall(x, ..), ChainType::Dotting) if x.args.is_empty() => (),
diff --git a/src/eval/target.rs b/src/eval/target.rs
index 6bff09c33..cdf54f7d9 100644
--- a/src/eval/target.rs
+++ b/src/eval/target.rs
@@ -162,6 +162,21 @@ pub enum Target<'a> {
         /// Offset index.
         index: usize,
     },
+    /// The target is a slice of a string.
+    /// This is necessary because directly pointing to a [`char`] inside a [`String`] is impossible.
+    #[cfg(not(feature = "no_index"))]
+    StringSlice {
+        /// Mutable reference to the source [`Dynamic`].
+        source: &'a mut Dynamic,
+        /// Copy of the character at the offset, as a [`Dynamic`].
+        value: Dynamic,
+        /// Offset index.
+        start: usize,
+        /// End index.
+        end: usize,
+        /// Is exclusive?
+        exclusive: bool,
+    },
 }
 
 impl<'a> Target<'a> {
@@ -174,6 +189,8 @@ impl<'a> Target<'a> {
             Self::RefMut(..) => true,
             #[cfg(not(feature = "no_closure"))]
             Self::SharedValue { .. } => true,
+            #[cfg(not(feature = "no_index"))]
+            Self::StringSlice { .. } => true,
             Self::TempValue(..) => false,
             #[cfg(not(feature = "no_index"))]
             Self::Bit { .. }
@@ -190,6 +207,8 @@ impl<'a> Target<'a> {
             Self::RefMut(..) => false,
             #[cfg(not(feature = "no_closure"))]
             Self::SharedValue { .. } => false,
+            #[cfg(not(feature = "no_index"))]
+            Self::StringSlice { .. } => true,
             Self::TempValue(..) => true,
             #[cfg(not(feature = "no_index"))]
             Self::Bit { .. }
@@ -206,6 +225,7 @@ impl<'a> Target<'a> {
         return match self {
             Self::RefMut(r) => r.is_shared(),
             Self::SharedValue { .. } => true,
+            Self::StringSlice { .. } => true,
             Self::TempValue(value) => value.is_shared(),
             #[cfg(not(feature = "no_index"))]
             Self::Bit { .. }
@@ -232,6 +252,11 @@ impl<'a> Target<'a> {
             Self::BlobByte { value, .. } => value, // byte is taken
             #[cfg(not(feature = "no_index"))]
             Self::StringChar { value, .. } => value, // char is taken
+            #[cfg(not(feature = "no_index"))]
+            Self::StringSlice { value, .. } => {
+                // Slice of a string is cloned
+                Dynamic::from(value.to_string())
+            }
         }
     }
     /// Take a `&mut Dynamic` reference from the `Target`.
@@ -270,6 +295,8 @@ impl<'a> Target<'a> {
             Self::BlobByte { source, .. } => source,
             #[cfg(not(feature = "no_index"))]
             Self::StringChar { source, .. } => source,
+            #[cfg(not(feature = "no_index"))]
+            Self::StringSlice { source, .. } => source,
         }
     }
     /// Propagate a changed value back to the original source.
@@ -372,6 +399,27 @@ impl<'a> Target<'a> {
                     .map(|(i, ch)| if i == *index { new_ch } else { ch })
                     .collect();
             }
+            #[cfg(not(feature = "no_index"))]
+            Self::StringSlice {
+                source,
+                ref start,
+                ref end,
+                ref value,
+                exclusive,
+            } => {
+                let s = &mut *source.write_lock::<crate::ImmutableString>().unwrap();
+
+                let n = s.chars().count();
+                let vs = s.chars().take(*start);
+                let ve = if *exclusive {
+                    let end = if *end > n { n } else { *end };
+                    s.chars().skip(end)
+                } else {
+                    let end = if *end >= n { n - 1 } else { *end };
+                    s.chars().skip(end + 1)
+                };
+                *s = vs.chain(value.to_string().chars()).chain(ve).collect();
+            }
         }
 
         Ok(())
@@ -405,6 +453,8 @@ impl AsRef<Dynamic> for Target<'_> {
             Self::SharedValue { guard, .. } => guard,
             Self::TempValue(ref value) => value,
             #[cfg(not(feature = "no_index"))]
+            Self::StringSlice { ref value, .. } => value,
+            #[cfg(not(feature = "no_index"))]
             Self::Bit { ref value, .. }
             | Self::BitField { ref value, .. }
             | Self::BlobByte { ref value, .. }
@@ -429,6 +479,8 @@ impl AsMut<Dynamic> for Target<'_> {
             Self::SharedValue { guard, .. } => &mut *guard,
             Self::TempValue(ref mut value) => value,
             #[cfg(not(feature = "no_index"))]
+            Self::StringSlice { ref mut value, .. } => value,
+            #[cfg(not(feature = "no_index"))]
             Self::Bit { ref mut value, .. }
             | Self::BitField { ref mut value, .. }
             | Self::BlobByte { ref mut value, .. }
diff --git a/src/parser.rs b/src/parser.rs
index 349b2bd17..edf7e7557 100644
--- a/src/parser.rs
+++ b/src/parser.rs
@@ -1344,6 +1344,7 @@ impl Engine {
                 Token::False => Expr::BoolConstant(false, settings.pos),
                 token => unreachable!("token is {:?}", token),
             },
+            Token::ExclusiveRange | Token::InclusiveRange => Expr::IntegerConstant(0, settings.pos),
             #[cfg(not(feature = "no_float"))]
             Token::FloatConstant(x) => {
                 let x = x.0;
@@ -2014,6 +2015,13 @@ impl Engine {
                 }
                 .into_fn_call_expr(pos))
             }
+            Token::RightBracket => {
+                let v = state.input.peek();
+                match v {
+                    Some((Token::RightBracket, _)) => Ok(Expr::Unit(settings.pos.clone())),
+                    _ => self.parse_primary(state, settings, ChainingFlags::empty()),
+                }
+            }
             // <EOF>
             Token::EOF => Err(PERR::UnexpectedEOF.into_err(settings.pos)),
             // All other tokens
@@ -2412,6 +2420,12 @@ impl Engine {
                         not_base.into_fn_call_expr(pos)
                     }
                 }
+                Token::ExclusiveRange | Token::InclusiveRange => {
+                    if op_base.args[1].is_unit() {
+                        let _ = op_base.args[1].take();
+                    }
+                    op_base.into_fn_call_expr(pos)
+                }
 
                 #[cfg(not(feature = "no_custom_syntax"))]
                 Token::Custom(s) if self.custom_keywords.contains_key(&*s) => {


Test patch:
diff --git a/tests/string.rs b/tests/string.rs
index 1af0083d2..1564508f4 100644
--- a/tests/string.rs
+++ b/tests/string.rs
@@ -53,7 +53,85 @@ fn test_string() {
     #[cfg(not(feature = "no_float"))]
     assert_eq!(engine.eval::<String>(r#""foo" + 123.4556"#).unwrap(), "foo123.4556");
 }
+#[cfg(not(feature = "no_index"))]
+#[test]
+fn test_string_index() {
+    let engine = Engine::new();
+    // char index
+    assert_eq!(engine.eval::<char>(r#"let y = "hello"; y[-4]"#).unwrap(), 'e');
 
+    // range index
+
+    // range index returns a string
+    assert_eq!(engine.eval::<char>(r#"let y = "hello"; y[1..2]"#).unwrap_err().to_string(), "Output type incorrect: string (expecting char)");
+
+    // 1..3
+    assert_eq!(engine.eval::<String>(r#"let y = "hello"; y[1..3]"#).unwrap(), "el");
+    // 0..5
+    assert_eq!(engine.eval::<String>(r#"let y = "hello"; y[..]"#).unwrap(), "hello");
+    // 0..2
+    assert_eq!(engine.eval::<String>(r#"let y = "hello"; y[..2]"#).unwrap(), "he");
+    // 1..4
+    assert_eq!(engine.eval::<String>(r#"let y = "hello"; y[1..-1]"#).unwrap(), "ell");
+    // 1..1
+    assert_eq!(engine.eval::<String>(r#"let y = "hello"; y[1..-4]"#).unwrap(), "");
+    // 2..1
+    assert_eq!(engine.eval::<String>(r#"let y = "hello"; y[2..-4]"#).unwrap(), "");
+    // overflow index
+    assert_eq!(engine.eval::<String>(r#"let y = "hello"; y[0..18]"#).unwrap(), "hello");
+    // overflow negative index
+    assert_eq!(engine.eval::<String>(r#"let y = "hello"; y[2..-18]"#).unwrap(), "");
+
+    // inclusive range
+    assert_eq!(engine.eval::<String>(r#"let y = "hello"; y[1..=3]"#).unwrap(), "ell");
+    // 0..=5
+    assert_eq!(engine.eval::<String>(r#"let y = "hello"; y[..=]"#).unwrap(), "hello");
+    // 0..=2
+    assert_eq!(engine.eval::<String>(r#"let y = "hello"; y[..=2]"#).unwrap(), "hel");
+    // 1..=4
+    assert_eq!(engine.eval::<String>(r#"let y = "hello"; y[1..=-1]"#).unwrap(), "ello");
+    // 1..=1
+    assert_eq!(engine.eval::<String>(r#"let y = "hello"; y[1..=-4]"#).unwrap(), "");
+    // 2..=1
+    assert_eq!(engine.eval::<String>(r#"let y = "hello"; y[2..-4]"#).unwrap(), "");
+    // overflow index
+    assert_eq!(engine.eval::<String>(r#"let y = "hello"; y[0..18]"#).unwrap(), "hello");
+    // overflow negative index
+    assert_eq!(engine.eval::<String>(r#"let y = "hello"; y[2..-18]"#).unwrap(), "");
+
+    // mut slice index
+    assert_eq!(engine.eval::<String>(r#"let y = "hello"; y[1] = 'i'; y"#).unwrap(), "hillo");
+    // mut slice index
+    assert_eq!(engine.eval::<String>(r#"let y = "hello"; y[1..2] = "i"; y"#).unwrap(), "hillo");
+    assert_eq!(engine.eval::<String>(r#"let y = "hello"; y[1..3] = "i"; y"#).unwrap(), "hilo");
+    assert_eq!(engine.eval::<String>(r#"let y = "hello"; y[1..3] = "iii"; y"#).unwrap(), "hiiilo");
+    assert_eq!(engine.eval::<String>(r#"let y = "hello"; y[1..=2] = "iii"; y"#).unwrap(), "hiiilo");
+    assert_eq!(engine.eval::<String>(r#"let y = "hello"; y[1..=2] = y[2..]; y"#).unwrap(), "hllolo");
+
+    // new string will not be affected by mut slice index on old string.
+    assert_eq!(engine.eval::<String>(r#"let y = "hello"; y[1..=2] = y[2..]; let s2 = y[1..]; s2[1..20] = "abc"; y"#).unwrap(), "hllolo");
+
+    assert_eq!(
+        engine
+            .eval::<String>(
+                r#"
+                    let y = "hello";
+                    let s2 = y[1..];
+                    s2[1..20] = "abc";
+                    if (s2 == "eabc") {
+                        y[2] = 'd';
+                    }
+                    y[3..] = "xyz";
+                    y[4] = '\u2764';
+                    y[6..] = "\u2764\u2764";
+                    
+                    y
+                "#
+            )
+            .unwrap(),
+        "hedx❤z❤❤"
+    );
+}
 #[test]
 fn test_string_dynamic() {
     let engine = Engine::new();