Support negative index counting from end.

CHANGELOG.md

@@ -1,29 +1,39 @@
 Rhai Release Notes
 ==================
 
-This version adds string interpolation with `` `... ${`` ... ``} ...` `` syntax.
-
 Version 0.19.16
 ===============
 
+This version adds string interpolation with `` `... ${`` ... ``} ...` `` syntax.
+
+Negative indices for arrays and strings are allowed and now count from the end (-1 = last item/character).
+
 Bug fixes
 ---------
 
 * Property setter op-assignments now work properly.
+* Off-by-one bug in `Array::drain` method with range is fixed.
 
 Breaking changes
 ----------------
 
+* Negative index to an array or string yields the appropriate element/character counting from the _end_.
 * `ModuleResolver` trait methods take an additional parameter `source_path` that contains the path of the current environment. This is to facilitate loading other script files always from the current directory.
 * `FileModuleResolver` now resolves relative paths under the source path if there is no base path set.
 * `FileModuleResolver::base_path` now returns `Option<&str>` which is `None` if there is no base path set.
 * Doc-comments now require the `metadata` feature.
 
+Enhancements
+------------
+
+* `Array::drain` and `Array::retain` methods with predicate now scan the array in forward order instead of in reverse.
+
 New features
 ------------
 
 * String interpolation support is added via the `` `... ${`` ... ``} ...` `` syntax.
 * `FileModuleResolver` resolves relative paths under the parent path (i.e. the path holding the script that does the loading). This allows seamless cross-loading of scripts from a directory hierarchy instead of having all relative paths load from the current working directory.
+* Negative index to an array or string yields the appropriate element/character counting from the _end_.
 
 
 Version 0.19.15

src/engine.rs

@@ -1598,15 +1598,29 @@ impl Engine {
 
                 let arr_len = arr.len();
 
-                if index >= 0 {
+                let arr_idx = if index < 0 {
-                    arr.get_mut(index as usize)
+                    // Count from end if negative
-                        .map(Target::from)
+                    arr_len
-                        .ok_or_else(|| {
+                        - index
-                            EvalAltResult::ErrorArrayBounds(arr_len, index, idx_pos).into()
+                            .checked_abs()
-                        })
+                            .ok_or_else(|| {
+                                Box::new(EvalAltResult::ErrorArrayBounds(arr_len, index, idx_pos))
+                            })
+                            .and_then(|n| {
+                                if n as usize > arr_len {
+                                    Err(EvalAltResult::ErrorArrayBounds(arr_len, index, idx_pos)
+                                        .into())
+                                } else {
+                                    Ok(n as usize)
+                                }
+                            })?
                 } else {
-                    EvalAltResult::ErrorArrayBounds(arr_len, index, idx_pos).into()
+                    index as usize
-                }
+                };
+
+                arr.get_mut(arr_idx)
+                    .map(Target::from)
+                    .ok_or_else(|| EvalAltResult::ErrorArrayBounds(arr_len, index, idx_pos).into())
             }
 
             #[cfg(not(feature = "no_object"))]
@@ -1634,15 +1648,27 @@ impl Engine {
                     .as_int()
                     .map_err(|err| self.make_type_mismatch_err::<crate::INT>(err, idx_pos))?;
 
-                if index >= 0 {
+                let (ch, offset) = if index >= 0 {
                     let offset = index as usize;
-                    let ch = s.chars().nth(offset).ok_or_else(|| {
+                    (
-                        EvalAltResult::ErrorStringBounds(chars_len, index, idx_pos)
+                        s.chars().nth(offset).ok_or_else(|| {
-                    })?;
+                            EvalAltResult::ErrorStringBounds(chars_len, index, idx_pos)
-                    Ok(Target::StringChar(target, offset, ch.into()))
+                        })?,
+                        offset,
+                    )
+                } else if let Some(index) = index.checked_abs() {
+                    let offset = index as usize;
+                    (
+                        s.chars().rev().nth(offset - 1).ok_or_else(|| {
+                            EvalAltResult::ErrorStringBounds(chars_len, index, idx_pos)
+                        })?,
+                        offset,
+                    )
                 } else {
-                    EvalAltResult::ErrorStringBounds(chars_len, index, idx_pos).into()
+                    return EvalAltResult::ErrorStringBounds(chars_len, index, idx_pos).into();
-                }
+                };
+
+                Ok(Target::StringChar(target, offset, ch.into()))
             }
 
             #[cfg(not(feature = "no_index"))]

