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) {
-        self.get_current_environment()
-            .borrow_mut()
-            .create_mutable_binding(name, deletion)
+    pub fn create_mutable_binding(&mut self, name: String, deletion: bool, scope: VariableScope) {
+        match scope {
+            VariableScope::Block => self
+                .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 {
-        self.get_current_environment()
-            .borrow_mut()
-            .create_immutable_binding(name, deletion)
+    pub fn create_immutable_binding(
+        &mut self,
+        name: String,
+        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 {
-        let env: Environment = self.get_current_environment().clone();
-        let borrowed_env = env.borrow();
-        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();
-            }
-        }
+    pub fn has_binding(&self, name: &str) -> bool {
+        self.environments()
+            .any(|env| env.borrow().has_binding(name))
+    }
 
-        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
-                        .environment
-                        .create_mutable_binding(name.clone().expect("No name was supplied"), false);
+                    self.realm.environment.create_mutable_binding(
+                        name.clone().expect("No name was supplied"),
+                        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
-                                .environment
-                                .create_mutable_binding(name.clone(), true);
+                            self.realm.environment.create_mutable_binding(
+                                name.clone(),
+                                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
-                        .environment
-                        .create_mutable_binding(name.clone(), false);
+                    self.realm.environment.create_mutable_binding(
+                        name.clone(),
+                        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
-                        .environment
-                        .create_mutable_binding(name.clone(), false);
+                    self.realm.environment.create_mutable_binding(
+                        name.clone(),
+                        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
-                        .environment
-                        .create_immutable_binding(name.clone(), false);
+                    self.realm.environment.create_immutable_binding(
+                        name.clone(),
+                        false,
+                        VariableScope::Block,
+                    );
                     let val = self.run(&value)?;
                     self.realm.environment.initialize_binding(&name, val);
                 }
@@ -485,19 +509,23 @@ 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
-                            .environment
-                            .create_mutable_binding(name.clone(), false);
+                        self.realm.environment.create_mutable_binding(
+                            name.clone(),
+                            false,
+                            VariableScope::Function,
+                        );
                         self.realm
                             .environment
                             .initialize_binding(name, expr.clone());
                     }
 
                     // Add arguments object
                     let arguments_obj = create_unmapped_arguments_object(arguments_list);
-                    self.realm
-                        .environment
-                        .create_mutable_binding("arguments".to_string(), false);
+                    self.realm.environment.create_mutable_binding(
+                        "arguments".to_string(),
+                        false,
+                        VariableScope::Function,
+                    );
                     self.realm
                         .environment
                         .initialize_binding("arguments", arguments_obj);