From ac092a1e043c834c99dd9bd81d4d6e6e6d32acbd Mon Sep 17 00:00:00 2001 From: Dylan Conway <35280289+dylan-conway@users.noreply.github.com> Date: Wed, 5 Apr 2023 13:39:51 -0700 Subject: 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 --- src/bun.js/bindings/bindings.cpp | 30 +++-- test/js/bun/test/test-test.test.ts | 229 +++++++++++++++++++++++++++++++++++++ 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 Bun__deepEquals(JSC__JSGlobalObject* globalObject, JSValue v1, JSValue v2, Vector, 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"); -- cgit v1.2.3