Add allow_shadowing.

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
 ------------

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;
+    }
 }

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 {

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))
         }
 

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)

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),
 

tests/options.rs

@@ -1,4 +1,4 @@
-use rhai::{Engine, EvalAltResult};
+use rhai::{Engine, EvalAltResult, Scope, INT};
 
 #[test]
 fn test_options_allow() -> Result<(), Box<EvalAltResult>> {
@@ -31,6 +31,20 @@ fn test_options_allow() -> Result<(), Box<EvalAltResult>> {
 
     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(())
 }