FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

assert: handle enumerable symbol keys · nodejs/node@db2e093 · GitHub

/ node Public

Commit db2e093

Browse files
BridgeAR
committed
assert: handle enumerable symbol keys
PR-URL: #15169 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Refael Ackermann <refack@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Tobias Nießen <tniessen@tnie.de>
1 parent a8c0a43 commit db2e093

4 files changed

Lines changed: 127 additions & 62 deletions

File tree

‎doc/api/assert.md‎

Lines changed: 18 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -46,12 +46,11 @@ Primitive values are compared with the [Abstract Equality Comparison][]
4646

4747
Only [enumerable "own" properties][] are considered. The
4848
[`assert.deepEqual()`][] implementation does not test the
49-
[`[[Prototype]]`][prototype-spec] of objects, attached symbols, or
50-
non-enumerable properties — for such checks, consider using
51-
[`assert.deepStrictEqual()`][] instead. This can lead to some
52-
potentially surprising results. For example, the following example does not
53-
throw an `AssertionError` because the properties on the [`RegExp`][] object are
54-
not enumerable:
49+
[`[[Prototype]]`][prototype-spec] of objects or enumerable own [`Symbol`][]
50+
properties. For such checks, consider using [assert.deepStrictEqual()][]
51+
instead. [`assert.deepEqual()`][] can have potentially surprising results. The
52+
following example does not throw an `AssertionError` because the properties on
53+
the [RegExp][] object are not enumerable:
5554

