From 18d0430a0b3dcd1acd6255415571cda45f01b445 Mon Sep 17 00:00:00 2001 From: Hans Larsen Date: Tue, 19 Mar 2024 12:18:29 -0700 Subject: [PATCH 1/6] Add a try_from_js implementation for Vec (accept any Array-like) --- .../src/value/conversions/try_from_js.rs | 69 ++++++++++++++++++- 1 file changed, 68 insertions(+), 1 deletion(-) diff --git a/core/engine/src/value/conversions/try_from_js.rs b/core/engine/src/value/conversions/try_from_js.rs index 55164e931f4..79bef950779 100644 --- a/core/engine/src/value/conversions/try_from_js.rs +++ b/core/engine/src/value/conversions/try_from_js.rs @@ -1,8 +1,9 @@ //! This module contains the [`TryFromJs`] trait, and conversions to basic Rust types. -use crate::{Context, JsBigInt, JsNativeError, JsResult, JsValue}; use num_bigint::BigInt; +use crate::{Context, js_string, JsBigInt, JsNativeError, JsResult, JsValue}; + /// This trait adds a fallible and efficient conversions from a [`JsValue`] to Rust types. pub trait TryFromJs: Sized { /// This function tries to convert a JavaScript value into `Self`. @@ -58,6 +59,33 @@ where } } +impl TryFromJs for Vec +where + T: TryFromJs, +{ + fn try_from_js(value: &JsValue, context: &mut Context) -> JsResult { + let object = match value { + JsValue::Object(o) => o, + _ => { + return Err(JsNativeError::typ() + .with_message("cannot convert value to a Vec") + .into()) + } + }; + + let length = object + .get(js_string!("length"), context)? + .to_length(context)?; + let mut vec = Vec::with_capacity(length as usize); + for i in 0..length { + let value = object.get(i, context)?; + vec.push(T::try_from_js(&value, context)?); + } + + Ok(vec) + } +} + impl TryFromJs for JsBigInt { fn try_from_js(value: &JsValue, _context: &mut Context) -> JsResult { match value { @@ -250,3 +278,42 @@ impl TryFromJs for u128 { } } } + +#[test] +fn value_into_vec() { + use boa_engine::{run_test_actions, TestAction}; + use indoc::indoc; + + #[derive(Debug, PartialEq, Eq, boa_macros::TryFromJs)] + struct TestStruct { + inner: bool, + my_int: i16, + my_vec: Vec, + } + + run_test_actions([TestAction::assert_with_op( + indoc! {r#" + let value = { + inner: true, + my_int: 11, + my_vec: ["a", "b", "c"] + }; + value + "#}, + |value, context| { + let value = TestStruct::try_from_js(&value, context); + eprintln!("{:?}", value); + match value { + Ok(value) => { + value + == TestStruct { + inner: true, + my_int: 11, + my_vec: vec!["a".to_string(), "b".to_string(), "c".to_string()], + } + } + _ => false, + } + }, + )]); +} From 725c2a1a8cb9a34d018abb192911475647636808 Mon Sep 17 00:00:00 2001 From: Hans Larsen Date: Tue, 19 Mar 2024 14:58:49 -0700 Subject: [PATCH 2/6] Use safe conversion to usize --- core/engine/src/value/conversions/try_from_js.rs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/core/engine/src/value/conversions/try_from_js.rs b/core/engine/src/value/conversions/try_from_js.rs index 79bef950779..0017ac7db4e 100644 --- a/core/engine/src/value/conversions/try_from_js.rs +++ b/core/engine/src/value/conversions/try_from_js.rs @@ -76,7 +76,12 @@ where let length = object .get(js_string!("length"), context)? .to_length(context)?; - let mut vec = Vec::with_capacity(length as usize); + let length = usize::try_from(length).map_err(|e| { + JsNativeError::typ() + .with_message(format!("could not convert length to usize: {e}")) + .into() + })?; + let mut vec = Vec::with_capacity(length); for i in 0..length { let value = object.get(i, context)?; vec.push(T::try_from_js(&value, context)?); From 10c3fee6d28c36eb95d278bf95f2c20037a30501 Mon Sep 17 00:00:00 2001 From: Hans Larsen Date: Tue, 19 Mar 2024 15:18:06 -0700 Subject: [PATCH 3/6] Fix typing compile error --- core/engine/src/value/conversions/try_from_js.rs | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/core/engine/src/value/conversions/try_from_js.rs b/core/engine/src/value/conversions/try_from_js.rs index 0017ac7db4e..f7fe7d1e832 100644 --- a/core/engine/src/value/conversions/try_from_js.rs +++ b/core/engine/src/value/conversions/try_from_js.rs @@ -76,11 +76,14 @@ where let length = object .get(js_string!("length"), context)? .to_length(context)?; - let length = usize::try_from(length).map_err(|e| { - JsNativeError::typ() - .with_message(format!("could not convert length to usize: {e}")) - .into() - })?; + let length = match usize::try_from(length) { + Ok(length) => length, + Err(e) => { + return Err(JsNativeError::typ() + .with_message(format!("could not convert length to usize: {e}")) + .into()); + } + }; let mut vec = Vec::with_capacity(length); for i in 0..length { let value = object.get(i, context)?; From fd8d272a4fd325194f419d1228c2be7015bbe44e Mon Sep 17 00:00:00 2001 From: Hans Larsen Date: Wed, 20 Mar 2024 09:10:38 -0700 Subject: [PATCH 4/6] Fix clippies and fmt --- core/engine/src/value/conversions/try_from_js.rs | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/core/engine/src/value/conversions/try_from_js.rs b/core/engine/src/value/conversions/try_from_js.rs index f7fe7d1e832..b4bf5f34396 100644 --- a/core/engine/src/value/conversions/try_from_js.rs +++ b/core/engine/src/value/conversions/try_from_js.rs @@ -2,7 +2,7 @@ use num_bigint::BigInt; -use crate::{Context, js_string, JsBigInt, JsNativeError, JsResult, JsValue}; +use crate::{js_string, Context, JsBigInt, JsNativeError, JsResult, JsValue}; /// This trait adds a fallible and efficient conversions from a [`JsValue`] to Rust types. pub trait TryFromJs: Sized { @@ -64,13 +64,10 @@ where T: TryFromJs, { fn try_from_js(value: &JsValue, context: &mut Context) -> JsResult { - let object = match value { - JsValue::Object(o) => o, - _ => { - return Err(JsNativeError::typ() - .with_message("cannot convert value to a Vec") - .into()) - } + let JsValue::Object(object) = value else { + return Err(JsNativeError::typ() + .with_message("cannot convert value to a Vec") + .into()); }; let length = object From 91cc279fe9774984e8c71b2e92dd67a5f834ad09 Mon Sep 17 00:00:00 2001 From: Hans Larsen Date: Wed, 20 Mar 2024 11:48:20 -0700 Subject: [PATCH 5/6] Remove println from test --- core/engine/src/value/conversions/try_from_js.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/engine/src/value/conversions/try_from_js.rs b/core/engine/src/value/conversions/try_from_js.rs index b4bf5f34396..6f23d00b134 100644 --- a/core/engine/src/value/conversions/try_from_js.rs +++ b/core/engine/src/value/conversions/try_from_js.rs @@ -307,7 +307,7 @@ fn value_into_vec() { "#}, |value, context| { let value = TestStruct::try_from_js(&value, context); - eprintln!("{:?}", value); + match value { Ok(value) => { value From 439009b11e7b24816a3a8c8e105dd135c8cd0ce4 Mon Sep 17 00:00:00 2001 From: Hans Larsen Date: Fri, 22 Mar 2024 19:33:12 -0700 Subject: [PATCH 6/6] Add a negative test --- .../src/value/conversions/try_from_js.rs | 52 +++++++++++++------ 1 file changed, 36 insertions(+), 16 deletions(-) diff --git a/core/engine/src/value/conversions/try_from_js.rs b/core/engine/src/value/conversions/try_from_js.rs index 6f23d00b134..07d0fadeb7f 100644 --- a/core/engine/src/value/conversions/try_from_js.rs +++ b/core/engine/src/value/conversions/try_from_js.rs @@ -296,8 +296,9 @@ fn value_into_vec() { my_vec: Vec, } - run_test_actions([TestAction::assert_with_op( - indoc! {r#" + run_test_actions([ + TestAction::assert_with_op( + indoc! {r#" let value = { inner: true, my_int: 11, @@ -305,20 +306,39 @@ fn value_into_vec() { }; value "#}, - |value, context| { - let value = TestStruct::try_from_js(&value, context); + |value, context| { + let value = TestStruct::try_from_js(&value, context); - match value { - Ok(value) => { - value - == TestStruct { - inner: true, - my_int: 11, - my_vec: vec!["a".to_string(), "b".to_string(), "c".to_string()], - } + match value { + Ok(value) => { + value + == TestStruct { + inner: true, + my_int: 11, + my_vec: vec!["a".to_string(), "b".to_string(), "c".to_string()], + } + } + _ => false, } - _ => false, - } - }, - )]); + }, + ), + TestAction::assert_with_op( + indoc!( + r#" + let wrong = { + inner: false, + my_int: 22, + my_vec: [{}, "e", "f"] + }; + wrong"# + ), + |value, context| { + let Err(value) = TestStruct::try_from_js(&value, context) else { + return false; + }; + assert!(value.to_string().contains("TypeError")); + true + }, + ), + ]); }