Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix Error constructors to return rather than throw #749

Merged
merged 10 commits into from
Oct 5, 2020
36 changes: 32 additions & 4 deletions boa/src/builtins/error/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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()`
Expand All @@ -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<Value> {
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())
}
}
}
2 changes: 1 addition & 1 deletion boa/src/builtins/error/range.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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())
}
}
2 changes: 1 addition & 1 deletion boa/src/builtins/error/reference.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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())
}
}
2 changes: 1 addition & 1 deletion boa/src/builtins/error/syntax.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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())
}
}
30 changes: 30 additions & 0 deletions boa/src/builtins/error/tests.rs
Original file line number Diff line number Diff line change
@@ -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\"");
}
2 changes: 1 addition & 1 deletion boa/src/builtins/error/type.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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())
}
}
3 changes: 3 additions & 0 deletions boa/src/builtins/object/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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]\"");
Expand Down
8 changes: 4 additions & 4 deletions boa/src/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -293,7 +293,7 @@ impl Context {
vec![Const::from(message.into()).into()],
))
.run(self)
.expect_err("RangeError should always throw")
.expect("Into<String> used as message")
}

/// Throws a `RangeError` with the specified message.
Expand All @@ -315,7 +315,7 @@ impl Context {
vec![Const::from(message.into()).into()],
))
.run(self)
.expect_err("TypeError should always throw")
.expect("Into<String> used as message")
}

/// Throws a `TypeError` with the specified message.
Expand All @@ -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<String> used as message")
}

/// Throws a `ReferenceError` with the specified message.
Expand All @@ -357,7 +357,7 @@ impl Context {
vec![Const::from(message.into()).into()],
))
.run(self)
.expect_err("SyntaxError should always throw")
.expect("Into<String> used as message")
}

/// Throws a `SyntaxError` with the specified message.
Expand Down