fix `deepEquals` with array holes and accessors (#2557)

* `deepEqual` handles slow array indexes

* another test

* oops

* remove bad test

* compare indexes in non-strict mode

* more tests

2 files changed, 249 insertions, 10 deletions

diff --git a/src/bun.js/bindings/bindings.cpp b/src/bun.js/bindings/bindings.cpp
index 7c1b3d82a..8c1712338 100644
--- a/src/bun.js/bindings/bindings.cpp
+++ b/src/bun.js/bindings/bindings.cpp
@@ -167,6 +167,22 @@ static bool canPerformFastPropertyEnumerationForIterationBun(Structure* s)
     return true;
 }
 
+JSValue getIndexWithoutAccessors(JSGlobalObject* globalObject, JSObject* obj, uint64_t i)
+{
+    if (obj->canGetIndexQuickly(i)) {
+        return obj->tryGetIndexQuickly(i);
+    }
+
+    PropertySlot slot(obj, PropertySlot::InternalMethodType::Get);
+    if (obj->methodTable()->getOwnPropertySlotByIndex(obj, globalObject, i, slot)) {
+        if (!slot.isAccessor()) {
+            return slot.getValue(globalObject, i);
+        }
+    }
+
+    return JSValue();
+}
+
 template<bool isStrict>
 bool Bun__deepEquals(JSC__JSGlobalObject* globalObject, JSValue v1, JSValue v2, Vector<std::pair<JSC::JSValue, JSC::JSValue>, 16>& stack, ThrowScope* scope, bool addToStack)
 {
@@ -478,13 +494,9 @@ bool Bun__deepEquals(JSC__JSGlobalObject* globalObject, JSValue v1, JSValue v2,
         }
 
         for (uint64_t i = 0; i < length; i++) {
-            JSValue left = o1->canGetIndexQuickly(i)
-                ? o1->getIndexQuickly(i)
-                : o1->tryGetIndexQuickly(i);
+            JSValue left = getIndexWithoutAccessors(globalObject, o1, i);
             RETURN_IF_EXCEPTION(*scope, false);
-            JSValue right = o2->canGetIndexQuickly(i)
-                ? o2->getIndexQuickly(i)
-                : o2->tryGetIndexQuickly(i);
+            JSValue right = getIndexWithoutAccessors(globalObject, o2, i);
             RETURN_IF_EXCEPTION(*scope, false);
 
             if constexpr (isStrict) {
@@ -651,10 +663,8 @@ bool Bun__deepEquals(JSC__JSGlobalObject* globalObject, JSValue v1, JSValue v2,
     o2->getPropertyNames(globalObject, a2, DontEnumPropertiesMode::Exclude);
 
     const size_t propertyArrayLength = a1.size();
-    if constexpr (isStrict) {
-        if (propertyArrayLength != a2.size()) {
-            return false;
-        }
+    if (propertyArrayLength != a2.size()) {
+        return false;
     }
 
     // take a property name from one, try to get it from both
diff --git a/test/js/bun/test/test-test.test.ts b/test/js/bun/test/test-test.test.ts
index 90a305283..2777b35fa 100644
--- a/test/js/bun/test/test-test.test.ts
+++ b/test/js/bun/test/test-test.test.ts
@@ -187,6 +187,183 @@ test("deepEquals regex", () => {
   expect(new RegExp("s", "g")).not.toEqual(new RegExp("s", "i"));
 });
 
+test("deepEquals works with accessors", () => {
+  {
+    let l1 = [1, undefined, 2];
+    let l2 = [1, undefined, 2];
+    Object.defineProperty(l1, 6, { get: () => 1 });
+    Object.defineProperty(l2, 6, { get: () => 1 });
+    expect(l1).toEqual(l2);
+    expect(l1).toStrictEqual(l2);
+  }
+  {
+    let l1 = [1, , 2];
+    let l2 = [1, undefined, 2];
+    Object.defineProperty(l1, 6, { get: () => 1 });
+    Object.defineProperty(l2, 6, { get: () => 2 });
+    expect(l1).toEqual(l2);
+    expect(l1).not.toStrictEqual(l2);
+  }
+  {
+    let l1 = [1, , 2];
+    let l2 = [1, , 2];
+    Object.defineProperty(l1, "hi", { get: () => 4 });
+    Object.defineProperty(l2, "hi", { get: () => 5 });
+    expect(l1).toEqual(l2);
+    expect(l1).toStrictEqual(l2);
+  }
+
+  {
+    let l1 = [1, , 2];
+    let l2 = [1, , 2];
+    Object.defineProperty(l1, "hi", { set: () => 4 });
+    Object.defineProperty(l2, "hi", { set: () => 5 });
+    expect(l1).toEqual(l2);
+    expect(l1).toStrictEqual(l2);
+  }
+
+  {
+    let o1 = { a: 1, c: undefined, b: 2 };
+    let o2 = { a: 1, c: undefined, b: 2 };
+    Object.defineProperty(o1, 6, { get: () => 1 });
+    Object.defineProperty(o2, 6, { get: () => 1 });
+    expect(o1).toEqual(o2);
+    expect(o1).toStrictEqual(o2);
+  }
+  {
+    let o1 = { a: 1, c: undefined, b: 2 };
+    let o2 = { a: 1, c: undefined, b: 2 };
+    Object.defineProperty(o1, 6, { get: () => 1 });
+    Object.defineProperty(o2, 6, { get: () => 2 });
+    expect(o1).toEqual(o2);
+    expect(o1).toStrictEqual(o2);
+  }
+  {
+    let o1 = { a: 1, c: undefined, b: 2 };
+    let o2 = { a: 1, c: undefined, b: 2 };
+    Object.defineProperty(o1, "hi", { get: () => 4 });
+    Object.defineProperty(o2, "hi", { get: () => 5 });
+    expect(o1).toEqual(o2);
+    expect(o1).toStrictEqual(o2);
+  }
+
+  {
+    let o1 = { a: 1, c: undefined, b: 2 };
+    let o2 = { a: 1, c: undefined, b: 2 };
+    Object.defineProperty(o1, "hi", { set: () => 4 });
+    Object.defineProperty(o2, "hi", { set: () => 5 });
+    expect(o1).toEqual(o2);
+    expect(o1).toStrictEqual(o2);
+  }
+});
+
+test("deepEquals works with proxies", () => {
+  {
+    let p1 = new Proxy({ a: 1, b: 2 }, {});
+    let p2 = new Proxy({ a: 1, b: 2 }, {});
+    expect(p1).toEqual(p2);
+    expect(p1).toStrictEqual(p2);
+    let p3 = new Proxy({ a: 1, b: 2 }, {});
+    let p4 = new Proxy({ a: 1, b: 3 }, {});
+    expect(p3).not.toEqual(p4);
+    expect(p3).not.toStrictEqual(p4);
+  }
+  {
+    let t1 = { a: 1, b: 2 };
+    let h1 = { get: (t, k) => t[k] };
+    let p1 = new Proxy(t1, h1);
+    let t2 = { a: 1, b: 2 };
+    let h2 = { get: (t, k) => 0 };
+    let p2 = new Proxy(t2, h2);
+    expect(p1).not.toEqual(p2);
+    expect(p1).not.toStrictEqual(p2);
+  }
+  {
+    let t1 = { a: 1, b: 2 };
+    let h1 = { get: (t, k) => t[k] + 2 };
+    let p1 = new Proxy(t1, h1);
+    let t2 = { a: 1, b: 2 };
+    let h2 = { get: (t, k) => t[k] + 2 };
+    let p2 = new Proxy(t2, h2);
+    expect(p1).toEqual(p2);
+    expect(p1).toStrictEqual(p2);
+  }
+  {
+    let t1 = { a: 1, b: 2 };
+    let h1 = { get: (t, k) => t[k] + 2 };
+    let p1 = new Proxy(t1, h1);
+    let t2 = { a: 1, b: 2 };
+    let h2 = { get: (t, k) => t[k] + 3 };
+    let p2 = new Proxy(t2, h2);
+    expect(p1).not.toEqual(p2);
+    expect(p1).not.toStrictEqual(p2);
+  }
+  {
+    // same handlers, different targets
+    let t1 = { a: 1, b: 2 };
+    let t2 = { a: 1, b: 2 };
+    let h1 = { get: (t, k) => t[k] + 2 };
+    let p1 = new Proxy(t1, h1);
+    let p2 = new Proxy(t2, h1);
+    expect(p1).toEqual(p2);
+    expect(p1).toStrictEqual(p2);
+  }
+  {
+    // same targets, different handlers
+    let t1 = { a: 1, b: 2 };
+    let h1 = { get: (t, k) => t[k] + 2 };
+    let h2 = { get: (t, k) => t[k] + 3 };
+    let p1 = new Proxy(t1, h1);
+    let p2 = new Proxy(t1, h2);
+    expect(p1).not.toEqual(p2);
+    expect(p1).not.toStrictEqual(p2);
+  }
+  {
+    // property with object
+    let t1 = { a: { b: 3 } };
+    let h1 = { get: (t, k) => t[k] };
+    let p1 = new Proxy(t1, h1);
+
+    let t2 = { a: { b: 3 } };
+    let h2 = { get: (t, k) => t[k] };
+    let p2 = new Proxy(t2, h2);
+
+    expect(p1).toEqual(p2);
+    expect(p1).toStrictEqual(p2);
+
+    let t3 = { a: { b: 3 } };
+    let h3 = { get: (t, k) => t[k] };
+    let p3 = new Proxy(t3, h3);
+
+    let t4 = { a: { b: 4 } };
+    let h4 = { get: (t, k) => t[k] };
+    let p4 = new Proxy(t4, h4);
+
+    expect(p3).not.toEqual(p4);
+    expect(p3).not.toStrictEqual(p4);
+  }
+  {
+    // proxy object equals itself
+    let t1 = { a: 1, b: 2 };
+    let h1 = { get: (t, k) => t[k] + 2 };
+    let p1 = new Proxy(t1, h1);
+    expect(p1).toEqual(p1);
+    expect(p1).toStrictEqual(p1);
+  }
+  {
+    let t1 = { a: 1, b: 2 };
+    let h1 = { get: (t, k) => k };
+    let p1 = new Proxy(t1, h1);
+
+    let t2 = { a: 1, b: 2 };
+    let h2 = { get: (t, k) => k };
+    let p2 = new Proxy(t2, h2);
+
+    expect(p1).toEqual(p2);
+    expect(p1).toStrictEqual(p2);
+  }
+});
+
 test("toThrow", () => {
   expect(() => {
     throw new Error("hello");
@@ -601,6 +778,58 @@ test("deepEquals - symbols", () => {
 });
 
 test("toEqual objects and arrays", () => {
+  {
+    let o1 = { 1: 4, 6: 3 };
+    let o2 = { 1: 4, 6: 3 };
+    expect(o1).toEqual(o2);
+    expect(o1).toStrictEqual(o2);
+  }
+  {
+    let o1 = { 1: 4, 6: 2 };
+    let o2 = { 1: 4, 6: 3 };
+    expect(o1).not.toEqual(o2);
+    expect(o1).not.toStrictEqual(o2);
+  }
+
+  {
+    let o1 = { a: 1, 3: 0 };
+    let o2 = { a: 1, 3: 0 };
+    expect(o1).toEqual(o2);
+    expect(o1).toStrictEqual(o2);
+  }
+  {
+    let o1 = { a: 1, 3: 0 };
+    let o2 = { a: 1, 3: 1 };
+    expect(o1).not.toEqual(o2);
+    expect(o1).not.toStrictEqual(o2);
+  }
+  {
+    let o1 = { a: {}, 4: { b: 3, c: { 9: 2 } } };
+    let o2 = { a: {}, 4: { b: 3, c: { 9: 2 } } };
+    expect(o1).toEqual(o2);
+    expect(o1).toStrictEqual(o2);
+  }
+  {
+    let o1 = { a: {}, 4: { b: 3, c: { 9: 2 } } };
+    let o2 = { a: {}, 4: { b: 3, c: { 9: 3 } } };
+    expect(o1).not.toEqual(o2);
+    expect(o1).not.toStrictEqual(o2);
+  }
+
+  {
+    let o1 = { a: 1, b: 2, c: 3 };
+    let o2 = { a: 1, b: 2, c: 3, 0: 1 };
+    expect(o1).not.toEqual(o2);
+    expect(o1).not.toStrictEqual(o2);
+  }
+
+  {
+    let o1 = { a: 1, b: 2, c: 3, 0: 1 };
+    let o2 = { a: 1, b: 2, c: 3 };
+    expect(o1).not.toEqual(o2);
+    expect(o1).not.toStrictEqual(o2);
+  }
+
   expect("hello").toEqual("hello");
   const s1 = Symbol("test1");
   const s2 = Symbol("test2");