From ee40d9c3b49465563d53c3e1ff8d909aa7c7e4b3 Mon Sep 17 00:00:00 2001
From: Jarred Sumner <709451+Jarred-Sumner@users.noreply.github.com>
Date: Sat, 20 May 2023 22:01:26 -0700
Subject: [bun:test] Make forEachPropertyOrdered behave closer to
 forEachProperty

When diffing, we were missing some properties due to not calling getters and other types of properties

This led to situations where Bun.deepEquals would report false and then display no difference

This aligns the behavior closer to forEachProperty

cc @dylan-conway please let me know if this is going to break anything
---
 src/bun.js/bindings/bindings.cpp | 45 ++++++++++++++++++++++++++++++++++++----
 1 file changed, 41 insertions(+), 4 deletions(-)

(limited to 'src')

diff --git a/src/bun.js/bindings/bindings.cpp b/src/bun.js/bindings/bindings.cpp
index e76f75205..a72fba992 100644
--- a/src/bun.js/bindings/bindings.cpp
+++ b/src/bun.js/bindings/bindings.cpp
@@ -3882,17 +3882,54 @@ void JSC__JSValue__forEachPropertyOrdered(JSC__JSValue JSValue0, JSC__JSGlobalOb
         const WTF::StringImpl* bImpl = b.isSymbol() && !b.isPrivateName() ? b.impl() : b.string().impl();
         return codePointCompare(aImpl, bImpl) < 0;
     });
+    auto clientData = WebCore::clientData(vm);
 
     for (auto property : vector) {
-        const WTF::StringImpl* name = property.isSymbol() && !property.isPrivateName() ? property.impl() : property.string().impl();
-        ZigString key = toZigString(name);
+        if (UNLIKELY(property.isEmpty() || property.isNull()))
+            continue;
+
+        // ignore constructor
+        if (property == vm.propertyNames->constructor || clientData->builtinNames().bunNativePtrPrivateName() == property)
+            continue;
+
+        JSC::PropertySlot slot(object, PropertySlot::InternalMethodType::Get);
+        if (!object->getPropertySlot(globalObject, property, slot))
+            continue;
+
+        if ((slot.attributes() & PropertyAttribute::DontEnum) != 0) {
+            if (property == vm.propertyNames->underscoreProto
+                || property == vm.propertyNames->toStringTagSymbol)
+                continue;
+        }
+
+        JSC::JSValue propertyValue = jsUndefined();
         auto scope = DECLARE_CATCH_SCOPE(vm);
-        JSValue propertyValue = object->get(globalObject, property);
-        if (scope.exception()) {
+        if ((slot.attributes() & PropertyAttribute::DontEnum) != 0) {
+            if ((slot.attributes() & PropertyAttribute::Accessor) != 0) {
+                propertyValue = slot.getPureResult();
+            } else if (slot.attributes() & PropertyAttribute::BuiltinOrFunction) {
+                propertyValue = slot.getValue(globalObject, property);
+            } else if (slot.isCustom()) {
+                propertyValue = slot.getValue(globalObject, property);
+            } else if (slot.isValue()) {
+                propertyValue = slot.getValue(globalObject, property);
+            } else if (object->getOwnPropertySlot(object, globalObject, property, slot)) {
+                propertyValue = slot.getValue(globalObject, property);
+            }
+        } else if ((slot.attributes() & PropertyAttribute::Accessor) != 0) {
+            propertyValue = slot.getPureResult();
+        } else {
+            propertyValue = slot.getValue(globalObject, property);
+        }
+
+        if (UNLIKELY(scope.exception())) {
             scope.clearException();
             propertyValue = jsUndefined();
         }
 
+        const WTF::StringImpl* name = property.isSymbol() && !property.isPrivateName() ? property.impl() : property.string().impl();
+        ZigString key = toZigString(name);
+
         JSC::EnsureStillAliveScope ensureStillAliveScope(propertyValue);
         iter(globalObject, arg2, &key, JSC::JSValue::encode(propertyValue), property.isSymbol());
     }
-- 
cgit v1.2.3