Skip to content

Commit 7b4d288

Browse files
puskin94aduh95
authored andcommitted
assert: make partialDeepStrictEqual throw when comparing [0] with [-0]
Fixes: #56230 PR-URL: #56237 Reviewed-By: James M Snell <[email protected]> Reviewed-By: LiviaMedeiros <[email protected]> Reviewed-By: Xuguang Mei <[email protected]> Reviewed-By: Jordan Harband <[email protected]>
1 parent 20ace0b commit 7b4d288

File tree

2 files changed

+100
-19
lines changed

2 files changed

+100
-19
lines changed

lib/assert.js

+50-19
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,7 @@ const {
5656
StringPrototypeIndexOf,
5757
StringPrototypeSlice,
5858
StringPrototypeSplit,
59+
Symbol,
5960
SymbolIterator,
6061
TypedArrayPrototypeGetLength,
6162
Uint8Array,
@@ -497,6 +498,17 @@ function partiallyCompareSets(actual, expected, comparedObjects) {
497498
return true;
498499
}
499500

501+
const minusZeroSymbol = Symbol('-0');
502+
const zeroSymbol = Symbol('0');
503+
504+
// Helper function to get a unique key for 0, -0 to avoid collisions
505+
function getZeroKey(item) {
506+
if (item === 0) {
507+
return ObjectIs(item, -0) ? minusZeroSymbol : zeroSymbol;
508+
}
509+
return item;
510+
}
511+
500512
function partiallyCompareArrays(actual, expected, comparedObjects) {
501513
if (expected.length > actual.length) {
502514
return false;
@@ -506,39 +518,58 @@ function partiallyCompareArrays(actual, expected, comparedObjects) {
506518

507519
// Create a map to count occurrences of each element in the expected array
508520
const expectedCounts = new SafeMap();
509-
for (const expectedItem of expected) {
510-
let found = false;
511-
for (const { 0: key, 1: count } of expectedCounts) {
512-
if (isDeepStrictEqual(key, expectedItem)) {
513-
expectedCounts.set(key, count + 1);
514-
found = true;
515-
break;
521+
const safeExpected = new SafeArrayIterator(expected);
522+
523+
for (const expectedItem of safeExpected) {
524+
// Check if the item is a zero or a -0, as these need to be handled separately
525+
if (expectedItem === 0) {
526+
const zeroKey = getZeroKey(expectedItem);
527+
expectedCounts.set(zeroKey, (expectedCounts.get(zeroKey)?.count || 0) + 1);
528+
} else {
529+
let found = false;
530+
for (const { 0: key, 1: count } of expectedCounts) {
531+
if (isDeepStrictEqual(key, expectedItem)) {
532+
expectedCounts.set(key, count + 1);
533+
found = true;
534+
break;
535+
}
536+
}
537+
if (!found) {
538+
expectedCounts.set(expectedItem, 1);
516539
}
517-
}
518-
if (!found) {
519-
expectedCounts.set(expectedItem, 1);
520540
}
521541
}
522542

523543
const safeActual = new SafeArrayIterator(actual);
524544

525-
// Create a map to count occurrences of relevant elements in the actual array
526545
for (const actualItem of safeActual) {
527-
for (const { 0: key, 1: count } of expectedCounts) {
528-
if (isDeepStrictEqual(key, actualItem)) {
546+
// Check if the item is a zero or a -0, as these need to be handled separately
547+
if (actualItem === 0) {
548+
const zeroKey = getZeroKey(actualItem);
549+
550+
if (expectedCounts.has(zeroKey)) {
551+
const count = expectedCounts.get(zeroKey);
529552
if (count === 1) {
530-
expectedCounts.delete(key);
553+
expectedCounts.delete(zeroKey);
531554
} else {
532-
expectedCounts.set(key, count - 1);
555+
expectedCounts.set(zeroKey, count - 1);
556+
}
557+
}
558+
} else {
559+
for (const { 0: expectedItem, 1: count } of expectedCounts) {
560+
if (isDeepStrictEqual(expectedItem, actualItem)) {
561+
if (count === 1) {
562+
expectedCounts.delete(expectedItem);
563+
} else {
564+
expectedCounts.set(expectedItem, count - 1);
565+
}
566+
break;
533567
}
534-
break;
535568
}
536569
}
537570
}
538571

539-
const { size } = expectedCounts;
540-
expectedCounts.clear();
541-
return size === 0;
572+
return expectedCounts.size === 0;
542573
}
543574

544575
/**

test/parallel/test-assert-objects.js

+50
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,41 @@ describe('Object Comparison Tests', () => {
9797
actual: [1, 'two', true],
9898
expected: [1, 'two', false],
9999
},
100+
{
101+
description: 'throws when comparing [0] with [-0]',
102+
actual: [0],
103+
expected: [-0],
104+
},
105+
{
106+
description: 'throws when comparing [0, 0, 0] with [0, -0]',
107+
actual: [0, 0, 0],
108+
expected: [0, -0],
109+
},
110+
{
111+
description: 'throws when comparing ["-0"] with [-0]',
112+
actual: ['-0'],
113+
expected: [-0],
114+
},
115+
{
116+
description: 'throws when comparing [-0] with [0]',
117+
actual: [-0],
118+
expected: [0],
119+
},
120+
{
121+
description: 'throws when comparing [-0] with ["-0"]',
122+
actual: [-0],
123+
expected: ['-0'],
124+
},
125+
{
126+
description: 'throws when comparing ["0"] with [0]',
127+
actual: ['0'],
128+
expected: [0],
129+
},
130+
{
131+
description: 'throws when comparing [0] with ["0"]',
132+
actual: [0],
133+
expected: ['0'],
134+
},
100135
{
101136
description:
102137
'throws when comparing two Date objects with different times',
@@ -385,6 +420,21 @@ describe('Object Comparison Tests', () => {
385420
actual: [1, 'two', true],
386421
expected: [1, 'two', true],
387422
},
423+
{
424+
description: 'compares [0] with [0]',
425+
actual: [0],
426+
expected: [0],
427+
},
428+
{
429+
description: 'compares [-0] with [-0]',
430+
actual: [-0],
431+
expected: [-0],
432+
},
433+
{
434+
description: 'compares [0, -0, 0] with [0, 0]',
435+
actual: [0, -0, 0],
436+
expected: [0, 0],
437+
},
388438
{
389439
description: 'compares two Date objects with the same time',
390440
actual: new Date(0),

0 commit comments

Comments
 (0)