Skip to content
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.

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

assert: partialDeepStrictEqual now handles comparisons of ArrayBuffers, SharedArrayBuffers and Int16Arrays #56098

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
255 changes: 179 additions & 76 deletions lib/assert.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,35 +21,44 @@
'use strict';

const {
ArrayBufferIsView,
ArrayBufferPrototypeGetByteLength,
ArrayFrom,
ArrayIsArray,
ArrayPrototypeIndexOf,
ArrayPrototypeJoin,
ArrayPrototypePush,
ArrayPrototypeSlice,
DataViewPrototypeGetBuffer,
DataViewPrototypeGetByteLength,
DataViewPrototypeGetByteOffset,
Error,
FunctionPrototypeCall,
MapPrototypeDelete,
MapPrototypeGet,
MapPrototypeGetSize,
MapPrototypeHas,
MapPrototypeSet,
NumberIsNaN,
ObjectAssign,
ObjectIs,
ObjectKeys,
ObjectPrototypeIsPrototypeOf,
ObjectPrototypeToString,
ReflectApply,
ReflectHas,
ReflectOwnKeys,
RegExpPrototypeExec,
SafeArrayIterator,
SafeMap,
SafeSet,
SafeWeakSet,
SetPrototypeGetSize,
String,
StringPrototypeIndexOf,
StringPrototypeSlice,
StringPrototypeSplit,
SymbolIterator,
TypedArrayPrototypeGetLength,
Uint8Array,
} = primordials;

