LibJS: Bring ForIn body evaluation closer to the specification

This fixes 2 bugs in our current implementation:
 * Properties deleted during iteration were still being iterated
 * Properties with the same name in both the object and it's prototype
   were iterated twice

2 files changed, 40 insertions, 20 deletions

diff --git a/Userland/Libraries/LibJS/AST.cpp b/Userland/Libraries/LibJS/AST.cpp
index 9b16c99711..141f4ae590 100644
--- a/Userland/Libraries/LibJS/AST.cpp
+++ b/Userland/Libraries/LibJS/AST.cpp
@@ -1021,30 +1021,29 @@ Completion ForInStatement::loop_evaluation(Interpreter& interpreter, GlobalObjec
     // 3. Let V be undefined.
     auto last_value = js_undefined();
 
-    while (object) {
-        auto property_names = TRY(object->enumerable_own_property_names(Object::PropertyKind::Key));
-        for (auto& value : property_names) {
-            TRY(for_in_head_state.execute_head(interpreter, global_object, value));
+    auto result = object->enumerate_object_properties([&](auto value) -> Optional<Completion> {
+        TRY(for_in_head_state.execute_head(interpreter, global_object, value));
 
-            // l. Let result be the result of evaluating stmt.
-            auto result = m_body->execute(interpreter, global_object);
-
-            // m. Set the running execution context's LexicalEnvironment to oldEnv.
-            interpreter.vm().running_execution_context().lexical_environment = old_environment;
+        // l. Let result be the result of evaluating stmt.
+        auto result = m_body->execute(interpreter, global_object);
 
-            // n. If LoopContinues(result, labelSet) is false, then
-            if (!loop_continues(result, label_set)) {
-                // 1. Return Completion(UpdateEmpty(result, V)).
-                return result.update_empty(last_value);
-            }
+        // m. Set the running execution context's LexicalEnvironment to oldEnv.
+        interpreter.vm().running_execution_context().lexical_environment = old_environment;
 
-            // o. If result.[[Value]] is not empty, set V to result.[[Value]].
-            if (result.value().has_value())
-                last_value = *result.value();
+        // n. If LoopContinues(result, labelSet) is false, then
+        if (!loop_continues(result, label_set)) {
+            // 1. Return Completion(UpdateEmpty(result, V)).
+            return result.update_empty(last_value);
         }
-        object = TRY(object->internal_get_prototype_of());
-    }
-    return last_value;
+
+        // o. If result.[[Value]] is not empty, set V to result.[[Value]].
+        if (result.value().has_value())
+            last_value = *result.value();
+
+        return {};
+    });
+
+    return result.value_or(last_value);
 }
 
 // 14.1.1 Runtime Semantics: Evaluation, https://tc39.es/ecma262/#sec-statement-semantics-runtime-semantics-evaluation
diff --git a/Userland/Libraries/LibJS/Tests/loops/for-in-basic.js b/Userland/Libraries/LibJS/Tests/loops/for-in-basic.js
index 6795c4baa8..f7b018ca38 100644
--- a/Userland/Libraries/LibJS/Tests/loops/for-in-basic.js
+++ b/Userland/Libraries/LibJS/Tests/loops/for-in-basic.js
@@ -99,3 +99,24 @@ describe("special left hand sides", () => {
         }).toThrowWithMessage(ReferenceError, "Invalid left-hand side in assignment");
     });
 });
+
+test("remove properties while iterating", () => {
+    const from = [1, 2, 3];
+    const to = [];
+    for (const prop in from) {
+        to.push(prop);
+        from.pop();
+    }
+    expect(to).toEqual(["0", "1"]);
+});
+
+test("duplicated properties in prototype", () => {
+    const object = { a: 1 };
+    const proto = { a: 2 };
+    Object.setPrototypeOf(object, proto);
+    const a = [];
+    for (const prop in object) {
+        a.push(prop);
+    }
+    expect(a).toEqual(["a"]);
+});