From 65e1141deb56cfe778b61daecaeb2391cb4ebb4c Mon Sep 17 00:00:00 2001 From: Giovanni Date: Thu, 12 Dec 2024 11:54:48 +0100 Subject: [PATCH] assert: make partialDeepStrictEqual throw when comparing [0] with [-0] Fixes: https://github.com/nodejs/node/issues/56230 --- lib/assert.js | 79 +++++++++++++++++++++------- test.js | 6 +++ test/parallel/test-assert-objects.js | 50 ++++++++++++++++++ 3 files changed, 115 insertions(+), 20 deletions(-) create mode 100644 test.js diff --git a/lib/assert.js b/lib/assert.js index a2991a096ac081..c06991ad864fab 100644 --- a/lib/assert.js +++ b/lib/assert.js @@ -497,6 +497,17 @@ function partiallyCompareSets(actual, expected, comparedObjects) { return true; } +function isZeroOrMinusZero(item) { + return ObjectIs(item, -0) || ObjectIs(item, 0); +} + +// Helper function to get a unique key for 0, -0 to avoid collisions +function getZeroKey(item) { + if (ObjectIs(item, 0)) return '0'; + if (ObjectIs(item, -0)) return '-0'; + return item; +} + function partiallyCompareArrays(actual, expected, comparedObjects) { if (expected.length > actual.length) { return false; @@ -507,38 +518,66 @@ function partiallyCompareArrays(actual, expected, comparedObjects) { // Create a map to count occurrences of each element in the expected array const expectedCounts = new SafeMap(); for (const expectedItem of expected) { - let found = false; - for (const { 0: key, 1: count } of expectedCounts) { - if (isDeepStrictEqual(key, expectedItem)) { - expectedCounts.set(key, count + 1); - found = true; - break; + // Check if the item is a zero or a -0, as these need to be handled separately + if (isZeroOrMinusZero(expectedItem)) { + const zeroKey = getZeroKey(expectedItem); + expectedCounts.set(zeroKey, { + count: (expectedCounts.get(zeroKey)?.count || 0) + 1, + expectedType: typeof expectedItem, + }); + } else { + let found = false; + for (const { 0: key, 1: { count, expectedType } } of expectedCounts) { + // This is a very ugly solution to not make eslint valid-typeof rule whine + const haveSameType = expectedType === (typeof expectedItem === 'number' ? 'number' : typeof expectedItem); + if (isDeepStrictEqual(key, expectedItem) && haveSameType) { + expectedCounts.set(key, { count: count + 1, expectedType }); + found = true; + break; + } + } + if (!found) { + expectedCounts.set(expectedItem, { count: 1, expectedType: typeof expectedItem }); } - } - if (!found) { - expectedCounts.set(expectedItem, 1); } } const safeActual = new SafeArrayIterator(actual); - // Create a map to count occurrences of relevant elements in the actual array for (const actualItem of safeActual) { - for (const { 0: key, 1: count } of expectedCounts) { - if (isDeepStrictEqual(key, actualItem)) { - if (count === 1) { - expectedCounts.delete(key); - } else { - expectedCounts.set(key, count - 1); + // Check if the item is a zero or a -0, as these need to be handled separately + if (isZeroOrMinusZero(actualItem)) { + const zeroKey = getZeroKey(actualItem); + + if (expectedCounts.has(zeroKey)) { + const { count, expectedType } = expectedCounts.get(zeroKey); + // This is a very ugly solution to not make eslint valid-typeof rule whine + const haveSameType = expectedType === (typeof actualItem === 'number' ? 'number' : typeof actualItem); + if (haveSameType) { + if (count === 1) { + expectedCounts.delete(zeroKey); + } else { + expectedCounts.set(zeroKey, { count: count - 1, expectedType }); + } + } + } + } else { + for (const { 0: expectedItem, 1: { count, expectedType } } of expectedCounts) { + // This is a very ugly solution to not make eslint valid-typeof rule whine + const haveSameType = expectedType === (typeof actualItem === 'number' ? 'number' : typeof actualItem); + if (isDeepStrictEqual(expectedItem, actualItem) && haveSameType) { + if (count === 1) { + expectedCounts.delete(expectedItem); + } else { + expectedCounts.set(expectedItem, { count: count - 1, expectedType }); + } + break; } - break; } } } - const { size } = expectedCounts; - expectedCounts.clear(); - return size === 0; + return expectedCounts.size === 0; } /** diff --git a/test.js b/test.js new file mode 100644 index 00000000000000..fd1c5ec2b94e04 --- /dev/null +++ b/test.js @@ -0,0 +1,6 @@ +const assert = require('assert'); + +const actual = ['-0']; +const expected = [-0]; + +assert.partialDeepStrictEqual(actual, expected); \ No newline at end of file diff --git a/test/parallel/test-assert-objects.js b/test/parallel/test-assert-objects.js index 3f02ff3c274daa..af0dd5f553b615 100644 --- a/test/parallel/test-assert-objects.js +++ b/test/parallel/test-assert-objects.js @@ -97,6 +97,41 @@ describe('Object Comparison Tests', () => { actual: [1, 'two', true], expected: [1, 'two', false], }, + { + description: 'throws when comparing [0] with [-0]', + actual: [0], + expected: [-0], + }, + { + description: 'throws when comparing [0, 0, 0] with [0, -0]', + actual: [0, 0, 0], + expected: [0, -0], + }, + { + description: 'throws when comparing [-0] with [0]', + actual: [0], + expected: [-0], + }, + { + description: 'throws when comparing ["-0"] with [-0]', + actual: ['-0'], + expected: [-0], + }, + { + description: 'throws when comparing [-0] with ["-0"]', + actual: [-0], + expected: ['-0'], + }, + { + description: 'throws when comparing ["0"] with [0]', + actual: ['0'], + expected: [0], + }, + { + description: 'throws when comparing [0] with ["0"]', + actual: [0], + expected: ['0'], + }, { description: 'throws when comparing two Date objects with different times', @@ -385,6 +420,21 @@ describe('Object Comparison Tests', () => { actual: [1, 'two', true], expected: [1, 'two', true], }, + { + description: 'compares [0] with [0]', + actual: [0], + expected: [0], + }, + { + description: 'compares [-0] with [-0]', + actual: [-0], + expected: [-0], + }, + { + description: 'compares [0, -0, 0] with [0, 0]', + actual: [0, -0, 0], + expected: [0, 0], + }, { description: 'compares two Date objects with the same time', actual: new Date(0),