5655
```js
5756
// WARNING: This does not throw an AssertionError!
@@ -109,6 +108,9 @@ parameter is an instance of an `Error` then it will be thrown instead of the
109108
<!-- YAML
110109
added: v1.2.0
111110
changes:
111+
- version: REPLACEME
112+
pr-url: https://github.com/nodejs/node/pull/15169
113+
description: Enumerable symbol properties are now compared.
112114
- version: REPLACEME
113115
pr-url: https://github.com/nodejs/node/pull/15036
114116
description: NaN is now compared using the [SameValueZero][] comparison.
@@ -132,7 +134,7 @@ changes:
132134
* `expected` {any}
133135
* `message` {any}
134136

135-
Generally identical to `assert.deepEqual()` with a few exceptions:
137+
Similar to `assert.deepEqual()` with the following exceptions:
136138

137139
1. Primitive values besides `NaN` are compared using the [Strict Equality
138140
Comparison][] ( `===` ). Set and Map values, Map keys and `NaN` are compared
@@ -143,6 +145,7 @@ Generally identical to `assert.deepEqual()` with a few exceptions:
143145
3. [Type tags][Object.prototype.toString()] of objects should be the same.
144146
4. [Object wrappers][] are compared both as objects and unwrapped values.
145147
5. `0` and `-0` are not considered equal.
148+
6. Enumerable own [`Symbol`][] properties are compared as well.
146149

147150
```js
148151
const assert = require('assert');
@@ -185,6 +188,13 @@ assert.deepStrictEqual(-0, -0);
185188
// OK
186189
assert.deepStrictEqual(0, -0);
187190
// AssertionError: 0 deepStrictEqual -0
191+
192+
const symbol1 = Symbol();
193+
const symbol2 = Symbol();
194+
assert.deepStrictEqual({ [symbol1]: 1 }, { [symbol1]: 1 });
195+
// OK, because it is the same symbol on both objects.
196+
assert.deepStrictEqual({ [symbol1]: 1 }, { [symbol2]: 1 });
197+
// Fails because symbol1 !== symbol2!
188198
```
189199

190200
If the values are not equal, an `AssertionError` is thrown with a `message`
@@ -712,6 +722,7 @@ For more information, see
712722
[`Object.is()`]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Object/is
713723
[`RegExp`]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Guide/Regular_Expressions
714724
[`Set`]: https://developer.mozilla.org/en/docs/Web/JavaScript/Reference/Global_Objects/Set
725+
[`Symbol`]: https://developer.mozilla.org/en/docs/Web/JavaScript/Reference/Global_Objects/Symbol
715726
[`TypeError`]: errors.html#errors_class_typeerror
716727
[`assert.deepEqual()`]: #assert_assert_deepequal_actual_expected_message
717728
[`assert.deepStrictEqual()`]: #assert_assert_deepstrictequal_actual_expected_message

‎lib/assert.js‎

Lines changed: 74 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ const { compare } = process.binding('buffer');
2424
const { isSet, isMap, isDate, isRegExp } = process.binding('util');
2525
const { objectToString } = require('internal/util');
2626
const errors = require('internal/errors');
27+
const { propertyIsEnumerable } = Object.prototype;
2728

2829
// The assert module provides functions that throw
2930
// AssertionError's when particular conditions are not met. The
@@ -165,7 +166,7 @@ function isObjectOrArrayTag(tag) {
165166
// For strict comparison, objects should have
166167
// a) The same built-in type tags
167168
// b) The same prototypes.
168-
function strictDeepEqual(actual, expected) {
169+
function strictDeepEqual(actual, expected, memos) {
169170
if (typeof actual !== 'object') {
170171
return typeof actual === 'number' && Number.isNaN(actual) &&
171172
Number.isNaN(expected);
@@ -186,12 +187,12 @@ function strictDeepEqual(actual, expected) {
186187
// Check for sparse arrays and general fast path
187188
if (actual.length !== expected.length)
188189
return false;
189-
// Skip testing the part below and continue in the callee function.
190-
return;
190+
// Skip testing the part below and continue with the keyCheck.
191+
return keyCheck(actual, expected, true, memos);
191192
}
192193
if (actualTag === '[object Object]') {
193-
// Skip testing the part below and continue in the callee function.
194-
return;
194+
// Skip testing the part below and continue with the keyCheck.
195+
return keyCheck(actual, expected, true, memos);
195196
}
196197
if (isDate(actual)) {
197198
if (actual.getTime() !== expected.getTime()) {
@@ -215,10 +216,8 @@ function strictDeepEqual(actual, expected) {
215216
}
216217
// Buffer.compare returns true, so actual.length === expected.length
217218
// if they both only contain numeric keys, we don't need to exam further
218-
if (Object.keys(actual).length === actual.length &&
219-
Object.keys(expected).length === expected.length) {
220-
return true;
221-
}
219+
return keyCheck(actual, expected, true, memos, actual.length,
220+
expected.length);
222221
} else if (typeof actual.valueOf === 'function') {
223222
const actualValue = actual.valueOf();
224223
// Note: Boxed string keys are going to be compared again by Object.keys
@@ -232,15 +231,14 @@ function strictDeepEqual(actual, expected) {
232231
lengthActual = actual.length;
233232
lengthExpected = expected.length;
234233
}
235-
if (Object.keys(actual).length === lengthActual &&
236-
Object.keys(expected).length === lengthExpected) {
237-
return true;
238-
}
234+
return keyCheck(actual, expected, true, memos, lengthActual,
235+
lengthExpected);
239236
}
240237
}
238+
return keyCheck(actual, expected, true, memos);
241239
}
242240

243-
function looseDeepEqual(actual, expected) {
241+
function looseDeepEqual(actual, expected, memos) {
244242
if (actual === null || typeof actual !== 'object') {
245243
if (expected === null || typeof expected !== 'object') {
246244
// eslint-disable-next-line eqeqeq
@@ -274,33 +272,62 @@ function looseDeepEqual(actual, expected) {
274272
} else if (isArguments(actualTag) || isArguments(expectedTag)) {
275273
return false;
276274
}
275+
return keyCheck(actual, expected, false, memos);
277276
}
278277

279-
function innerDeepEqual(actual, expected, strict, memos) {
280-
// All identical values are equivalent, as determined by ===.
281-
if (actual === expected) {
282-
if (actual !== 0)
283-
return true;
284-
return strict ? Object.is(actual, expected) : true;
285-
}
286-
287-
// Returns a boolean if (not) equal and undefined in case we have to check
288-
// further.
289-
const partialCheck = strict ?
290-
strictDeepEqual(actual, expected) :
291-
looseDeepEqual(actual, expected);
292-
293-
if (partialCheck !== undefined) {
294-
return partialCheck;
295-
}
296-
278+
function keyCheck(actual, expected, strict, memos, lengthA, lengthB) {
297279
// For all remaining Object pairs, including Array, objects and Maps,
298280
// equivalence is determined by having:
299281
// a) The same number of owned enumerable properties
300282
// b) The same set of keys/indexes (although not necessarily the same order)
301283
// c) Equivalent values for every corresponding key/index
302284
// d) For Sets and Maps, equal contents
303285
// Note: this accounts for both named and indexed properties on Arrays.
286+
var aKeys = Object.keys(actual);
287+
var bKeys = Object.keys(expected);
288+
var i;
289+
290+
// The pair must have the same number of owned properties.
291+
if (aKeys.length !== bKeys.length)
292+
return false;
293+
294+
if (strict) {
295+
var symbolKeysA = Object.getOwnPropertySymbols(actual);
296+
var symbolKeysB = Object.getOwnPropertySymbols(expected);
297+
if (symbolKeysA.length !== 0) {
298+
symbolKeysA = symbolKeysA.filter((k) =>
299+
propertyIsEnumerable.call(actual, k));
300+
symbolKeysB = symbolKeysB.filter((k) =>
301+
propertyIsEnumerable.call(expected, k));
302+
if (symbolKeysA.length !== symbolKeysB.length)
303+
return false;
304+
} else if (symbolKeysB.length !== 0 && symbolKeysB.filter((k) =>
305+
propertyIsEnumerable.call(expected, k)).length !== 0) {
306+
return false;
307+
}
308+
if (lengthA !== undefined) {
309+
if (aKeys.length !== lengthA || bKeys.length !== lengthB)
310+
return false;
311+
if (symbolKeysA.length === 0)
312+
return true;
313+
aKeys = [];
314+
bKeys = [];
315+
}
316+
if (symbolKeysA.length !== 0) {
317+
aKeys.push(...symbolKeysA);
318+
bKeys.push(...symbolKeysB);
319+
}
320+
}
321+
322+
// Cheap key test:
323+
const keys = {};
324+
for (i = 0; i < aKeys.length; i++) {
325+
keys[aKeys[i]] = true;
326+
}
327+
for (i = 0; i < aKeys.length; i++) {
328+
if (keys[bKeys[i]] === undefined)
329+
return false;
330+
}
304331

305332
// Use memos to handle cycles.
306333
if (memos === undefined) {
@@ -323,25 +350,6 @@ function innerDeepEqual(actual, expected, strict, memos) {
323350
memos.position++;
324351
}
325352

326-
const aKeys = Object.keys(actual);
327-
const bKeys = Object.keys(expected);
328-
var i;
329-
330-
// The pair must have the same number of owned properties
331-
// (keys incorporates hasOwnProperty).
332-
if (aKeys.length !== bKeys.length)
333-
return false;
334-
335-
// Cheap key test:
336-
const keys = {};
337-
for (i = 0; i < aKeys.length; i++) {
338-
keys[aKeys[i]] = true;
339-
}
340-
for (i = 0; i < aKeys.length; i++) {
341-
if (keys[bKeys[i]] === undefined)
342-
return false;
343-
}
344-
345353
memos.actual.set(actual, memos.position);
346354
memos.expected.set(expected, memos.position);
347355

@@ -353,6 +361,21 @@ function innerDeepEqual(actual, expected, strict, memos) {
353361
return areEq;
354362
}
355363

364+
function innerDeepEqual(actual, expected, strict, memos) {
365+
// All identical values are equivalent, as determined by ===.
366+
if (actual === expected) {
367+
if (actual !== 0)
368+
return true;
369+
return strict ? Object.is(actual, expected) : true;
370+
}
371+
372+
// Check more closely if actual and expected are equal.
373+
if (strict === true)
374+
return strictDeepEqual(actual, expected, memos);
375+
376+
return looseDeepEqual(actual, expected, memos);
377+
}
378+
356379
function setHasEqualElement(set, val1, strict, memo) {
357380
// Go looking.
358381
for (const val2 of set) {

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

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -507,8 +507,36 @@ assert.doesNotThrow(
507507
boxedSymbol.slow = true;
508508
assertNotDeepOrStrict(boxedSymbol, {});
509509
}
510+
510511
// Minus zero
511512
assertOnlyDeepEqual(0, -0);
512513
assertDeepAndStrictEqual(-0, -0);
513514

515+
// Handle symbols (enumerable only)
516+
{
517+
const symbol1 = Symbol();
518+
const obj1 = { [symbol1]: 1 };
519+
const obj2 = { [symbol1]: 1 };
520+
const obj3 = { [Symbol()]: 1 };
521+
// Add a non enumerable symbol as well. It is going to be ignored!
522+
Object.defineProperty(obj2, Symbol(), { value: 1 });
523+
assertOnlyDeepEqual(obj1, obj3);
524+
assertDeepAndStrictEqual(obj1, obj2);
525+
// TypedArrays have a fast path. Test for this as well.
526+
const a = new Uint8Array(4);
527+
const b = new Uint8Array(4);
528+
a[symbol1] = true;
529+
b[symbol1] = false;
530+
assertOnlyDeepEqual(a, b);
531+
b[symbol1] = true;
532+
assertDeepAndStrictEqual(a, b);
533+
// The same as TypedArrays is valid for boxed primitives
534+
const boxedStringA = new String('test');
535+
const boxedStringB = new String('test');
536+
boxedStringA[symbol1] = true;
537+
assertOnlyDeepEqual(boxedStringA, boxedStringB);
538+
boxedStringA[symbol1] = true;
539+
assertDeepAndStrictEqual(a, b);
540+
}
541+
514542
/* eslint-enable */

‎test/parallel/test-net-normalize-args.js‎

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -13,10 +13,13 @@ function validateNormalizedArgs(input, output) {
1313
}
1414

1515
// Test creation of normalized arguments.
16-
validateNormalizedArgs([], [{}, null]);
17-
validateNormalizedArgs([{ port: 1234 }], [{ port: 1234 }, null]);
18-
validateNormalizedArgs([{ port: 1234 }, assert.fail],
19-
[{ port: 1234 }, assert.fail]);
16+
const res = [{}, null];
17+
res[normalizedArgsSymbol] = true;
18+
validateNormalizedArgs([], res);
19+
res[0].port = 1234;
20+
validateNormalizedArgs([{ port: 1234 }], res);
21+
res[1] = assert.fail;
22+
validateNormalizedArgs([{ port: 1234 }, assert.fail], res);
2023

2124
// Connecting to the server should fail with a standard array.
2225
{

0 commit comments

Comments
 (0)
Back | FazBrowse Home | New Git URL