Skip to content

bun:test: run an asymmetric matcher at a key or index the other side lacks #42536

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
robobun wants to merge 8 commits into
base: main
Choose a base branch
from
Open

bun:test: run an asymmetric matcher at a key or index the other side lacks #42536

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
c1a268e
bun:test: run an asymmetric matcher at a key or index the other side …
robobun Sep 13, 2026
6ecd792
bun:test: apply the missing-key matcher rule to Error properties and …
robobun Sep 13, 2026
ae98ead
ci: retrigger
robobun Sep 13, 2026
394dc92
bun:test: shorten comments in Bun__deepEquals
robobun Sep 13, 2026
e287ff9
bun:test: shorten the matcherOnlyKeys comment
robobun Sep 13, 2026
d51e22d
Merge branch 'main' into robobun/148ab1ee/toequal-matcher-missing-key
robobun Sep 30, 2026
39b2f77
bun:test: walk the second object's extra names by name, keep the fast…
robobun Oct 1, 2026
b95a1da
bun:test: shorten two comments in the name walks
robobun Oct 1, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
175 changes: 157 additions & 18 deletions src/jsc/bindings/bindings.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -627,6 +627,24 @@ AsymmetricMatcherResult matchAsymmetricMatcher(JSGlobalObject* globalObject, JSV
return result;
}

// The values matchAsymmetricMatcherAndGetFlags handles. Runs no user code.
static bool isAsymmetricMatcher(JSValue value)
{
if (value.isEmpty() || !value.isCell())
return false;
JSCell* cell = value.asCell();
if (cell->type() != JSC::JSType(JSDOMWrapperType))
return false;
return cell->inherits<JSExpectAnything>()
|| cell->inherits<JSExpectAny>()
|| cell->inherits<JSExpectStringContaining>()
|| cell->inherits<JSExpectStringMatching>()
|| cell->inherits<JSExpectArrayContaining>()
|| cell->inherits<JSExpectObjectContaining>()
|| cell->inherits<JSExpectCloseTo>()
|| cell->inherits<JSExpectCustomAsymmetricMatcher>();
}

