From be9356727fc11bf3367f81263472f9329ee26f41 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Fri, 4 Feb 2022 22:59:41 +0800 Subject: [PATCH] Add variable definition filter. --- CHANGELOG.md | 1 + src/api/events.rs | 63 ++++++++++++++++++++++++- src/engine.rs | 8 +++- src/eval/stmt.rs | 113 +++++++++++++++++++++++++++++---------------- src/func/native.rs | 8 ++++ tests/var_scope.rs | 23 +++++++++ 6 files changed, 173 insertions(+), 43 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index fcc67f65..571ec17e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -27,6 +27,7 @@ New features * 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_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. Enhancements ------------ diff --git a/src/api/events.rs b/src/api/events.rs index 53de16f5..0a7b5eaf 100644 --- a/src/api/events.rs +++ b/src/api/events.rs @@ -15,12 +15,12 @@ impl Engine { /// > `Fn(name: &str, index: usize, context: &EvalContext) -> Result, Box>` /// /// where: + /// * `name`: name of the variable. /// * `index`: an offset from the bottom of the current [`Scope`][crate::Scope] that the /// variable is supposed to reside. Offsets start from 1, with 1 meaning the last variable in /// the current [`Scope`][crate::Scope]. Essentially the correct variable is at position /// `scope.len() - index`. If `index` is zero, then there is no pre-calculated offset position /// and a search through the current [`Scope`][crate::Scope] must be performed. - /// /// * `context`: the current [evaluation context][`EvalContext`]. /// /// ## Return value @@ -63,6 +63,67 @@ impl Engine { self.resolve_var = Some(Box::new(callback)); self } + /// Provide a callback that will be invoked before the definition of each variable . + /// + /// # Callback Function Signature + /// + /// The callback function signature takes the following form: + /// + /// > `Fn(name: &str, is_const: bool, block_level: usize, will_shadow: bool, context: &EvalContext) -> Result>` + /// + /// where: + /// * `name`: name of the variable to be defined. + /// * `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 + /// * `will_shadow`: will the variable _shadow_ an existing variable? + /// * `context`: the current [evaluation context][`EvalContext`]. + /// + /// ## Return value + /// + /// * `Ok(true)`: continue with normal variable definition. + /// * `Ok(false)`: deny the variable definition with an [runtime error][EvalAltResult::ErrorRuntime]. + /// + /// ## Raising errors + /// + /// Return `Err(...)` if there is an error. + /// + /// # Example + /// + /// ```should_panic + /// # fn main() -> Result<(), Box> { + /// use rhai::Engine; + /// + /// let mut engine = Engine::new(); + /// + /// // Register a variable definition filter. + /// engine.on_def_var(|name, is_const, _, _, _| { + /// // Disallow defining MYSTIC_NUMBER as a constant + /// if name == "MYSTIC_NUMBER" && is_const { + /// Ok(false) + /// } else { + /// Ok(true) + /// } + /// }); + /// + /// // The following runs fine: + /// engine.eval::("let MYSTIC_NUMBER = 42;")?; + /// + /// // The following will cause an error: + /// engine.eval::("const MYSTIC_NUMBER = 42;")?; + /// + /// # Ok(()) + /// # } + /// ``` + #[inline(always)] + pub fn on_def_var( + &mut self, + callback: impl Fn(&str, bool, usize, bool, &EvalContext) -> RhaiResultOf + + SendSync + + 'static, + ) -> &mut Self { + self.def_var_filter = Some(Box::new(callback)); + self + } /// _(internals)_ Register a callback that will be invoked during parsing to remap certain tokens. /// Exported under the `internals` feature only. /// diff --git a/src/engine.rs b/src/engine.rs index a08e13f4..86fb6859 100644 --- a/src/engine.rs +++ b/src/engine.rs @@ -1,7 +1,9 @@ //! Main module defining the script evaluation [`Engine`]. use crate::api::custom_syntax::CustomSyntax; -use crate::func::native::{OnDebugCallback, OnParseTokenCallback, OnPrintCallback, OnVarCallback}; +use crate::func::native::{ + OnDebugCallback, OnDefVarCallback, OnParseTokenCallback, OnPrintCallback, OnVarCallback, +}; use crate::packages::{Package, StandardPackage}; use crate::tokenizer::Token; use crate::types::dynamic::Union; @@ -113,6 +115,8 @@ pub struct Engine { pub(crate) custom_keywords: BTreeMap>, /// Custom syntax. pub(crate) custom_syntax: BTreeMap>, + /// Callback closure for filtering variable definition. + pub(crate) def_var_filter: Option>, /// Callback closure for resolving variable access. pub(crate) resolve_var: Option>, /// Callback closure to remap tokens during parsing. @@ -160,6 +164,7 @@ impl fmt::Debug for Engine { .field("disabled_symbols", &self.disabled_symbols) .field("custom_keywords", &self.custom_keywords) .field("custom_syntax", &(!self.custom_syntax.is_empty())) + .field("def_var_filter", &self.def_var_filter.is_some()) .field("resolve_var", &self.resolve_var.is_some()) .field("token_mapper", &self.token_mapper.is_some()) .field("print", &self.print.is_some()) @@ -272,6 +277,7 @@ impl Engine { custom_keywords: BTreeMap::new(), custom_syntax: BTreeMap::new(), + def_var_filter: None, resolve_var: None, token_mapper: None, diff --git a/src/eval/stmt.rs b/src/eval/stmt.rs index f37cd574..1c4a06dc 100644 --- a/src/eval/stmt.rs +++ b/src/eval/stmt.rs @@ -1,6 +1,6 @@ //! Module defining functions for evaluating a statement. -use super::{EvalState, GlobalRuntimeState, Target}; +use super::{EvalContext, EvalState, GlobalRuntimeState, Target}; use crate::ast::{ BinaryExpr, Expr, Ident, OpAssignment, Stmt, SwitchCases, TryCatchBlock, AST_OPTION_FLAGS::*, }; @@ -240,7 +240,7 @@ impl Engine { if let Ok(rhs_val) = rhs_result { let search_result = - self.search_namespace(scope, global, state, lib, this_ptr, lhs); + self.search_namespace(scope, global, state, lib, this_ptr, lhs, level); if let Ok(search_val) = search_result { let (mut lhs_ptr, pos) = search_val; @@ -807,7 +807,7 @@ impl Engine { Err(ERR::ErrorVariableExists(x.name.to_string(), *pos).into()) } // Let/const statement - Stmt::Var(expr, x, options, _) => { + Stmt::Var(expr, x, options, pos) => { let var_name = &x.name; let entry_type = if options.contains(AST_OPTION_CONSTANT) { @@ -817,48 +817,79 @@ impl Engine { }; let export = options.contains(AST_OPTION_EXPORTED); - let value_result = self - .eval_expr(scope, global, state, lib, this_ptr, expr, level) - .map(Dynamic::flatten); - - if let Ok(value) = value_result { - let _alias = if !rewind_scope { - #[cfg(not(feature = "no_function"))] - #[cfg(not(feature = "no_module"))] - if state.scope_level == 0 - && entry_type == AccessMode::ReadOnly - && lib.iter().any(|&m| !m.is_empty()) - { - if global.constants.is_none() { - global.constants = Some(crate::Shared::new(crate::Locked::new( - std::collections::BTreeMap::new(), - ))); - } - crate::func::locked_write(global.constants.as_ref().unwrap()) - .insert(var_name.clone(), value.clone()); - } - - if export { - Some(var_name) - } else { - None - } - } else if export { - unreachable!("exported variable not on global level"); - } else { - None + let result = if let Some(ref filter) = self.def_var_filter { + let shadowing = scope.contains(var_name); + let scope_level = state.scope_level; + let is_const = entry_type == AccessMode::ReadOnly; + let context = EvalContext { + engine: self, + scope, + global, + state, + lib, + this_ptr, + level: level, }; - scope.push_dynamic_value(var_name.clone(), entry_type, value); - - #[cfg(not(feature = "no_module"))] - if let Some(alias) = _alias { - scope.add_entry_alias(scope.len() - 1, alias.clone()); + match filter(var_name, 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())), + err @ Err(_) => Some(err), } - - Ok(Dynamic::UNIT) } else { - value_result + None + }; + + if let Some(result) = result { + result.map(|_| Dynamic::UNIT) + } else { + let value_result = self + .eval_expr(scope, global, state, lib, this_ptr, expr, level) + .map(Dynamic::flatten); + + if let Ok(value) = value_result { + let _alias = if !rewind_scope { + #[cfg(not(feature = "no_function"))] + #[cfg(not(feature = "no_module"))] + if state.scope_level == 0 + && entry_type == AccessMode::ReadOnly + && lib.iter().any(|&m| !m.is_empty()) + { + if global.constants.is_none() { + global.constants = Some(crate::Shared::new( + crate::Locked::new(std::collections::BTreeMap::new()), + )); + } + crate::func::locked_write(global.constants.as_ref().unwrap()) + .insert(var_name.clone(), value.clone()); + } + + if export { + Some(var_name) + } else { + None + } + } else if export { + unreachable!("exported variable not on global level"); + } else { + None + }; + + scope.push_dynamic_value(var_name.clone(), entry_type, value); + + #[cfg(not(feature = "no_module"))] + if let Some(alias) = _alias { + scope.add_entry_alias(scope.len() - 1, alias.clone()); + } + + Ok(Dynamic::UNIT) + } else { + value_result + } } } diff --git a/src/func/native.rs b/src/func/native.rs index 43de5bcb..380098fd 100644 --- a/src/func/native.rs +++ b/src/func/native.rs @@ -444,3 +444,11 @@ pub type OnVarCallback = dyn Fn(&str, usize, &EvalContext) -> RhaiResultOf RhaiResultOf> + Send + Sync; + +/// Callback function for variable definition. +#[cfg(not(feature = "sync"))] +pub type OnDefVarCallback = dyn Fn(&str, 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; diff --git a/tests/var_scope.rs b/tests/var_scope.rs index 8efbc5f4..7948833b 100644 --- a/tests/var_scope.rs +++ b/tests/var_scope.rs @@ -120,3 +120,26 @@ fn test_var_resolver() -> Result<(), Box> { Ok(()) } + +#[test] +fn test_var_def_filter() -> Result<(), Box> { + let mut engine = Engine::new(); + + engine.on_def_var(|name, scope_level, _, _| match (name, scope_level) { + ("x", 0 | 1) => Ok(false), + _ => Ok(true), + }); + + assert_eq!( + engine.eval::("let y = 42; let y = 123; let z = y + 1; z")?, + 124 + ); + + assert!(engine.run("let x = 42;").is_err()); + 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()); + engine.run("let y = 42; { let z = y + 1; { let x = z + 1; } }")?; + + Ok(()) +}