const {
Expand All @@ -65,6 +74,8 @@ const AssertionError = require('internal/assert/assertion_error');
const { inspect } = require('internal/util/inspect');
const { Buffer } = require('buffer');
const {
isArrayBuffer,
isDataView,
isKeyObject,
isPromise,
isRegExp,
Expand All @@ -73,6 +84,8 @@ const {
isDate,
isWeakSet,
isWeakMap,
isSharedArrayBuffer,
isAnyArrayBuffer,
} = require('internal/util/types');
const { isError, deprecate, emitExperimentalWarning } = require('internal/util');
const { innerOk } = require('internal/assert/utils');
Expand Down Expand Up @@ -369,9 +382,161 @@ function isSpecial(obj) {
}

const typesToCallDeepStrictEqualWith = [
isKeyObject, isWeakSet, isWeakMap, Buffer.isBuffer,
isKeyObject, isWeakSet, isWeakMap, Buffer.isBuffer, isSharedArrayBuffer,
];

function compareMaps(actual, expected, comparedObjects) {
if (MapPrototypeGetSize(actual) !== MapPrototypeGetSize(expected)) {
return false;
}
const safeIterator = FunctionPrototypeCall(SafeMap.prototype[SymbolIterator], actual);

comparedObjects ??= new SafeWeakSet();

for (const { 0: key, 1: val } of safeIterator) {
if (!MapPrototypeHas(expected, key)) {
return false;
}
if (!compareBranch(val, MapPrototypeGet(expected, key), comparedObjects)) {
return false;
}
}
return true;
}

function partiallyCompareArrayBuffersOrViews(actual, expected) {
let actualView, expectedView, expectedViewLength;

if (!ArrayBufferIsView(actual)) {
aduh95 marked this conversation as resolved.
Show resolved Hide resolved
let actualViewLength;

if (isArrayBuffer(actual) && isArrayBuffer(expected)) {
actualViewLength = ArrayBufferPrototypeGetByteLength(actual);
expectedViewLength = ArrayBufferPrototypeGetByteLength(expected);
} else if (isSharedArrayBuffer(actual) && isSharedArrayBuffer(expected)) {
actualViewLength = actual.byteLength;
expectedViewLength = expected.byteLength;
} else {
// Cannot compare ArrayBuffers with SharedArrayBuffers
return false;
}
Comment on lines +413 to +422
Copy link
Contributor

@aduh95 aduh95 Dec 4, 2024

Choose a reason for hiding this comment

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

We don't need to compute the value twice

    const isActualSharedArrayBuffer = isSharedArrayBuffer(actual);

    if (isActualSharedArrayBuffer !== isSharedArrayBuffer(expected)) return false;
    
    if (isActualSharedArrayBuffer) {
      // SharedArrayBuffer is not available in primordials because it can be
      // disabled with --no-harmony-sharedarraybuffer CLI flag.
      actualViewLength = actual.byteLength;
      expectedViewLength = expected.byteLength;
    } else {
      // If it is a BufferView and not a SharedArrayBuffer, it has to be an ArrayBuffer.
      actualViewLength = ArrayBufferPrototypeGetByteLength(actual);
      expectedViewLength = ArrayBufferPrototypeGetByteLength(expected);
    }

Copy link
Contributor

@aduh95 aduh95 Dec 4, 2024

Choose a reason for hiding this comment

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

Hum actually we do want to validate that expected is indeed a BufferView

Suggested change
if (isArrayBuffer(actual) && isArrayBuffer(expected)) {
actualViewLength = ArrayBufferPrototypeGetByteLength(actual);
expectedViewLength = ArrayBufferPrototypeGetByteLength(expected);
} else if (isSharedArrayBuffer(actual) && isSharedArrayBuffer(expected)) {
actualViewLength = actual.byteLength;
expectedViewLength = expected.byteLength;
} else {
// Cannot compare ArrayBuffers with SharedArrayBuffers
return false;
}
const isActualSharedArrayBuffer = isSharedArrayBuffer(actual);
if (!isActualSharedArrayBuffer && isArrayBuffer(expected)) {
actualViewLength = ArrayBufferPrototypeGetByteLength(actual);
expectedViewLength = ArrayBufferPrototypeGetByteLength(expected);
} else if (isActualSharedArrayBuffer && isSharedArrayBuffer(expected)) {
// SharedArrayBuffer is not available in primordials because it can be
// disabled with --no-harmony-sharedarraybuffer CLI flag.
actualViewLength = actual.byteLength;
expectedViewLength = expected.byteLength;
} else {
// Cannot compare ArrayBuffers with SharedArrayBuffers
return false;
}

Not sure if it actually matters for a performance PoV, feel free to ignore


if (expectedViewLength > actualViewLength) {
return false;
}
actualView = new Uint8Array(actual);
expectedView = new Uint8Array(expected);

} else if (isDataView(actual)) {
aduh95 marked this conversation as resolved.
Show resolved Hide resolved
if (!isDataView(expected)) {
return false;
}
const actualByteLength = DataViewPrototypeGetByteLength(actual);
expectedViewLength = DataViewPrototypeGetByteLength(expected);
if (expectedViewLength > actualByteLength) {
return false;
}

actualView = new Uint8Array(
DataViewPrototypeGetBuffer(actual),
DataViewPrototypeGetByteOffset(actual),
actualByteLength,
);
expectedView = new Uint8Array(
DataViewPrototypeGetBuffer(expected),
DataViewPrototypeGetByteOffset(expected),
expectedViewLength,
);
} else {
if (ObjectPrototypeToString(actual) !== ObjectPrototypeToString(expected)) {
return false;
}
actualView = actual;
expectedView = expected;
expectedViewLength = TypedArrayPrototypeGetLength(expected);

if (expectedViewLength > TypedArrayPrototypeGetLength(actual)) {
return false;
}
}

for (let i = 0; i < expectedViewLength; i++) {
if (actualView[i] !== expectedView[i]) {
aduh95 marked this conversation as resolved.
Show resolved Hide resolved
return false;
}
}

return true;
}

function partiallyCompareSets(actual, expected, comparedObjects) {
if (SetPrototypeGetSize(expected) > SetPrototypeGetSize(actual)) {
return false; // `expected` can't be a subset if it has more elements
}

if (isDeepEqual === undefined) lazyLoadComparison();

const actualArray = ArrayFrom(FunctionPrototypeCall(SafeSet.prototype[SymbolIterator], actual));
const expectedIterator = FunctionPrototypeCall(SafeSet.prototype[SymbolIterator], expected);
const usedIndices = new SafeSet();

expectedIteration: for (const expectedItem of expectedIterator) {
for (let actualIdx = 0; actualIdx < actualArray.length; actualIdx++) {
if (!usedIndices.has(actualIdx) && isDeepStrictEqual(actualArray[actualIdx], expectedItem)) {
usedIndices.add(actualIdx);
continue expectedIteration;
}
}
return false;
}

return true;
}

function partiallyCompareArrays(actual, expected, comparedObjects) {
if (expected.length > actual.length) {
return false;
}

if (isDeepEqual === undefined) lazyLoadComparison();

// 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;
}
}
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);
}
break;
}
}
}

