diff --git a/CHANGELOG.md b/CHANGELOG.md index a09f3801..f5d2e8ca 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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. diff --git a/src/api/options.rs b/src/api/options.rs index 4fe6a4d7..230df25c 100644 --- a/src/api/options.rs +++ b/src/api/options.rs @@ -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; + } } diff --git a/src/eval/chaining.rs b/src/eval/chaining.rs index 898ab201..a9b05825 100644 --- a/src/eval/chaining.rs +++ b/src/eval/chaining.rs @@ -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!( diff --git a/src/packages/debugging.rs b/src/packages/debugging.rs index ab24bc3a..a476604c 100644 --- a/src/packages/debugging.rs +++ b/src/packages/debugging.rs @@ -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; diff --git a/src/types/error.rs b/src/types/error.rs index 32d3195b..58236d7f 100644 --- a/src/types/error.rs +++ b/src/types/error.rs @@ -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) diff --git a/src/types/parse_error.rs b/src/types/parse_error.rs index 4ae403ba..8f129676 100644 --- a/src/types/parse_error.rs +++ b/src/types/parse_error.rs @@ -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, diff --git a/tests/maps.rs b/tests/maps.rs index 5dceecb4..36ff7e84 100644 --- a/tests/maps.rs +++ b/tests/maps.rs @@ -107,6 +107,23 @@ b`: 1}; y["a\nb"] Ok(()) } +#[test] +fn test_map_prop() -> Result<(), Box> { + 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> { let engine = Engine::new();