Error handling in environment (#659)

* Map ErrorKind to JS Errors

* Fix fmt

* Modify code with suggestions

* rebase and fix compilation errors

* Remove redundant clone

* Remove duplicate file

* cargo fmt

boa/src/builtins/function/mod.rs

@@ -124,12 +124,14 @@ impl Function {
         // Create binding
         local_env
             .borrow_mut()
-            .create_mutable_binding(param.name().to_owned(), false);
+            .create_mutable_binding(param.name().to_owned(), false)
+            .expect("Failed to create binding for rest param");
 
         // Set Binding to value
         local_env
             .borrow_mut()
-            .initialize_binding(param.name(), array);
+            .initialize_binding(param.name(), array)
+            .expect("Failed to initialize rest param");
     }
 
     // Adds an argument to the environment
@@ -142,12 +144,14 @@ impl Function {
         // Create binding
         local_env
             .borrow_mut()
-            .create_mutable_binding(param.name().to_owned(), false);
+            .create_mutable_binding(param.name().to_owned(), false)
+            .expect("Failed to create binding");
 
         // Set Binding to value
         local_env
             .borrow_mut()
-            .initialize_binding(param.name(), value);
+            .initialize_binding(param.name(), value)
+            .expect("Failed to intialize binding");
     }
 
     /// Returns true if the function object is callable.

boa/src/context.rs

@@ -342,7 +342,7 @@ impl Context {
     #[inline]
     pub fn construct_range_error<M>(&mut self, message: M) -> Value
     where
-        M: Into<String>,
+        M: Into<Box<str>>,
     {
         // Runs a `new RangeError(message)`.
         New::from(Call::new(
@@ -357,7 +357,7 @@ impl Context {
     #[inline]
     pub fn throw_range_error<M>(&mut self, message: M) -> Result<Value>
     where
-        M: Into<String>,
+        M: Into<Box<str>>,
     {
         Err(self.construct_range_error(message))
     }
@@ -366,7 +366,7 @@ impl Context {
     #[inline]
     pub fn construct_type_error<M>(&mut self, message: M) -> Value
     where
-        M: Into<String>,
+        M: Into<Box<str>>,
     {
         // Runs a `new TypeError(message)`.
         New::from(Call::new(
@@ -381,7 +381,7 @@ impl Context {
     #[inline]
     pub fn throw_type_error<M>(&mut self, message: M) -> Result<Value>
     where
-        M: Into<String>,
+        M: Into<Box<str>>,
     {
         Err(self.construct_type_error(message))
     }
@@ -390,11 +390,11 @@ impl Context {
     #[inline]
     pub fn construct_reference_error<M>(&mut self, message: M) -> Value
     where
-        M: Into<String>,
+        M: Into<Box<str>>,
     {
         New::from(Call::new(
             Identifier::from("ReferenceError"),
-            vec![Const::from(message.into() + " is not defined").into()],
+            vec![Const::from(message.into()).into()],
         ))
         .run(self)
         .expect("Into<String> used as message")
@@ -404,7 +404,7 @@ impl Context {
     #[inline]
     pub fn throw_reference_error<M>(&mut self, message: M) -> Result<Value>
     where
-        M: Into<String>,
+        M: Into<Box<str>>,
     {
         Err(self.construct_reference_error(message))
     }
@@ -413,7 +413,7 @@ impl Context {
     #[inline]
     pub fn construct_syntax_error<M>(&mut self, message: M) -> Value
     where
-        M: Into<String>,
+        M: Into<Box<str>>,
     {
         New::from(Call::new(
             Identifier::from("SyntaxError"),
@@ -427,15 +427,15 @@ impl Context {
     #[inline]
     pub fn throw_syntax_error<M>(&mut self, message: M) -> Result<Value>
     where
-        M: Into<String>,
+        M: Into<Box<str>>,
     {
         Err(self.construct_syntax_error(message))
     }
 
     /// Constructs a `EvalError` with the specified message.
     pub fn construct_eval_error<M>(&mut self, message: M) -> Value
     where
-        M: Into<String>,
+        M: Into<Box<str>>,
     {
         New::from(Call::new(
             Identifier::from("EvalError"),
@@ -448,7 +448,7 @@ impl Context {
     /// Constructs a `URIError` with the specified message.
     pub fn construct_uri_error<M>(&mut self, message: M) -> Value
     where
-        M: Into<String>,
+        M: Into<Box<str>>,
     {
         New::from(Call::new(
             Identifier::from("URIError"),
@@ -461,15 +461,15 @@ impl Context {
     /// Throws a `EvalError` with the specified message.
     pub fn throw_eval_error<M>(&mut self, message: M) -> Result<Value>
     where
-        M: Into<String>,
+        M: Into<Box<str>>,
     {
         Err(self.construct_eval_error(message))
     }
 
     /// Throws a `URIError` with the specified message.
     pub fn throw_uri_error<M>(&mut self, message: M) -> Result<Value>
     where
-        M: Into<String>,
+        M: Into<Box<str>>,
     {
         Err(self.construct_uri_error(message))
     }
@@ -619,7 +619,8 @@ impl Context {
             Node::Identifier(ref name) => {
                 self.realm
                     .environment
-                    .set_mutable_binding(name.as_ref(), value.clone(), true);
+                    .set_mutable_binding(name.as_ref(), value.clone(), true)
+                    .map_err(|e| e.to_error(self))?;
                 Ok(value)
             }
             Node::GetConstField(ref get_const_field_node) => Ok(get_const_field_node

boa/src/environment/declarative_environment_record.rs

@@ -5,6 +5,7 @@
 //! A declarative Environment Record binds the set of identifiers defined by the declarations contained within its scope.
 //! More info:  [ECMA-262 sec-declarative-environment-records](https://tc39.es/ecma262/#sec-declarative-environment-records)
 
+use super::ErrorKind;
 use crate::{
     environment::{
         environment_record_trait::EnvironmentRecordTrait,
@@ -41,11 +42,12 @@ impl EnvironmentRecordTrait for DeclarativeEnvironmentRecord {
         self.env_rec.contains_key(name)
     }
 
-    fn create_mutable_binding(&mut self, name: String, deletion: bool) {
-        if self.env_rec.contains_key(&name) {
-            // TODO: change this when error handling comes into play
-            panic!("Identifier {} has already been declared", name);
-        }
+    fn create_mutable_binding(&mut self, name: String, deletion: bool) -> Result<(), ErrorKind> {
+        assert!(
+            !self.env_rec.contains_key(&name),
+            "Identifier {} has already been declared",
+            name
+        );
 
         self.env_rec.insert(
             name,
@@ -56,13 +58,15 @@ impl EnvironmentRecordTrait for DeclarativeEnvironmentRecord {
                 strict: false,
             },
         );
+        Ok(())
     }
 
-    fn create_immutable_binding(&mut self, name: String, strict: bool) -> bool {
-        if self.env_rec.contains_key(&name) {
-            // TODO: change this when error handling comes into play
-            panic!("Identifier {} has already been declared", name);
-        }
+    fn create_immutable_binding(&mut self, name: String, strict: bool) -> Result<(), ErrorKind> {
+        assert!(
+            !self.env_rec.contains_key(&name),
+            "Identifier {} has already been declared",
+            name
+        );
 
         self.env_rec.insert(
             name,
@@ -73,61 +77,73 @@ impl EnvironmentRecordTrait for DeclarativeEnvironmentRecord {
                 strict,
             },
         );
-
-        true
+        Ok(())
     }
 
-    fn initialize_binding(&mut self, name: &str, value: Value) {
+    fn initialize_binding(&mut self, name: &str, value: Value) -> Result<(), ErrorKind> {
         if let Some(ref mut record) = self.env_rec.get_mut(name) {
             if record.value.is_none() {
                 record.value = Some(value);
-            } else {
-                // TODO: change this when error handling comes into play
-                panic!("Identifier {} has already been defined", name);
+                return Ok(());
             }
         }
+        panic!("record must have binding for {}", name);
     }
 
     #[allow(clippy::else_if_without_else)]
-    fn set_mutable_binding(&mut self, name: &str, value: Value, mut strict: bool) {
+    fn set_mutable_binding(
+        &mut self,
+        name: &str,
+        value: Value,
+        mut strict: bool,
+    ) -> Result<(), ErrorKind> {
         if self.env_rec.get(name).is_none() {
             if strict {
-                // TODO: change this when error handling comes into play
-                panic!("Reference Error: Cannot set mutable binding for {}", name);
+                return Err(ErrorKind::new_reference_error(format!(
+                    "{} not found",
+                    name
+                )));
             }
 
-            self.create_mutable_binding(name.to_owned(), true);
-            self.initialize_binding(name, value);
-            return;
+            self.create_mutable_binding(name.to_owned(), true)?;
+            self.initialize_binding(name, value)?;
+            return Ok(());
         }
 
         let record: &mut DeclarativeEnvironmentRecordBinding = self.env_rec.get_mut(name).unwrap();
         if record.strict {
             strict = true
         }
         if record.value.is_none() {
-            // TODO: change this when error handling comes into play
-            panic!("Reference Error: Cannot set mutable binding for {}", name);
+            return Err(ErrorKind::new_reference_error(format!(
+                "{} has not been initialized",
+                name
+            )));
         }
-
         if record.mutable {
             record.value = Some(value);
         } else if strict {
-            // TODO: change this when error handling comes into play
-            panic!("TypeError: Cannot mutate an immutable binding {}", name);
+            return Err(ErrorKind::new_type_error(format!(
+                "Cannot mutate an immutable binding {}",
+                name
+            )));
         }
+
+        Ok(())
     }
 
-    fn get_binding_value(&self, name: &str, _strict: bool) -> Value {
+    fn get_binding_value(&self, name: &str, _strict: bool) -> Result<Value, ErrorKind> {
         if let Some(binding) = self.env_rec.get(name) {
-            binding
-                .value
-                .as_ref()
-                .expect("Could not get record as reference")
-                .clone()
+            if let Some(ref val) = binding.value {
+                Ok(val.clone())
+            } else {
+                Err(ErrorKind::new_reference_error(format!(
+                    "{} is an uninitialized binding",
+                    name
+                )))
+            }
         } else {
-            // TODO: change this when error handling comes into play
-            panic!("ReferenceError: Cannot get binding value for {}", name);
+            panic!("Cannot get binding value for {}", name);
         }
     }
 
@@ -141,16 +157,16 @@ impl EnvironmentRecordTrait for DeclarativeEnvironmentRecord {
                     false
                 }
             }
-            None => false,
+            None => panic!("env_rec has no binding for {}", name),
         }
     }
 
     fn has_this_binding(&self) -> bool {
         false
     }
 
-    fn get_this_binding(&self) -> Value {
-        Value::undefined()
+    fn get_this_binding(&self) -> Result<Value, ErrorKind> {
+        Ok(Value::undefined())
     }
 
     fn has_super_binding(&self) -> bool {

boa/src/environment/environment_record_trait.rs

@@ -8,6 +8,7 @@
 //!
 //! There are 5 Environment record kinds. They all have methods in common, these are implemented as a the `EnvironmentRecordTrait`
 //!
+use super::ErrorKind;
 use crate::{
     environment::lexical_environment::{Environment, EnvironmentType},
     gc::{Finalize, Trace},
@@ -25,30 +26,35 @@ pub trait EnvironmentRecordTrait: Debug + Trace + Finalize {
 
     /// Create a new but uninitialized mutable binding in an Environment Record. The String value N is the text of the bound name.
     /// If the Boolean argument deletion is true the binding may be subsequently deleted.
-    fn create_mutable_binding(&mut self, name: String, deletion: bool);
+    fn create_mutable_binding(&mut self, name: String, deletion: bool) -> Result<(), ErrorKind>;
 
     /// Create a new but uninitialized immutable binding in an Environment Record.
     /// The String value N is the text of the bound name.
     /// If strict is true then attempts to set it after it has been initialized will always throw an exception,
     /// regardless of the strict mode setting of operations that reference that binding.
-    fn create_immutable_binding(&mut self, name: String, strict: bool) -> bool;
+    fn create_immutable_binding(&mut self, name: String, strict: bool) -> Result<(), ErrorKind>;
 
     /// Set the value of an already existing but uninitialized binding in an Environment Record.
     /// The String value N is the text of the bound name.
     /// V is the value for the binding and is a value of any ECMAScript language type.
-    fn initialize_binding(&mut self, name: &str, value: Value);
+    fn initialize_binding(&mut self, name: &str, value: Value) -> Result<(), ErrorKind>;
 
     /// Set the value of an already existing mutable binding in an Environment Record.
     /// The String value `name` is the text of the bound name.
     /// value is the `value` for the binding and may be a value of any ECMAScript language type. S is a Boolean flag.
     /// If `strict` is true and the binding cannot be set throw a TypeError exception.
-    fn set_mutable_binding(&mut self, name: &str, value: Value, strict: bool);
+    fn set_mutable_binding(
+        &mut self,
+        name: &str,
+        value: Value,
+        strict: bool,
+    ) -> Result<(), ErrorKind>;
 
     /// Returns the value of an already existing binding from an Environment Record.
     /// The String value N is the text of the bound name.
     /// S is used to identify references originating in strict mode code or that
     /// otherwise require strict mode reference semantics.
-    fn get_binding_value(&self, name: &str, strict: bool) -> Value;
+    fn get_binding_value(&self, name: &str, strict: bool) -> Result<Value, ErrorKind>;
 
     /// Delete a binding from an Environment Record.
     /// The String value name is the text of the bound name.
@@ -61,7 +67,7 @@ pub trait EnvironmentRecordTrait: Debug + Trace + Finalize {
     fn has_this_binding(&self) -> bool;
 
     /// Return the `this` binding from the environment
-    fn get_this_binding(&self) -> Value;
+    fn get_this_binding(&self) -> Result<Value, ErrorKind>;
 
     /// Determine if an Environment Record establishes a super method binding.
     /// Return true if it does and false if it does not.