TryFromJs from JsMap for HashMap & BtreeMap (#3998)

* `TryFromJs` from `JsMap` for `HashMap` & `BtreeMap`

* fix `clippy` warn

* use `IteratorResult` instead of `as_object`

* `JsMap` impl `rust_for_each`

* fix: initial `JsMap` can be changed in `for_each`

* better naming

Author: Nikita-str

core/engine/src/builtins/map/mod.rs

@@ -503,6 +503,53 @@ impl Map {
         }
     }
 
+    /// Call `f` for each `(key, value)` in the `Map`.
+    ///
+    /// Can not be used in [`Self::for_each`] because in that case will be
+    /// incorrect order for next steps of the algo:
+    /// ```txt
+    /// 2. Perform ? RequireInternalSlot(M, [[MapData]]).
+    /// 3. If IsCallable(callbackfn) is false, throw a TypeError exception.
+    /// ```
+    pub(crate) fn for_each_native<F>(this: &JsValue, mut f: F) -> JsResult<()>
+    where
+        F: FnMut(JsValue, JsValue) -> JsResult<()>,
+    {
+        // See `Self::for_each` for comments on the algo.
+
+        let map = this
+            .as_object()
+            .filter(|obj| obj.is::<OrderedMap<JsValue>>())
+            .ok_or_else(|| JsNativeError::typ().with_message("`this` is not a Map"))?;
+
+        let _lock = map
+            .downcast_mut::<OrderedMap<JsValue>>()
+            .expect("checked that `this` was a map")
+            .lock(map.clone());
+
+        let mut index = 0;
+        loop {
+            let (k, v) = {
+                let map = map
+                    .downcast_ref::<OrderedMap<JsValue>>()
+                    .expect("checked that `this` was a map");
+
+                if index < map.full_len() {
+                    if let Some((k, v)) = map.get_index(index) {
+                        (k.clone(), v.clone())
+                    } else {
+                        continue;
+                    }
+                } else {
+                    return Ok(());
+                }
+            };
+
+            f(k, v)?;
+            index += 1;
+        }
+    }
+
     /// `Map.prototype.values()`
     ///
     /// Returns a new Iterator object that contains the values for each element in the Map object in insertion order.

core/engine/src/object/builtins/jsmap.rs

@@ -402,6 +402,16 @@ impl JsMap {
         )
     }
 
+    /// Executes the provided callback function for each key-value pair within the [`JsMap`].
+    #[inline]
+    pub fn for_each_native<F>(&self, f: F) -> JsResult<()>
+    where
+        F: FnMut(JsValue, JsValue) -> JsResult<()>,
+    {
+        let this = self.inner.clone().into();
+        Map::for_each_native(&this, f)
+    }
+
     /// Returns a new [`JsMapIterator`] object that yields the `value` for each element within the [`JsMap`] in insertion order.
     #[inline]
     pub fn values(&self, context: &mut Context) -> JsResult<JsMapIterator> {

core/engine/src/value/conversions/try_from_js.rs

@@ -565,3 +565,26 @@ fn value_into_map() {
         }),
     ]);
 }
+
+#[test]
+fn js_map_into_rust_map() -> JsResult<()> {
+    use boa_engine::Source;
+    use std::collections::{BTreeMap, HashMap};
+
+    let js_code = "new Map([['a', 1], ['b', 3], ['aboba', 42024]])";
+    let mut context = Context::default();
+
+    let js_value = context.eval(Source::from_bytes(js_code))?;
+
+    let hash_map = HashMap::<String, i32>::try_from_js(&js_value, &mut context)?;
+    let btree_map = BTreeMap::<String, i32>::try_from_js(&js_value, &mut context)?;
+
+    let expect = [("a".into(), 1), ("aboba".into(), 42024), ("b".into(), 3)];
+
+    let expected_hash_map: HashMap<String, _> = expect.iter().cloned().collect();
+    assert_eq!(expected_hash_map, hash_map);
+
+    let expected_btree_map: BTreeMap<String, _> = expect.iter().cloned().collect();
+    assert_eq!(expected_btree_map, btree_map);
+    Ok(())
+}

core/engine/src/value/conversions/try_from_js/collections.rs

@@ -3,6 +3,7 @@
 use std::collections::{BTreeMap, HashMap};
 use std::hash::Hash;
 
+use crate::object::JsMap;
 use crate::value::TryFromJs;
 use crate::{Context, JsNativeError, JsResult, JsValue};
 
@@ -18,6 +19,20 @@ where
                 .into());
         };
 
+        // JsMap case
+        if let Ok(js_map) = JsMap::from_object(object.clone()) {
+            let mut map = Self::default();
+            js_map.for_each_native(|key, value| {
+                map.insert(
+                    K::try_from_js(&key, context)?,
+                    V::try_from_js(&value, context)?,
+                );
+                Ok(())
+            })?;
+            return Ok(map);
+        }
+
+        // key-valued JsObject case:
         let keys = object.__own_property_keys__(context)?;
 
         keys.into_iter()
@@ -47,6 +62,20 @@ where
                 .into());
         };
 
+        // JsMap case
+        if let Ok(js_map) = JsMap::from_object(object.clone()) {
+            let mut map = Self::default();
+            js_map.for_each_native(|key, value| {
+                map.insert(
+                    K::try_from_js(&key, context)?,
+                    V::try_from_js(&value, context)?,
+                );
+                Ok(())
+            })?;
+            return Ok(map);
+        }
+
+        // key-valued JsObject case:
         let keys = object.__own_property_keys__(context)?;
 
         keys.into_iter()