Skip to content

Commit

Permalink
assert: fix boxed primitives in deepStrictEqual
Browse files Browse the repository at this point in the history
Unbox all primitives and compare them as well instead of
only comparing boxed strings.

PR-URL: nodejs#15050
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
  • Loading branch information
BridgeAR authored and addaleax committed Sep 13, 2017
1 parent 2a206e8 commit 225bc1f
Show file tree
Hide file tree
Showing 3 changed files with 46 additions and 2 deletions.
10 changes: 9 additions & 1 deletion doc/api/assert.md
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ changes:
* `expected` {any}
* `message` {any}

Generally identical to `assert.deepEqual()` with three exceptions:
Generally identical to `assert.deepEqual()` with a few exceptions:

1. Primitive values besides `NaN` are compared using the [Strict Equality
Comparison][] ( `===` ). Set and Map values, Map keys and `NaN` are compared
Expand All @@ -143,6 +143,7 @@ Generally identical to `assert.deepEqual()` with three exceptions:
2. [`[[Prototype]]`][prototype-spec] of objects are compared using
the [Strict Equality Comparison][] too.
3. [Type tags][Object.prototype.toString()] of objects should be the same.
4. [Object wrappers][] are compared both as objects and unwrapped values.

```js
const assert = require('assert');
Expand Down Expand Up @@ -172,8 +173,14 @@ assert.deepEqual(date, fakeDate);
assert.deepStrictEqual(date, fakeDate);
// AssertionError: 2017-03-11T14:25:31.849Z deepStrictEqual Date {}
// Different type tags

assert.deepStrictEqual(NaN, NaN);
// OK, because of the SameValueZero comparison

assert.deepStrictEqual(new Number(1), new Number(2));
// Fails because the wrapped number is unwrapped and compared as well.
assert.deepStrictEqual(new String('foo'), Object('foo'));
// OK because the object and the string are identical when unwrapped.
```

If the values are not equal, an `AssertionError` is thrown with a `message`
Expand Down Expand Up @@ -711,3 +718,4 @@ For more information, see
[enumerable "own" properties]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Enumerability_and_ownership_of_properties
[mdn-equality-guide]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Equality_comparisons_and_sameness
[prototype-spec]: https://tc39.github.io/ecma262/#sec-ordinary-object-internal-methods-and-internal-slots
[Object wrappers]: https://developer.mozilla.org/en-US/docs/Glossary/Primitive#Primitive_wrapper_objects_in_JavaScript
19 changes: 18 additions & 1 deletion lib/assert.js
Original file line number Diff line number Diff line change
Expand Up @@ -215,13 +215,30 @@ function strictDeepEqual(actual, expected) {
if (!areSimilarTypedArrays(actual, expected)) {
return false;
}

// Buffer.compare returns true, so actual.length === expected.length
// if they both only contain numeric keys, we don't need to exam further
if (Object.keys(actual).length === actual.length &&
Object.keys(expected).length === expected.length) {
return true;
}
} else if (typeof actual.valueOf === 'function') {
const actualValue = actual.valueOf();
// Note: Boxed string keys are going to be compared again by Object.keys
if (actualValue !== actual) {
if (!innerDeepEqual(actualValue, expected.valueOf(), true))
return false;
// Fast path for boxed primitives
var lengthActual = 0;
var lengthExpected = 0;
if (typeof actualValue === 'string') {
lengthActual = actual.length;
lengthExpected = expected.length;
}
if (Object.keys(actual).length === lengthActual &&
Object.keys(expected).length === lengthExpected) {
return true;
}
}
}
}

Expand Down
19 changes: 19 additions & 0 deletions test/parallel/test-assert-deep.js
Original file line number Diff line number Diff line change
Expand Up @@ -489,4 +489,23 @@ assert.doesNotThrow(() => { assert.deepStrictEqual({ a: NaN }, { a: NaN }); });
assert.doesNotThrow(
() => { assert.deepStrictEqual([ 1, 2, NaN, 4 ], [ 1, 2, NaN, 4 ]); });

// Handle boxed primitives
{
const boxedString = new String('test');
const boxedSymbol = Object(Symbol());
assertOnlyDeepEqual(new Boolean(true), Object(false));
assertOnlyDeepEqual(Object(true), new Number(1));
assertOnlyDeepEqual(new Number(2), new Number(1));
assertOnlyDeepEqual(boxedSymbol, Object(Symbol()));
assertOnlyDeepEqual(boxedSymbol, {});
assertDeepAndStrictEqual(boxedSymbol, boxedSymbol);
assertDeepAndStrictEqual(Object(true), Object(true));
assertDeepAndStrictEqual(Object(2), Object(2));
assertDeepAndStrictEqual(boxedString, Object('test'));
boxedString.slow = true;
assertNotDeepOrStrict(boxedString, Object('test'));
boxedSymbol.slow = true;
assertNotDeepOrStrict(boxedSymbol, {});
}

/* eslint-enable */

0 comments on commit 225bc1f

Please sign in to comment.