Skip to content

Commit 7493db2

Browse files
BridgeARBridgeAR
committed
assert: adjust loose assertions
This changes the loose deep equal comparison by using the same logic as done in the strict deep equal comparison besides comparing primitives loosely, not comparing symbol properties and not comparing the prototype. `assert.deepEqual` is still commenly used and this is likely the biggest pitfall. Most changes are only minor and won't have a big impact besides likely fixing user expectations. PR-URL: #25008 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Daniel Bevenius <[email protected]>
1 parent 5cb1964 commit 7493db2

File tree

6 files changed

+116
-161
lines changed

6 files changed

+116
-161
lines changed

‎doc/api/assert.md‎

Lines changed: 32 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -163,6 +163,10 @@ An alias of [`assert.ok()`][].
163163
<!-- YAML
164164
added: v0.1.21
165165
changes:
166+
- version: REPLACEME
167+
pr-url: https://github.com/nodejs/node/pull/25008
168+
description: The type tags are now properly compared and there are a couple
169+
minor comparison adjustments to make the check less surprising.
166170
- version: v9.0.0
167171
pr-url: https://github.com/nodejs/node/pull/15001
168172
description: The `Error` names and messages are now properly compared
@@ -191,26 +195,39 @@ An alias of [`assert.deepStrictEqual()`][].
191195

192196
> Stability: 0 - Deprecated: Use [`assert.deepStrictEqual()`][] instead.
193197
194-
Tests for deep equality between the `actual` and `expected` parameters.
195-
Primitive values are compared with the [Abstract Equality Comparison][]
196-
( `==` ).
198+
Tests for deep equality between the `actual` and `expected` parameters. Consider
199+
using [`assert.deepStrictEqual()`][] instead. [`assert.deepEqual()`][] can have
200+
potentially surprising results.
201+
202+
"Deep" equality means that the enumerable "own" properties of child objects
203+
are also recursively evaluated by the following rules.
204+
205+
### Comparison details
206+
207+
* Primitive values are compared with the [Abstract Equality Comparison][]
208+
( `==` ).
209+
* [Type tags][Object.prototype.toString()] of objects should be the same.
210+
* Only [enumerable "own" properties][] are considered.
211+
* [`Error`][] names and messages are always compared, even if these are not
212+
enumerable properties.
213+
* [Object wrappers][] are compared both as objects and unwrapped values.
214+
* `Object` properties are compared unordered.
215+
* [`Map`][] keys and [`Set`][] items are compared unordered.
216+
* Recursion stops when both sides differ or both sides encounter a circular
217+
reference.
218+
* Implementation does not test the [`[[Prototype]]`][prototype-spec] of
219+
objects.
220+
* [`Symbol`][] properties are not compared.
221+
* [`WeakMap`][] and [`WeakSet`][] comparison does not rely on their values.
197222

198-
Only [enumerable "own" properties][] are considered. The
199-
[`assert.deepEqual()`][] implementation does not test the
200-
[`[[Prototype]]`][prototype-spec] of objects or enumerable own [`Symbol`][]
201-
properties. For such checks, consider using [`assert.deepStrictEqual()`][]
202-
instead. [`assert.deepEqual()`][] can have potentially surprising results. The
203-
following example does not throw an `AssertionError` because the properties on
204-
the [`RegExp`][] object are not enumerable:
223+
The following example does not throw an `AssertionError` because the primitives
224+
are considered equal by the [Abstract Equality Comparison][] ( `==` ).
205225

206226
```js
207227
// WARNING: This does not throw an AssertionError!
208-
assert.deepEqual(/a/gi, new Date());
228+
assert.deepEqual('+00000000', false);
209229
```
210230

211-
An exception is made for [`Map`][] and [`Set`][]. `Map`s and `Set`s have their
212-
contained items compared too, as expected.
213-
214231
"Deep" equality means that the enumerable "own" properties of child objects
215232
are evaluated also:
216233

