From 3458b3de17a08f02deedf79b5cde66e25ce22995 Mon Sep 17 00:00:00 2001 From: RageKnify Date: Thu, 1 Oct 2020 12:22:36 +0100 Subject: [PATCH 1/8] Fix: Make SyntaxError.toString use to_string SyntaxError.toString was using Value::display while all other error types use Value::to_string --- boa/src/builtins/error/syntax.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/boa/src/builtins/error/syntax.rs b/boa/src/builtins/error/syntax.rs index 086b558def8..009189f2dc4 100644 --- a/boa/src/builtins/error/syntax.rs +++ b/boa/src/builtins/error/syntax.rs @@ -52,11 +52,11 @@ impl SyntaxError { /// [spec]: https://tc39.es/ecma262/#sec-error.prototype.tostring /// [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], _: &mut Context) -> Result { - let name = this.get_field("name"); - let message = this.get_field("message"); - // FIXME: This should not use `.display()` - Ok(format!("{}: {}", name.display(), message.display()).into()) + pub(crate) fn to_string(this: &Value, _: &[Value], ctx: &mut Context) -> Result { + let name = this.get_field("name").to_string(ctx)?; + let message = this.get_field("message").to_string(ctx)?; + + Ok(Value::from(format!("{}: {}", name, message))) } /// Initialise the global object with the `SyntaxError` object. From be659d9109ef9290d90409e0231a1a22a31d3a7d Mon Sep 17 00:00:00 2001 From: RageKnify Date: Thu, 1 Oct 2020 19:05:31 +0100 Subject: [PATCH 2/8] Refactor: Error constructors to return instead of "throwing" Introduces some changes accross files to accommodate for the change Fix: Correct Error.prototype.toString to use Value::to_string rather than Value::Display Test: Add a test case for Error Objects in Object.prototype.toString --- boa/src/builtins/bigint/mod.rs | 2 +- boa/src/builtins/boolean/mod.rs | 2 +- boa/src/builtins/date/mod.rs | 2 +- boa/src/builtins/error/mod.rs | 15 ++++++--------- 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/type.rs | 2 +- boa/src/builtins/map/mod.rs | 20 ++++++++++---------- boa/src/builtins/number/mod.rs | 2 +- boa/src/builtins/object/tests.rs | 3 +++ boa/src/builtins/string/mod.rs | 2 +- boa/src/builtins/symbol/mod.rs | 2 +- boa/src/context.rs | 20 ++++++++------------ boa/src/exec/identifier/mod.rs | 6 +++++- boa/src/exec/operator/mod.rs | 6 +++++- boa/src/value/mod.rs | 24 ++++++++++++------------ 17 files changed, 59 insertions(+), 55 deletions(-) diff --git a/boa/src/builtins/bigint/mod.rs b/boa/src/builtins/bigint/mod.rs index 3fa59a5162f..2ab92fb81ae 100644 --- a/boa/src/builtins/bigint/mod.rs +++ b/boa/src/builtins/bigint/mod.rs @@ -75,7 +75,7 @@ impl BigInt { } // 3. Throw a TypeError exception. - Err(ctx.construct_type_error("'this' is not a BigInt")) + Err(ctx.construct_type_error("'this' is not a BigInt")?) } /// `BigInt()` diff --git a/boa/src/builtins/boolean/mod.rs b/boa/src/builtins/boolean/mod.rs index 7ccb02b72b6..0e7a71e8674 100644 --- a/boa/src/builtins/boolean/mod.rs +++ b/boa/src/builtins/boolean/mod.rs @@ -44,7 +44,7 @@ impl Boolean { _ => {} } - Err(ctx.construct_type_error("'this' is not a boolean")) + Err(ctx.construct_type_error("'this' is not a boolean")?) } /// `[[Construct]]` Create a new boolean object diff --git a/boa/src/builtins/date/mod.rs b/boa/src/builtins/date/mod.rs index 9a6779aa903..ddb5816fd7c 100644 --- a/boa/src/builtins/date/mod.rs +++ b/boa/src/builtins/date/mod.rs @@ -1584,5 +1584,5 @@ pub fn this_time_value(value: &Value, ctx: &mut Context) -> Result { return Ok(*date); } } - Err(ctx.construct_type_error("'this' is not a Date")) + Err(ctx.construct_type_error("'this' is not a Date")?) } diff --git a/boa/src/builtins/error/mod.rs b/boa/src/builtins/error/mod.rs index 5321b62c7cf..a1f61137dc8 100644 --- a/boa/src/builtins/error/mod.rs +++ b/boa/src/builtins/error/mod.rs @@ -51,7 +51,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()` @@ -65,14 +65,11 @@ impl Error { /// [spec]: https://tc39.es/ecma262/#sec-error.prototype.tostring /// [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], _: &mut Context) -> Result { - let name = this.get_field("name"); - let message = this.get_field("message"); - Ok(Value::from(format!( - "{}: {}", - name.display(), - message.display() - ))) + pub(crate) fn to_string(this: &Value, _: &[Value], ctx: &mut Context) -> Result { + let name = this.get_field("name").to_string(ctx)?; + let message = this.get_field("message").to_string(ctx)?; + + Ok(Value::from(format!("{}: {}", name, message))) } /// Initialise the global object with the `Error` object. diff --git a/boa/src/builtins/error/range.rs b/boa/src/builtins/error/range.rs index 9fea28d9a1c..4c07dba9184 100644 --- a/boa/src/builtins/error/range.rs +++ b/boa/src/builtins/error/range.rs @@ -36,7 +36,7 @@ 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()) } /// `Error.prototype.toString()` diff --git a/boa/src/builtins/error/reference.rs b/boa/src/builtins/error/reference.rs index 144e055fc02..a7fa4608aed 100644 --- a/boa/src/builtins/error/reference.rs +++ b/boa/src/builtins/error/reference.rs @@ -35,7 +35,7 @@ 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()) } /// `Error.prototype.toString()` diff --git a/boa/src/builtins/error/syntax.rs b/boa/src/builtins/error/syntax.rs index 009189f2dc4..45c831a1bd0 100644 --- a/boa/src/builtins/error/syntax.rs +++ b/boa/src/builtins/error/syntax.rs @@ -38,7 +38,7 @@ 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()) } /// `Error.prototype.toString()` diff --git a/boa/src/builtins/error/type.rs b/boa/src/builtins/error/type.rs index e2e301c12f2..2d8047f7a01 100644 --- a/boa/src/builtins/error/type.rs +++ b/boa/src/builtins/error/type.rs @@ -41,7 +41,7 @@ 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()) } /// `Error.prototype.toString()` diff --git a/boa/src/builtins/map/mod.rs b/boa/src/builtins/map/mod.rs index 98918556c55..586a7488730 100644 --- a/boa/src/builtins/map/mod.rs +++ b/boa/src/builtins/map/mod.rs @@ -53,10 +53,10 @@ impl Map { map.insert(key, value); map.len() } else { - return Err(ctx.construct_type_error("'this' is not a Map")); + return Err(ctx.construct_type_error("'this' is not a Map")?); } } else { - return Err(ctx.construct_type_error("'this' is not a Map")); + return Err(ctx.construct_type_error("'this' is not a Map")?); }; Self::set_size(this, size); @@ -86,10 +86,10 @@ impl Map { let deleted = map.remove(key).is_some(); (deleted, map.len()) } else { - return Err(ctx.construct_type_error("'this' is not a Map")); + return Err(ctx.construct_type_error("'this' is not a Map")?); } } else { - return Err(ctx.construct_type_error("'this' is not a Map")); + return Err(ctx.construct_type_error("'this' is not a Map")?); }; Self::set_size(this, size); Ok(deleted.into()) @@ -123,7 +123,7 @@ impl Map { } } - Err(ctx.construct_type_error("'this' is not a Map")) + Err(ctx.construct_type_error("'this' is not a Map")?) } /// `Map.prototype.clear( )` @@ -168,7 +168,7 @@ impl Map { } } - Err(ctx.construct_type_error("'this' is not a Map")) + Err(ctx.construct_type_error("'this' is not a Map")?) } /// `Map.prototype.forEach( callbackFn [ , thisArg ] )` @@ -253,6 +253,7 @@ impl Map { ctx.construct_type_error( "iterable for Map should have array-like objects", ) + .expect("&str used as message") })?; map.insert(key, value); } @@ -260,13 +261,12 @@ impl Map { } else { return Err(ctx.construct_type_error( "iterable for Map should have array-like objects", - )); + )?); } } _ => { - return Err( - ctx.construct_type_error("iterable for Map should have array-like objects") - ) + return Err(ctx + .construct_type_error("iterable for Map should have array-like objects")?) } }, }; diff --git a/boa/src/builtins/number/mod.rs b/boa/src/builtins/number/mod.rs index 2a92f63d10a..598bfe65a0d 100644 --- a/boa/src/builtins/number/mod.rs +++ b/boa/src/builtins/number/mod.rs @@ -114,7 +114,7 @@ impl Number { _ => {} } - Err(ctx.construct_type_error("'this' is not a number")) + Err(ctx.construct_type_error("'this' is not a number")?) } /// Helper function that formats a float as a ES6-style exponential number string. diff --git a/boa/src/builtins/object/tests.rs b/boa/src/builtins/object/tests.rs index 5c42f85fc93..096108a0698 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/builtins/string/mod.rs b/boa/src/builtins/string/mod.rs index b69487e23f2..702121059db 100644 --- a/boa/src/builtins/string/mod.rs +++ b/boa/src/builtins/string/mod.rs @@ -57,7 +57,7 @@ impl String { _ => {} } - Err(ctx.construct_type_error("'this' is not a string")) + Err(ctx.construct_type_error("'this' is not a string")?) } /// [[Construct]] - Creates a new instance `this` diff --git a/boa/src/builtins/symbol/mod.rs b/boa/src/builtins/symbol/mod.rs index 3eae3a8d610..d92bcbf3ead 100644 --- a/boa/src/builtins/symbol/mod.rs +++ b/boa/src/builtins/symbol/mod.rs @@ -261,7 +261,7 @@ impl Symbol { _ => {} } - Err(ctx.construct_type_error("'this' is not a Symbol")) + Err(ctx.construct_type_error("'this' is not a Symbol")?) } /// The `Symbol()` constructor returns a value of type symbol. diff --git a/boa/src/context.rs b/boa/src/context.rs index cb2b6abf9f6..36da57cb71d 100644 --- a/boa/src/context.rs +++ b/boa/src/context.rs @@ -149,7 +149,7 @@ impl Context { } /// Constructs a `RangeError` with the specified message. - pub fn construct_range_error(&mut self, message: M) -> Value + pub fn construct_range_error(&mut self, message: M) -> Result where M: Into, { @@ -159,7 +159,6 @@ impl Context { vec![Const::from(message.into()).into()], )) .run(self) - .expect_err("RangeError should always throw") } /// Throws a `RangeError` with the specified message. @@ -167,11 +166,11 @@ impl Context { where M: Into, { - Err(self.construct_range_error(message)) + Err(self.construct_range_error(message)?) } /// Constructs a `TypeError` with the specified message. - pub fn construct_type_error(&mut self, message: M) -> Value + pub fn construct_type_error(&mut self, message: M) -> Result where M: Into, { @@ -181,7 +180,6 @@ impl Context { vec![Const::from(message.into()).into()], )) .run(self) - .expect_err("TypeError should always throw") } /// Throws a `TypeError` with the specified message. @@ -189,11 +187,11 @@ impl Context { where M: Into, { - Err(self.construct_type_error(message)) + Err(self.construct_type_error(message)?) } /// Constructs a `ReferenceError` with the specified message. - pub fn construct_reference_error(&mut self, message: M) -> Value + pub fn construct_reference_error(&mut self, message: M) -> Result where M: Into, { @@ -202,7 +200,6 @@ impl Context { vec![Const::from(message.into() + " is not defined").into()], )) .run(self) - .expect_err("ReferenceError should always throw") } /// Throws a `ReferenceError` with the specified message. @@ -210,11 +207,11 @@ impl Context { where M: Into, { - Err(self.construct_reference_error(message)) + Err(self.construct_reference_error(message)?) } /// Constructs a `SyntaxError` with the specified message. - pub fn construct_syntax_error(&mut self, message: M) -> Value + pub fn construct_syntax_error(&mut self, message: M) -> Result where M: Into, { @@ -223,7 +220,6 @@ impl Context { vec![Const::from(message.into()).into()], )) .run(self) - .expect_err("SyntaxError should always throw") } /// Throws a `SyntaxError` with the specified message. @@ -231,7 +227,7 @@ impl Context { where M: Into, { - Err(self.construct_syntax_error(message)) + Err(self.construct_syntax_error(message)?) } /// Utility to create a function Value for Function Declarations, Arrow Functions or Function Expressions diff --git a/boa/src/exec/identifier/mod.rs b/boa/src/exec/identifier/mod.rs index 6b113acc57b..6f8b99b5d10 100644 --- a/boa/src/exec/identifier/mod.rs +++ b/boa/src/exec/identifier/mod.rs @@ -7,6 +7,10 @@ impl Executable for Identifier { .realm() .environment .get_binding_value(self.as_ref()) - .ok_or_else(|| interpreter.construct_reference_error(self.as_ref())) + .ok_or_else(|| { + interpreter + .construct_reference_error(self.as_ref()) + .expect("Identifer used as message") + }) } } diff --git a/boa/src/exec/operator/mod.rs b/boa/src/exec/operator/mod.rs index abb07ae9d27..9bf218f23b1 100644 --- a/boa/src/exec/operator/mod.rs +++ b/boa/src/exec/operator/mod.rs @@ -130,7 +130,11 @@ impl Executable for BinOp { .realm() .environment .get_binding_value(name.as_ref()) - .ok_or_else(|| interpreter.construct_reference_error(name.as_ref()))?; + .ok_or_else(|| { + interpreter + .construct_reference_error(name.as_ref()) + .expect("Identifer used as message") + })?; let v_b = self.rhs().run(interpreter)?; let value = Self::run_assign(op, v_a, v_b, interpreter)?; interpreter.realm_mut().environment.set_mutable_binding( diff --git a/boa/src/value/mod.rs b/boa/src/value/mod.rs index cb3d53e3628..76df9d3568c 100644 --- a/boa/src/value/mod.rs +++ b/boa/src/value/mod.rs @@ -260,7 +260,7 @@ impl Value { .unwrap_or(JSONValue::Null)), Self::Integer(val) => Ok(JSONValue::Number(JSONNumber::from(val))), Self::BigInt(_) => { - Err(interpreter.construct_type_error("BigInt value can't be serialized in JSON")) + Err(interpreter.construct_type_error("BigInt value can't be serialized in JSON")?) } Self::Symbol(_) | Self::Undefined => { unreachable!("Symbols and Undefined JSON Values depend on parent type"); @@ -595,9 +595,9 @@ impl Value { /// This function is equivelent to `BigInt(value)` in JavaScript. pub fn to_bigint(&self, ctx: &mut Context) -> Result { match self { - Value::Null => Err(ctx.construct_type_error("cannot convert null to a BigInt")), + Value::Null => Err(ctx.construct_type_error("cannot convert null to a BigInt")?), Value::Undefined => { - Err(ctx.construct_type_error("cannot convert undefined to a BigInt")) + Err(ctx.construct_type_error("cannot convert undefined to a BigInt")?) } Value::String(ref string) => Ok(RcBigInt::from(BigInt::from_string(string, ctx)?)), Value::Boolean(true) => Ok(RcBigInt::from(BigInt::from(1))), @@ -610,14 +610,14 @@ impl Value { Err(ctx.construct_type_error(format!( "The number {} cannot be converted to a BigInt because it is not an integer", num - ))) + ))?) } Value::BigInt(b) => Ok(b.clone()), Value::Object(_) => { let primitive = self.to_primitive(ctx, PreferredType::Number)?; primitive.to_bigint(ctx) } - Value::Symbol(_) => Err(ctx.construct_type_error("cannot convert Symbol to a BigInt")), + Value::Symbol(_) => Err(ctx.construct_type_error("cannot convert Symbol to a BigInt")?), } } @@ -648,7 +648,7 @@ impl Value { Value::Rational(rational) => Ok(Number::to_native_string(*rational).into()), Value::Integer(integer) => Ok(integer.to_string().into()), Value::String(string) => Ok(string.clone()), - Value::Symbol(_) => Err(ctx.construct_type_error("can't convert symbol to string")), + Value::Symbol(_) => Err(ctx.construct_type_error("can't convert symbol to string")?), Value::BigInt(ref bigint) => Ok(bigint.to_string().into()), Value::Object(_) => { let primitive = self.to_primitive(ctx, PreferredType::String)?; @@ -665,7 +665,7 @@ impl Value { pub fn to_object(&self, ctx: &mut Context) -> Result { match self { Value::Undefined | Value::Null => { - Err(ctx.construct_type_error("cannot convert 'null' or 'undefined' to object")) + Err(ctx.construct_type_error("cannot convert 'null' or 'undefined' to object")?) } Value::Boolean(boolean) => { let proto = ctx @@ -815,11 +815,11 @@ impl Value { let integer_index = self.to_integer(ctx)?; if integer_index < 0.0 { - return Err(ctx.construct_range_error("Integer index must be >= 0")); + return Err(ctx.construct_range_error("Integer index must be >= 0")?); } if integer_index > Number::MAX_SAFE_INTEGER { - return Err(ctx.construct_range_error("Integer index must be less than 2**(53) - 1")); + return Err(ctx.construct_range_error("Integer index must be less than 2**(53) - 1")?); } Ok(integer_index as usize) @@ -882,8 +882,8 @@ impl Value { } Value::Rational(number) => Ok(number), Value::Integer(integer) => Ok(f64::from(integer)), - Value::Symbol(_) => Err(ctx.construct_type_error("argument must not be a symbol")), - Value::BigInt(_) => Err(ctx.construct_type_error("argument must not be a bigint")), + Value::Symbol(_) => Err(ctx.construct_type_error("argument must not be a symbol")?), + Value::BigInt(_) => Err(ctx.construct_type_error("argument must not be a bigint")?), Value::Object(_) => { let primitive = self.to_primitive(ctx, PreferredType::Number)?; primitive.to_number(ctx) @@ -918,7 +918,7 @@ impl Value { #[inline] pub fn require_object_coercible<'a>(&'a self, ctx: &mut Context) -> Result<&'a Value> { if self.is_null_or_undefined() { - Err(ctx.construct_type_error("cannot convert null or undefined to Object")) + Err(ctx.construct_type_error("cannot convert null or undefined to Object")?) } else { Ok(self) } From 02cd2f83ae8d19b2578279bb36e50220b8cf89ba Mon Sep 17 00:00:00 2001 From: RageKnify Date: Fri, 2 Oct 2020 00:19:25 +0100 Subject: [PATCH 3/8] Fix: Make Error.prototype.toString spec compliant Noted by @HalidOdat in boa-dev/boa#749 --- boa/src/builtins/error/mod.rs | 30 ++++++++++++++++++++++++++--- boa/src/builtins/error/range.rs | 30 ++++++++++++++++++++++++++--- boa/src/builtins/error/reference.rs | 30 ++++++++++++++++++++++++++--- boa/src/builtins/error/syntax.rs | 30 ++++++++++++++++++++++++++--- boa/src/builtins/error/type.rs | 30 ++++++++++++++++++++++++++--- 5 files changed, 135 insertions(+), 15 deletions(-) diff --git a/boa/src/builtins/error/mod.rs b/boa/src/builtins/error/mod.rs index a1f61137dc8..8d36fa84284 100644 --- a/boa/src/builtins/error/mod.rs +++ b/boa/src/builtins/error/mod.rs @@ -66,10 +66,34 @@ 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], ctx: &mut Context) -> Result { - let name = this.get_field("name").to_string(ctx)?; - let message = this.get_field("message").to_string(ctx)?; + if !this.is_object() { + return ctx.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(ctx)?; + 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(ctx)?; + message_to_string.as_str() + }; - Ok(Value::from(format!("{}: {}", name, message))) + if name == "" { + Ok(Value::from(message)) + } else if message == "" { + Ok(Value::from(name)) + } else { + Ok(Value::from(format!("{}: {}", name, message))) + } } /// Initialise the global object with the `Error` object. diff --git a/boa/src/builtins/error/range.rs b/boa/src/builtins/error/range.rs index 4c07dba9184..989ef20be11 100644 --- a/boa/src/builtins/error/range.rs +++ b/boa/src/builtins/error/range.rs @@ -51,10 +51,34 @@ impl RangeError { /// [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], ctx: &mut Context) -> Result { - let name = this.get_field("name").to_string(ctx)?; - let message = this.get_field("message").to_string(ctx)?; + if !this.is_object() { + return ctx.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(ctx)?; + 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(ctx)?; + message_to_string.as_str() + }; - Ok(Value::from(format!("{}: {}", name, message))) + if name == "" { + Ok(Value::from(message)) + } else if message == "" { + Ok(Value::from(name)) + } else { + Ok(Value::from(format!("{}: {}", name, message))) + } } /// Initialise the global object with the `RangeError` object. diff --git a/boa/src/builtins/error/reference.rs b/boa/src/builtins/error/reference.rs index a7fa4608aed..9a44b8928d1 100644 --- a/boa/src/builtins/error/reference.rs +++ b/boa/src/builtins/error/reference.rs @@ -50,10 +50,34 @@ impl ReferenceError { /// [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], ctx: &mut Context) -> Result { - let name = this.get_field("name").to_string(ctx)?; - let message = this.get_field("message").to_string(ctx)?; + if !this.is_object() { + return ctx.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(ctx)?; + 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(ctx)?; + message_to_string.as_str() + }; - Ok(Value::from(format!("{}: {}", name, message))) + if name == "" { + Ok(Value::from(message)) + } else if message == "" { + Ok(Value::from(name)) + } else { + Ok(Value::from(format!("{}: {}", name, message))) + } } /// Initialise the global object with the `ReferenceError` object. diff --git a/boa/src/builtins/error/syntax.rs b/boa/src/builtins/error/syntax.rs index 45c831a1bd0..72525ecc095 100644 --- a/boa/src/builtins/error/syntax.rs +++ b/boa/src/builtins/error/syntax.rs @@ -53,10 +53,34 @@ impl SyntaxError { /// [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], ctx: &mut Context) -> Result { - let name = this.get_field("name").to_string(ctx)?; - let message = this.get_field("message").to_string(ctx)?; + if !this.is_object() { + return ctx.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(ctx)?; + 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(ctx)?; + message_to_string.as_str() + }; - Ok(Value::from(format!("{}: {}", name, message))) + if name == "" { + Ok(Value::from(message)) + } else if message == "" { + Ok(Value::from(name)) + } else { + Ok(Value::from(format!("{}: {}", name, message))) + } } /// Initialise the global object with the `SyntaxError` object. diff --git a/boa/src/builtins/error/type.rs b/boa/src/builtins/error/type.rs index 2d8047f7a01..519fe8627f0 100644 --- a/boa/src/builtins/error/type.rs +++ b/boa/src/builtins/error/type.rs @@ -56,10 +56,34 @@ impl TypeError { /// [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], ctx: &mut Context) -> Result { - let name = this.get_field("name").to_string(ctx)?; - let message = this.get_field("message").to_string(ctx)?; + if !this.is_object() { + return ctx.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(ctx)?; + 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(ctx)?; + message_to_string.as_str() + }; - Ok(Value::from(format!("{}: {}", name, message))) + if name == "" { + Ok(Value::from(message)) + } else if message == "" { + Ok(Value::from(name)) + } else { + Ok(Value::from(format!("{}: {}", name, message))) + } } /// Initialise the global object with the `RangeError` object. From 21526fb0f4528f79e3fe75ba1f7774d6772aa3f9 Mon Sep 17 00:00:00 2001 From: RageKnify Date: Fri, 2 Oct 2020 00:51:13 +0100 Subject: [PATCH 4/8] Test: Add a test for Error.toString --- boa/src/builtins/error/mod.rs | 3 +++ boa/src/builtins/error/tests.rs | 25 +++++++++++++++++++++++++ 2 files changed, 28 insertions(+) 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 8d36fa84284..0096497a26d 100644 --- a/boa/src/builtins/error/mod.rs +++ b/boa/src/builtins/error/mod.rs @@ -24,6 +24,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; diff --git a/boa/src/builtins/error/tests.rs b/boa/src/builtins/error/tests.rs new file mode 100644 index 00000000000..39d4b9c0c6c --- /dev/null +++ b/boa/src/builtins/error/tests.rs @@ -0,0 +1,25 @@ +use crate::{forward, Context}; + +#[test] +fn error_to_string() { + let mut ctx = Context::new(); + let init = r#" + let e = new Error('1'); + 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, "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\""); +} From cc9f366b9e6e5954a6c99a282bf58e3cd6cbe456 Mon Sep 17 00:00:00 2001 From: RageKnify Date: Sun, 4 Oct 2020 20:05:14 +0100 Subject: [PATCH 5/8] Refactor: Use expect rather than ? where valid --- boa/src/builtins/bigint/mod.rs | 4 ++- boa/src/builtins/boolean/mod.rs | 4 ++- boa/src/builtins/date/mod.rs | 4 ++- boa/src/builtins/error/mod.rs | 8 ++--- boa/src/builtins/map/mod.rs | 33 +++++++++++++------ boa/src/builtins/number/mod.rs | 4 ++- boa/src/builtins/string/mod.rs | 4 ++- boa/src/builtins/symbol/mod.rs | 4 ++- boa/src/value/mod.rs | 56 ++++++++++++++++++++++----------- 9 files changed, 82 insertions(+), 39 deletions(-) diff --git a/boa/src/builtins/bigint/mod.rs b/boa/src/builtins/bigint/mod.rs index 0dd092d7385..c4513fe643e 100644 --- a/boa/src/builtins/bigint/mod.rs +++ b/boa/src/builtins/bigint/mod.rs @@ -120,7 +120,9 @@ impl BigInt { } // 3. Throw a TypeError exception. - Err(ctx.construct_type_error("'this' is not a BigInt")?) + Err(ctx + .construct_type_error("'this' is not a BigInt") + .expect("&str used as message")) } /// `BigInt.prototype.toString( [radix] )` diff --git a/boa/src/builtins/boolean/mod.rs b/boa/src/builtins/boolean/mod.rs index 98456b8d327..a772492f2fe 100644 --- a/boa/src/builtins/boolean/mod.rs +++ b/boa/src/builtins/boolean/mod.rs @@ -82,7 +82,9 @@ impl Boolean { _ => {} } - Err(ctx.construct_type_error("'this' is not a boolean")?) + Err(ctx + .construct_type_error("'this' is not a boolean") + .expect("&str used as message")) } /// The `toString()` method returns a string representing the specified `Boolean` object. diff --git a/boa/src/builtins/date/mod.rs b/boa/src/builtins/date/mod.rs index bd956371d1e..295a03540f1 100644 --- a/boa/src/builtins/date/mod.rs +++ b/boa/src/builtins/date/mod.rs @@ -1337,5 +1337,7 @@ pub fn this_time_value(value: &Value, ctx: &mut Context) -> Result { return Ok(*date); } } - Err(ctx.construct_type_error("'this' is not a Date")?) + Err(ctx + .construct_type_error("'this' is not a Date") + .expect("&str used as message")) } diff --git a/boa/src/builtins/error/mod.rs b/boa/src/builtins/error/mod.rs index 15524f385e5..8910cea12af 100644 --- a/boa/src/builtins/error/mod.rs +++ b/boa/src/builtins/error/mod.rs @@ -95,16 +95,16 @@ impl Error { /// [spec]: https://tc39.es/ecma262/#sec-error.prototype.tostring /// [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], ctx: &mut Context) -> Result { + pub(crate) fn to_string(this: &Value, _: &[Value], context: &mut Context) -> Result { if !this.is_object() { - return ctx.throw_type_error("'this' is not an 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(ctx)?; + name_to_string = name.to_string(context)?; name_to_string.as_str() }; @@ -113,7 +113,7 @@ impl Error { let message = if message.is_undefined() { "" } else { - message_to_string = message.to_string(ctx)?; + message_to_string = message.to_string(context)?; message_to_string.as_str() }; diff --git a/boa/src/builtins/map/mod.rs b/boa/src/builtins/map/mod.rs index 1d34e71b84d..fec82e469d3 100644 --- a/boa/src/builtins/map/mod.rs +++ b/boa/src/builtins/map/mod.rs @@ -78,14 +78,15 @@ impl Map { } map } else { - return Err(ctx.construct_type_error( - "iterable for Map should have array-like objects", - )?); + return Err(ctx + .construct_type_error("iterable for Map should have array-like objects") + .expect("&str used as message")); } } _ => { return Err(ctx - .construct_type_error("iterable for Map should have array-like objects")?) + .construct_type_error("iterable for Map should have array-like objects") + .expect("&str used as message")) } }, }; @@ -131,10 +132,14 @@ impl Map { map.insert(key, value); map.len() } else { - return Err(ctx.construct_type_error("'this' is not a Map")?); + return Err(ctx + .construct_type_error("'this' is not a Map") + .expect("&str used as message")); } } else { - return Err(ctx.construct_type_error("'this' is not a Map")?); + return Err(ctx + .construct_type_error("'this' is not a Map") + .expect("&str used as message")); }; Self::set_size(this, size); @@ -164,10 +169,14 @@ impl Map { let deleted = map.remove(key).is_some(); (deleted, map.len()) } else { - return Err(ctx.construct_type_error("'this' is not a Map")?); + return Err(ctx + .construct_type_error("'this' is not a Map") + .expect("&str used as message")); } } else { - return Err(ctx.construct_type_error("'this' is not a Map")?); + return Err(ctx + .construct_type_error("'this' is not a Map") + .expect("&str used as message")); }; Self::set_size(this, size); Ok(deleted.into()) @@ -201,7 +210,9 @@ impl Map { } } - Err(ctx.construct_type_error("'this' is not a Map")?) + Err(ctx + .construct_type_error("'this' is not a Map") + .expect("&str used as message")) } /// `Map.prototype.clear( )` @@ -246,7 +257,9 @@ impl Map { } } - Err(ctx.construct_type_error("'this' is not a Map")?) + Err(ctx + .construct_type_error("'this' is not a Map") + .expect("&str used as message")) } /// `Map.prototype.forEach( callbackFn [ , thisArg ] )` diff --git a/boa/src/builtins/number/mod.rs b/boa/src/builtins/number/mod.rs index acb340c9d45..12240f53a29 100644 --- a/boa/src/builtins/number/mod.rs +++ b/boa/src/builtins/number/mod.rs @@ -184,7 +184,9 @@ impl Number { _ => {} } - Err(ctx.construct_type_error("'this' is not a number")?) + Err(ctx + .construct_type_error("'this' is not a number") + .expect("&str used as message")) } /// Helper function that formats a float as a ES6-style exponential number string. diff --git a/boa/src/builtins/string/mod.rs b/boa/src/builtins/string/mod.rs index b390c86bfa9..1b973c52160 100644 --- a/boa/src/builtins/string/mod.rs +++ b/boa/src/builtins/string/mod.rs @@ -156,7 +156,9 @@ impl String { _ => {} } - Err(ctx.construct_type_error("'this' is not a string")?) + Err(ctx + .construct_type_error("'this' is not a string") + .expect("&str used as message")) } /// Get the string value to a primitive string diff --git a/boa/src/builtins/symbol/mod.rs b/boa/src/builtins/symbol/mod.rs index 14d9d2d0a1a..16909da16c9 100644 --- a/boa/src/builtins/symbol/mod.rs +++ b/boa/src/builtins/symbol/mod.rs @@ -336,7 +336,9 @@ impl Symbol { _ => {} } - Err(ctx.construct_type_error("'this' is not a Symbol")?) + Err(ctx + .construct_type_error("'this' is not a Symbol") + .expect("&str used as message")) } /// `Symbol.prototype.toString()` diff --git a/boa/src/value/mod.rs b/boa/src/value/mod.rs index 1b7016d132b..fcf64b44ee5 100644 --- a/boa/src/value/mod.rs +++ b/boa/src/value/mod.rs @@ -256,9 +256,9 @@ impl Value { .map(JSONValue::Number) .unwrap_or(JSONValue::Null)), Self::Integer(val) => Ok(JSONValue::Number(JSONNumber::from(val))), - Self::BigInt(_) => { - Err(interpreter.construct_type_error("BigInt value can't be serialized in JSON")?) - } + Self::BigInt(_) => Err(interpreter + .construct_type_error("BigInt value can't be serialized in JSON") + .expect("&str used as message")), Self::Symbol(_) | Self::Undefined => { unreachable!("Symbols and Undefined JSON Values depend on parent type"); } @@ -615,10 +615,12 @@ impl Value { /// This function is equivelent to `BigInt(value)` in JavaScript. pub fn to_bigint(&self, ctx: &mut Context) -> Result { match self { - Value::Null => Err(ctx.construct_type_error("cannot convert null to a BigInt")?), - Value::Undefined => { - Err(ctx.construct_type_error("cannot convert undefined to a BigInt")?) - } + Value::Null => Err(ctx + .construct_type_error("cannot convert null to a BigInt") + .expect("&str used as message")), + Value::Undefined => Err(ctx + .construct_type_error("cannot convert undefined to a BigInt") + .expect("&str used as message")), Value::String(ref string) => Ok(RcBigInt::from(BigInt::from_string(string, ctx)?)), Value::Boolean(true) => Ok(RcBigInt::from(BigInt::from(1))), Value::Boolean(false) => Ok(RcBigInt::from(BigInt::from(0))), @@ -627,17 +629,21 @@ impl Value { if let Ok(bigint) = BigInt::try_from(*num) { return Ok(RcBigInt::from(bigint)); } - Err(ctx.construct_type_error(format!( + Err(ctx + .construct_type_error(format!( "The number {} cannot be converted to a BigInt because it is not an integer", num - ))?) + )) + .expect("String used as message")) } Value::BigInt(b) => Ok(b.clone()), Value::Object(_) => { let primitive = self.to_primitive(ctx, PreferredType::Number)?; primitive.to_bigint(ctx) } - Value::Symbol(_) => Err(ctx.construct_type_error("cannot convert Symbol to a BigInt")?), + Value::Symbol(_) => Err(ctx + .construct_type_error("cannot convert Symbol to a BigInt") + .expect("&str used as message")), } } @@ -668,7 +674,9 @@ impl Value { Value::Rational(rational) => Ok(Number::to_native_string(*rational).into()), Value::Integer(integer) => Ok(integer.to_string().into()), Value::String(string) => Ok(string.clone()), - Value::Symbol(_) => Err(ctx.construct_type_error("can't convert symbol to string")?), + Value::Symbol(_) => Err(ctx + .construct_type_error("can't convert symbol to string") + .expect("&str used as message")), Value::BigInt(ref bigint) => Ok(bigint.to_string().into()), Value::Object(_) => { let primitive = self.to_primitive(ctx, PreferredType::String)?; @@ -684,9 +692,9 @@ impl Value { /// See: pub fn to_object(&self, ctx: &mut Context) -> Result { match self { - Value::Undefined | Value::Null => { - Err(ctx.construct_type_error("cannot convert 'null' or 'undefined' to object")?) - } + Value::Undefined | Value::Null => Err(ctx + .construct_type_error("cannot convert 'null' or 'undefined' to object") + .expect("&str used as message")), Value::Boolean(boolean) => { let prototype = ctx.standard_objects().boolean_object().prototype(); Ok(GcObject::new(Object::with_prototype( @@ -802,11 +810,15 @@ impl Value { let integer_index = self.to_integer(ctx)?; if integer_index < 0.0 { - return Err(ctx.construct_range_error("Integer index must be >= 0")?); + return Err(ctx + .construct_range_error("Integer index must be >= 0") + .expect("&str used as message")); } if integer_index > Number::MAX_SAFE_INTEGER { - return Err(ctx.construct_range_error("Integer index must be less than 2**(53) - 1")?); + return Err(ctx + .construct_range_error("Integer index must be less than 2**(53) - 1") + .expect("&str used as message")); } Ok(integer_index as usize) @@ -869,8 +881,12 @@ impl Value { } Value::Rational(number) => Ok(number), Value::Integer(integer) => Ok(f64::from(integer)), - Value::Symbol(_) => Err(ctx.construct_type_error("argument must not be a symbol")?), - Value::BigInt(_) => Err(ctx.construct_type_error("argument must not be a bigint")?), + Value::Symbol(_) => Err(ctx + .construct_type_error("argument must not be a symbol") + .expect("&str used as message")), + Value::BigInt(_) => Err(ctx + .construct_type_error("argument must not be a bigint") + .expect("&str used as message")), Value::Object(_) => { let primitive = self.to_primitive(ctx, PreferredType::Number)?; primitive.to_number(ctx) @@ -905,7 +921,9 @@ impl Value { #[inline] pub fn require_object_coercible(&self, ctx: &mut Context) -> Result<&Value> { if self.is_null_or_undefined() { - Err(ctx.construct_type_error("cannot convert null or undefined to Object")?) + Err(ctx + .construct_type_error("cannot convert null or undefined to Object") + .expect("&str used as message")) } else { Ok(self) } From 10dc0f1d20c91a8e1020e8bd40127cb0c4fdddf4 Mon Sep 17 00:00:00 2001 From: RageKnify Date: Sun, 4 Oct 2020 20:49:41 +0100 Subject: [PATCH 6/8] Test: Add 2 test cases for Error.prototype.toString --- boa/src/builtins/error/tests.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/boa/src/builtins/error/tests.rs b/boa/src/builtins/error/tests.rs index 39d4b9c0c6c..c0c1d4c9970 100644 --- a/boa/src/builtins/error/tests.rs +++ b/boa/src/builtins/error/tests.rs @@ -5,6 +5,9 @@ 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'); @@ -12,6 +15,8 @@ fn error_to_string() { "#; 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()"), From 8975e5550981270b83e2716e013d7f52603b8f81 Mon Sep 17 00:00:00 2001 From: RageKnify Date: Mon, 5 Oct 2020 00:33:38 +0100 Subject: [PATCH 7/8] Refactor: Change construct_$TYPE_error's return back to Value Discussed in boa-dev/boa#749 --- boa/src/builtins/array/array_iterator.rs | 6 +- boa/src/builtins/bigint/mod.rs | 4 +- boa/src/builtins/boolean/mod.rs | 4 +- boa/src/builtins/date/mod.rs | 4 +- boa/src/builtins/iterable/mod.rs | 15 +---- boa/src/builtins/map/mod.rs | 37 ++++-------- boa/src/builtins/number/mod.rs | 4 +- boa/src/builtins/string/mod.rs | 4 +- boa/src/builtins/symbol/mod.rs | 4 +- boa/src/context.rs | 20 ++++--- boa/src/syntax/ast/node/identifier/mod.rs | 6 +- .../syntax/ast/node/operator/bin_op/mod.rs | 6 +- boa/src/value/mod.rs | 56 +++++++------------ 13 files changed, 56 insertions(+), 114 deletions(-) diff --git a/boa/src/builtins/array/array_iterator.rs b/boa/src/builtins/array/array_iterator.rs index 9c4b91aa0d7..55593ce96a9 100644 --- a/boa/src/builtins/array/array_iterator.rs +++ b/boa/src/builtins/array/array_iterator.rs @@ -79,10 +79,8 @@ impl ArrayIterator { .array .get_field("length") .as_number() - .ok_or_else(|| { - ctx.construct_type_error("Not an array") - .expect("&str used as message") - })? as u32; + .ok_or_else(|| ctx.construct_type_error("Not an array"))? + as u32; if array_iterator.next_index >= len { array_iterator.array = Value::undefined(); return Ok(create_iter_result_object(ctx, Value::undefined(), true)); diff --git a/boa/src/builtins/bigint/mod.rs b/boa/src/builtins/bigint/mod.rs index c4513fe643e..82d2c629034 100644 --- a/boa/src/builtins/bigint/mod.rs +++ b/boa/src/builtins/bigint/mod.rs @@ -120,9 +120,7 @@ impl BigInt { } // 3. Throw a TypeError exception. - Err(ctx - .construct_type_error("'this' is not a BigInt") - .expect("&str used as message")) + Err(ctx.construct_type_error("'this' is not a BigInt")) } /// `BigInt.prototype.toString( [radix] )` diff --git a/boa/src/builtins/boolean/mod.rs b/boa/src/builtins/boolean/mod.rs index a772492f2fe..2a1357c0265 100644 --- a/boa/src/builtins/boolean/mod.rs +++ b/boa/src/builtins/boolean/mod.rs @@ -82,9 +82,7 @@ impl Boolean { _ => {} } - Err(ctx - .construct_type_error("'this' is not a boolean") - .expect("&str used as message")) + Err(ctx.construct_type_error("'this' is not a boolean")) } /// The `toString()` method returns a string representing the specified `Boolean` object. diff --git a/boa/src/builtins/date/mod.rs b/boa/src/builtins/date/mod.rs index 295a03540f1..f604f6a4933 100644 --- a/boa/src/builtins/date/mod.rs +++ b/boa/src/builtins/date/mod.rs @@ -1337,7 +1337,5 @@ pub fn this_time_value(value: &Value, ctx: &mut Context) -> Result { return Ok(*date); } } - Err(ctx - .construct_type_error("'this' is not a Date") - .expect("&str used as message")) + Err(ctx.construct_type_error("'this' is not a Date")) } diff --git a/boa/src/builtins/iterable/mod.rs b/boa/src/builtins/iterable/mod.rs index 44c1da83c4e..3f3ab44209a 100644 --- a/boa/src/builtins/iterable/mod.rs +++ b/boa/src/builtins/iterable/mod.rs @@ -59,18 +59,12 @@ pub fn get_iterator(ctx: &mut Context, iterable: Value) -> Result { - return Err(ctx - .construct_type_error("iterable for Map should have array-like objects") - .expect("&str used as message")) + return Err( + ctx.construct_type_error("iterable for Map should have array-like objects") + ) } }, }; @@ -132,14 +131,10 @@ impl Map { map.insert(key, value); map.len() } else { - return Err(ctx - .construct_type_error("'this' is not a Map") - .expect("&str used as message")); + return Err(ctx.construct_type_error("'this' is not a Map")); } } else { - return Err(ctx - .construct_type_error("'this' is not a Map") - .expect("&str used as message")); + return Err(ctx.construct_type_error("'this' is not a Map")); }; Self::set_size(this, size); @@ -169,14 +164,10 @@ impl Map { let deleted = map.remove(key).is_some(); (deleted, map.len()) } else { - return Err(ctx - .construct_type_error("'this' is not a Map") - .expect("&str used as message")); + return Err(ctx.construct_type_error("'this' is not a Map")); } } else { - return Err(ctx - .construct_type_error("'this' is not a Map") - .expect("&str used as message")); + return Err(ctx.construct_type_error("'this' is not a Map")); }; Self::set_size(this, size); Ok(deleted.into()) @@ -210,9 +201,7 @@ impl Map { } } - Err(ctx - .construct_type_error("'this' is not a Map") - .expect("&str used as message")) + Err(ctx.construct_type_error("'this' is not a Map")) } /// `Map.prototype.clear( )` @@ -257,9 +246,7 @@ impl Map { } } - Err(ctx - .construct_type_error("'this' is not a Map") - .expect("&str used as message")) + Err(ctx.construct_type_error("'this' is not a Map")) } /// `Map.prototype.forEach( callbackFn [ , thisArg ] )` diff --git a/boa/src/builtins/number/mod.rs b/boa/src/builtins/number/mod.rs index 12240f53a29..4d5cb218bc7 100644 --- a/boa/src/builtins/number/mod.rs +++ b/boa/src/builtins/number/mod.rs @@ -184,9 +184,7 @@ impl Number { _ => {} } - Err(ctx - .construct_type_error("'this' is not a number") - .expect("&str used as message")) + Err(ctx.construct_type_error("'this' is not a number")) } /// Helper function that formats a float as a ES6-style exponential number string. diff --git a/boa/src/builtins/string/mod.rs b/boa/src/builtins/string/mod.rs index 1b973c52160..a1a76c12b9b 100644 --- a/boa/src/builtins/string/mod.rs +++ b/boa/src/builtins/string/mod.rs @@ -156,9 +156,7 @@ impl String { _ => {} } - Err(ctx - .construct_type_error("'this' is not a string") - .expect("&str used as message")) + Err(ctx.construct_type_error("'this' is not a string")) } /// Get the string value to a primitive string diff --git a/boa/src/builtins/symbol/mod.rs b/boa/src/builtins/symbol/mod.rs index 16909da16c9..04dd9f08ce7 100644 --- a/boa/src/builtins/symbol/mod.rs +++ b/boa/src/builtins/symbol/mod.rs @@ -336,9 +336,7 @@ impl Symbol { _ => {} } - Err(ctx - .construct_type_error("'this' is not a Symbol") - .expect("&str used as message")) + Err(ctx.construct_type_error("'this' is not a Symbol")) } /// `Symbol.prototype.toString()` diff --git a/boa/src/context.rs b/boa/src/context.rs index ee36c999599..5ddf3037e0b 100644 --- a/boa/src/context.rs +++ b/boa/src/context.rs @@ -283,7 +283,7 @@ impl Context { } /// Constructs a `RangeError` with the specified message. - pub fn construct_range_error(&mut self, message: M) -> Result + pub fn construct_range_error(&mut self, message: M) -> Value where M: Into, { @@ -293,6 +293,7 @@ impl Context { vec![Const::from(message.into()).into()], )) .run(self) + .expect("Into used as message") } /// Throws a `RangeError` with the specified message. @@ -300,11 +301,11 @@ impl Context { where M: Into, { - Err(self.construct_range_error(message)?) + Err(self.construct_range_error(message)) } /// Constructs a `TypeError` with the specified message. - pub fn construct_type_error(&mut self, message: M) -> Result + pub fn construct_type_error(&mut self, message: M) -> Value where M: Into, { @@ -314,6 +315,7 @@ impl Context { vec![Const::from(message.into()).into()], )) .run(self) + .expect("Into used as message") } /// Throws a `TypeError` with the specified message. @@ -321,11 +323,11 @@ impl Context { where M: Into, { - Err(self.construct_type_error(message)?) + Err(self.construct_type_error(message)) } /// Constructs a `ReferenceError` with the specified message. - pub fn construct_reference_error(&mut self, message: M) -> Result + pub fn construct_reference_error(&mut self, message: M) -> Value where M: Into, { @@ -334,6 +336,7 @@ impl Context { vec![Const::from(message.into() + " is not defined").into()], )) .run(self) + .expect("Into used as message") } /// Throws a `ReferenceError` with the specified message. @@ -341,11 +344,11 @@ impl Context { where M: Into, { - Err(self.construct_reference_error(message)?) + Err(self.construct_reference_error(message)) } /// Constructs a `SyntaxError` with the specified message. - pub fn construct_syntax_error(&mut self, message: M) -> Result + pub fn construct_syntax_error(&mut self, message: M) -> Value where M: Into, { @@ -354,6 +357,7 @@ impl Context { vec![Const::from(message.into()).into()], )) .run(self) + .expect("Into used as message") } /// Throws a `SyntaxError` with the specified message. @@ -361,7 +365,7 @@ impl Context { where M: Into, { - Err(self.construct_syntax_error(message)?) + Err(self.construct_syntax_error(message)) } /// Utility to create a function Value for Function Declarations, Arrow Functions or Function Expressions diff --git a/boa/src/syntax/ast/node/identifier/mod.rs b/boa/src/syntax/ast/node/identifier/mod.rs index 868064665ee..fe0378d1b62 100644 --- a/boa/src/syntax/ast/node/identifier/mod.rs +++ b/boa/src/syntax/ast/node/identifier/mod.rs @@ -36,11 +36,7 @@ impl Executable for Identifier { .realm() .environment .get_binding_value(self.as_ref()) - .ok_or_else(|| { - interpreter - .construct_reference_error(self.as_ref()) - .expect("Identifier used as message") - }) + .ok_or_else(|| interpreter.construct_reference_error(self.as_ref())) } } diff --git a/boa/src/syntax/ast/node/operator/bin_op/mod.rs b/boa/src/syntax/ast/node/operator/bin_op/mod.rs index 6e0d8c43d18..db8985eec2a 100644 --- a/boa/src/syntax/ast/node/operator/bin_op/mod.rs +++ b/boa/src/syntax/ast/node/operator/bin_op/mod.rs @@ -157,11 +157,7 @@ impl Executable for BinOp { .realm() .environment .get_binding_value(name.as_ref()) - .ok_or_else(|| { - interpreter - .construct_reference_error(name.as_ref()) - .expect("Identifer used as message") - })?; + .ok_or_else(|| interpreter.construct_reference_error(name.as_ref()))?; let v_b = self.rhs().run(interpreter)?; let value = Self::run_assign(op, v_a, v_b, interpreter)?; interpreter.realm_mut().environment.set_mutable_binding( diff --git a/boa/src/value/mod.rs b/boa/src/value/mod.rs index fcf64b44ee5..c89b8b187f7 100644 --- a/boa/src/value/mod.rs +++ b/boa/src/value/mod.rs @@ -256,9 +256,9 @@ impl Value { .map(JSONValue::Number) .unwrap_or(JSONValue::Null)), Self::Integer(val) => Ok(JSONValue::Number(JSONNumber::from(val))), - Self::BigInt(_) => Err(interpreter - .construct_type_error("BigInt value can't be serialized in JSON") - .expect("&str used as message")), + Self::BigInt(_) => { + Err(interpreter.construct_type_error("BigInt value can't be serialized in JSON")) + } Self::Symbol(_) | Self::Undefined => { unreachable!("Symbols and Undefined JSON Values depend on parent type"); } @@ -615,12 +615,10 @@ impl Value { /// This function is equivelent to `BigInt(value)` in JavaScript. pub fn to_bigint(&self, ctx: &mut Context) -> Result { match self { - Value::Null => Err(ctx - .construct_type_error("cannot convert null to a BigInt") - .expect("&str used as message")), - Value::Undefined => Err(ctx - .construct_type_error("cannot convert undefined to a BigInt") - .expect("&str used as message")), + Value::Null => Err(ctx.construct_type_error("cannot convert null to a BigInt")), + Value::Undefined => { + Err(ctx.construct_type_error("cannot convert undefined to a BigInt")) + } Value::String(ref string) => Ok(RcBigInt::from(BigInt::from_string(string, ctx)?)), Value::Boolean(true) => Ok(RcBigInt::from(BigInt::from(1))), Value::Boolean(false) => Ok(RcBigInt::from(BigInt::from(0))), @@ -629,21 +627,17 @@ impl Value { if let Ok(bigint) = BigInt::try_from(*num) { return Ok(RcBigInt::from(bigint)); } - Err(ctx - .construct_type_error(format!( + Err(ctx.construct_type_error(format!( "The number {} cannot be converted to a BigInt because it is not an integer", num - )) - .expect("String used as message")) + ))) } Value::BigInt(b) => Ok(b.clone()), Value::Object(_) => { let primitive = self.to_primitive(ctx, PreferredType::Number)?; primitive.to_bigint(ctx) } - Value::Symbol(_) => Err(ctx - .construct_type_error("cannot convert Symbol to a BigInt") - .expect("&str used as message")), + Value::Symbol(_) => Err(ctx.construct_type_error("cannot convert Symbol to a BigInt")), } } @@ -674,9 +668,7 @@ impl Value { Value::Rational(rational) => Ok(Number::to_native_string(*rational).into()), Value::Integer(integer) => Ok(integer.to_string().into()), Value::String(string) => Ok(string.clone()), - Value::Symbol(_) => Err(ctx - .construct_type_error("can't convert symbol to string") - .expect("&str used as message")), + Value::Symbol(_) => Err(ctx.construct_type_error("can't convert symbol to string")), Value::BigInt(ref bigint) => Ok(bigint.to_string().into()), Value::Object(_) => { let primitive = self.to_primitive(ctx, PreferredType::String)?; @@ -692,9 +684,9 @@ impl Value { /// See: pub fn to_object(&self, ctx: &mut Context) -> Result { match self { - Value::Undefined | Value::Null => Err(ctx - .construct_type_error("cannot convert 'null' or 'undefined' to object") - .expect("&str used as message")), + Value::Undefined | Value::Null => { + Err(ctx.construct_type_error("cannot convert 'null' or 'undefined' to object")) + } Value::Boolean(boolean) => { let prototype = ctx.standard_objects().boolean_object().prototype(); Ok(GcObject::new(Object::with_prototype( @@ -810,15 +802,11 @@ impl Value { let integer_index = self.to_integer(ctx)?; if integer_index < 0.0 { - return Err(ctx - .construct_range_error("Integer index must be >= 0") - .expect("&str used as message")); + return Err(ctx.construct_range_error("Integer index must be >= 0")); } if integer_index > Number::MAX_SAFE_INTEGER { - return Err(ctx - .construct_range_error("Integer index must be less than 2**(53) - 1") - .expect("&str used as message")); + return Err(ctx.construct_range_error("Integer index must be less than 2**(53) - 1")); } Ok(integer_index as usize) @@ -881,12 +869,8 @@ impl Value { } Value::Rational(number) => Ok(number), Value::Integer(integer) => Ok(f64::from(integer)), - Value::Symbol(_) => Err(ctx - .construct_type_error("argument must not be a symbol") - .expect("&str used as message")), - Value::BigInt(_) => Err(ctx - .construct_type_error("argument must not be a bigint") - .expect("&str used as message")), + Value::Symbol(_) => Err(ctx.construct_type_error("argument must not be a symbol")), + Value::BigInt(_) => Err(ctx.construct_type_error("argument must not be a bigint")), Value::Object(_) => { let primitive = self.to_primitive(ctx, PreferredType::Number)?; primitive.to_number(ctx) @@ -921,9 +905,7 @@ impl Value { #[inline] pub fn require_object_coercible(&self, ctx: &mut Context) -> Result<&Value> { if self.is_null_or_undefined() { - Err(ctx - .construct_type_error("cannot convert null or undefined to Object") - .expect("&str used as message")) + Err(ctx.construct_type_error("cannot convert null or undefined to Object")) } else { Ok(self) } From d4b9824aec12c56d3fe395debaa4a49deaac7f02 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Borges?= Date: Mon, 5 Oct 2020 02:54:46 +0100 Subject: [PATCH 8/8] Clean up as suggested by @HalidOdat Co-authored-by: Halid Odat --- boa/src/builtins/error/mod.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/boa/src/builtins/error/mod.rs b/boa/src/builtins/error/mod.rs index 8910cea12af..ee819458afc 100644 --- a/boa/src/builtins/error/mod.rs +++ b/boa/src/builtins/error/mod.rs @@ -117,12 +117,12 @@ impl Error { message_to_string.as_str() }; - if name == "" { - Ok(Value::from(message)) - } else if message == "" { - Ok(Value::from(name)) + if name.is_empty() { + Ok(message.into()) + } else if message.is_empty() { + Ok(name.into()) } else { - Ok(Value::from(format!("{}: {}", name, message))) + Ok(format!("{}: {}", name, message).into()) } } }