From dc82aa29dc3cc8425d857a3f461e13c8c4f31e14 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Borges?= Date: Mon, 5 Oct 2020 19:48:44 +0100 Subject: [PATCH] Fix Error constructors to return rather than throw (#749) * Refactor: Error constructors to return instead of "throwing" * Test: Add a test case for Error Objects in Object.prototype.toString * Fix: Make Error.prototype.toString spec compliant * Test: Add tests for Error.prototype.toString --- boa/src/builtins/error/mod.rs | 36 +++++++++++++++++++++++++---- boa/src/builtins/error/range.rs | 2 +- boa/src/builtins/error/reference.rs | 2 +- boa/src/builtins/error/syntax.rs | 2 +- boa/src/builtins/error/tests.rs | 30 ++++++++++++++++++++++++ boa/src/builtins/error/type.rs | 2 +- boa/src/builtins/object/tests.rs | 3 +++ boa/src/context.rs | 8 +++---- 8 files changed, 73 insertions(+), 12 deletions(-) create mode 100644 boa/src/builtins/error/tests.rs diff --git a/boa/src/builtins/error/mod.rs b/boa/src/builtins/error/mod.rs index 61d73599a5..ee819458af 100644 --- a/boa/src/builtins/error/mod.rs +++ b/boa/src/builtins/error/mod.rs @@ -25,6 +25,9 @@ pub(crate) mod r#type; // pub(crate) mod eval; // pub(crate) mod uri; +#[cfg(test)] +mod tests; + pub(crate) use self::r#type::TypeError; pub(crate) use self::range::RangeError; pub(crate) use self::reference::ReferenceError; @@ -78,7 +81,7 @@ impl Error { // This value is used by console.log and other routines to match Object type // to its Javascript Identifier (global constructor method name) this.set_data(ObjectData::Error); - Err(this.clone()) + Ok(this.clone()) } /// `Error.prototype.toString()` @@ -93,8 +96,33 @@ impl Error { /// [mdn]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Error/toString #[allow(clippy::wrong_self_convention)] pub(crate) fn to_string(this: &Value, _: &[Value], context: &mut Context) -> Result { - let name = this.get_field("name").to_string(context)?; - let message = this.get_field("message").to_string(context)?; - Ok(format!("{}: {}", name, message).into()) + if !this.is_object() { + return context.throw_type_error("'this' is not an Object"); + } + let name = this.get_field("name"); + let name_to_string; + let name = if name.is_undefined() { + "Error" + } else { + name_to_string = name.to_string(context)?; + name_to_string.as_str() + }; + + let message = this.get_field("message"); + let message_to_string; + let message = if message.is_undefined() { + "" + } else { + message_to_string = message.to_string(context)?; + message_to_string.as_str() + }; + + if name.is_empty() { + Ok(message.into()) + } else if message.is_empty() { + Ok(name.into()) + } else { + Ok(format!("{}: {}", name, message).into()) + } } } diff --git a/boa/src/builtins/error/range.rs b/boa/src/builtins/error/range.rs index e8dd56e3f7..e4eb1e95bf 100644 --- a/boa/src/builtins/error/range.rs +++ b/boa/src/builtins/error/range.rs @@ -62,6 +62,6 @@ impl RangeError { // This value is used by console.log and other routines to match Object type // to its Javascript Identifier (global constructor method name) this.set_data(ObjectData::Error); - Err(this.clone()) + Ok(this.clone()) } } diff --git a/boa/src/builtins/error/reference.rs b/boa/src/builtins/error/reference.rs index 3144f7a47e..fe74caa9f5 100644 --- a/boa/src/builtins/error/reference.rs +++ b/boa/src/builtins/error/reference.rs @@ -61,6 +61,6 @@ impl ReferenceError { // This value is used by console.log and other routines to match Object type // to its Javascript Identifier (global constructor method name) this.set_data(ObjectData::Error); - Err(this.clone()) + Ok(this.clone()) } } diff --git a/boa/src/builtins/error/syntax.rs b/boa/src/builtins/error/syntax.rs index 55dd588660..1d68b7d170 100644 --- a/boa/src/builtins/error/syntax.rs +++ b/boa/src/builtins/error/syntax.rs @@ -64,6 +64,6 @@ impl SyntaxError { // This value is used by console.log and other routines to match Object type // to its Javascript Identifier (global constructor method name) this.set_data(ObjectData::Error); - Err(this.clone()) + Ok(this.clone()) } } diff --git a/boa/src/builtins/error/tests.rs b/boa/src/builtins/error/tests.rs new file mode 100644 index 0000000000..c0c1d4c997 --- /dev/null +++ b/boa/src/builtins/error/tests.rs @@ -0,0 +1,30 @@ +use crate::{forward, Context}; + +#[test] +fn error_to_string() { + let mut ctx = Context::new(); + let init = r#" + let e = new Error('1'); + let name = new Error(); + let message = new Error('message'); + message.name = ''; + let range_e = new RangeError('2'); + let ref_e = new ReferenceError('3'); + let syntax_e = new SyntaxError('4'); + let type_e = new TypeError('5'); + "#; + forward(&mut ctx, init); + assert_eq!(forward(&mut ctx, "e.toString()"), "\"Error: 1\""); + assert_eq!(forward(&mut ctx, "name.toString()"), "\"Error\""); + assert_eq!(forward(&mut ctx, "message.toString()"), "\"message\""); + assert_eq!(forward(&mut ctx, "range_e.toString()"), "\"RangeError: 2\""); + assert_eq!( + forward(&mut ctx, "ref_e.toString()"), + "\"ReferenceError: 3\"" + ); + assert_eq!( + forward(&mut ctx, "syntax_e.toString()"), + "\"SyntaxError: 4\"" + ); + assert_eq!(forward(&mut ctx, "type_e.toString()"), "\"TypeError: 5\""); +} diff --git a/boa/src/builtins/error/type.rs b/boa/src/builtins/error/type.rs index fc0ec4aa9f..bc2e6f4f14 100644 --- a/boa/src/builtins/error/type.rs +++ b/boa/src/builtins/error/type.rs @@ -67,6 +67,6 @@ impl TypeError { // This value is used by console.log and other routines to match Object type // to its Javascript Identifier (global constructor method name) this.set_data(ObjectData::Error); - Err(this.clone()) + Ok(this.clone()) } } diff --git a/boa/src/builtins/object/tests.rs b/boa/src/builtins/object/tests.rs index a13abf1d9b..9491c08db7 100644 --- a/boa/src/builtins/object/tests.rs +++ b/boa/src/builtins/object/tests.rs @@ -146,6 +146,8 @@ fn object_to_string() { Array.prototype.toString = Object.prototype.toString; let f = () => {}; Function.prototype.toString = Object.prototype.toString; + let e = new Error('test'); + Error.prototype.toString = Object.prototype.toString; let b = Boolean(); Boolean.prototype.toString = Object.prototype.toString; let i = Number(42); @@ -170,6 +172,7 @@ fn object_to_string() { // ); assert_eq!(forward(&mut ctx, "a.toString()"), "\"[object Array]\""); assert_eq!(forward(&mut ctx, "f.toString()"), "\"[object Function]\""); + assert_eq!(forward(&mut ctx, "e.toString()"), "\"[object Error]\""); assert_eq!(forward(&mut ctx, "b.toString()"), "\"[object Boolean]\""); assert_eq!(forward(&mut ctx, "i.toString()"), "\"[object Number]\""); assert_eq!(forward(&mut ctx, "s.toString()"), "\"[object String]\""); diff --git a/boa/src/context.rs b/boa/src/context.rs index 642746934b..d68faacc1e 100644 --- a/boa/src/context.rs +++ b/boa/src/context.rs @@ -293,7 +293,7 @@ impl Context { vec![Const::from(message.into()).into()], )) .run(self) - .expect_err("RangeError should always throw") + .expect("Into used as message") } /// Throws a `RangeError` with the specified message. @@ -315,7 +315,7 @@ impl Context { vec![Const::from(message.into()).into()], )) .run(self) - .expect_err("TypeError should always throw") + .expect("Into used as message") } /// Throws a `TypeError` with the specified message. @@ -336,7 +336,7 @@ impl Context { vec![Const::from(message.into() + " is not defined").into()], )) .run(self) - .expect_err("ReferenceError should always throw") + .expect("Into used as message") } /// Throws a `ReferenceError` with the specified message. @@ -357,7 +357,7 @@ impl Context { vec![Const::from(message.into()).into()], )) .run(self) - .expect_err("SyntaxError should always throw") + .expect("Into used as message") } /// Throws a `SyntaxError` with the specified message.