diff --git a/CHANGELOG.md b/CHANGELOG.md index 2ebebf9d..fc71773b 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/src/engine.rs b/src/engine.rs index ff549a1c..3c2775d2 100644 --- a/src/engine.rs +++ b/src/engine.rs @@ -1598,15 +1598,29 @@ impl Engine { let arr_len = arr.len(); - if index >= 0 { - arr.get_mut(index as usize) - .map(Target::from) - .ok_or_else(|| { - EvalAltResult::ErrorArrayBounds(arr_len, index, idx_pos).into() - }) + let arr_idx = if index < 0 { + // Count from end if negative + arr_len + - index + .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::(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) - })?; - Ok(Target::StringChar(target, offset, ch.into())) + ( + s.chars().nth(offset).ok_or_else(|| { + EvalAltResult::ErrorStringBounds(chars_len, index, idx_pos) + })?, + 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"))] diff --git a/src/optimize.rs b/src/optimize.rs index 7e4bcb88..49a58a1c 100644 --- a/src/optimize.rs +++ b/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] diff --git a/src/packages/array_basic.rs b/src/packages/array_basic.rs index 2708305a..1703194e 100644 --- a/src/packages/array_basic.rs +++ b/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> { - 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> { + 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> { - 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> { + 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> { let mut drained = Array::with_capacity(array.len()); - let mut i = array.len(); - - while i > 0 { - i -= 1; + let mut i = 0; + let mut x = 0; + 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> { let mut drained = Array::new(); - let mut i = array.len(); - - while i > 0 { - i -= 1; + let mut i = 0; + let mut x = 0; + 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::(); - drained.extend(array.drain(..start)); + let mut drained = array.drain(..start).collect::(); + drained.extend(array.drain(len..)); drained } diff --git a/src/packages/string_more.rs b/src/packages/string_more.rs index 109549fd..3efd6dd2 100644 --- a/src/packages/string_more.rs +++ b/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::>(); + let num_chars = chars.len(); + if n as usize > num_chars { + 0 + } else { + chars + .into_iter() + .take(num_chars - n as usize) + .collect::() + .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::>(); + let num_chars = chars.len(); + if n as usize > num_chars { + 0 + } else { + chars + .into_iter() + .take(num_chars - n as usize) + .collect::() + .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(); diff --git a/src/parser.rs b/src/parser.rs index 9bac4053..18de1c75 100644 --- a/src/parser.rs +++ b/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(_) => (), @@ -1047,6 +1039,9 @@ fn parse_primary( segments.push(Expr::StringConstant(s.into(), pos)); } } + (Token::LexError(err @ LexError::UnterminatedString), pos) => { + return Err(err.into_err(pos)) + } (token, _) => unreachable!( "expected a string within an interpolated string literal, but gets {:?}", token diff --git a/src/result.rs b/src/result.rs index 0e008cad..4652aaa1 100644 --- a/src/result.rs +++ b/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 => { - write!(f, "{}: {} < 0", desc, index)? + Self::ErrorArrayBounds(0, 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 => { - write!(f, "{}: {} < 0", desc, index)? + Self::ErrorStringBounds(0, 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)?, diff --git a/src/token.rs b/src/token.rs index 4d7bbdb1..197ed303 100644 --- a/src/token.rs +++ b/src/token.rs @@ -871,6 +871,18 @@ pub trait InputStream { /// # Volatile API /// /// This function is volatile and may change. +/// +/// # Returns +/// +/// |Type |Return Value |`state.is_within_text_terminated_by`| +/// |---------------------------------|----------------------------|------------------------------------| +/// |`"hello"` |`StringConstant("hello")` |`None` | +/// |`"hello`_{LF}_ or _{EOF}_ |`LexError` |`None` | +/// |`"hello\`_{EOF}_ or _{LF}{EOF}_ |`StringConstant("hello")` |`Some('"')` | +/// |`` `hello``_{EOF}_ |`StringConstant("hello")` |``Some('`')`` | +/// |`` `hello``_{LF}{EOF}_ |`StringConstant("hello\n")` |``Some('`')`` | +/// |`` `hello ${`` |`InterpolatedString("hello ")`
next token is `{`|`None` | +/// |`` } hello` `` |`StringConstant(" hello")` |``Some('`')`` | pub fn parse_string_literal( stream: &mut impl InputStream, state: &mut TokenizeState, diff --git a/tests/arrays.rs b/tests/arrays.rs index 0c154cd6..0d080557 100644 --- a/tests/arrays.rs +++ b/tests/arrays.rs @@ -12,6 +12,8 @@ fn test_arrays() -> Result<(), Box> { engine.eval::(r#"let y = [1, [ 42, 88, "93" ], 3]; y[1][2][1]"#)?, '3' ); + assert_eq!(engine.eval::("let y = [1, 2, 3]; y[0]")?, 1); + assert_eq!(engine.eval::("let y = [1, 2, 3]; y[-1]")?, 3); assert!(engine.eval::("let y = [1, 2, 3]; 2 in y")?); assert_eq!(engine.eval::("let y = [1, 2, 3]; y += 4; y[3]")?, 4); diff --git a/tests/string.rs b/tests/string.rs index abcea364..99e51606 100644 --- a/tests/string.rs +++ b/tests/string.rs @@ -46,7 +46,11 @@ fn test_string() -> Result<(), Box> { assert_eq!(engine.eval::("to_string(42)")?, "42"); #[cfg(not(feature = "no_index"))] - assert_eq!(engine.eval::(r#"let y = "hello"; y[1]"#)?, 'e'); + { + assert_eq!(engine.eval::(r#"let y = "hello"; y[1]"#)?, 'e'); + assert_eq!(engine.eval::(r#"let y = "hello"; y[-1]"#)?, 'o'); + assert_eq!(engine.eval::(r#"let y = "hello"; y[-4]"#)?, 'e'); + } #[cfg(not(feature = "no_object"))] assert_eq!(engine.eval::(r#"let y = "hello"; y.len"#)?, 5); @@ -109,9 +113,7 @@ fn test_string_substring() -> Result<(), Box> { let engine = Engine::new(); assert_eq!( - engine.eval::( - r#"let x = "\u2764\u2764\u2764 hello! \u2764\u2764\u2764"; x.sub_string(-1, 2)"# - )?, + engine.eval::(r#"let x = "hello! \u2764\u2764\u2764"; x.sub_string(-2, 2)"#)?, "❤❤" ); @@ -200,9 +202,9 @@ fn test_string_substring() -> Result<(), Box> { assert_eq!( engine.eval::( - 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!(