const { size } = expectedCounts;
expectedCounts.clear();
return size === 0;
}

/**
* Compares two objects or values recursively to check if they are equal.
* @param {any} actual - The actual value to compare.
Expand All @@ -388,22 +553,16 @@ function compareBranch(
) {
// Check for Map object equality
if (isMap(actual) && isMap(expected)) {
if (actual.size !== expected.size) {
return false;
}
const safeIterator = FunctionPrototypeCall(SafeMap.prototype[SymbolIterator], actual);

comparedObjects ??= new SafeWeakSet();
return compareMaps(actual, expected, comparedObjects);
}

for (const { 0: key, 1: val } of safeIterator) {
if (!MapPrototypeHas(expected, key)) {
return false;
}
if (!compareBranch(val, MapPrototypeGet(expected, key), comparedObjects)) {
return false;
}
}
return true;
if (
ArrayBufferIsView(actual) ||
isAnyArrayBuffer(actual) ||
ArrayBufferIsView(expected) ||
isAnyArrayBuffer(expected)
) {
return partiallyCompareArrayBuffersOrViews(actual, expected);
}

for (const type of typesToCallDeepStrictEqualWith) {
Expand All @@ -415,68 +574,12 @@ function compareBranch(

// Check for Set object equality
if (isSet(actual) && isSet(expected)) {
if (expected.size > actual.size) {
return false; // `expected` can't be a subset if it has more elements
}

if (isDeepEqual === undefined) lazyLoadComparison();

const actualArray = ArrayFrom(FunctionPrototypeCall(SafeSet.prototype[SymbolIterator], actual));
const expectedIterator = FunctionPrototypeCall(SafeSet.prototype[SymbolIterator], expected);
const usedIndices = new SafeSet();

expectedIteration: for (const expectedItem of expectedIterator) {
for (let actualIdx = 0; actualIdx < actualArray.length; actualIdx++) {
if (!usedIndices.has(actualIdx) && isDeepStrictEqual(actualArray[actualIdx], expectedItem)) {
usedIndices.add(actualIdx);
continue expectedIteration;
}
}
return false;
}

return true;
return partiallyCompareSets(actual, expected, comparedObjects);
}

// Check if expected array is a subset of actual array
if (ArrayIsArray(actual) && ArrayIsArray(expected)) {
if (expected.length > actual.length) {
return false;
}

if (isDeepEqual === undefined) lazyLoadComparison();

// 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)) {
MapPrototypeSet(expectedCounts, key, count + 1);
found = true;
break;
}
}
if (!found) {
MapPrototypeSet(expectedCounts, expectedItem, 1);
}
}

// Create a map to count occurrences of relevant elements in the actual array
for (const actualItem of actual) {
for (const { 0: key, 1: count } of expectedCounts) {
if (isDeepStrictEqual(key, actualItem)) {
if (count === 1) {
MapPrototypeDelete(expectedCounts, key);
} else {
MapPrototypeSet(expectedCounts, key, count - 1);
}
break;
}
}
}

return !expectedCounts.size;
return partiallyCompareArrays(actual, expected, comparedObjects);
}

// Comparison done when at least one of the values is not an object
Expand Down
Loading
Loading