Merge pull request #393 from schungx/master

@schungx Fix panic with unterminated interpolated string.
This commit is contained in:
Stephen Chung 2021-04-10 17:57:29 +08:00 committed by GitHub
commit b658ca0b4b
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
10 changed files with 288 additions and 83 deletions

View File

@ -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

View File

@ -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::<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)
})?;
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"))]

View File

@ -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]

View File

@ -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();
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<Array, Box<EvalAltResult>> {
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::<Array>();
drained.extend(array.drain(..start));
let mut drained = array.drain(..start).collect::<Array>();
drained.extend(array.drain(len..));
drained
}

View File

@ -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();

View File

@ -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

View File

@ -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)?,

View File

@ -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 ")`<br/>next token is `{`|`None` |
/// |`` } hello` `` |`StringConstant(" hello")` |``Some('`')`` |
pub fn parse_string_literal(
stream: &mut impl InputStream,
state: &mut TokenizeState,

View File

@ -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);

View File

@ -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>(
r#"let x = "\u2764\u2764\u2764 hello! \u2764\u2764\u2764"; x.sub_string(-1, 2)"#
)?,
engine.eval::<String>(r#"let x = "hello! \u2764\u2764\u2764"; x.sub_string(-2, 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!(