Add fail on invalid property for maps.

This commit is contained in:
Stephen Chung 2022-02-09 13:12:43 +08:00
parent 6acc486e00
commit 340a047369
7 changed files with 175 additions and 130 deletions

View File

@ -6,7 +6,11 @@ Version 1.5.0
This version adds a debugging interface, which can be used to integrate a debugger.
The `REPL` tool uses [`rustyline`](https://crates.io/crates/rustyline) for line editing and history.
Based on popular demand, an option is added to throw exceptions when invalid properties are accessed
on object maps (default is to return `()`).
Also based on popular demand, the `REPL` tool now uses
[`rustyline`](https://crates.io/crates/rustyline) for line editing and history.
Bug fixes
---------
@ -17,6 +21,7 @@ Bug fixes
* Globally-defined constants are now encapsulated correctly inside a loaded module and no longer spill across call boundaries.
* Type names display is fixed.
* Exceptions thrown inside function calls now unwrap correctly when `catch`-ed.
* Error messages for certain invalid property accesses are fixed.
Script-breaking changes
-----------------------
@ -29,6 +34,7 @@ New features
* A debugging interface is added.
* A new bin tool, `rhai-dbg` (aka _The Rhai Debugger_), is added to showcase the debugging interface.
* A new package, `DebuggingPackage`, is added which contains the `back_trace` function to get the current call stack anywhere in a script.
* `Engine::set_fail_on_invalid_map_property` is added to control whether to raise an error (new `EvalAltResult::ErrorPropertyNotFound`) when invalid properties are accessed on object maps.
* `Engine::set_allow_shadowing` is added to allow/disallow variables _shadowing_, with new errors `EvalAltResult::ErrorVariableExists` and `ParseErrorType::VariableExists`.
* `Engine::on_def_var` allows registering a closure which can decide whether a variable definition is allow to continue, or should fail with an error.

View File

@ -18,10 +18,14 @@ pub struct LanguageOptions {
pub allow_anonymous_fn: bool,
/// Is looping allowed?
pub allow_looping: bool,
/// Strict variables mode?
pub strict_var: bool,
/// Is variables shadowing allowed?
pub allow_shadowing: bool,
/// Strict variables mode?
pub strict_var: bool,
/// Raise error if an object map property does not exist?
/// Returns `()` if `false`.
#[cfg(not(feature = "no_object"))]
pub fail_on_invalid_map_property: bool,
}
impl LanguageOptions {
@ -37,6 +41,8 @@ impl LanguageOptions {
allow_looping: true,
strict_var: false,
allow_shadowing: true,
#[cfg(not(feature = "no_object"))]
fail_on_invalid_map_property: false,
}
}
}
@ -49,6 +55,7 @@ impl Default for LanguageOptions {
impl Engine {
/// Is `if`-expression allowed?
/// Default is `true`.
#[inline(always)]
pub fn allow_if_expression(&self) -> bool {
self.options.allow_if_expr
@ -59,6 +66,7 @@ impl Engine {
self.options.allow_if_expr = enable;
}
/// Is `switch` expression allowed?
/// Default is `true`.
#[inline(always)]
pub fn allow_switch_expression(&self) -> bool {
self.options.allow_switch_expr
@ -69,6 +77,7 @@ impl Engine {
self.options.allow_switch_expr = enable;
}
/// Is statement-expression allowed?
/// Default is `true`.
#[inline(always)]
pub fn allow_statement_expression(&self) -> bool {
self.options.allow_stmt_expr
@ -79,6 +88,7 @@ impl Engine {
self.options.allow_stmt_expr = enable;
}
/// Is anonymous function allowed?
/// Default is `true`.
///
/// Not available under `no_function`.
#[cfg(not(feature = "no_function"))]
@ -95,6 +105,7 @@ impl Engine {
self.options.allow_anonymous_fn = enable;
}
/// Is looping allowed?
/// Default is `true`.
#[inline(always)]
pub fn allow_looping(&self) -> bool {
self.options.allow_looping
@ -104,17 +115,8 @@ impl Engine {
pub fn set_allow_looping(&mut self, enable: bool) {
self.options.allow_looping = enable;
}
/// Is strict variables mode enabled?
#[inline(always)]
pub fn strict_variables(&self) -> bool {
self.options.strict_var
}
/// Set whether strict variables mode is enabled.
#[inline(always)]
pub fn set_strict_variables(&mut self, enable: bool) {
self.options.strict_var = enable;
}
/// Is variables shadowing allowed?
/// Default is `true`.
#[inline(always)]
pub fn allow_shadowing(&self) -> bool {
self.options.allow_shadowing
@ -124,4 +126,32 @@ impl Engine {
pub fn set_allow_shadowing(&mut self, enable: bool) {
self.options.allow_shadowing = enable;
}
/// Is strict variables mode enabled?
/// Default is `false`.
#[inline(always)]
pub fn strict_variables(&self) -> bool {
self.options.strict_var
}
/// Set whether strict variables mode is enabled.
#[inline(always)]
pub fn set_strict_variables(&mut self, enable: bool) {
self.options.strict_var = enable;
}
/// Raise error if an object map property does not exist?
/// Default is `false`.
///
/// Not available under `no_object`.
#[cfg(not(feature = "no_object"))]
#[inline(always)]
pub fn fail_on_invalid_map_property(&self) -> bool {
self.options.fail_on_invalid_map_property
}
/// Set whether to raise error if an object map property does not exist.
///
/// Not available under `no_object`.
#[cfg(not(feature = "no_object"))]
#[inline(always)]
pub fn set_fail_on_invalid_map_property(&mut self, enable: bool) {
self.options.fail_on_invalid_map_property = enable;
}
}

View File

@ -4,9 +4,7 @@
use super::{EvalState, GlobalRuntimeState, Target};
use crate::ast::{Expr, OpAssignment};
use crate::types::dynamic::Union;
use crate::{
Dynamic, Engine, Module, Position, RhaiError, RhaiResult, RhaiResultOf, Scope, StaticVec, ERR,
};
use crate::{Dynamic, Engine, Module, Position, RhaiResult, RhaiResultOf, Scope, StaticVec, ERR};
use std::hash::Hash;
#[cfg(feature = "no_std")]
use std::prelude::v1::*;
@ -185,20 +183,15 @@ impl Engine {
if let Some(mut new_val) = try_setter {
// Try to call index setter if value is changed
let hash_set =
crate::ast::FnCallHashes::from_native(global.hash_idx_set());
let args = &mut [target, &mut idx_val_for_setter, &mut new_val];
let fn_name = crate::engine::FN_IDX_SET;
if let Err(err) = self.exec_fn_call(
None, global, state, lib, fn_name, hash_set, args, is_ref_mut,
true, root_pos, level,
) {
// Just ignore if there is no index setter
if !matches!(*err, ERR::ErrorFunctionNotFound(..)) {
return Err(err);
}
}
let idx = &mut idx_val_for_setter;
let new_val = &mut new_val;
self.call_indexer_set(
global, state, lib, target, idx, new_val, is_ref_mut, level,
)
.or_else(|idx_err| match *idx_err {
ERR::ErrorIndexingType(..) => Ok((Dynamic::UNIT, false)),
_ => Err(idx_err.fill_position(root_pos)),
})?;
}
Ok(result)
@ -234,15 +227,12 @@ impl Engine {
if let Some(mut new_val) = try_setter {
// Try to call index setter
let hash_set =
crate::ast::FnCallHashes::from_native(global.hash_idx_set());
let args = &mut [target, &mut idx_val_for_setter, &mut new_val];
let fn_name = crate::engine::FN_IDX_SET;
self.exec_fn_call(
None, global, state, lib, fn_name, hash_set, args, is_ref_mut,
true, root_pos, level,
)?;
let idx = &mut idx_val_for_setter;
let new_val = &mut new_val;
self.call_indexer_set(
global, state, lib, target, idx, new_val, is_ref_mut, level,
)
.map_err(|err| err.fill_position(root_pos))?;
}
Ok((Dynamic::UNIT, true))
@ -341,12 +331,10 @@ impl Engine {
.or_else(|err| match *err {
// Try an indexer if property does not exist
ERR::ErrorDotExpr(..) => {
let prop = name.into();
self.get_indexed_mut(
global, state, lib, target, prop, *pos, false, true,
level,
let mut prop = name.into();
self.call_indexer_get(
global, state, lib, target, &mut prop, level,
)
.map(|v| (v.take_or_clone(), false))
.map_err(
|idx_err| match *idx_err {
ERR::ErrorIndexingType(..) => err,
@ -382,15 +370,10 @@ impl Engine {
.or_else(|err| match *err {
// Try an indexer if property does not exist
ERR::ErrorDotExpr(..) => {
let args = &mut [target, &mut name.into(), &mut new_val];
let fn_name = crate::engine::FN_IDX_SET;
let hash_set =
crate::ast::FnCallHashes::from_native(global.hash_idx_set());
let pos = Position::NONE;
self.exec_fn_call(
None, global, state, lib, fn_name, hash_set, args, is_ref_mut,
true, pos, level,
let idx = &mut name.into();
let new_val = &mut new_val;
self.call_indexer_set(
global, state, lib, target, idx, new_val, is_ref_mut, level,
)
.map_err(
|idx_err| match *idx_err {
@ -418,11 +401,10 @@ impl Engine {
|err| match *err {
// Try an indexer if property does not exist
ERR::ErrorDotExpr(..) => {
let prop = name.into();
self.get_indexed_mut(
global, state, lib, target, prop, *pos, false, true, level,
let mut prop = name.into();
self.call_indexer_get(
global, state, lib, target, &mut prop, level,
)
.map(|v| (v.take_or_clone(), false))
.map_err(|idx_err| {
match *idx_err {
ERR::ErrorIndexingType(..) => err,
@ -517,16 +499,14 @@ impl Engine {
.or_else(|err| match *err {
// Try an indexer if property does not exist
ERR::ErrorDotExpr(..) => {
let prop = name.into();
self.get_indexed_mut(
global, state, lib, target, prop, pos, false, true,
level,
let mut prop = name.into();
self.call_indexer_get(
global, state, lib, target, &mut prop, level,
)
.map(|v| (v.take_or_clone(), false))
.map_err(
|idx_err| match *idx_err {
ERR::ErrorIndexingType(..) => err,
_ => idx_err,
_ => idx_err.fill_position(pos),
},
)
}
@ -566,16 +546,11 @@ impl Engine {
|err| match *err {
// Try an indexer if property does not exist
ERR::ErrorDotExpr(..) => {
let args =
&mut [target.as_mut(), &mut name.into(), val];
let fn_name = crate::engine::FN_IDX_SET;
let hash_set =
crate::ast::FnCallHashes::from_native(
global.hash_idx_set(),
);
self.exec_fn_call(
None, global, state, lib, fn_name, hash_set,
args, is_ref_mut, true, pos, level,
let idx = &mut name.into();
let new_val = val;
self.call_indexer_set(
global, state, lib, target, idx, new_val,
is_ref_mut, level,
)
.or_else(|idx_err| match *idx_err {
ERR::ErrorIndexingType(..) => {
@ -583,7 +558,7 @@ impl Engine {
// the property is read-only
Ok((Dynamic::UNIT, false))
}
_ => Err(idx_err),
_ => Err(idx_err.fill_position(pos)),
})
}
_ => Err(err),
@ -743,7 +718,7 @@ impl Engine {
pos = arg_pos;
}
values.push(value.flatten());
Ok::<_, RhaiError>((values, pos))
Ok::<_, crate::RhaiError>((values, pos))
},
)?;
@ -789,7 +764,7 @@ impl Engine {
pos = arg_pos
}
values.push(value.flatten());
Ok::<_, RhaiError>((values, pos))
Ok::<_, crate::RhaiError>((values, pos))
},
)?;
super::ChainArgument::from_fn_call_args(values, pos)
@ -842,6 +817,52 @@ impl Engine {
Ok(())
}
/// Call a get indexer.
/// [`Position`] in [`EvalAltResult`] may be [`NONE`][Position::NONE] and should be set afterwards.
#[inline(always)]
fn call_indexer_get(
&self,
global: &mut GlobalRuntimeState,
state: &mut EvalState,
lib: &[&Module],
target: &mut Dynamic,
idx: &mut Dynamic,
level: usize,
) -> RhaiResultOf<(Dynamic, bool)> {
let args = &mut [target, idx];
let hash_get = crate::ast::FnCallHashes::from_native(global.hash_idx_get());
let fn_name = crate::engine::FN_IDX_GET;
let pos = Position::NONE;
self.exec_fn_call(
None, global, state, lib, fn_name, hash_get, args, true, true, pos, level,
)
}
/// Call a set indexer.
/// [`Position`] in [`EvalAltResult`] may be [`NONE`][Position::NONE] and should be set afterwards.
#[inline(always)]
fn call_indexer_set(
&self,
global: &mut GlobalRuntimeState,
state: &mut EvalState,
lib: &[&Module],
target: &mut Dynamic,
idx: &mut Dynamic,
new_val: &mut Dynamic,
is_ref_mut: bool,
level: usize,
) -> RhaiResultOf<(Dynamic, bool)> {
let hash_set = crate::ast::FnCallHashes::from_native(global.hash_idx_set());
let args = &mut [target, idx, new_val];
let fn_name = crate::engine::FN_IDX_SET;
let pos = Position::NONE;
self.exec_fn_call(
None, global, state, lib, fn_name, hash_set, args, is_ref_mut, true, pos, level,
)
}
/// Get the value at the indexed position of a base type.
/// [`Position`] in [`EvalAltResult`] may be [`NONE`][Position::NONE] and should be set afterwards.
fn get_indexed_mut<'t>(
@ -908,10 +929,13 @@ impl Engine {
map.insert(index.clone().into(), Dynamic::UNIT);
}
Ok(map
.get_mut(index.as_str())
.map(Target::from)
.unwrap_or_else(|| Target::from(Dynamic::UNIT)))
if let Some(value) = map.get_mut(index.as_str()) {
Ok(Target::from(value))
} else if self.fail_on_invalid_map_property() {
Err(ERR::ErrorPropertyNotFound(index.to_string(), pos).into())
} else {
Ok(Target::from(Dynamic::UNIT))
}
}
#[cfg(not(feature = "no_index"))]
@ -1045,17 +1069,9 @@ impl Engine {
})
}
_ if use_indexers => {
let args = &mut [target, &mut idx];
let hash_get = crate::ast::FnCallHashes::from_native(global.hash_idx_get());
let fn_name = crate::engine::FN_IDX_GET;
let pos = Position::NONE;
self.exec_fn_call(
None, global, state, lib, fn_name, hash_get, args, true, true, pos, level,
)
.map(|(v, ..)| v.into())
}
_ if use_indexers => self
.call_indexer_get(global, state, lib, target, &mut idx, level)
.map(|(v, ..)| v.into()),
_ => Err(ERR::ErrorIndexingType(
format!(

View File

@ -6,13 +6,11 @@ use crate::plugin::*;
use std::prelude::v1::*;
#[cfg(not(feature = "no_function"))]
use crate::{Dynamic, NativeCallContext};
#[cfg(not(feature = "no_index"))]
use crate::{Array, Dynamic, NativeCallContext};
#[cfg(not(feature = "no_function"))]
#[cfg(not(feature = "no_index"))]
use crate::Array;
#[cfg(not(feature = "no_function"))]
#[cfg(not(feature = "no_object"))]
use crate::Map;

View File

@ -13,6 +13,10 @@ use std::prelude::v1::*;
///
/// All wrapped [`Position`] values represent the location in the script where the error occurs.
///
/// Some errors never appear when certain features are turned on.
/// They still exist so that the application can turn features on and off without going through
/// massive code changes to remove/add back enum variants in match statements.
///
/// # Thread Safety
///
/// Currently, [`EvalAltResult`] is neither [`Send`] nor [`Sync`].
@ -32,8 +36,10 @@ pub enum EvalAltResult {
/// Shadowing of an existing variable disallowed. Wrapped value is the variable name.
ErrorVariableExists(String, Position),
/// Usage of an unknown variable. Wrapped value is the variable name.
/// Access of an unknown variable. Wrapped value is the variable name.
ErrorVariableNotFound(String, Position),
/// Access of an unknown object map property. Wrapped value is the property name.
ErrorPropertyNotFound(String, Position),
/// Call to an unknown function. Wrapped value is the function signature.
ErrorFunctionNotFound(String, Position),
/// Usage of an unknown [module][crate::Module]. Wrapped value is the [module][crate::Module] name.
@ -143,6 +149,7 @@ impl fmt::Display for EvalAltResult {
Self::ErrorVariableExists(s, ..) => write!(f, "Variable is already defined: {}", s)?,
Self::ErrorVariableNotFound(s, ..) => write!(f, "Variable not found: {}", s)?,
Self::ErrorPropertyNotFound(s, ..) => write!(f, "Property not found: {}", s)?,
Self::ErrorFunctionNotFound(s, ..) => write!(f, "Function not found: {}", s)?,
Self::ErrorModuleNotFound(s, ..) => write!(f, "Module not found: {}", s)?,
Self::ErrorDataRace(s, ..) => {
@ -279,6 +286,7 @@ impl EvalAltResult {
| Self::ErrorFor(..)
| Self::ErrorVariableExists(..)
| Self::ErrorVariableNotFound(..)
| Self::ErrorPropertyNotFound(..)
| Self::ErrorModuleNotFound(..)
| Self::ErrorDataRace(..)
| Self::ErrorAssignmentToConstant(..)
@ -370,6 +378,7 @@ impl EvalAltResult {
}
Self::ErrorVariableExists(v, ..)
| Self::ErrorVariableNotFound(v, ..)
| Self::ErrorPropertyNotFound(v, ..)
| Self::ErrorDataRace(v, ..)
| Self::ErrorAssignmentToConstant(v, ..) => {
map.insert("variable".into(), v.into());
@ -432,6 +441,7 @@ impl EvalAltResult {
| Self::ErrorFor(pos)
| Self::ErrorVariableExists(.., pos)
| Self::ErrorVariableNotFound(.., pos)
| Self::ErrorPropertyNotFound(.., pos)
| Self::ErrorModuleNotFound(.., pos)
| Self::ErrorDataRace(.., pos)
| Self::ErrorAssignmentToConstant(.., pos)
@ -481,6 +491,7 @@ impl EvalAltResult {
| Self::ErrorFor(pos)
| Self::ErrorVariableExists(.., pos)
| Self::ErrorVariableNotFound(.., pos)
| Self::ErrorPropertyNotFound(.., pos)
| Self::ErrorModuleNotFound(.., pos)
| Self::ErrorDataRace(.., pos)
| Self::ErrorAssignmentToConstant(.., pos)

View File

@ -89,21 +89,12 @@ pub enum ParseErrorType {
MalformedCallExpr(String),
/// An expression in indexing brackets `[]` has syntax error. Wrapped value is the error
/// description (if any).
///
/// Never appears under the `no_index` feature.
MalformedIndexExpr(String),
/// An expression in an `in` expression has syntax error. Wrapped value is the error description
/// (if any).
///
/// Never appears under the `no_object` and `no_index` features combination.
/// An expression in an `in` expression has syntax error. Wrapped value is the error description (if any).
MalformedInExpr(String),
/// A capturing has syntax error. Wrapped value is the error description (if any).
///
/// Never appears under the `no_closure` feature.
MalformedCapture(String),
/// A map definition has duplicated property names. Wrapped value is the property name.
///
/// Never appears under the `no_object` feature.
DuplicatedProperty(String),
/// A `switch` case is duplicated.
DuplicatedSwitchCase,
@ -116,8 +107,6 @@ pub enum ParseErrorType {
/// The case condition of a `switch` statement is not appropriate.
WrongSwitchCaseCondition,
/// Missing a property name for custom types and maps.
///
/// Never appears under the `no_object` feature.
PropertyExpected,
/// Missing a variable name after the `let`, `const`, `for` or `catch` keywords.
VariableExpected,
@ -129,38 +118,22 @@ pub enum ParseErrorType {
/// Missing an expression. Wrapped value is the expression type.
ExprExpected(String),
/// Defining a doc-comment in an appropriate place (e.g. not at global level).
///
/// Never appears under the `no_function` feature.
WrongDocComment,
/// Defining a function `fn` in an appropriate place (e.g. inside another function).
///
/// Never appears under the `no_function` feature.
WrongFnDefinition,
/// Defining a function with a name that conflicts with an existing function.
/// Wrapped values are the function name and number of parameters.
///
/// Never appears under the `no_object` feature.
FnDuplicatedDefinition(String, usize),
/// Missing a function name after the `fn` keyword.
///
/// Never appears under the `no_function` feature.
FnMissingName,
/// A function definition is missing the parameters list. Wrapped value is the function name.
///
/// Never appears under the `no_function` feature.
FnMissingParams(String),
/// A function definition has duplicated parameters. Wrapped values are the function name and
/// parameter name.
///
/// Never appears under the `no_function` feature.
FnDuplicatedParam(String, String),
/// A function definition is missing the body. Wrapped value is the function name.
///
/// Never appears under the `no_function` feature.
FnMissingBody(String),
/// Export statement not at global level.
///
/// Never appears under the `no_module` feature.
WrongExport,
/// Assignment to an a constant variable. Wrapped value is the constant variable name.
AssignmentToConstant(String),
@ -178,16 +151,10 @@ pub enum ParseErrorType {
/// An imported module is not found.
///
/// Only appears when strict variables mode is enabled.
///
/// Never appears under the `no_module` feature.
ModuleUndefined(String),
/// Expression exceeding the maximum levels of complexity.
///
/// Never appears under the `unchecked` feature.
ExprTooDeep,
/// Literal exceeding the maximum size. Wrapped values are the data type name and the maximum size.
///
/// Never appears under the `unchecked` feature.
LiteralTooLarge(String, usize),
/// Break statement not inside a loop.
LoopBreak,

View File

@ -107,6 +107,23 @@ b`: 1}; y["a\nb"]
Ok(())
}
#[test]
fn test_map_prop() -> Result<(), Box<EvalAltResult>> {
let mut engine = Engine::new();
assert_eq!(engine.eval::<()>("let x = #{a: 42}; x.b")?, ());
engine.set_fail_on_invalid_map_property(true);
assert!(
matches!(*engine.eval::<()>("let x = #{a: 42}; x.b").expect_err("should error"),
EvalAltResult::ErrorPropertyNotFound(prop, _) if prop == "b"
)
);
Ok(())
}
#[test]
fn test_map_assign() -> Result<(), Box<EvalAltResult>> {
let engine = Engine::new();