diff --git a/CHANGELOG.md b/CHANGELOG.md index 43efe92b..17c0f8fa 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -38,7 +38,7 @@ New features * 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. +* `Engine::on_def_var` allows registering a closure which can decide whether a variable definition is allow to continue, during compilation or runtime, or should fail with an error (`ParseErrorType::ForbiddenVariable` or `EvalAltResult::ErrorForbiddenVariable`). * A new syntax for defining custom packages is introduced that removes the need to specify the Rhai crate name (internally uses the `$crate` meta variable). Enhancements diff --git a/src/api/events.rs b/src/api/events.rs index c927f358..134d9486 100644 --- a/src/api/events.rs +++ b/src/api/events.rs @@ -73,8 +73,9 @@ impl Engine { /// /// where: /// * `name`: name of the variable to be defined. + /// * `is_runtime`: `true` if the variable definition event happens during runtime, `false` if during compilation. /// * `is_const`: `true` if the statement is `const`, otherwise it is `let`. - /// * `block_level`: the current nesting level of statement blocks, with zero being the global level + /// * `block_level`: the current nesting level of statement blocks, with zero being the global level. /// * `will_shadow`: will the variable _shadow_ an existing variable? /// * `context`: the current [evaluation context][`EvalContext`]. /// @@ -96,7 +97,7 @@ impl Engine { /// let mut engine = Engine::new(); /// /// // Register a variable definition filter. - /// engine.on_def_var(|name, is_const, _, _, _| { + /// engine.on_def_var(|name, _, is_const, _, _, _| { /// // Disallow defining MYSTIC_NUMBER as a constant /// if name == "MYSTIC_NUMBER" && is_const { /// Ok(false) @@ -117,7 +118,7 @@ impl Engine { #[inline(always)] pub fn on_def_var( &mut self, - callback: impl Fn(&str, bool, usize, bool, &EvalContext) -> RhaiResultOf + callback: impl Fn(&str, bool, bool, usize, bool, &EvalContext) -> RhaiResultOf + SendSync + 'static, ) -> &mut Self { diff --git a/src/eval/stmt.rs b/src/eval/stmt.rs index 5aa179eb..3e04d3e8 100644 --- a/src/eval/stmt.rs +++ b/src/eval/stmt.rs @@ -831,13 +831,13 @@ impl Engine { level: level, }; - match filter(var_name, is_const, scope_level, shadowing, &context) { + match filter(var_name, true, is_const, scope_level, shadowing, &context) { Ok(true) => None, - Ok(false) => Some(Err(ERR::ErrorRuntime( - format!("Variable cannot be defined: {}", var_name).into(), - *pos, - ) - .into())), + Ok(false) => { + Some(Err( + ERR::ErrorForbiddenVariable(var_name.to_string(), *pos).into() + )) + } err @ Err(_) => Some(err), } } else { diff --git a/src/func/native.rs b/src/func/native.rs index 375f6f81..9d97eb96 100644 --- a/src/func/native.rs +++ b/src/func/native.rs @@ -447,8 +447,9 @@ pub type OnVarCallback = /// Callback function for variable definition. #[cfg(not(feature = "sync"))] -pub type OnDefVarCallback = dyn Fn(&str, bool, usize, bool, &EvalContext) -> RhaiResultOf; +pub type OnDefVarCallback = + dyn Fn(&str, bool, bool, usize, bool, &EvalContext) -> RhaiResultOf; /// Callback function for variable definition. #[cfg(feature = "sync")] pub type OnDefVarCallback = - dyn Fn(&str, bool, usize, bool, &EvalContext) -> RhaiResultOf + Send + Sync; + dyn Fn(&str, bool, bool, usize, bool, &EvalContext) -> RhaiResultOf + Send + Sync; diff --git a/src/parser.rs b/src/parser.rs index b57eed9a..2bcabfe0 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -7,6 +7,7 @@ use crate::ast::{ OpAssignment, ScriptFnDef, Stmt, SwitchCases, TryCatchBlock, AST_OPTION_FLAGS::*, }; use crate::engine::{Precedence, KEYWORD_THIS, OP_CONTAINS}; +use crate::eval::{EvalState, GlobalRuntimeState}; use crate::func::hashing::get_hasher; use crate::tokenizer::{ is_keyword_function, is_valid_function_name, is_valid_identifier, Span, Token, TokenStream, @@ -15,8 +16,9 @@ use crate::tokenizer::{ use crate::types::dynamic::AccessMode; use crate::types::StringsInterner; use crate::{ - calc_fn_hash, Dynamic, Engine, ExclusiveRange, Identifier, ImmutableString, InclusiveRange, - LexError, OptimizationLevel, ParseError, Position, Scope, Shared, StaticVec, AST, INT, PERR, + calc_fn_hash, Dynamic, Engine, EvalAltResult, EvalContext, ExclusiveRange, Identifier, + ImmutableString, InclusiveRange, LexError, OptimizationLevel, ParseError, Position, Scope, + Shared, StaticVec, AST, INT, PERR, }; #[cfg(feature = "no_std")] use std::prelude::v1::*; @@ -47,7 +49,7 @@ pub struct ParseState<'e> { /// Interned strings. pub interned_strings: StringsInterner, /// Encapsulates a local stack with variable names to simulate an actual runtime scope. - pub stack: StaticVec<(Identifier, AccessMode)>, + pub stack: Scope<'e>, /// Size of the local variables stack upon entry of the current block scope. pub entry_stack_len: usize, /// Tracks a list of external variables (variables that are not explicitly declared in the scope). @@ -89,7 +91,7 @@ impl<'e> ParseState<'e> { #[cfg(not(feature = "no_closure"))] allow_capture: true, interned_strings: StringsInterner::new(), - stack: StaticVec::new_const(), + stack: Scope::new(), entry_stack_len: 0, #[cfg(not(feature = "no_module"))] imports: StaticVec::new_const(), @@ -112,10 +114,9 @@ impl<'e> ParseState<'e> { let index = self .stack - .iter() - .rev() + .iter_rev_raw() .enumerate() - .find(|(.., (n, ..))| { + .find(|&(.., (n, ..))| { if n == SCOPE_SEARCH_BARRIER_MARKER { // Do not go beyond the barrier barrier = true; @@ -1337,13 +1338,11 @@ fn parse_primary( } if segments.is_empty() { - segments.push(Expr::StringConstant( - state.get_interned_string("", ""), - settings.pos, - )); + Expr::StringConstant(state.get_interned_string("", ""), settings.pos) + } else { + segments.shrink_to_fit(); + Expr::InterpolatedString(segments.into(), settings.pos) } - segments.shrink_to_fit(); - Expr::InterpolatedString(segments.into(), settings.pos) } // Array literal @@ -1802,7 +1801,11 @@ fn make_assignment_stmt( || index.expect("either long or short index is `None`").get(), |n| n.get() as usize, ); - match state.stack[state.stack.len() - index].1 { + match state + .stack + .get_mut_by_index(state.stack.len() - index) + .access_mode() + { AccessMode::ReadWrite => Ok(Stmt::Assignment( (op_info, (lhs, rhs).into()).into(), op_pos, @@ -2195,7 +2198,7 @@ fn parse_custom_syntax( // Add a barrier variable to the stack so earlier variables will not be matched. // Variable searches stop at the first barrier. let marker = state.get_identifier("", SCOPE_SEARCH_BARRIER_MARKER); - state.stack.push((marker, AccessMode::ReadWrite)); + state.stack.push(marker, ()); } let parse_func = syntax.parse.as_ref(); @@ -2562,21 +2565,24 @@ fn parse_for( let counter_var = counter_name.map(|name| { let name = state.get_identifier("", name); let pos = counter_pos.expect("`Some`"); - state.stack.push((name.clone(), AccessMode::ReadWrite)); - Ident { name, pos } + state.stack.push(name.clone(), ()); + Ident { + name: name.clone(), + pos, + } }); let loop_var = state.get_identifier("", name); - state.stack.push((loop_var.clone(), AccessMode::ReadWrite)); + state.stack.push(loop_var.clone(), ()); let loop_var = Ident { - name: loop_var, + name: loop_var.clone(), pos: name_pos, }; settings.is_breakable = true; let body = parse_block(input, state, lib, settings.level_up())?; - state.stack.truncate(prev_stack_len); + state.stack.rewind(prev_stack_len); Ok(Stmt::For( expr, @@ -2610,6 +2616,30 @@ fn parse_let( return Err(PERR::VariableExists(name.to_string()).into_err(pos)); } + if let Some(ref filter) = state.engine.def_var_filter { + let shadowing = state.stack.iter().any(|(v, ..)| v == name.as_ref()); + let level = settings.level; + let is_const = var_type == AccessMode::ReadOnly; + let context = EvalContext { + engine: state.engine, + scope: &mut state.stack, + global: &mut GlobalRuntimeState::new(state.engine), + state: &mut EvalState::new(), + lib: &[], + this_ptr: &mut None, + level, + }; + + match filter(&name, false, is_const, level, shadowing, &context) { + Ok(true) => (), + Ok(false) => return Err(PERR::ForbiddenVariable(name.to_string()).into_err(pos)), + Err(err) => match *err { + EvalAltResult::ErrorParsing(perr, pos) => return Err(perr.into_err(pos)), + _ => return Err(PERR::ForbiddenVariable(name.to_string()).into_err(pos)), + }, + } + } + let name = state.get_identifier("", name); let var_def = Ident { name: name.clone(), @@ -2624,8 +2654,6 @@ fn parse_let( Expr::Unit(Position::NONE) }; - state.stack.push((name, var_type)); - let export = if is_export { AST_OPTION_EXPORTED } else { @@ -2634,14 +2662,20 @@ fn parse_let( match var_type { // let name = expr - AccessMode::ReadWrite => Ok(Stmt::Var(expr, var_def.into(), export, settings.pos)), + AccessMode::ReadWrite => { + state.stack.push(name, ()); + Ok(Stmt::Var(expr, var_def.into(), export, settings.pos)) + } // const name = { expr:constant } - AccessMode::ReadOnly => Ok(Stmt::Var( - expr, - var_def.into(), - AST_OPTION_CONSTANT + export, - settings.pos, - )), + AccessMode::ReadOnly => { + state.stack.push_constant(name, ()); + Ok(Stmt::Var( + expr, + var_def.into(), + AST_OPTION_CONSTANT + export, + settings.pos, + )) + } } } @@ -2820,7 +2854,7 @@ fn parse_block( } }; - state.stack.truncate(state.entry_stack_len); + state.stack.rewind(state.entry_stack_len); state.entry_stack_len = prev_entry_stack_len; #[cfg(not(feature = "no_module"))] @@ -3106,7 +3140,7 @@ fn parse_try_catch( } let name = state.get_identifier("", name); - state.stack.push((name.clone(), AccessMode::ReadWrite)); + state.stack.push(name.clone(), ()); Some(Ident { name, pos }) } else { None @@ -3117,7 +3151,7 @@ fn parse_try_catch( if catch_var.is_some() { // Remove the error variable from the stack - state.stack.pop().unwrap(); + state.stack.rewind(state.stack.len() - 1); } Ok(Stmt::TryCatch( @@ -3176,7 +3210,7 @@ fn parse_fn( ); } let s = state.get_identifier("", s); - state.stack.push((s.clone(), AccessMode::ReadWrite)); + state.stack.push(s.clone(), ()); params.push((s, pos)) } (Token::LexError(err), pos) => return Err(err.into_err(pos)), @@ -3319,7 +3353,7 @@ fn parse_anon_fn( ); } let s = state.get_identifier("", s); - state.stack.push((s.clone(), AccessMode::ReadWrite)); + state.stack.push(s.clone(), ()); params_list.push(s) } (Token::LexError(err), pos) => return Err(err.into_err(pos)), diff --git a/src/types/error.rs b/src/types/error.rs index 58236d7f..9eaf7907 100644 --- a/src/types/error.rs +++ b/src/types/error.rs @@ -36,6 +36,8 @@ pub enum EvalAltResult { /// Shadowing of an existing variable disallowed. Wrapped value is the variable name. ErrorVariableExists(String, Position), + /// Forbidden variable name. Wrapped value is the variable name. + ErrorForbiddenVariable(String, Position), /// 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. @@ -148,6 +150,7 @@ impl fmt::Display for EvalAltResult { Self::ErrorInModule(s, err, ..) => write!(f, "Error in module {}: {}", s, err)?, Self::ErrorVariableExists(s, ..) => write!(f, "Variable is already defined: {}", s)?, + Self::ErrorForbiddenVariable(s, ..) => write!(f, "Forbidden variable name: {}", 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)?, @@ -285,6 +288,7 @@ impl EvalAltResult { | Self::ErrorIndexingType(..) | Self::ErrorFor(..) | Self::ErrorVariableExists(..) + | Self::ErrorForbiddenVariable(..) | Self::ErrorVariableNotFound(..) | Self::ErrorPropertyNotFound(..) | Self::ErrorModuleNotFound(..) @@ -377,6 +381,7 @@ impl EvalAltResult { map.insert("type".into(), t.into()); } Self::ErrorVariableExists(v, ..) + | Self::ErrorForbiddenVariable(v, ..) | Self::ErrorVariableNotFound(v, ..) | Self::ErrorPropertyNotFound(v, ..) | Self::ErrorDataRace(v, ..) @@ -440,6 +445,7 @@ impl EvalAltResult { | Self::ErrorIndexingType(.., pos) | Self::ErrorFor(pos) | Self::ErrorVariableExists(.., pos) + | Self::ErrorForbiddenVariable(.., pos) | Self::ErrorVariableNotFound(.., pos) | Self::ErrorPropertyNotFound(.., pos) | Self::ErrorModuleNotFound(.., pos) @@ -490,6 +496,7 @@ impl EvalAltResult { | Self::ErrorIndexingType(.., pos) | Self::ErrorFor(pos) | Self::ErrorVariableExists(.., pos) + | Self::ErrorForbiddenVariable(.., pos) | Self::ErrorVariableNotFound(.., pos) | Self::ErrorPropertyNotFound(.., pos) | Self::ErrorModuleNotFound(.., pos) diff --git a/src/types/parse_error.rs b/src/types/parse_error.rs index 7d5835ec..fb922e15 100644 --- a/src/types/parse_error.rs +++ b/src/types/parse_error.rs @@ -109,6 +109,8 @@ pub enum ParseErrorType { PropertyExpected, /// Missing a variable name after the `let`, `const`, `for` or `catch` keywords. VariableExpected, + /// Forbidden variable name. Wrapped value is the variable name. + ForbiddenVariable(String), /// An identifier is a reserved symbol. Reserved(String), /// An expression is of the wrong type. @@ -240,6 +242,7 @@ impl fmt::Display for ParseErrorType { Self::WrongSwitchCaseCondition => f.write_str("This switch case cannot have a condition"), Self::PropertyExpected => f.write_str("Expecting name of a property"), Self::VariableExpected => f.write_str("Expecting name of a variable"), + Self::ForbiddenVariable(s) => write!(f, "Forbidden variable name: {}", s), Self::WrongFnDefinition => f.write_str("Function definitions must be at global level and cannot be inside a block or another function"), Self::FnMissingName => f.write_str("Expecting function name in function declaration"), Self::WrongDocComment => f.write_str("Doc-comment must be followed immediately by a function definition"), diff --git a/src/types/scope.rs b/src/types/scope.rs index 74a667e7..85fd70d4 100644 --- a/src/types/scope.rs +++ b/src/types/scope.rs @@ -595,6 +595,16 @@ impl Scope<'_> { .zip(self.values.iter()) .map(|((name, ..), value)| (name.as_ref(), value.is_read_only(), value)) } + /// Get a reverse iterator to entries in the [`Scope`]. + /// Shared values are not expanded. + #[inline] + pub(crate) fn iter_rev_raw(&self) -> impl Iterator { + self.names + .iter() + .rev() + .zip(self.values.iter().rev()) + .map(|((name, ..), value)| (name.as_ref(), value.is_read_only(), value)) + } /// Remove a range of entries within the [`Scope`]. /// /// # Panics diff --git a/tests/var_scope.rs b/tests/var_scope.rs index d8543b81..4b07ab71 100644 --- a/tests/var_scope.rs +++ b/tests/var_scope.rs @@ -1,4 +1,4 @@ -use rhai::{Engine, EvalAltResult, Position, Scope, INT}; +use rhai::{Engine, EvalAltResult, ParseErrorType, Position, Scope, INT}; #[test] fn test_var_scope() -> Result<(), Box> { @@ -125,7 +125,10 @@ fn test_var_resolver() -> Result<(), Box> { fn test_var_def_filter() -> Result<(), Box> { let mut engine = Engine::new(); - engine.on_def_var(|name, _, scope_level, _, _| match (name, scope_level) { + let ast = engine.compile("let x = 42;")?; + engine.run_ast(&ast)?; + + engine.on_def_var(|name, _, _, scope_level, _, _| match (name, scope_level) { ("x", 0 | 1) => Ok(false), _ => Ok(true), }); @@ -135,7 +138,14 @@ fn test_var_def_filter() -> Result<(), Box> { 124 ); - assert!(engine.run("let x = 42;").is_err()); + assert!(matches!( + *engine.compile("let x = 42;").expect_err("should error").0, + ParseErrorType::ForbiddenVariable(s) if s == "x" + )); + assert!(matches!( + *engine.run_ast(&ast).expect_err("should err"), + EvalAltResult::ErrorForbiddenVariable(s, _) if s == "x" + )); assert!(engine.run("const x = 42;").is_err()); assert!(engine.run("let y = 42; { let x = y + 1; }").is_err()); assert!(engine.run("let y = 42; { let x = y + 1; }").is_err());