template<typename PromiseType, bool isInternal>
static void handlePromise(PromiseType* promise, JSC::JSGlobalObject* globalObject, JSC::EncodedJSValue ctx, Zig::FFIFunction resolverFunction, Zig::FFIFunction rejecterFunction)
{
Expand Down Expand Up @@ -1002,7 +1020,12 @@ bool Bun__deepEquals(JSC::JSGlobalObject* globalObject, JSValue v1, JSValue v2,
}

if constexpr (!isStrict) {
if (((left.isEmpty() || right.isEmpty()) && (left.isUndefined() || right.isUndefined()))) {
// a hole or an index past the end reads as undefined
if (left.isEmpty())
left = jsUndefined();
if (right.isEmpty())
right = jsUndefined();
if (left.isUndefined() && right.isUndefined()) {
continue;
}
}
Expand All @@ -1020,6 +1043,15 @@ bool Bun__deepEquals(JSC::JSGlobalObject* globalObject, JSValue v1, JSValue v2,
continue;
}

if constexpr (!isStrict && enableAsymmetricMatchers) {
if (isAsymmetricMatcher(right)) {
auto eql = Bun__deepEquals<isStrict, enableAsymmetricMatchers, checkPrototypes, skipPrototypeIdentity>(globalObject, jsUndefined(), right, gcBuffer, stack, scope, true);
RETURN_IF_EXCEPTION(scope, false);
if (!eql) return false;
continue;
}
}

return false;
}

Expand Down Expand Up @@ -1062,6 +1094,11 @@ bool Bun__deepEquals(JSC::JSGlobalObject* globalObject, JSValue v1, JSValue v2,
if (prop1.isUndefined() && prop2.isEmpty()) {
continue;
}
if constexpr (enableAsymmetricMatchers) {
if (prop2.isEmpty() && isAsymmetricMatcher(prop1)) {
prop2 = jsUndefined();
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
}
}

if (!prop2) {
Comment on lines 1094 to 1104

@claude claude Bot Sep 30, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟣 pre-existing, not blocking: pre-existing: a matcher at a symbol key that only the expected array has is never evaluated, so toEqual passes where Jest fails. The array own-property walk at bindings.cpp:1079-1111 iterates a1 only; unlike the object paths there is no "remaining properties in the other object" pass, so a2-only keys are ignored and bindings.cpp:1113 returns true. expect([1]).toEqual(Object.assign([1], { [sym]: expect.any(Function) })) passes; Jest pushes the matcher-only key and fails. Fix: after the a1 loop, walk a2's keys absent from a1 and require each to be undefined or a matcher that accepts o1->get(key), mirroring bindings.cpp:1355-1378.
A small fix can ride a push you are already making; otherwise a short reply is enough.

Why this was flagged

Two arrays whose index elements match, where the expected (v2) array carries an own enumerable symbol-keyed property that the received (v1) array lacks, e.g. expect([1]).toEqual(Object.assign([1], { [Symbol("s")]: expect.any(Function) })), reaching Bun__deepEquals via JSC__JSValue__jestDeepEquals (bindings.cpp:3325). After the index loops, bindings.cpp:1064-1069 collects symbol keys of both arrays but bindings.cpp:1071 sets propertyLength = a1.size() and the loop at :1079 walks only a1; there is no pass over a2's extra keys, so :1113 returns true. Jest's eq adds every asymmetric-matcher key of b that a lacks to aKeys and evaluates eq(undefined, any(Function)) → false. This is pre-existing: the base also returns true for Object.assign([1], { [sym]: 5 }) as the expected side. The PR's new code at :1097-1101 handles the mirror case (matcher on o1, key missing on o2) at this site but does not add the reverse pass that every object path (:1355-1378, :1263-1273, :1717-1741) has, so the fix is incomplete for arrays with symbol keys.

Verification: pre-existing. Trigger: non-strict toEqual on two arrays where only the expected array (v2) carries an own enumerable symbol-keyed property. The loop at bindings.cpp:1079 walks only a1's keys; the only a2 check is the isStrict size comparison at :1072-1076, which is compiled out for toEqual, so a2-only keys are never read and :1113 return true; fires. Base branch is identical, so merging makes nothing worse.

Expand Down Expand Up @@ -1095,6 +1132,8 @@ bool Bun__deepEquals(JSC::JSGlobalObject* globalObject, JSValue v1, JSValue v2,
bool sameStructure = o2Structure->id() == o1Structure->id();
// Comparing values runs user getters that can rehash this PropertyTable mid-walk (use-after-free), so collect the pairs first and compare after.
MarkedArgumentBuffer pairs;
// matcher on one side, no own property on the other: read with get() after the walks
Vector<Identifier, 4> matcherOnlyKeys;
if (sameStructure) {
o1Structure->forEachProperty(vm, [&](const PropertyTableEntry& entry) -> bool {
if (entry.attributes() & PropertyAttribute::DontEnum || PropertyName(entry.key()).isPrivateName()) {
Expand Down Expand Up @@ -1125,7 +1164,6 @@ bool Bun__deepEquals(JSC::JSGlobalObject* globalObject, JSValue v1, JSValue v2,
if (entry.attributes() & PropertyAttribute::DontEnum || PropertyName(entry.key()).isPrivateName()) {
return true;
}
count++;

JSValue left = o1->getDirect(entry.offset());
JSValue right;
Expand All @@ -1147,13 +1185,21 @@ bool Bun__deepEquals(JSC::JSGlobalObject* globalObject, JSValue v1, JSValue v2,
if (left.isUndefined() && right.isEmpty()) {
return true;
}
if constexpr (enableAsymmetricMatchers) {
if (right.isEmpty() && isAsymmetricMatcher(left)) {
matcherOnlyKeys.append(Identifier::fromUid(vm, entry.key()));
return true;
Comment thread
claude[bot] marked this conversation as resolved.
}
}
}

if (!right) {
result = false;
return false;
}

// `remain` below counts only the properties in `pairs`.
count++;
pairs.appendWithCrashOnOverflow(left);
pairs.appendWithCrashOnOverflow(right);
return true;
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Expand All @@ -1174,6 +1220,12 @@ bool Bun__deepEquals(JSC::JSGlobalObject* globalObject, JSValue v1, JSValue v2,

// Membership check only; every left property is in `pairs` and compared below.
if (o1->getDirectOffset(vm, JSC::PropertyName(entry.key())) == invalidOffset) {
if constexpr (!isStrict && enableAsymmetricMatchers) {
if (isAsymmetricMatcher(o2->getDirect(entry.offset()))) {
matcherOnlyKeys.append(Identifier::fromUid(vm, entry.key()));
return true;
}
}
result = false;
return false;
}
Expand Down Expand Up @@ -1209,6 +1261,18 @@ bool Bun__deepEquals(JSC::JSGlobalObject* globalObject, JSValue v1, JSValue v2,
}
}

for (const Identifier& key : matcherOnlyKeys) {
JSValue left = o1->get(globalObject, key);
RETURN_IF_EXCEPTION(scope, false);
JSValue right = o2->get(globalObject, key);
RETURN_IF_EXCEPTION(scope, false);
auto eql = Bun__deepEquals<isStrict, enableAsymmetricMatchers, checkPrototypes, skipPrototypeIdentity>(globalObject, left, right, gcBuffer, stack, scope, true);
RETURN_IF_EXCEPTION(scope, false);
if (!eql) {
return false;
}
}

return true;
}
}
Expand Down Expand Up @@ -1238,8 +1302,7 @@ bool Bun__deepEquals(JSC::JSGlobalObject* globalObject, JSValue v1, JSValue v2,
}

// take a property name from one, try to get it from both
size_t i;
for (i = 0; i < propertyArrayLength1; i++) {
for (size_t i = 0; i < propertyArrayLength1; i++) {
Identifier i1 = a1[i];
PropertyName propertyName1 = PropertyName(i1);

Expand Down Expand Up @@ -1271,6 +1334,11 @@ bool Bun__deepEquals(JSC::JSGlobalObject* globalObject, JSValue v1, JSValue v2,
if (prop1.isUndefined() && prop2.isEmpty()) {
continue;
}
if constexpr (enableAsymmetricMatchers) {
if (prop2.isEmpty() && isAsymmetricMatcher(prop1)) {
prop2 = jsUndefined();
}
}
}

if (!prop2) {
Expand All @@ -1282,15 +1350,52 @@ bool Bun__deepEquals(JSC::JSGlobalObject* globalObject, JSValue v1, JSValue v2,
if (!eql) return false;
}

// for the remaining properties in the other object, make sure they are undefined
for (; i < propertyArrayLength2; i++) {
Identifier i2 = a2[i];
PropertyName propertyName2 = PropertyName(i2);
// names only the second object enumerates must be undefined or a matcher that accepts o1's read
if constexpr (!isStrict) {
for (size_t j = 0; j < propertyArrayLength2; j++) {
Identifier i2 = a2[j];
PropertyName propertyName2 = PropertyName(i2);

JSValue prop2 = o2->getIfPropertyExists(globalObject, propertyName2);
RETURN_IF_EXCEPTION(scope, false);
// same name at the same position: compared in the first loop
if (j < propertyArrayLength1 && a1[j] == i2) {
continue;
}
// the chain lookup matches how a1 was built (the node entry point is strict only)
static_assert(!checkPrototypes);
PropertySlot slot1(o1, PropertySlot::InternalMethodType::HasProperty);
bool has1 = o1->getPropertySlot(globalObject, propertyName2, slot1);
RETURN_IF_EXCEPTION(scope, false);
if (has1 && !(slot1.attributes() & PropertyAttribute::DontEnum)) {
continue;
}
Comment on lines +1365 to +1370

@claude claude Bot Oct 1, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 When the received value is a Proxy whose has trap answers true for names it does not own, toEqual now passes with extra keys on expected that base and Jest reject. The a2 walk decides "already compared" from a HasProperty chain probe at bindings.cpp:1366-1369, and a Proxy has hit reports non-DontEnum attributes, so the extra name is skipped and its value never read. Fix: treat a name as compared only when it is in a1 (build a name set from a1 once) and run the existing own/matcher/mismatch checks on every other name, in both the generic walk and the Error walk at bindings.cpp:1747-1751, so presence is never inferred from a HasProperty probe.

Why this was flagged

Input: received = new Proxy({ a: 1 }, { has: () => true }), expected = { a: 1, b: 2 }, through expect(received).toEqual(expected). A Proxy overrides getOwnPropertySlot so the structure fast path at bindings.cpp:1123 is skipped. a1 is built from the proxy's ownKeys at bindings.cpp:1290 = ['a']; a2 = ['a', 'b']. The first loop compares a. In the new a2 walk, j = 1 is 'b': bindings.cpp:1366 calls o1->getPropertySlot with InternalMethodType::HasProperty, which for a ProxyObject runs the has trap and on true sets the slot with PropertyAttribute::None; has1 is true and the DontEnum test at bindings.cpp:1368 fails, so the loop continues without reading expected's 'b' and the function returns true at bindings.cpp:1403. On the base branch the trailing loop at i = 1 reads o2->getIfPropertyExists('b') = 2, which is not undefined, and returns false; Jest's eq uses hasOwnProperty for hasKey so 'b' is counted as missing and the key counts differ, returning false. No later check reads prop2 for a name that took the continue at bindings.cpp:1369.

Verification: Triggered when received is a Proxy whose [[HasProperty]] answers true for a name its own-keys enumeration does not report, and expected carries an extra non-matcher key at that name. At bindings.cpp:1366 the HasProperty probe runs the has trap, so :1368 is true and the loop continues; 'b' is never read from o2 and :1403 returns true where base returned false.


JSValue prop2 = o2->getIfPropertyExists(globalObject, propertyName2);
RETURN_IF_EXCEPTION(scope, false);

if (prop2.isUndefined()) {
continue;
}

if constexpr (enableAsymmetricMatchers) {
// Jest counts an own non-enumerable key as present, so it stays a mismatch
if (has1) {
PropertySlot ownSlot(o1, PropertySlot::InternalMethodType::GetOwnProperty);
bool own1 = o1->methodTable()->getOwnPropertySlot(o1, globalObject, propertyName2, ownSlot);
RETURN_IF_EXCEPTION(scope, false);
if (own1) {
return false;
}
}
if (isAsymmetricMatcher(prop2)) {
JSValue prop1 = o1->get(globalObject, propertyName2);
RETURN_IF_EXCEPTION(scope, false);
auto eql = Bun__deepEquals<isStrict, enableAsymmetricMatchers, checkPrototypes, skipPrototypeIdentity>(globalObject, prop1, prop2, gcBuffer, stack, scope, true);
RETURN_IF_EXCEPTION(scope, false);
if (!eql) return false;
continue;
}
}

if (!prop2.isUndefined()) {
return false;
}
}
Expand Down Expand Up @@ -1597,8 +1702,7 @@ static std::optional<bool> specialObjectsDequalSlow(const DeepEqualsMode& mode,
}

// take a property name from one, try to get it from both
size_t i;
for (i = 0; i < propertyArrayLength1; i++) {
for (size_t i = 0; i < propertyArrayLength1; i++) {
Identifier i1 = a1[i];
if (i1 == vm.propertyNames->stack) continue;
PropertyName propertyName1 = PropertyName(i1);
Expand All @@ -1614,6 +1718,9 @@ static std::optional<bool> specialObjectsDequalSlow(const DeepEqualsMode& mode,
if (prop1.isUndefined() && prop2.isEmpty()) {
continue;
}
if (mode.enableAsymmetricMatchers && prop2.isEmpty() && isAsymmetricMatcher(prop1)) {
prop2 = jsUndefined();
}
}

if (!prop2) {
Expand All @@ -1627,18 +1734,50 @@ static std::optional<bool> specialObjectsDequalSlow(const DeepEqualsMode& mode,
}
}

// for the remaining properties in the other object, make sure they are undefined
for (; i < propertyArrayLength2; i++) {
Identifier i2 = a2[i];
// names only the right Error enumerates must be undefined or a matcher that accepts left's read
for (size_t j = 0; !mode.isStrict && j < propertyArrayLength2; j++) {
Identifier i2 = a2[j];
if (i2 == vm.propertyNames->stack) continue;
PropertyName propertyName2 = PropertyName(i2);

if (j < propertyArrayLength1 && a1[j] == i2) {
continue;
}
PropertySlot slot1(left, PropertySlot::InternalMethodType::HasProperty);
bool has1 = left->getPropertySlot(globalObject, propertyName2, slot1);
RETURN_IF_EXCEPTION(scope, {});
if (has1 && !(slot1.attributes() & PropertyAttribute::DontEnum)) {
continue;
}

JSValue prop2 = right->getIfPropertyExists(globalObject, propertyName2);
RETURN_IF_EXCEPTION(scope, {});

if (!prop2.isUndefined()) {
return false;
if (prop2.isUndefined()) {
continue;
}

if (mode.enableAsymmetricMatchers && isAsymmetricMatcher(prop2)) {
// Jest counts an own non-enumerable key as present, so it stays a mismatch
if (has1) {
PropertySlot ownSlot(left, PropertySlot::InternalMethodType::GetOwnProperty);
bool own1 = left->methodTable()->getOwnPropertySlot(left, globalObject, propertyName2, ownSlot);
RETURN_IF_EXCEPTION(scope, {});
if (own1) {
return false;
}
}
JSValue prop1 = left->get(globalObject, propertyName2);
RETURN_IF_EXCEPTION(scope, {});
bool propertiesEqual = mode.deepEquals(globalObject, prop1, prop2, gcBuffer, stack, scope, true);
RETURN_IF_EXCEPTION(scope, {});
if (!propertiesEqual) {
return false;
}
continue;
}

return false;
}

return true;
Expand Down
Loading
Loading