diff --git a/CHANGELOG.md b/CHANGELOG.md index 3f7a2997..fcc67f65 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -26,6 +26,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_allow_shadowing` is added to allow/disallow variables _shadowing_, with new errors `EvalAltResult::ErrorVariableExists` and `ParseErrorType::VariableExists`. Enhancements ------------ diff --git a/src/api/options.rs b/src/api/options.rs index a6e1f59b..4fe6a4d7 100644 --- a/src/api/options.rs +++ b/src/api/options.rs @@ -17,9 +17,11 @@ pub struct LanguageOptions { #[cfg(not(feature = "no_function"))] pub allow_anonymous_fn: bool, /// Is looping allowed? - pub allow_loop: bool, + pub allow_looping: bool, /// Strict variables mode? pub strict_var: bool, + /// Is variables shadowing allowed? + pub allow_shadowing: bool, } impl LanguageOptions { @@ -32,8 +34,9 @@ impl LanguageOptions { allow_stmt_expr: true, #[cfg(not(feature = "no_function"))] allow_anonymous_fn: true, - allow_loop: true, + allow_looping: true, strict_var: false, + allow_shadowing: true, } } } @@ -94,12 +97,12 @@ impl Engine { /// Is looping allowed? #[inline(always)] pub fn allow_looping(&self) -> bool { - self.options.allow_loop + self.options.allow_looping } /// Set whether looping is allowed. #[inline(always)] pub fn set_allow_looping(&mut self, enable: bool) { - self.options.allow_loop = enable; + self.options.allow_looping = enable; } /// Is strict variables mode enabled? #[inline(always)] @@ -111,4 +114,14 @@ impl Engine { pub fn set_strict_variables(&mut self, enable: bool) { self.options.strict_var = enable; } + /// Is variables shadowing allowed? + #[inline(always)] + pub fn allow_shadowing(&self) -> bool { + self.options.allow_shadowing + } + /// Set whether variables shadowing is allowed. + #[inline(always)] + pub fn set_allow_shadowing(&mut self, enable: bool) { + self.options.allow_shadowing = enable; + } } diff --git a/src/eval/stmt.rs b/src/eval/stmt.rs index 8e1e7c5c..f37cd574 100644 --- a/src/eval/stmt.rs +++ b/src/eval/stmt.rs @@ -802,9 +802,14 @@ impl Engine { // Empty return Stmt::Return(_, None, pos) => Err(ERR::Return(Dynamic::UNIT, *pos).into()), + // Let/const statement - shadowing disallowed + Stmt::Var(_, x, _, pos) if !self.allow_shadowing() && scope.contains(&x.name) => { + Err(ERR::ErrorVariableExists(x.name.to_string(), *pos).into()) + } // Let/const statement Stmt::Var(expr, x, options, _) => { let var_name = &x.name; + let entry_type = if options.contains(AST_OPTION_CONSTANT) { AccessMode::ReadOnly } else { diff --git a/src/parser.rs b/src/parser.rs index 878e7ddd..85046218 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -16,7 +16,7 @@ use crate::types::dynamic::AccessMode; use crate::types::StringsInterner; use crate::{ calc_fn_hash, Dynamic, Engine, ExclusiveRange, Identifier, ImmutableString, InclusiveRange, - LexError, ParseError, Position, Scope, Shared, StaticVec, AST, INT, PERR, + LexError, ParseError, Position, Scope, Shared, StaticVec, Variant, AST, INT, PERR, }; #[cfg(feature = "no_std")] use std::prelude::v1::*; @@ -2591,6 +2591,12 @@ fn parse_let( // let name ... let (name, pos) = parse_var_name(input)?; + if !settings.default_options.allow_shadowing + && state.stack.iter().any(|(v, _)| v == name.as_ref()) + { + return Err(PERR::VariableExists(name.to_string()).into_err(pos)); + } + let name = state.get_identifier("", name); let var_def = Ident { name: name.clone(), @@ -2973,25 +2979,25 @@ fn parse_stmt( Token::If => parse_if(input, state, lib, settings.level_up()), Token::Switch => parse_switch(input, state, lib, settings.level_up()), - Token::While | Token::Loop if settings.default_options.allow_loop => { + Token::While | Token::Loop if settings.default_options.allow_looping => { parse_while_loop(input, state, lib, settings.level_up()) } - Token::Do if settings.default_options.allow_loop => { + Token::Do if settings.default_options.allow_looping => { parse_do(input, state, lib, settings.level_up()) } - Token::For if settings.default_options.allow_loop => { + Token::For if settings.default_options.allow_looping => { parse_for(input, state, lib, settings.level_up()) } - Token::Continue if settings.default_options.allow_loop && settings.is_breakable => { + Token::Continue if settings.default_options.allow_looping && settings.is_breakable => { let pos = eat_token(input, Token::Continue); Ok(Stmt::BreakLoop(AST_OPTION_NONE, pos)) } - Token::Break if settings.default_options.allow_loop && settings.is_breakable => { + Token::Break if settings.default_options.allow_looping && settings.is_breakable => { let pos = eat_token(input, Token::Break); Ok(Stmt::BreakLoop(AST_OPTION_BREAK, pos)) } - Token::Continue | Token::Break if settings.default_options.allow_loop => { + Token::Continue | Token::Break if settings.default_options.allow_looping => { Err(PERR::LoopBreak.into_err(token_pos)) } diff --git a/src/types/error.rs b/src/types/error.rs index 8131b7b0..60fc5d41 100644 --- a/src/types/error.rs +++ b/src/types/error.rs @@ -30,6 +30,8 @@ pub enum EvalAltResult { /// Syntax error. ErrorParsing(ParseErrorType, Position), + /// 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. ErrorVariableNotFound(String, Position), /// Call to an unknown function. Wrapped value is the function signature. @@ -139,8 +141,9 @@ impl fmt::Display for EvalAltResult { } Self::ErrorInModule(s, err, _) => write!(f, "Error in module {}: {}", s, err)?, - Self::ErrorFunctionNotFound(s, _) => write!(f, "Function not found: {}", s)?, + Self::ErrorVariableExists(s, _) => write!(f, "Variable is already defined: {}", s)?, Self::ErrorVariableNotFound(s, _) => write!(f, "Variable not found: {}", s)?, + Self::ErrorFunctionNotFound(s, _) => write!(f, "Function not found: {}", s)?, Self::ErrorModuleNotFound(s, _) => write!(f, "Module not found: {}", s)?, Self::ErrorDataRace(s, _) => { write!(f, "Data race detected when accessing variable: {}", s)? @@ -274,6 +277,7 @@ impl EvalAltResult { | Self::ErrorBitFieldBounds(_, _, _) | Self::ErrorIndexingType(_, _) | Self::ErrorFor(_) + | Self::ErrorVariableExists(_, _) | Self::ErrorVariableNotFound(_, _) | Self::ErrorModuleNotFound(_, _) | Self::ErrorDataRace(_, _) @@ -364,7 +368,8 @@ impl EvalAltResult { Self::ErrorIndexingType(t, _) => { map.insert("type".into(), t.into()); } - Self::ErrorVariableNotFound(v, _) + Self::ErrorVariableExists(v, _) + | Self::ErrorVariableNotFound(v, _) | Self::ErrorDataRace(v, _) | Self::ErrorAssignmentToConstant(v, _) => { map.insert("variable".into(), v.into()); @@ -415,6 +420,7 @@ impl EvalAltResult { | Self::ErrorBitFieldBounds(_, _, pos) | Self::ErrorIndexingType(_, pos) | Self::ErrorFor(pos) + | Self::ErrorVariableExists(_, pos) | Self::ErrorVariableNotFound(_, pos) | Self::ErrorModuleNotFound(_, pos) | Self::ErrorDataRace(_, pos) @@ -463,6 +469,7 @@ impl EvalAltResult { | Self::ErrorBitFieldBounds(_, _, pos) | Self::ErrorIndexingType(_, pos) | Self::ErrorFor(pos) + | Self::ErrorVariableExists(_, pos) | Self::ErrorVariableNotFound(_, pos) | Self::ErrorModuleNotFound(_, pos) | Self::ErrorDataRace(_, pos) diff --git a/src/types/parse_error.rs b/src/types/parse_error.rs index 50ab93da..d416043b 100644 --- a/src/types/parse_error.rs +++ b/src/types/parse_error.rs @@ -167,6 +167,10 @@ pub enum ParseErrorType { /// Assignment to an inappropriate LHS (left-hand-side) expression. /// Wrapped value is the error message (if any). AssignmentToInvalidLHS(String), + /// A variable is already defined. + /// + /// Only appears when variables shadowing is disabled. + VariableExists(String), /// A variable is not found. /// /// Only appears when strict variables mode is enabled. @@ -241,6 +245,7 @@ impl fmt::Display for ParseErrorType { Self::DuplicatedSwitchCase => f.write_str("Duplicated switch case"), Self::DuplicatedVariable(s) => write!(f, "Duplicated variable name: {}", s), + Self::VariableExists(s) => write!(f, "Variable already defined: {}", s), Self::VariableUndefined(s) => write!(f, "Undefined variable: {}", s), Self::ModuleUndefined(s) => write!(f, "Undefined module: {}", s), diff --git a/tests/options.rs b/tests/options.rs index 336a2856..d7e45780 100644 --- a/tests/options.rs +++ b/tests/options.rs @@ -1,4 +1,4 @@ -use rhai::{Engine, EvalAltResult}; +use rhai::{Engine, EvalAltResult, Scope, INT}; #[test] fn test_options_allow() -> Result<(), Box> { @@ -31,6 +31,20 @@ fn test_options_allow() -> Result<(), Box> { assert!(engine.compile("while x > y { foo(z); }").is_err()); + engine.compile("let x = 42; let x = 123;")?; + + engine.set_allow_shadowing(false); + + assert!(engine.compile("let x = 42; let x = 123;").is_err()); + assert!(engine.compile("const x = 42; let x = 123;").is_err()); + assert!(engine.compile("let x = 42; const x = 123;").is_err()); + assert!(engine.compile("const x = 42; const x = 123;").is_err()); + + let mut scope = Scope::new(); + scope.push("x", 42 as INT); + + assert!(engine.run_with_scope(&mut scope, "let x = 42;").is_err()); + Ok(()) }