Refactor iterator APIs to be on parity with the latest spec (#3962)

* The Great Refactoring Of Iterators

* Update core/engine/src/builtins/promise/mod.rs

Co-authored-by: raskad <32105367+raskad@users.noreply.github.com>

* Update core/engine/src/builtins/promise/mod.rs

Co-authored-by: raskad <32105367+raskad@users.noreply.github.com>

---------

Co-authored-by: raskad <32105367+raskad@users.noreply.github.com>

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

@@ -15,10 +15,7 @@ use boa_profiler::Profiler;
 use thin_vec::ThinVec;
 
 use crate::{
-    builtins::{
-        iterable::{if_abrupt_close_iterator, IteratorHint},
-        BuiltInObject, Number,
-    },
+    builtins::{iterable::if_abrupt_close_iterator, BuiltInObject, Number},
     context::intrinsics::{Intrinsics, StandardConstructor, StandardConstructors},
     error::JsNativeError,
     js_string,
@@ -610,44 +607,41 @@ impl Array {
             _ => Self::array_create(0, None, context)?,
         };
 
-        // c. Let iteratorRecord be ? GetIterator(items, sync, usingIterator).
-        let mut iterator_record =
-            items.get_iterator(context, Some(IteratorHint::Sync), Some(using_iterator))?;
+        // c. Let iteratorRecord be ? GetIteratorFromMethod(items, usingIterator).
+        let mut iterator_record = items.get_iterator_from_method(&using_iterator, context)?;
 
         // d. Let k be 0.
         // e. Repeat,
         //     i. If k ≥ 2^53 - 1 (MAX_SAFE_INTEGER), then
         //     ...
-        //     x. Set k to k + 1.
+        //     ix. Set k to k + 1.
         for k in 0..9_007_199_254_740_991_u64 {
-            // iii. Let next be ? IteratorStep(iteratorRecord).
-            if iterator_record.step(context)? {
-                // 1. Perform ? Set(A, "length", 𝔽(k), true).
+            // iii. Let next be ? IteratorStepValue(iteratorRecord).
+            let Some(next) = iterator_record.step_value(context)? else {
+                // iv. If next is done, then
+                //     1. Perform ? Set(A, "length", 𝔽(k), true).
                 a.set(StaticJsStrings::LENGTH, k, true, context)?;
-                // 2. Return A.
+                //     2. Return A.
                 return Ok(a.into());
-            }
-
-            // iv. If next is false, then
-            // v. Let nextValue be ? IteratorValue(next).
-            let next_value = iterator_record.value(context)?;
+            };
 
-            // vi. If mapping is true, then
+            // v. If mapping is true, then
             let mapped_value = if let Some(mapfn) = mapping {
-                // 1. Let mappedValue be Call(mapfn, thisArg, « nextValue, 𝔽(k) »).
-                let mapped_value = mapfn.call(this_arg, &[next_value, k.into()], context);
+                // 1. Let mappedValue be Completion(Call(mapper, thisArg, « next, 𝔽(k) »)).
+                let mapped_value = mapfn.call(this_arg, &[next, k.into()], context);
 
                 // 2. IfAbruptCloseIterator(mappedValue, iteratorRecord).
                 if_abrupt_close_iterator!(mapped_value, iterator_record, context)
             } else {
-                // vii. Else, let mappedValue be nextValue.
-                next_value
+                // vi. Else,
+                //     1. Let mappedValue be next.
+                next
             };
 
-            // viii. Let defineStatus be CreateDataPropertyOrThrow(A, Pk, mappedValue).
+            // vii. Let defineStatus be Completion(CreateDataPropertyOrThrow(A, Pk, mappedValue)).
             let define_status = a.create_data_property_or_throw(k, mapped_value, context);
 
-            // ix. IfAbruptCloseIterator(defineStatus, iteratorRecord).
+            // viii. IfAbruptCloseIterator(defineStatus, iteratorRecord).
             if_abrupt_close_iterator!(define_status, iterator_record, context);
         }
 

core/engine/src/builtins/error/aggregate.rs

@@ -9,7 +9,7 @@
 
 use crate::{
     builtins::{
-        iterable::iterable_to_list, Array, BuiltInBuilder, BuiltInConstructor, BuiltInObject,
+        iterable::IteratorHint, Array, BuiltInBuilder, BuiltInConstructor, BuiltInObject,
         IntrinsicObject,
     },
     context::intrinsics::{Intrinsics, StandardConstructor, StandardConstructors},
@@ -56,7 +56,11 @@ impl BuiltInConstructor for AggregateError {
     const STANDARD_CONSTRUCTOR: fn(&StandardConstructors) -> &StandardConstructor =
         StandardConstructors::aggregate_error;
 
-    /// Create a new aggregate error object.
+    /// [`AggregateError ( errors, message [ , options ] )`][spec]
+    ///
+    /// Creates a new aggregate error object.
+    ///
+    /// [spec]: AggregateError ( errors, message [ , options ] )
     fn constructor(
         new_target: &JsValue,
         args: &[JsValue],
@@ -102,9 +106,12 @@ impl BuiltInConstructor for AggregateError {
         // 4. Perform ? InstallErrorCause(O, options).
         Error::install_error_cause(&o, args.get_or_undefined(2), context)?;
 
-        // 5. Let errorsList be ? IterableToList(errors).
+        // 5. Let errorsList be ? IteratorToList(? GetIterator(errors, sync)).
         let errors = args.get_or_undefined(0);
-        let errors_list = iterable_to_list(context, errors, None)?;
+        let errors_list = errors
+            .get_iterator(IteratorHint::Sync, context)?
+            .into_list(context)?;
+
         // 6. Perform ! DefinePropertyOrThrow(O, "errors",
         //    PropertyDescriptor {
         //      [[Configurable]]: true,

core/engine/src/builtins/intl/list_format/mod.rs

@@ -9,6 +9,7 @@ use icu_provider::DataLocale;
 
 use crate::{
     builtins::{
+        iterable::IteratorHint,
         options::{get_option, get_options_object},
         Array, BuiltInBuilder, BuiltInConstructor, BuiltInObject, IntrinsicObject, OrdinaryObject,
     },
@@ -486,37 +487,35 @@ fn string_list_from_iterable(iterable: &JsValue, context: &mut Context) -> JsRes
         return Ok(Vec::new());
     }
 
-    // 2. Let iteratorRecord be ? GetIterator(iterable).
-    let mut iterator = iterable.get_iterator(context, None, None)?;
+    // 2. Let iteratorRecord be ? GetIterator(iterable, sync).
+    let mut iterator = iterable.get_iterator(IteratorHint::Sync, context)?;
 
     // 3. Let list be a new empty List.
     let mut list = Vec::new();
 
     // 4. Let next be true.
     // 5. Repeat, while next is not false,
-    //     a. Set next to ? IteratorStep(iteratorRecord).
-    //     b. If next is not false, then
-    while !iterator.step(context)? {
-        let item = iterator.value(context)?;
-        //    i. Let nextValue be ? IteratorValue(next).
-        //    ii. If Type(nextValue) is not String, then
-        let Some(s) = item.as_string().cloned() else {
-            //    1. Let error be ThrowCompletion(a newly created TypeError object).
-            //    2. Return ? IteratorClose(iteratorRecord, error).
+    //     a. Let next be ? IteratorStepValue(iteratorRecord).
+    while let Some(next) = iterator.step_value(context)? {
+        // c. If next is not a String, then
+        let Some(s) = next.as_string().cloned() else {
+            // i. Let error be ThrowCompletion(a newly created TypeError object).
+            // ii. Return ? IteratorClose(iteratorRecord, error).
             return Err(iterator
                 .close(
                     Err(JsNativeError::typ()
                         .with_message("StringListFromIterable: can only format strings into a list")
                         .into()),
                     context,
                 )
-                .expect_err("Should return the provided error"));
+                .expect_err("`close` should return the provided error"));
         };
 
-        //    iii. Append nextValue to the end of the List list.
+        // d. Append next to list.
         list.push(s);
     }
 
-    // 6. Return list.
+    //     b. If next is done, then
+    //         i. Return list.
     Ok(list)
 }

core/engine/src/builtins/iterable/async_from_sync_iterator.rs

@@ -98,7 +98,7 @@ impl AsyncFromSyncIterator {
         // 1. Let O be the this value.
         // 2. Assert: O is an Object that has a [[SyncIteratorRecord]] internal slot.
         // 4. Let syncIteratorRecord be O.[[SyncIteratorRecord]].
-        let sync_iterator_record = this
+        let mut sync_iterator_record = this
             .as_object()
             .and_then(JsObject::downcast_ref::<Self>)
             .expect("async from sync iterator prototype must be object")
@@ -113,18 +113,10 @@ impl AsyncFromSyncIterator {
         .expect("cannot fail with promise constructor");
 
         // 5. If value is present, then
-        // a. Let result be Completion(IteratorNext(syncIteratorRecord, value)).
+        //     a. Let result be Completion(IteratorNext(syncIteratorRecord, value)).
         // 6. Else,
-        // a. Let result be Completion(IteratorNext(syncIteratorRecord)).
-        let iterator = sync_iterator_record.iterator().clone();
-        let next = sync_iterator_record.next_method();
-        let result = next
-            .call(
-                &iterator.into(),
-                args.first().map_or(&[], std::slice::from_ref),
-                context,
-            )
-            .and_then(IteratorResult::from_value);
+        //     a. Let result be Completion(IteratorNext(syncIteratorRecord)).
+        let result = sync_iterator_record.next(args.first(), context);
 
         // 7. IfAbruptRejectPromise(result, promiseCapability).
         let result = if_abrupt_reject_promise!(result, promise_capability, context);