src/optimize.rs

@@ -654,6 +654,17 @@ fn optimize_expr(expr: &mut Expr, state: &mut State) {
                 result.set_position(*pos);
                 *expr = result;
             }
+            // array[-int]
+            (Expr::Array(a, pos), Expr::IntegerConstant(i, _))
+                if *i < 0 && i.checked_abs().map(|n| n as usize <= a.len()).unwrap_or(false) && a.iter().all(Expr::is_pure) =>
+            {
+                // Array literal where everything is pure - promote the indexed item.
+                // All other items can be thrown away.
+                state.set_dirty();
+                let mut result = a.remove(a.len() - i.abs() as usize);
+                result.set_position(*pos);
+                *expr = result;
+            }
             // map[string]
             (Expr::Map(m, pos), Expr::StringConstant(s, _)) if m.0.iter().all(|(_, x)| x.is_pure()) => {
                 // Map literal where everything is pure - promote the indexed item.
@@ -669,6 +680,12 @@ fn optimize_expr(expr: &mut Expr, state: &mut State) {
                 state.set_dirty();
                 *expr = Expr::CharConstant(s.chars().nth(*i as usize).unwrap(), *pos);
             }
+            // string[-int]
+            (Expr::StringConstant(s, pos), Expr::IntegerConstant(i, _)) if *i < 0 && i.checked_abs().map(|n| n as usize <= s.chars().count()).unwrap_or(false) => {
+                // String literal indexing - get the character
+                state.set_dirty();
+                *expr = Expr::CharConstant(s.chars().rev().nth(i.abs() as usize - 1).unwrap(), *pos);
+            }
             // var[rhs]
             (Expr::Variable(_, _, _), rhs) => optimize_expr(rhs, state),
             // lhs[rhs]

src/packages/array_basic.rs

@@ -34,7 +34,15 @@ mod array_functions {
     }
     pub fn insert(array: &mut Array, position: INT, item: Dynamic) {
         if position <= 0 {
-            array.insert(0, item);
+            if let Some(n) = position.checked_abs() {
+                if n as usize > array.len() {
+                    array.insert(0, item);
+                } else {
+                    array.insert(array.len() - n as usize, item);
+                }
+            } else {
+                array.insert(0, item);
+            }
         } else if (position as usize) >= array.len() {
             push(array, item);
         } else {
@@ -104,9 +112,13 @@ mod array_functions {
     }
     pub fn splice(array: &mut Array, start: INT, len: INT, replace: Array) {
         let start = if start < 0 {
-            0
+            let arr_len = array.len();
+            start
+                .checked_abs()
+                .map_or(0, |n| arr_len - (n as usize).min(arr_len))
         } else if start as usize >= array.len() {
-            array.len() - 1
+            array.extend(replace.into_iter());
+            return;
         } else {
             start as usize
         };
@@ -123,9 +135,12 @@ mod array_functions {
     }
     pub fn extract(array: &mut Array, start: INT, len: INT) -> Array {
         let start = if start < 0 {
-            0
+            let arr_len = array.len();
+            start
+                .checked_abs()
+                .map_or(0, |n| arr_len - (n as usize).min(arr_len))
         } else if start as usize >= array.len() {
-            array.len() - 1
+            return Default::default();
         } else {
             start as usize
         };
@@ -143,9 +158,12 @@ mod array_functions {
     #[rhai_fn(name = "extract")]
     pub fn extract_tail(array: &mut Array, start: INT) -> Array {
         let start = if start < 0 {
-            0
+            let arr_len = array.len();
+            start
+                .checked_abs()
+                .map_or(0, |n| arr_len - (n as usize).min(arr_len))
         } else if start as usize >= array.len() {
-            array.len() - 1
+            return Default::default();
         } else {
             start as usize
         };
@@ -155,7 +173,17 @@ mod array_functions {
     #[rhai_fn(name = "split")]
     pub fn split_at(array: &mut Array, start: INT) -> Array {
         if start <= 0 {
-            mem::take(array)
+            if let Some(n) = start.checked_abs() {
+                if n as usize > array.len() {
+                    mem::take(array)
+                } else {
+                    let mut result: Array = Default::default();
+                    result.extend(array.drain(array.len() - n as usize..));
+                    result
+                }
+            } else {
+                mem::take(array)
+            }
         } else if start as usize >= array.len() {
             Default::default()
         } else {
@@ -270,7 +298,27 @@ mod array_functions {
         array: &mut Array,
         value: Dynamic,
     ) -> Result<INT, Box<EvalAltResult>> {
-        for (i, item) in array.iter_mut().enumerate() {
+        index_of_starting_from(ctx, array, value, 0)
+    }
+    #[rhai_fn(name = "index_of", return_raw, pure)]
+    pub fn index_of_starting_from(
+        ctx: NativeCallContext,
+        array: &mut Array,
+        value: Dynamic,
+        start: INT,
+    ) -> Result<INT, Box<EvalAltResult>> {
+        let start = if start < 0 {
+            let arr_len = array.len();
+            start
+                .checked_abs()
+                .map_or(0, |n| arr_len - (n as usize).min(arr_len))
+        } else if start as usize >= array.len() {
+            return Ok(-1);
+        } else {
+            start as usize
+        };
+
+        for (i, item) in array.iter_mut().enumerate().skip(start) {
             if ctx
                 .call_fn_dynamic_raw(OP_EQUALS, true, &mut [item, &mut value.clone()])
                 .or_else(|err| match *err {
@@ -301,7 +349,27 @@ mod array_functions {
         array: &mut Array,
         filter: FnPtr,
     ) -> Result<INT, Box<EvalAltResult>> {
-        for (i, item) in array.iter().enumerate() {
+        index_of_filter_starting_from(ctx, array, filter, 0)
+    }
+    #[rhai_fn(name = "index_of", return_raw, pure)]
+    pub fn index_of_filter_starting_from(
+        ctx: NativeCallContext,
+        array: &mut Array,
+        filter: FnPtr,
+        start: INT,
+    ) -> Result<INT, Box<EvalAltResult>> {
+        let start = if start < 0 {
+            let arr_len = array.len();
+            start
+                .checked_abs()
+                .map_or(0, |n| arr_len - (n as usize).min(arr_len))
+        } else if start as usize >= array.len() {
+            return Ok(-1);
+        } else {
+            start as usize
+        };
+
+        for (i, item) in array.iter().enumerate().skip(start) {
             if filter
                 .call_dynamic(&ctx, None, [item.clone()])
                 .or_else(|err| match *err {
@@ -567,18 +635,17 @@ mod array_functions {
     ) -> Result<Array, Box<EvalAltResult>> {
         let mut drained = Array::with_capacity(array.len());
 
-        let mut i = array.len();
+        let mut i = 0;
-
+        let mut x = 0;
-        while i > 0 {
-            i -= 1;
 
+        while x < array.len() {
             if filter
-                .call_dynamic(&ctx, None, [array[i].clone()])
+                .call_dynamic(&ctx, None, [array[x].clone()])
                 .or_else(|err| match *err {
                     EvalAltResult::ErrorFunctionNotFound(fn_sig, _)
                         if fn_sig.starts_with(filter.fn_name()) =>
                     {
-                        filter.call_dynamic(&ctx, None, [array[i].clone(), (i as INT).into()])
+                        filter.call_dynamic(&ctx, None, [array[x].clone(), (i as INT).into()])
                     }
                     _ => Err(err),
                 })
@@ -593,8 +660,12 @@ mod array_functions {
                 .as_bool()
                 .unwrap_or(false)
             {
-                drained.push(array.remove(i));
+                drained.push(array.remove(x));
+            } else {
+                x += 1;
             }
+
+            i += 1;
         }
 
         Ok(drained)
@@ -602,9 +673,12 @@ mod array_functions {
     #[rhai_fn(name = "drain")]
     pub fn drain_range(array: &mut Array, start: INT, len: INT) -> Array {
         let start = if start < 0 {
-            0
+            let arr_len = array.len();
+            start
+                .checked_abs()
+                .map_or(0, |n| arr_len - (n as usize).min(arr_len))
         } else if start as usize >= array.len() {
-            array.len() - 1
+            return Default::default();
         } else {
             start as usize
         };
@@ -617,7 +691,7 @@ mod array_functions {
             len as usize
         };
 
-        array.drain(start..start + len - 1).collect()
+        array.drain(start..start + len).collect()
     }
     #[rhai_fn(return_raw)]
     pub fn retain(
@@ -627,18 +701,17 @@ mod array_functions {
     ) -> Result<Array, Box<EvalAltResult>> {
         let mut drained = Array::new();
 
-        let mut i = array.len();
+        let mut i = 0;
-
+        let mut x = 0;
-        while i > 0 {
-            i -= 1;
 
+        while x < array.len() {
             if !filter
-                .call_dynamic(&ctx, None, [array[i].clone()])
+                .call_dynamic(&ctx, None, [array[x].clone()])
                 .or_else(|err| match *err {
                     EvalAltResult::ErrorFunctionNotFound(fn_sig, _)
                         if fn_sig.starts_with(filter.fn_name()) =>
                     {
-                        filter.call_dynamic(&ctx, None, [array[i].clone(), (i as INT).into()])
+                        filter.call_dynamic(&ctx, None, [array[x].clone(), (i as INT).into()])
                     }
                     _ => Err(err),
                 })
@@ -653,8 +726,12 @@ mod array_functions {
                 .as_bool()
                 .unwrap_or(false)
             {
-                drained.push(array.remove(i));
+                drained.push(array.remove(x));
+            } else {
+                x += 1;
             }
+
+            i += 1;
         }
 
         Ok(drained)
@@ -662,9 +739,12 @@ mod array_functions {
     #[rhai_fn(name = "retain")]
     pub fn retain_range(array: &mut Array, start: INT, len: INT) -> Array {
         let start = if start < 0 {
-            0
+            let arr_len = array.len();
+            start
+                .checked_abs()
+                .map_or(0, |n| arr_len - (n as usize).min(arr_len))
         } else if start as usize >= array.len() {
-            array.len() - 1
+            return mem::take(array);
         } else {
             start as usize
         };
@@ -677,8 +757,8 @@ mod array_functions {
             len as usize
         };
 
-        let mut drained = array.drain(start + len..).collect::<Array>();
+        let mut drained = array.drain(..start).collect::<Array>();
-        drained.extend(array.drain(..start));
+        drained.extend(array.drain(len..));
 
         drained
     }

src/packages/string_more.rs

@@ -109,6 +109,22 @@ mod string_functions {
     #[rhai_fn(name = "index_of")]
     pub fn index_of_char_starting_from(string: &str, character: char, start: INT) -> INT {
         let start = if start < 0 {
+            if let Some(n) = start.checked_abs() {
+                let chars = string.chars().collect::<Vec<_>>();
+                let num_chars = chars.len();
+                if n as usize > num_chars {
+                    0
+                } else {
+                    chars
+                        .into_iter()
+                        .take(num_chars - n as usize)
+                        .collect::<String>()
+                        .len()
+                }
+            } else {
+                0
+            }
+        } else if start == 0 {
             0
         } else if start as usize >= string.chars().count() {
             return -1 as INT;
@@ -135,6 +151,22 @@ mod string_functions {
     #[rhai_fn(name = "index_of")]
     pub fn index_of_string_starting_from(string: &str, find_string: &str, start: INT) -> INT {
         let start = if start < 0 {
+            if let Some(n) = start.checked_abs() {
+                let chars = string.chars().collect::<Vec<_>>();
+                let num_chars = chars.len();
+                if n as usize > num_chars {
+                    0
+                } else {
+                    chars
+                        .into_iter()
+                        .take(num_chars - n as usize)
+                        .collect::<String>()
+                        .len()
+                }
+            } else {
+                0
+            }
+        } else if start == 0 {
             0
         } else if start as usize >= string.chars().count() {
             return -1 as INT;
@@ -165,17 +197,30 @@ mod string_functions {
         start: INT,
         len: INT,
     ) -> ImmutableString {
+        let mut chars = StaticVec::with_capacity(string.len());
+
         let offset = if string.is_empty() || len <= 0 {
             return ctx.engine().empty_string.clone().into();
         } else if start < 0 {
-            0
+            if let Some(n) = start.checked_abs() {
+                chars.extend(string.chars());
+                if n as usize > chars.len() {
+                    0
+                } else {
+                    chars.len() - n as usize
+                }
+            } else {
+                0
+            }
         } else if start as usize >= string.chars().count() {
             return ctx.engine().empty_string.clone().into();
         } else {
             start as usize
         };
 
-        let chars: StaticVec<_> = string.chars().collect();
+        if chars.is_empty() {
+            chars.extend(string.chars());
+        }
 
         let len = if offset + len as usize > chars.len() {
             chars.len() - offset
@@ -203,11 +248,22 @@ mod string_functions {
 
     #[rhai_fn(name = "crop")]
     pub fn crop(string: &mut ImmutableString, start: INT, len: INT) {
+        let mut chars = StaticVec::with_capacity(string.len());
+
         let offset = if string.is_empty() || len <= 0 {
             string.make_mut().clear();
             return;
         } else if start < 0 {
-            0
+            if let Some(n) = start.checked_abs() {
+                chars.extend(string.chars());
+                if n as usize > chars.len() {
+                    0
+                } else {
+                    chars.len() - n as usize
+                }
+            } else {
+                0
+            }
         } else if start as usize >= string.chars().count() {
             string.make_mut().clear();
             return;
@@ -215,7 +271,9 @@ mod string_functions {
             start as usize
         };
 
-        let chars: StaticVec<_> = string.chars().collect();
+        if chars.is_empty() {
+            chars.extend(string.chars());
+        }
 
         let len = if offset + len as usize > chars.len() {
             chars.len() - offset
@@ -374,7 +432,18 @@ mod string_functions {
         #[rhai_fn(name = "split")]
         pub fn split_at(ctx: NativeCallContext, string: ImmutableString, start: INT) -> Array {
             if start <= 0 {
-                vec![ctx.engine().empty_string.clone().into(), string.into()]
+                if let Some(n) = start.checked_abs() {
+                    let num_chars = string.chars().count();
+                    if n as usize > num_chars {
+                        vec![ctx.engine().empty_string.clone().into(), string.into()]
+                    } else {
+                        let prefix: String = string.chars().take(num_chars - n as usize).collect();
+                        let prefix_len = prefix.len();
+                        vec![prefix.into(), string[prefix_len..].into()]
+                    }
+                } else {
+                    vec![ctx.engine().empty_string.clone().into(), string.into()]
+                }
             } else {
                 let prefix: String = string.chars().take(start as usize).collect();
                 let prefix_len = prefix.len();

src/parser.rs

@@ -451,14 +451,6 @@ fn parse_index_chain(
 
     // Check type of indexing - must be integer or string
     match &idx_expr {
-        // lhs[int]
-        Expr::IntegerConstant(x, pos) if *x < 0 => {
-            return Err(PERR::MalformedIndexExpr(format!(
-                "Array access expects non-negative index: {} < 0",
-                *x
-            ))
-            .into_err(*pos))
-        }
         Expr::IntegerConstant(_, pos) => match lhs {
             Expr::Array(_, _) | Expr::StringConstant(_, _) | Expr::InterpolatedString(_) => (),
 

src/result.rs

@@ -104,14 +104,8 @@ impl EvalAltResult {
             Self::ErrorIndexingType(_, _) => {
                 "Indexing can only be performed on an array, an object map, a string, or a type with an indexer function defined"
             }
-            Self::ErrorArrayBounds(_, index, _) if *index < 0 => {
-                "Array access expects non-negative index"
-            }
             Self::ErrorArrayBounds(0, _, _) => "Empty array has nothing to access",
             Self::ErrorArrayBounds(_, _, _) => "Array index out of bounds",
-            Self::ErrorStringBounds(_, index, _) if *index < 0 => {
-                "Indexing a string expects a non-negative index"
-            }
             Self::ErrorStringBounds(0, _, _) => "Empty string has nothing to index",
             Self::ErrorStringBounds(_, _, _) => "String index out of bounds",
             Self::ErrorFor(_) => "For loop expects an array, object map, or range",
@@ -210,32 +204,30 @@ impl fmt::Display for EvalAltResult {
             Self::LoopBreak(_, _) => f.write_str(desc)?,
             Self::Return(_, _) => f.write_str(desc)?,
 
-            Self::ErrorArrayBounds(_, index, _) if *index < 0 => {
+            Self::ErrorArrayBounds(0, index, _) => {
-                write!(f, "{}: {} < 0", desc, index)?
+                write!(f, "Array index {} out of bounds: array is empty", index)?
             }
-            Self::ErrorArrayBounds(0, _, _) => f.write_str(desc)?,
             Self::ErrorArrayBounds(1, index, _) => write!(
                 f,
-                "Array index {} is out of bounds: only one element in the array",
+                "Array index {} out of bounds: only 1 element in the array",
                 index
             )?,
             Self::ErrorArrayBounds(max, index, _) => write!(
                 f,
-                "Array index {} is out of bounds: only {} elements in the array",
+                "Array index {} out of bounds: only {} elements in the array",
                 index, max
             )?,
-            Self::ErrorStringBounds(_, index, _) if *index < 0 => {
+            Self::ErrorStringBounds(0, index, _) => {
-                write!(f, "{}: {} < 0", desc, index)?
+                write!(f, "String index {} out of bounds: string is empty", index)?
             }
-            Self::ErrorStringBounds(0, _, _) => f.write_str(desc)?,
             Self::ErrorStringBounds(1, index, _) => write!(
                 f,
-                "String index {} is out of bounds: only one character in the string",
+                "String index {} out of bounds: only 1 character in the string",
                 index
             )?,
             Self::ErrorStringBounds(max, index, _) => write!(
                 f,
-                "String index {} is out of bounds: only {} characters in the string",
+                "String index {} out of bounds: only {} characters in the string",
                 index, max
             )?,
             Self::ErrorDataTooLarge(typ, _) => write!(f, "{} exceeds maximum limit", typ)?,

tests/arrays.rs

@@ -12,6 +12,8 @@ fn test_arrays() -> Result<(), Box<EvalAltResult>> {
         engine.eval::<char>(r#"let y = [1, [ 42, 88, "93" ], 3]; y[1][2][1]"#)?,
         '3'
     );
+    assert_eq!(engine.eval::<INT>("let y = [1, 2, 3]; y[0]")?, 1);
+    assert_eq!(engine.eval::<INT>("let y = [1, 2, 3]; y[-1]")?, 3);
     assert!(engine.eval::<bool>("let y = [1, 2, 3]; 2 in y")?);
     assert_eq!(engine.eval::<INT>("let y = [1, 2, 3]; y += 4; y[3]")?, 4);
 

tests/string.rs

@@ -46,7 +46,11 @@ fn test_string() -> Result<(), Box<EvalAltResult>> {
     assert_eq!(engine.eval::<String>("to_string(42)")?, "42");
 
     #[cfg(not(feature = "no_index"))]
-    assert_eq!(engine.eval::<char>(r#"let y = "hello"; y[1]"#)?, 'e');
+    {
+        assert_eq!(engine.eval::<char>(r#"let y = "hello"; y[1]"#)?, 'e');
+        assert_eq!(engine.eval::<char>(r#"let y = "hello"; y[-1]"#)?, 'o');
+        assert_eq!(engine.eval::<char>(r#"let y = "hello"; y[-4]"#)?, 'e');
+    }
 
     #[cfg(not(feature = "no_object"))]
     assert_eq!(engine.eval::<INT>(r#"let y = "hello"; y.len"#)?, 5);
@@ -109,9 +113,7 @@ fn test_string_substring() -> Result<(), Box<EvalAltResult>> {
     let engine = Engine::new();
 
     assert_eq!(
-        engine.eval::<String>(
+        engine.eval::<String>(r#"let x = "hello! \u2764\u2764\u2764"; x.sub_string(-2, 2)"#)?,
-            r#"let x = "\u2764\u2764\u2764 hello! \u2764\u2764\u2764"; x.sub_string(-1, 2)"#
-        )?,
         "❤❤"
     );
 
@@ -200,9 +202,9 @@ fn test_string_substring() -> Result<(), Box<EvalAltResult>> {
 
     assert_eq!(
         engine.eval::<INT>(
-            r#"let x = "\u2764\u2764\u2764 hello! \u2764\u2764\u2764"; x.index_of('\u2764', -1)"#
+            r#"let x = "\u2764\u2764\u2764 hello! \u2764\u2764\u2764"; x.index_of('\u2764', -6)"#
         )?,
-        0
+        11
     );
 
     assert_eq!(