Implement block scoped variable declarations (#173)

* Implement block scoped variable declarations

`const` and `let` are now scoped to the block, while `var` is scoped to
the surronding function (or global).

Another bigger change is that all tests have been changed to use `var`
instead of `let` or `const`. This is because every time `forward` is
called with some new Javascript to evaluate, we parse it as a block,
hence variables can only persist across calls to `forward` if they are
defined using `var`. I assume it is intentional to parse each call as a
separate block, because a block is the only `ExprDef` which can contain
multiple statements.

Closes #39

* Prefer environment parent over environment_stack

Instead of iterating over the `environment_stack` we rather use
`get_outer_environment` as it will be a better fit when asyncronous
functions are implemented.

* Ensure variable from outer scope is assigned

Variables that are defined outside a block should be changeable within
the scope. Just because variable is undefined does not mean it is not
initialized.

src/lib/environment/declarative_environment_record.rs

@@ -161,7 +161,7 @@ impl EnvironmentRecordTrait for DeclarativeEnvironmentRecord {
     }
 
     fn get_outer_environment(&self) -> Option<Environment> {
-        None
+        self.outer_env.as_ref().cloned()
     }
 
     fn set_outer_environment(&mut self, env: Environment) {

src/lib/environment/lexical_environment.rs

@@ -32,6 +32,15 @@ pub enum EnvironmentType {
     Object,
 }
 
+/// The scope of a given variable
+#[derive(Debug, Clone, Copy)]
+pub enum VariableScope {
+    /// The variable declaration is scoped to the current block (`let` and `const`)
+    Block,
+    /// The variable declaration is scoped to the current function (`var`)
+    Function,
+}
+
 #[derive(Debug)]
 pub struct LexicalEnvironment {
     environment_stack: VecDeque<Environment>,
@@ -90,6 +99,12 @@ impl LexicalEnvironment {
         self.environment_stack.pop_back();
     }
 
+    pub fn environments(&self) -> impl Iterator<Item = Environment> {
+        std::iter::successors(Some(self.get_current_environment_ref().clone()), |env| {
+            env.borrow().get_outer_environment()
+        })
+    }
+
     pub fn get_global_object(&self) -> Option<Value> {
         self.environment_stack
             .get(0)
@@ -98,25 +113,74 @@ impl LexicalEnvironment {
             .get_global_object()
     }
 
-    pub fn create_mutable_binding(&mut self, name: String, deletion: bool) {
+    pub fn create_mutable_binding(&mut self, name: String, deletion: bool, scope: VariableScope) {
-        self.get_current_environment()
+        match scope {
-            .borrow_mut()
+            VariableScope::Block => self
-            .create_mutable_binding(name, deletion)
+                .get_current_environment()
+                .borrow_mut()
+                .create_mutable_binding(name, deletion),
+            VariableScope::Function => {
+                // Find the first function or global environment (from the top of the stack)
+                let env = self
+                    .environments()
+                    .find(|env| match env.borrow().get_environment_type() {
+                        EnvironmentType::Function | EnvironmentType::Global => true,
+                        _ => false,
+                    })
+                    .expect("No function or global environment");
+
+                env.borrow_mut().create_mutable_binding(name, deletion);
+            }
+        }
     }
 
-    pub fn create_immutable_binding(&mut self, name: String, deletion: bool) -> bool {
+    pub fn create_immutable_binding(
-        self.get_current_environment()
+        &mut self,
-            .borrow_mut()
+        name: String,
-            .create_immutable_binding(name, deletion)
+        deletion: bool,
+        scope: VariableScope,
+    ) -> bool {
+        match scope {
+            VariableScope::Block => self
+                .get_current_environment()
+                .borrow_mut()
+                .create_immutable_binding(name, deletion),
+            VariableScope::Function => {
+                // Find the first function or global environment (from the top of the stack)
+                let env = self
+                    .environments()
+                    .find(|env| match env.borrow().get_environment_type() {
+                        EnvironmentType::Function | EnvironmentType::Global => true,
+                        _ => false,
+                    })
+                    .expect("No function or global environment");
+
+                #[allow(clippy::let_and_return)]
+                // FIXME need to assign result to a variable to avoid borrow checker error
+                // (borrowed value `env` does not live long enough)
+                let b = env.borrow_mut().create_immutable_binding(name, deletion);
+                b
+            }
+        }
     }
 
     pub fn set_mutable_binding(&mut self, name: &str, value: Value, strict: bool) {
-        let env = self.get_current_environment();
+        // Find the first environment which has the given binding
+        let env = self
+            .environments()
+            .find(|env| env.borrow().has_binding(name))
+            .expect("Binding does not exists"); // TODO graceful error handling
+
         env.borrow_mut().set_mutable_binding(name, value, strict);
     }
 
     pub fn initialize_binding(&mut self, name: &str, value: Value) {
-        let env = self.get_current_environment();
+        // Find the first environment which has the given binding
+        let env = self
+            .environments()
+            .find(|env| env.borrow().has_binding(name))
+            .expect("Binding does not exists"); // TODO graceful error handling
+
         env.borrow_mut().initialize_binding(name, value);
     }
 
@@ -138,37 +202,16 @@ impl LexicalEnvironment {
             .expect("Could not get mutable reference to back object")
     }
 
-    pub fn get_binding_value(&mut self, name: &str) -> Value {
+    pub fn has_binding(&self, name: &str) -> bool {
-        let env: Environment = self.get_current_environment().clone();
+        self.environments()
-        let borrowed_env = env.borrow();
+            .any(|env| env.borrow().has_binding(name))
-        let result = borrowed_env.has_binding(name);
+    }
-        if result {
-            return borrowed_env.get_binding_value(name, false);
-        }
-
-        // Check outer scope
-        if borrowed_env.get_outer_environment().is_some() {
-            let mut outer: Option<Environment> = borrowed_env.get_outer_environment();
-            while outer.is_some() {
-                if outer
-                    .as_ref()
-                    .expect("Could not get outer as reference")
-                    .borrow()
-                    .has_binding(&name)
-                {
-                    return outer
-                        .expect("Outer was None")
-                        .borrow()
-                        .get_binding_value(name, false);
-                }
-                outer = outer
-                    .expect("Outer was None")
-                    .borrow()
-                    .get_outer_environment();
-            }
-        }
 
-        Gc::new(ValueData::Undefined)
+    pub fn get_binding_value(&mut self, name: &str) -> Value {
+        self.environments()
+            .find(|env| env.borrow().has_binding(name))
+            .map(|env| env.borrow().get_binding_value(name, false))
+            .unwrap_or_else(|| Gc::new(ValueData::Undefined))
     }
 }
 
@@ -236,3 +279,70 @@ pub fn new_global_environment(global: Value, this_value: Value) -> Environment {
         var_names: HashSet::new(),
     })))
 }
+
+#[cfg(test)]
+mod tests {
+    use crate::exec;
+
+    #[test]
+    fn let_is_blockscoped() {
+        let scenario = r#"
+          {
+            let bar = "bar";
+          }
+          bar == undefined;
+        "#;
+
+        assert_eq!(&exec(scenario), "true");
+    }
+
+    #[test]
+    fn const_is_blockscoped() {
+        let scenario = r#"
+          {
+            const bar = "bar";
+          }
+          bar == undefined;
+        "#;
+
+        assert_eq!(&exec(scenario), "true");
+    }
+
+    #[test]
+    fn var_not_blockscoped() {
+        let scenario = r#"
+          {
+            var bar = "bar";
+          }
+          bar == "bar";
+        "#;
+
+        assert_eq!(&exec(scenario), "true");
+    }
+
+    #[test]
+    fn set_outer_var_in_blockscope() {
+        let scenario = r#"
+          var bar;
+          {
+            bar = "foo";
+          }
+          bar == "foo";
+        "#;
+
+        assert_eq!(&exec(scenario), "true");
+    }
+
+    #[test]
+    fn set_outer_let_in_blockscope() {
+        let scenario = r#"
+          let bar;
+          {
+            bar = "foo";
+          }
+          bar == "foo";
+        "#;
+
+        assert_eq!(&exec(scenario), "true");
+    }
+}

src/lib/exec.rs

@@ -1,5 +1,7 @@
 use crate::{
-    environment::lexical_environment::new_function_environment,
+    environment::lexical_environment::{
+        new_declarative_environment, new_function_environment, VariableScope,
+    },
     js::{
         function::{create_unmapped_arguments_object, Function, RegularFunction},
         object::{ObjectKind, INSTANCE_PROTOTYPE, PROTOTYPE},
@@ -71,6 +73,13 @@ impl Executor for Interpreter {
             ExprDef::Const(Const::String(ref str)) => Ok(to_value(str.to_owned())),
             ExprDef::Const(Const::Bool(val)) => Ok(to_value(val)),
             ExprDef::Block(ref es) => {
+                {
+                    let env = &mut self.realm.environment;
+                    env.push(new_declarative_environment(Some(
+                        env.get_current_environment_ref().clone(),
+                    )));
+                }
+
                 let mut obj = to_value(None::<()>);
                 for e in es.iter() {
                     let val = self.run(e)?;
@@ -84,6 +93,8 @@ impl Executor for Interpreter {
                         obj = val;
                     }
                 }
+
+                self.realm.environment.pop();
                 Ok(obj)
             }
             ExprDef::Local(ref name) => {
@@ -215,9 +226,11 @@ impl Executor for Interpreter {
                     Function::RegularFunc(RegularFunction::new(*expr.clone(), args.clone()));
                 let val = Gc::new(ValueData::Function(Box::new(GcCell::new(function))));
                 if name.is_some() {
-                    self.realm
+                    self.realm.environment.create_mutable_binding(
-                        .environment
+                        name.clone().expect("No name was supplied"),
-                        .create_mutable_binding(name.clone().expect("No name was supplied"), false);
+                        false,
+                        VariableScope::Function,
+                    );
                     self.realm.environment.initialize_binding(
                         name.as_ref().expect("Could not get name as reference"),
                         val.clone(),
@@ -355,7 +368,11 @@ impl Executor for Interpreter {
                             for i in 0..data.args.len() {
                                 let name = data.args.get(i).expect("Could not get data argument");
                                 let expr = v_args.get(i).expect("Could not get argument");
-                                env.create_mutable_binding(name.clone(), false);
+                                env.create_mutable_binding(
+                                    name.clone(),
+                                    false,
+                                    VariableScope::Function,
+                                );
                                 env.initialize_binding(name, expr.to_owned());
                             }
                             let result = self.run(&data.expr);
@@ -380,16 +397,17 @@ impl Executor for Interpreter {
                 let val = self.run(val_e)?;
                 match ref_e.def {
                     ExprDef::Local(ref name) => {
-                        if *self.realm.environment.get_binding_value(&name) != ValueData::Undefined
+                        if self.realm.environment.has_binding(name) {
-                        {
                             // Binding already exists
                             self.realm
                                 .environment
                                 .set_mutable_binding(&name, val.clone(), true);
                         } else {
-                            self.realm
+                            self.realm.environment.create_mutable_binding(
-                                .environment
+                                name.clone(),
-                                .create_mutable_binding(name.clone(), true);
+                                true,
+                                VariableScope::Function,
+                            );
                             self.realm.environment.initialize_binding(name, val.clone());
                         }
                     }
@@ -408,9 +426,11 @@ impl Executor for Interpreter {
                         Some(v) => self.run(&v)?,
                         None => Gc::new(ValueData::Undefined),
                     };
-                    self.realm
+                    self.realm.environment.create_mutable_binding(
-                        .environment
+                        name.clone(),
-                        .create_mutable_binding(name.clone(), false);
+                        false,
+                        VariableScope::Function,
+                    );
                     self.realm.environment.initialize_binding(&name, val);
                 }
                 Ok(Gc::new(ValueData::Undefined))
@@ -422,18 +442,22 @@ impl Executor for Interpreter {
                         Some(v) => self.run(&v)?,
                         None => Gc::new(ValueData::Undefined),
                     };
-                    self.realm
+                    self.realm.environment.create_mutable_binding(
-                        .environment
+                        name.clone(),
-                        .create_mutable_binding(name.clone(), false);
+                        false,
+                        VariableScope::Block,
+                    );
                     self.realm.environment.initialize_binding(&name, val);
                 }
                 Ok(Gc::new(ValueData::Undefined))
             }
             ExprDef::ConstDecl(ref vars) => {
                 for (name, value) in vars.iter() {
-                    self.realm
+                    self.realm.environment.create_immutable_binding(
-                        .environment
+                        name.clone(),
-                        .create_immutable_binding(name.clone(), false);
+                        false,
+                        VariableScope::Block,
+                    );
                     let val = self.run(&value)?;
                     self.realm.environment.initialize_binding(&name, val);
                 }
@@ -485,9 +509,11 @@ impl Interpreter {
                     for i in 0..data.args.len() {
                         let name = data.args.get(i).expect("Could not get data argument");
                         let expr: &Value = arguments_list.get(i).expect("Could not get argument");
-                        self.realm
+                        self.realm.environment.create_mutable_binding(
-                            .environment
+                            name.clone(),
-                            .create_mutable_binding(name.clone(), false);
+                            false,
+                            VariableScope::Function,
+                        );
                         self.realm
                             .environment
                             .initialize_binding(name, expr.clone());
@@ -495,9 +521,11 @@ impl Interpreter {
 
                     // Add arguments object
                     let arguments_obj = create_unmapped_arguments_object(arguments_list);
-                    self.realm
+                    self.realm.environment.create_mutable_binding(
-                        .environment
+                        "arguments".to_string(),
-                        .create_mutable_binding("arguments".to_string(), false);
+                        false,
+                        VariableScope::Function,
+                    );
                     self.realm
                         .environment
                         .initialize_binding("arguments", arguments_obj);

src/lib/js/array.rs

@@ -528,8 +528,8 @@ mod tests {
         let realm = Realm::create();
         let mut engine = Executor::new(realm);
         let init = r#"
-        let empty = new Array();
+        var empty = new Array();
-        let one = new Array(1);
+        var one = new Array(1);
         "#;
         forward(&mut engine, init);
         // Empty ++ Empty
@@ -551,9 +551,9 @@ mod tests {
         let realm = Realm::create();
         let mut engine = Executor::new(realm);
         let init = r#"
-        let empty = [ ];
+        var empty = [ ];
-        let one = ["a"];
+        var one = ["a"];
-        let many = ["a", "b", "c"];
+        var many = ["a", "b", "c"];
         "#;
         forward(&mut engine, init);
         // Empty
@@ -573,9 +573,9 @@ mod tests {
         let mut engine = Executor::new(realm);
         // taken from https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/every
         let init = r#"
-        let empty = [];
+        var empty = [];
 
-        let array = [11, 23, 45];
+        var array = [11, 23, 45];
         function callback(element) {
             return element > 10;
         }
@@ -583,13 +583,13 @@ mod tests {
             return element < 10;
         }
 
-        let appendArray = [1,2,3,4];
+        var appendArray = [1,2,3,4];
         function appendingCallback(elem,index,arr) {
           arr.push('new');
           return elem !== "new";
         }
 
-        let delArray = [1,2,3,4];
+        var delArray = [1,2,3,4];
         function deletingCallback(elem,index,arr) {
           arr.pop()
           return elem < 3;
@@ -620,7 +620,7 @@ mod tests {
         function comp(a) {
             return a == "a";
         }
-        let many = ["a", "b", "c"];
+        var many = ["a", "b", "c"];
         "#;
         forward(&mut engine, init);
         let found = forward(&mut engine, "many.find(comp)");
@@ -658,10 +658,10 @@ mod tests {
         let realm = Realm::create();
         let mut engine = Executor::new(realm);
         let init = r#"
-        let empty = [ ];
+        var empty = [ ];
-        let one = ["a"];
+        var one = ["a"];
-        let many = ["a", "b", "c"];
+        var many = ["a", "b", "c"];
-        let duplicates = ["a", "b", "c", "a", "b"];
+        var duplicates = ["a", "b", "c", "a", "b"];
         "#;
         forward(&mut engine, init);
 
@@ -722,10 +722,10 @@ mod tests {
         let realm = Realm::create();
         let mut engine = Executor::new(realm);
         let init = r#"
-        let empty = [ ];
+        var empty = [ ];
-        let one = ["a"];
+        var one = ["a"];
-        let many = ["a", "b", "c"];
+        var many = ["a", "b", "c"];
-        let duplicates = ["a", "b", "c", "a", "b"];
+        var duplicates = ["a", "b", "c", "a", "b"];
         "#;
         forward(&mut engine, init);
 

src/lib/js/boolean.rs

@@ -108,8 +108,8 @@ mod tests {
         let realm = Realm::create();
         let mut engine = Executor::new(realm);
         let init = r#"
-        const one = new Boolean(1);
+        var one = new Boolean(1);
-        const zero = Boolean(0);
+        var zero = Boolean(0);
         "#;
         forward(&mut engine, init);
         let one = forward_val(&mut engine, "one").unwrap();
@@ -124,10 +124,10 @@ mod tests {
         let realm = Realm::create();
         let mut engine = Executor::new(realm);
         let init = r#"
-        const trueVal = new Boolean(true);
+        var trueVal = new Boolean(true);
-        const trueNum = new Boolean(1);
+        var trueNum = new Boolean(1);
-        const trueString = new Boolean("true");
+        var trueString = new Boolean("true");
-        const trueBool = new Boolean(trueVal);
+        var trueBool = new Boolean(trueVal);
         "#;
 
         forward(&mut engine, init);
@@ -154,8 +154,8 @@ mod tests {
         let realm = Realm::create();
         let mut engine = Executor::new(realm);
         let init = r#"
-        const boolInstance = new Boolean(true);
+        var boolInstance = new Boolean(true);
-        const boolProto = Boolean.prototype;
+        var boolProto = Boolean.prototype;
         "#;
 
         forward(&mut engine, init);

src/lib/js/function.rs

@@ -145,7 +145,7 @@ mod tests {
         function jason(a, b) {
             return arguments[0];
         }
-        const val = jason(100, 6);
+        var val = jason(100, 6);
         "#;
 
         forward(&mut engine, init);

src/lib/js/regexp.rs

@@ -365,9 +365,9 @@ mod tests {
         let realm = Realm::create();
         let mut engine = Executor::new(realm);
         let init = r#"
-        let constructed = new RegExp("[0-9]+(\\.[0-9]+)?");
+        var constructed = new RegExp("[0-9]+(\\.[0-9]+)?");
-        let literal = /[0-9]+(\.[0-9]+)?/;
+        var literal = /[0-9]+(\.[0-9]+)?/;
-        let ctor_literal = new RegExp(/[0-9]+(\.[0-9]+)?/);
+        var ctor_literal = new RegExp(/[0-9]+(\.[0-9]+)?/);
         "#;
 
         forward(&mut engine, init);
@@ -416,7 +416,7 @@ mod tests {
         let realm = Realm::create();
         let mut engine = Executor::new(realm);
         let init = r#"
-        let regex = /[0-9]+(\.[0-9]+)?/g;
+        var regex = /[0-9]+(\.[0-9]+)?/g;
         "#;
 
         forward(&mut engine, init);

src/lib/js/string.rs

@@ -822,9 +822,9 @@ mod tests {
         let realm = Realm::create();
         let mut engine = Executor::new(realm);
         let init = r#"
-        const hello = new String('Hello, ');
+        var hello = new String('Hello, ');
-        const world = new String('world! ');
+        var world = new String('world! ');
-        const nice = new String('Have a nice day.');
+        var nice = new String('Have a nice day.');
         "#;
         forward(&mut engine, init);
         let _a = forward(&mut engine, "hello.concat(world, nice)");
@@ -841,8 +841,8 @@ mod tests {
         let realm = Realm::create();
         let mut engine = Executor::new(realm);
         let init = r#"
-        const hello = new String('Hello');
+        var hello = new String('Hello');
-        const world = String('world');
+        var world = String('world');
         "#;
         forward(&mut engine, init);
         let hello = forward_val(&mut engine, "hello").unwrap();
@@ -857,9 +857,9 @@ mod tests {
         let realm = Realm::create();
         let mut engine = Executor::new(realm);
         let init = r#"
-        const empty = new String('');
+        var empty = new String('');
-        const en = new String('english');
+        var en = new String('english');
-        const zh = new String('中文');
+        var zh = new String('中文');
         "#;
         forward(&mut engine, init);
 
@@ -885,13 +885,13 @@ mod tests {
         let realm = Realm::create();
         let mut engine = Executor::new(realm);
         let init = r#"
-        const empty = new String('');
+        var empty = new String('');
-        const en = new String('english');
+        var en = new String('english');
-        const zh = new String('中文');
+        var zh = new String('中文');
 
-        const emptyLiteral = '';
+        var emptyLiteral = '';
-        const enLiteral = 'english';
+        var enLiteral = 'english';
-        const zhLiteral = '中文';
+        var zhLiteral = '中文';
         "#;
         forward(&mut engine, init);
         let pass = String::from("true");
@@ -909,13 +909,13 @@ mod tests {
         let realm = Realm::create();
         let mut engine = Executor::new(realm);
         let init = r#"
-        const empty = new String('');
+        var empty = new String('');
-        const en = new String('english');
+        var en = new String('english');
-        const zh = new String('中文');
+        var zh = new String('中文');
 
-        const emptyLiteral = '';
+        var emptyLiteral = '';
-        const enLiteral = 'english';
+        var enLiteral = 'english';
-        const zhLiteral = '中文';
+        var zhLiteral = '中文';
         "#;
         forward(&mut engine, init);
         let pass = String::from("true");
@@ -952,7 +952,7 @@ mod tests {
 
         forward(
             &mut engine,
-            "const groupMatches = 'test1test2'.matchAll(/t(e)(st(\\d?))/g)",
+            "var groupMatches = 'test1test2'.matchAll(/t(e)(st(\\d?))/g)",
         );
         assert_eq!(
             forward(&mut engine, "groupMatches.length"),
@@ -984,9 +984,9 @@ mod tests {
         );
 
         let init = r#"
-        const regexp = RegExp('foo[a-z]*','g');
+        var regexp = RegExp('foo[a-z]*','g');
-        const str = 'table football, foosball';
+        var str = 'table football, foosball';
-        const matches = str.matchAll(regexp);
+        var matches = str.matchAll(regexp);
         "#;
         forward(&mut engine, init);
         assert_eq!(