@@ -304,7 +321,7 @@ are recursively evaluated also by the following rules.
304321
* Enumerable own [`Symbol`][] properties are compared as well.
305322
* [Object wrappers][] are compared both as objects and unwrapped values.
306323
* `Object` properties are compared unordered.
307-
* `Map` keys and `Set` items are compared unordered.
324+
* [`Map`][] keys and [`Set`][] items are compared unordered.
308325
* Recursion stops when both sides differ or both sides encounter a circular
309326
reference.
310327
* [`WeakMap`][] and [`WeakSet`][] comparison does not rely on their values. See

‎lib/internal/util/comparisons.js‎

Lines changed: 47 additions & 106 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,9 @@ const {
1414
isStringObject,
1515
isBooleanObject,
1616
isBigIntObject,
17-
isSymbolObject
17+
isSymbolObject,
18+
isFloat32Array,
19+
isFloat64Array
1820
} = require('internal/util/types');
1921
const {
2022
getOwnNonIndexProperties,
@@ -84,18 +86,6 @@ function areEqualArrayBuffers(buf1, buf2) {
8486
compare(new Uint8Array(buf1), new Uint8Array(buf2)) === 0;
8587
}
8688

87-
function isFloatTypedArrayTag(tag) {
88-
return tag === '[object Float32Array]' || tag === '[object Float64Array]';
89-
}
90-
91-
function isArguments(tag) {
92-
return tag === '[object Arguments]';
93-
}
94-
95-
function isObjectOrArrayTag(tag) {
96-
return tag === '[object Array]' || tag === '[object Object]';
97-
}
98-
9989
function isEqualBoxedPrimitive(val1, val2) {
10090
if (isNumberObject(val1)) {
10191
return isNumberObject(val2) &&
@@ -132,23 +122,45 @@ function isEqualBoxedPrimitive(val1, val2) {
132122
// For strict comparison, objects should have
133123
// a) The same built-in type tags
134124
// b) The same prototypes.
135-
function strictDeepEqual(val1, val2, memos) {
136-
if (typeof val1 !== 'object') {
137-
return typeof val1 === 'number' && numberIsNaN(val1) &&
138-
numberIsNaN(val2);
125+
126+
function innerDeepEqual(val1, val2, strict, memos) {
127+
// All identical values are equivalent, as determined by ===.
128+
if (val1 === val2) {
129+
if (val1 !== 0)
130+
return true;
131+
return strict ? objectIs(val1, val2) : true;
139132
}
140-
if (typeof val2 !== 'object' || val1 === null || val2 === null) {
141-
return false;
133+
134+
// Check more closely if val1 and val2 are equal.
135+
if (strict) {
136+
if (typeof val1 !== 'object') {
137+
return typeof val1 === 'number' && numberIsNaN(val1) &&
138+
numberIsNaN(val2);
139+
}
140+
if (typeof val2 !== 'object' || val1 === null || val2 === null) {
141+
return false;
142+
}
143+
if (getPrototypeOf(val1) !== getPrototypeOf(val2)) {
144+
return false;
145+
}
146+
} else {
147+
if (val1 === null || typeof val1 !== 'object') {
148+
if (val2 === null || typeof val2 !== 'object') {
149+
// eslint-disable-next-line eqeqeq
150+
return val1 == val2;
151+
}
152+
return false;
153+
}
154+
if (val2 === null || typeof val2 !== 'object') {
155+
return false;
156+
}
142157
}
143158
const val1Tag = objectToString(val1);
144159
const val2Tag = objectToString(val2);
145160

146161
if (val1Tag !== val2Tag) {
147162
return false;
148163
}
149-
if (getPrototypeOf(val1) !== getPrototypeOf(val2)) {
150-
return false;
151-
}
152164
if (Array.isArray(val1)) {
153165
// Check for sparse arrays and general fast path
154166
if (val1.length !== val2.length) {
@@ -159,10 +171,10 @@ function strictDeepEqual(val1, val2, memos) {
159171
if (keys1.length !== keys2.length) {
160172
return false;
161173
}
162-
return keyCheck(val1, val2, kStrict, memos, kIsArray, keys1);
174+
return keyCheck(val1, val2, strict, memos, kIsArray, keys1);
163175
}
164176
if (val1Tag === '[object Object]') {
165-
return keyCheck(val1, val2, kStrict, memos, kNoIterator);
177+
return keyCheck(val1, val2, strict, memos, kNoIterator);
166178
}
167179
if (isDate(val1)) {
168180
if (dateGetTime(val1) !== dateGetTime(val2)) {
@@ -174,13 +186,16 @@ function strictDeepEqual(val1, val2, memos) {
174186
}
175187
} else if (isNativeError(val1) || val1 instanceof Error) {
176188
// Do not compare the stack as it might differ even though the error itself
177-
// is otherwise identical. The non-enumerable name should be identical as
178-
// the prototype is also identical. Otherwise this is caught later on.
179-
if (val1.message !== val2.message) {
189+
// is otherwise identical.
190+
if (val1.message !== val2.message || val1.name !== val2.name) {
180191
return false;
181192
}
182193
} else if (isArrayBufferView(val1)) {
183-
if (!areSimilarTypedArrays(val1, val2)) {
194+
if (!strict && (isFloat32Array(val1) || isFloat64Array(val1))) {
195+
if (!areSimilarFloatArrays(val1, val2)) {
196+
return false;
197+
}
198+
} else if (!areSimilarTypedArrays(val1, val2)) {
184199
return false;
185200
}
186201
// Buffer.compare returns true, so val1.length === val2.length. If they both
@@ -191,84 +206,25 @@ function strictDeepEqual(val1, val2, memos) {
191206
if (keys1.length !== keys2.length) {
192207
return false;
193208
}
194-
return keyCheck(val1, val2, kStrict, memos, kNoIterator, keys1);
209+
return keyCheck(val1, val2, strict, memos, kNoIterator, keys1);
195210
} else if (isSet(val1)) {
196211
if (!isSet(val2) || val1.size !== val2.size) {
197212
return false;
198213
}
199-
return keyCheck(val1, val2, kStrict, memos, kIsSet);
214+
return keyCheck(val1, val2, strict, memos, kIsSet);
200215
} else if (isMap(val1)) {
201216
if (!isMap(val2) || val1.size !== val2.size) {
202217
return false;
203218
}
204-
return keyCheck(val1, val2, kStrict, memos, kIsMap);
219+
return keyCheck(val1, val2, strict, memos, kIsMap);
205220
} else if (isAnyArrayBuffer(val1)) {
206221
if (!areEqualArrayBuffers(val1, val2)) {
207222
return false;
208223
}
209224
} else if (isBoxedPrimitive(val1) && !isEqualBoxedPrimitive(val1, val2)) {
210225
return false;
211226
}
212-
return keyCheck(val1, val2, kStrict, memos, kNoIterator);
213-
}
214-
215-
function looseDeepEqual(val1, val2, memos) {
216-
if (val1 === null || typeof val1 !== 'object') {
217-
if (val2 === null || typeof val2 !== 'object') {
218-
// eslint-disable-next-line eqeqeq
219-
return val1 == val2;
220-
}
221-
return false;
222-
}
223-
if (val2 === null || typeof val2 !== 'object') {
224-
return false;
225-
}
226-
const val1Tag = objectToString(val1);
227-
const val2Tag = objectToString(val2);
228-
if (val1Tag === val2Tag) {
229-
if (isObjectOrArrayTag(val1Tag)) {
230-
return keyCheck(val1, val2, kLoose, memos, kNoIterator);
231-
}
232-
if (isArrayBufferView(val1)) {
233-
if (isFloatTypedArrayTag(val1Tag)) {
234-
return areSimilarFloatArrays(val1, val2);
235-
}
236-
return areSimilarTypedArrays(val1, val2);
237-
}
238-
if (isDate(val1) && isDate(val2)) {
239-
return val1.getTime() === val2.getTime();
240-
}
241-
if (isRegExp(val1) && isRegExp(val2)) {
242-
return areSimilarRegExps(val1, val2);
243-
}
244-
if (val1 instanceof Error && val2 instanceof Error) {
245-
if (val1.message !== val2.message || val1.name !== val2.name)
246-
return false;
247-
}
248-
// Ensure reflexivity of deepEqual with `arguments` objects.
249-
// See https://github.com/nodejs/node-v0.x-archive/pull/7178
250-
} else if (isArguments(val1Tag) || isArguments(val2Tag)) {
251-
return false;
252-
}
253-
if (isSet(val1)) {
254-
if (!isSet(val2) || val1.size !== val2.size) {
255-
return false;
256-
}
257-
return keyCheck(val1, val2, kLoose, memos, kIsSet);
258-
} else if (isMap(val1)) {
259-
if (!isMap(val2) || val1.size !== val2.size) {
260-
return false;
261-
}
262-
return keyCheck(val1, val2, kLoose, memos, kIsMap);
263-
} else if (isSet(val2) || isMap(val2)) {
264-
return false;
265-
}
266-
if (isAnyArrayBuffer(val1) && isAnyArrayBuffer(val2)) {
267-
if (!areEqualArrayBuffers(val1, val2)) {
268-
return false;
269-
}
270-
}
271-
return keyCheck(val1, val2, kLoose, memos, kNoIterator);
227+
return keyCheck(val1, val2, strict, memos, kNoIterator);
272228
}
273229

274230
function getEnumerables(val, keys) {
@@ -370,21 +326,6 @@ function keyCheck(val1, val2, strict, memos, iterationType, aKeys) {
370326
return areEq;
371327
}
372328

373-
function innerDeepEqual(val1, val2, strict, memos) {
374-
// All identical values are equivalent, as determined by ===.
375-
if (val1 === val2) {
376-
if (val1 !== 0)
377-
return true;
378-
return strict ? objectIs(val1, val2) : true;
379-
}
380-
381-
// Check more closely if val1 and val2 are equal.
382-
if (strict === true)
383-
return strictDeepEqual(val1, val2, memos);
384-
385-
return looseDeepEqual(val1, val2, memos);
386-
}
387-
388329
function setHasEqualElement(set, val1, strict, memo) {
389330
// Go looking.
390331
for (const val2 of set) {

‎test/es-module/test-esm-dynamic-import.js‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -23,9 +23,12 @@ function expectMissingModuleError(result) {
2323
function expectOkNamespace(result) {
2424
Promise.resolve(result)
2525
.then(common.mustCall((ns) => {
26-
// Can't deepStrictEqual because ns isn't a normal object
27-
// eslint-disable-next-line no-restricted-properties
28-
assert.deepEqual(ns, { default: true });
26+
const expected = { default: true };
27+
Object.defineProperty(expected, Symbol.toStringTag, {
28+
value: 'Module'
29+
});
30+
Object.setPrototypeOf(expected, Object.getPrototypeOf(ns));
31+
assert.deepStrictEqual(ns, expected);
2932
}));
3033
}
3134

‎test/parallel/test-assert-checktag.js‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,8 @@ if (process.stdout.isTTY)
1818
FakeDate.prototype = Date.prototype;
1919
const fake = new FakeDate();
2020

21-
assert.deepEqual(date, fake);
22-
assert.deepEqual(fake, date);
21+
assert.notDeepEqual(date, fake);
22+
assert.notDeepEqual(fake, date);
2323

2424
// For deepStrictEqual we check the runtime type,
2525
// then reveal the fakeness of the fake date
@@ -45,7 +45,7 @@ if (process.stdout.isTTY)
4545
for (const prop of Object.keys(global)) {
4646
fakeGlobal[prop] = global[prop];
4747
}
48-
assert.deepEqual(fakeGlobal, global);
48+
assert.notDeepEqual(fakeGlobal, global);
4949
// Message will be truncated anyway, don't validate
5050
assert.throws(() => assert.deepStrictEqual(fakeGlobal, global),
5151
assert.AssertionError);
@@ -57,7 +57,7 @@ if (process.stdout.isTTY)
5757
for (const prop of Object.keys(process)) {
5858
fakeProcess[prop] = process[prop];
5959
}
60-
assert.deepEqual(fakeProcess, process);
60+
assert.notDeepEqual(fakeProcess, process);
6161
// Message will be truncated anyway, don't validate
6262
assert.throws(() => assert.deepStrictEqual(fakeProcess, process),
6363
assert.AssertionError);

0 commit comments

Comments
 (0)