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

[Merged by Bors] - Add proxy handling in isArray method #1777

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 11 additions & 12 deletions boa/src/builtins/array/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -291,10 +291,7 @@ impl Array {
return Ok(spreadable.to_boolean());
}
// 4. Return ? IsArray(O).
match this.as_object() {
Some(obj) => Ok(obj.is_array()),
_ => Ok(false),
}
this.is_array(context)
}

/// `get Array [ @@species ]`
Expand Down Expand Up @@ -323,7 +320,7 @@ impl Array {
) -> JsResult<JsObject> {
// 1. Let isArray be ? IsArray(originalArray).
// 2. If isArray is false, return ? ArrayCreate(length).
if !original_array.is_array() {
if !original_array.is_array_abstract(context)? {
return Self::array_create(length, None, context);
}
// 3. Let C be ? Get(originalArray, "constructor").
Expand Down Expand Up @@ -411,13 +408,15 @@ impl Array {
///
/// [spec]: https://tc39.es/ecma262/#sec-array.isarray
/// [mdn]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/isArray
pub(crate) fn is_array(_: &JsValue, args: &[JsValue], _: &mut Context) -> JsResult<JsValue> {
Ok(args
.get_or_undefined(0)
.as_object()
.map(|obj| obj.borrow().is_array())
.unwrap_or_default()
.into())
pub(crate) fn is_array(
_: &JsValue,
args: &[JsValue],
context: &mut Context,
) -> JsResult<JsValue> {
// 1. Return ? IsArray(arg).
args.get_or_undefined(0)
.is_array(context)
.map(|ok| ok.into())
}

/// `Array.of(...items)`
Expand Down
6 changes: 3 additions & 3 deletions boa/src/builtins/json/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ impl Json {
if let Some(obj) = val.as_object() {
// a. Let isArray be ? IsArray(val).
// b. If isArray is true, then
if obj.is_array() {
if obj.is_array_abstract(context)? {
// i. Let I be 0.
// ii. Let len be ? LengthOfArrayLike(val).
// iii. Repeat, while I < len,
Expand Down Expand Up @@ -237,7 +237,7 @@ impl Json {
} else {
// i. Let isArray be ? IsArray(replacer).
// ii. If isArray is true, then
if replacer_obj.is_array() {
if replacer_obj.is_array_abstract(context)? {
// 1. Set PropertyList to a new empty List.
let mut property_set = indexmap::IndexSet::new();

Expand Down Expand Up @@ -457,7 +457,7 @@ impl Json {
// a. Let isArray be ? IsArray(value).
// b. If isArray is true, return ? SerializeJSONArray(state, value).
// c. Return ? SerializeJSONObject(state, value).
return if obj.is_array() {
return if obj.is_array_abstract(context)? {
Ok(Some(Self::serialize_json_array(state, obj, context)?))
} else {
Ok(Some(Self::serialize_json_object(state, obj, context)?))
Expand Down
34 changes: 18 additions & 16 deletions boa/src/builtins/object/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -55,10 +55,10 @@ impl BuiltIn for Object {
.length(Self::LENGTH)
.inherit(None)
.method(Self::has_own_property, "hasOwnProperty", 1)
.method(Self::property_is_enumerable, "propertyIsEnumerable", 0)
.method(Self::property_is_enumerable, "propertyIsEnumerable", 1)
.method(Self::to_string, "toString", 0)
.method(Self::value_of, "valueOf", 0)
.method(Self::is_prototype_of, "isPrototypeOf", 0)
.method(Self::is_prototype_of, "isPrototypeOf", 1)
.static_method(Self::create, "create", 2)
.static_method(Self::set_prototype_of, "setPrototypeOf", 2)
.static_method(Self::get_prototype_of, "getPrototypeOf", 1)
Expand Down Expand Up @@ -482,20 +482,22 @@ impl Object {
return Ok("[object Null]".into());
}
// 3. Let O be ! ToObject(this value).
let o = this.to_object(context)?;
// TODO: 4. Let isArray be ? IsArray(O).
// TODO: 5. If isArray is true, let builtinTag be "Array".

// 6. Else if O has a [[ParameterMap]] internal slot, let builtinTag be "Arguments".
// 7. Else if O has a [[Call]] internal method, let builtinTag be "Function".
// 8. Else if O has an [[ErrorData]] internal slot, let builtinTag be "Error".
// 9. Else if O has a [[BooleanData]] internal slot, let builtinTag be "Boolean".
// 10. Else if O has a [[NumberData]] internal slot, let builtinTag be "Number".
// 11. Else if O has a [[StringData]] internal slot, let builtinTag be "String".
// 12. Else if O has a [[DateValue]] internal slot, let builtinTag be "Date".
// 13. Else if O has a [[RegExpMatcher]] internal slot, let builtinTag be "RegExp".
// 14. Else, let builtinTag be "Object".
let builtin_tag = {
let o = this.to_object(context).expect("toObject cannot fail here");

// 4. Let isArray be ? IsArray(O).
// 5. If isArray is true, let builtinTag be "Array".
let builtin_tag = if JsValue::from(o.clone()).is_array(context)? {
"Array"
} else {
// 6. Else if O has a [[ParameterMap]] internal slot, let builtinTag be "Arguments".
// 7. Else if O has a [[Call]] internal method, let builtinTag be "Function".
// 8. Else if O has an [[ErrorData]] internal slot, let builtinTag be "Error".
// 9. Else if O has a [[BooleanData]] internal slot, let builtinTag be "Boolean".
// 10. Else if O has a [[NumberData]] internal slot, let builtinTag be "Number".
// 11. Else if O has a [[StringData]] internal slot, let builtinTag be "String".
// 12. Else if O has a [[DateValue]] internal slot, let builtinTag be "Date".
// 13. Else if O has a [[RegExpMatcher]] internal slot, let builtinTag be "RegExp".
// 14. Else, let builtinTag be "Object".
let o = o.borrow();
match o.kind() {
ObjectKind::Array => "Array",
Expand Down
32 changes: 32 additions & 0 deletions boa/src/object/operations.rs
Original file line number Diff line number Diff line change
Expand Up @@ -559,6 +559,38 @@ impl JsObject {
}
}

/// Abstract operation `IsArray ( argument )`
///
/// Check if a value is an array.
///
/// More information:
/// - [ECMAScript reference][spec]
///
/// [spec]: https://tc39.es/ecma262/#sec-isarray
pub(crate) fn is_array_abstract(&self, context: &mut Context) -> JsResult<bool> {
// Note: The spec specifies this function for JsValue.
// It is implemented for JsObject for convenience.

// 2. If argument is an Array exotic object, return true.
if self.is_array() {
return Ok(true);
}

// 3. If argument is a Proxy exotic object, then
let object = self.borrow();
if let Some(proxy) = object.as_proxy() {
// a. If argument.[[ProxyHandler]] is null, throw a TypeError exception.
// b. Let target be argument.[[ProxyTarget]].
let (target, _) = proxy.try_data(context)?;

// c. Return ? IsArray(target).
return target.is_array_abstract(context);
}

// 4. Return false.
Ok(false)
}

// todo: GetFunctionRealm

// todo: CopyDataProperties
Expand Down
22 changes: 11 additions & 11 deletions boa/src/value/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1026,24 +1026,24 @@ impl JsValue {
.into()
}

/// Check if it is an array.
/// Abstract operation `IsArray ( argument )`
///
/// Check if a value is an array.
///
/// More information:
/// - [ECMAScript reference][spec]
///
/// [spec]: https://tc39.es/ecma262/#sec-isarray
pub(crate) fn is_array(&self, _context: &mut Context) -> JsResult<bool> {
pub(crate) fn is_array(&self, context: &mut Context) -> JsResult<bool> {
// Note: The spec specifies this function for JsValue.
// The main part of the function is implemented for JsObject.

// 1. If Type(argument) is not Object, return false.
if let Some(object) = self.as_object() {
// 2. If argument is an Array exotic object, return true.
// a. If argument.[[ProxyHandler]] is null, throw a TypeError exception.
// 3. If argument is a Proxy exotic object, then
// b. Let target be argument.[[ProxyTarget]].
// c. Return ? IsArray(target).
// TODO: handle ProxyObjects
Ok(object.is_array())
} else {
// 4. Return false.
object.is_array_abstract(context)
}
// 4. Return false.
else {
Ok(false)
}
}
Expand Down