diff --git a/src/jsc/bindings/bindings.cpp b/src/jsc/bindings/bindings.cpp index 31ba33ed07ef..b349481587ae 100644 --- a/src/jsc/bindings/bindings.cpp +++ b/src/jsc/bindings/bindings.cpp @@ -1793,8 +1793,9 @@ static std::optional specialObjectsDequalSlow(const DeepEqualsMode& mode, return false; } if (mode.checkPrototypes) { - // node also compares `lastIndex` and own enumerable properties. - bool sameLastIndex = JSC::sameValue(globalObject, left->getLastIndex(), right->getLastIndex()); + // node compares `lastIndex` with ===, not Object.is, then own enumerable properties: + // https://github.com/nodejs/node/blob/v26.12.1/lib/internal/util/comparisons.js#L148-L152 + bool sameLastIndex = JSC::JSValue::strictEqual(globalObject, left->getLastIndex(), right->getLastIndex()); RETURN_IF_EXCEPTION(scope, {}); if (!sameLastIndex) { return false; diff --git a/src/jsc/modules/NodeUtilTypesModule.cpp b/src/jsc/modules/NodeUtilTypesModule.cpp index 925fd83a6b7f..2e4f8cdb7e21 100644 --- a/src/jsc/modules/NodeUtilTypesModule.cpp +++ b/src/jsc/modules/NodeUtilTypesModule.cpp @@ -808,8 +808,9 @@ static bool compareBranch(JSC::JSGlobalObject* globalObject, JSC::MarkedArgument if (actualRegExp->regExp()->pattern() != expectedRegExp->regExp()->pattern() || actualRegExp->regExp()->flags() != expectedRegExp->regExp()->flags()) return false; - // node also compares lastIndex strictly (comparisons.js isRegExp arm). - bool lastIndexEqual = JSC::sameValue(globalObject, actualRegExp->getLastIndex(), expectedRegExp->getLastIndex()); + // node compares lastIndex with ===, not Object.is: + // https://github.com/nodejs/node/blob/v26.12.1/lib/internal/util/comparisons.js#L148-L152 + bool lastIndexEqual = JSC::JSValue::strictEqual(globalObject, actualRegExp->getLastIndex(), expectedRegExp->getLastIndex()); RETURN_IF_EXCEPTION(scope, false); if (!lastIndexEqual) return false; diff --git a/test/js/node/assert/assert.test.cjs b/test/js/node/assert/assert.test.cjs index a8a2043b730b..03b0449bd165 100644 --- a/test/js/node/assert/assert.test.cjs +++ b/test/js/node/assert/assert.test.cjs @@ -85,6 +85,45 @@ describe("assert.partialDeepStrictEqual", () => { assert.partialDeepStrictEqual({ arr: [shared, shared] }, { arr: [{ x: 1 }] }); }); + // Node's areSimilarRegExps() compares lastIndex with ===: -0 equals 0, NaN differs from NaN, "0" differs from 0. + // https://github.com/nodejs/node/blob/v26.12.1/lib/internal/util/comparisons.js#L148-L152 + test.each([ + ["at the top level", re => re], + ["in an array", re => [re]], + ["in an object property", re => ({ re })], + ["in a Map value", re => new Map([["k", re]])], + ["in a Map key", re => new Map([[re, 1]])], + ["in a Set", re => new Set([re])], + ["in an Error cause", re => new Error("e", { cause: re })], + ])("RegExp lastIndex is compared with === %s", (_name, wrap) => { + const withLastIndex = lastIndex => Object.assign(/a/g, { lastIndex }); + const partial = (actual, expected) => { + try { + assert.partialDeepStrictEqual(wrap(actual), wrap(expected)); + return "equal"; + } catch (e) { + return e.code; + } + }; + const sameObject = withLastIndex(NaN); + expect({ + negativeZeroAndZero: partial(withLastIndex(-0), withLastIndex(0)), + zeroAndNegativeZero: partial(withLastIndex(0), withLastIndex(-0)), + nanAndNaN: partial(withLastIndex(NaN), withLastIndex(NaN)), + stringZeroAndZero: partial(withLastIndex("0"), withLastIndex(0)), + threeAndZero: partial(withLastIndex(3), withLastIndex(0)), + // The identity shortcut answers before lastIndex is read. + sameObjectWithNaN: partial(sameObject, sameObject), + }).toEqual({ + negativeZeroAndZero: "equal", + zeroAndNegativeZero: "equal", + nanAndNaN: "ERR_ASSERTION", + stringZeroAndZero: "ERR_ASSERTION", + threeAndZero: "ERR_ASSERTION", + sameObjectWithNaN: "equal", + }); + }); + test("a descendant actual identical to an ancestor expected is not mistaken for a cycle", () => { const e = { k: {} }; assert.partialDeepStrictEqual({ k: e }, e); diff --git a/test/js/node/assert/deep-equal.test.ts b/test/js/node/assert/deep-equal.test.ts index abb31a1a0e3d..dc492f10699e 100644 --- a/test/js/node/assert/deep-equal.test.ts +++ b/test/js/node/assert/deep-equal.test.ts @@ -658,6 +658,62 @@ const cases: Case[] = [ // RegExp. { name: "two equal regexps", a: () => /a/g, b: () => /a/g, strict: true, loose: true }, { name: "regexps with different flags", a: () => /a/g, b: () => /a/i, strict: false, loose: false }, + // lastIndex is compared with ===, not Object.is, in every mode. + { + name: "regexps with lastIndex -0 and 0", + a: () => Object.assign(/a/g, { lastIndex: -0 }), + b: () => /a/g, + strict: true, + loose: true, + }, + { + name: "regexps with lastIndex NaN", + a: () => Object.assign(/a/g, { lastIndex: NaN }), + b: () => Object.assign(/a/g, { lastIndex: NaN }), + strict: false, + loose: false, + looseBug: "reports equal", + }, + // A lastIndex of 0 and of -0 make two deep-equal members. The Set and Map walks take the + // first deep-equal member and do not pair members one to one. + // https://github.com/oven-sh/bun/issues/28760 + { + name: "sets of three regexps with lastIndex 0, -0, 1 and 0, 1, 1", + a: () => + new Set([ + Object.assign(/a/g, { lastIndex: 0 }), + Object.assign(/a/g, { lastIndex: -0 }), + Object.assign(/a/g, { lastIndex: 1 }), + ]), + b: () => + new Set([ + Object.assign(/a/g, { lastIndex: 0 }), + Object.assign(/a/g, { lastIndex: 1 }), + Object.assign(/a/g, { lastIndex: 1 }), + ]), + strict: false, + loose: false, + strictBug: "reports equal", + looseBug: "reports equal", + }, + // https://github.com/oven-sh/bun/issues/34830 + { + name: "maps with two regexp keys whose lastIndex is 0 and -0", + a: () => + new Map([ + [Object.assign(/a/g, { lastIndex: 0 }), 1], + [Object.assign(/a/g, { lastIndex: -0 }), 2], + ]), + b: () => + new Map([ + [Object.assign(/a/g, { lastIndex: 0 }), 1], + [Object.assign(/a/g, { lastIndex: -0 }), 2], + ]), + strict: true, + loose: true, + strictBug: "reports not equal", + looseBug: "reports not equal", + }, { name: "regexps with different lastIndex", a: () => Object.assign(/a/g, { lastIndex: 3 }), @@ -1162,6 +1218,49 @@ describe("util.isDeepStrictEqual", () => { expect(calls).toBe(2); }); + // Node's areSimilarRegExps() compares lastIndex with ===: -0 equals 0, NaN differs from NaN, "0" differs from 0. + // https://github.com/nodejs/node/blob/v26.12.1/lib/internal/util/comparisons.js#L148-L152 + describe("RegExp lastIndex", () => { + const withLastIndex = (lastIndex: unknown) => Object.assign(/a/g, { lastIndex }); + const positions: Array<[string, (re: RegExp) => unknown]> = [ + ["at the top level", re => re], + ["in an array", re => [re]], + ["in an object property", re => ({ re })], + ["in a Map value", re => new Map([["k", re]])], + ["in a Map key", re => new Map([[re, 1]])], + ["in a Set", re => new Set([re])], + ["in an Error cause", re => new Error("e", { cause: re })], + ]; + + for (const [name, wrap] of positions) { + test(`is compared with === ${name}`, () => { + const sameObject = withLastIndex(NaN); + for (const skipPrototype of [false, true]) { + const equal = (a: RegExp, b: RegExp) => util.isDeepStrictEqual(wrap(a), wrap(b), skipPrototype as any); + assert.deepStrictEqual( + { + skipPrototype, + negativeZeroAndZero: equal(withLastIndex(-0), withLastIndex(0)), + zeroAndNegativeZero: equal(withLastIndex(0), withLastIndex(-0)), + nanAndNaN: equal(withLastIndex(NaN), withLastIndex(NaN)), + stringZeroAndZero: equal(withLastIndex("0"), withLastIndex(0)), + // The identity shortcut answers before lastIndex is read. + sameObjectWithNaN: equal(sameObject, sameObject), + }, + { + skipPrototype, + negativeZeroAndZero: true, + zeroAndNegativeZero: true, + nanAndNaN: false, + stringZeroAndZero: false, + sameObjectWithNaN: true, + }, + ); + } + }); + } + }); + // The third argument was added in Node v26. describe("skipPrototype", () => { class Foo {