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
6 changes: 4 additions & 2 deletions boa/src/builtins/array/array_iterator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -79,8 +79,10 @@ impl ArrayIterator {
.array
.get_field("length")
.as_number()
.ok_or_else(|| ctx.construct_type_error("Not an array"))?
as u32;
.ok_or_else(|| {
ctx.construct_type_error("Not an array")
.expect("&str used as message")
})? as u32;
if array_iterator.next_index >= len {
array_iterator.array = Value::undefined();
return Ok(create_iter_result_object(ctx, Value::undefined(), true));
Expand Down
4 changes: 3 additions & 1 deletion boa/src/builtins/bigint/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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] )`
Expand Down
4 changes: 3 additions & 1 deletion boa/src/builtins/boolean/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
4 changes: 3 additions & 1 deletion boa/src/builtins/date/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1337,5 +1337,7 @@ pub fn this_time_value(value: &Value, ctx: &mut Context) -> Result<Date> {
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"))
}
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 == "" {
Ok(Value::from(message))
} else if message == "" {
Ok(Value::from(name))
} else {
Ok(Value::from(format!("{}: {}", name, message)))
}
RageKnify marked this conversation as resolved.
Show resolved Hide resolved
}
}
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())
}
}
15 changes: 12 additions & 3 deletions boa/src/builtins/iterable/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -59,12 +59,18 @@ pub fn get_iterator(ctx: &mut Context, iterable: Value) -> Result<IteratorRecord
let iterator_function = iterable
.get_property(ctx.well_known_symbols().iterator_symbol())
.and_then(|mut p| p.value.take())
.ok_or_else(|| ctx.construct_type_error("Not an iterable"))?;
.ok_or_else(|| {
ctx.construct_type_error("Not an iterable")
.expect("&str used as message")
})?;
let iterator_object = ctx.call(&iterator_function, &iterable, &[])?;
let next_function = iterator_object
.get_property("next")
.and_then(|mut p| p.value.take())
.ok_or_else(|| ctx.construct_type_error("Could not find property `next`"))?;
.ok_or_else(|| {
ctx.construct_type_error("Could not find property `next`")
.expect("&str used as message")
})?;
Ok(IteratorRecord::new(iterator_object, next_function))
}

Expand Down Expand Up @@ -115,7 +121,10 @@ impl IteratorRecord {
.get_property("done")
.and_then(|mut p| p.value.take())
.and_then(|v| v.as_boolean())
.ok_or_else(|| ctx.construct_type_error("Could not find property `done`"))?;
.ok_or_else(|| {
ctx.construct_type_error("Could not find property `done`")
.expect("&str used as message")
})?;
let next_result = next
.get_property("value")
.and_then(|mut p| p.value.take())
Expand Down
37 changes: 25 additions & 12 deletions boa/src/builtins/map/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -72,20 +72,21 @@ impl Map {
ctx.construct_type_error(
"iterable for Map should have array-like objects",
)
.expect_err("&str used as message")
})?;
map.insert(key, value);
}
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")
)
return Err(ctx
.construct_type_error("iterable for Map should have array-like objects")
.expect("&str used as message"))
}
},
};
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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())
Expand Down Expand Up @@ -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( )`
Expand Down Expand Up @@ -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 ] )`
Expand Down
4 changes: 3 additions & 1 deletion boa/src/builtins/number/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
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
4 changes: 3 additions & 1 deletion boa/src/builtins/string/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 3 additions & 1 deletion boa/src/builtins/symbol/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()`
Expand Down
Loading