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: #15050
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
  • Loading branch information
BridgeAR authored and jasnell committed Sep 21, 2017
1 parent 59f1836 commit f6c65e6
Show file tree
Hide file tree
Showing 4 changed files with 45 additions and 4 deletions.
9 changes: 8 additions & 1 deletion doc/api/assert.md
Original file line number Diff line number Diff line change
Expand Up @@ -124,14 +124,15 @@ 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 are compared using the [Strict Equality Comparison][]
( `===` ). Set values and Map keys are compared using the [SameValueZero][]
comparison. (Which means they are free of the [caveats][]).
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 @@ -161,6 +162,11 @@ assert.deepEqual(date, fakeDate);
assert.deepStrictEqual(date, fakeDate);
// AssertionError: 2017-03-11T14:25:31.849Z deepStrictEqual Date {}
// Different type tags

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 @@ -641,3 +647,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 @@ -202,13 +202,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
2 changes: 0 additions & 2 deletions test/addons-napi/test_conversions/test.js
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,6 @@ assert.strictEqual(0, test.toNumber([]));
assert.strictEqual(0, test.toNumber(false));
assert.strictEqual(0, test.toNumber(null));
assert.strictEqual(0, test.toNumber(''));
assert.ok(Number.isNaN(test.toNumber(Number.NaN)));
assert.ok(Number.isNaN(test.toNumber({})));
assert.ok(Number.isNaN(test.toNumber(undefined)));
assert.throws(() => test.toNumber(testSym), TypeError);
Expand All @@ -116,7 +115,6 @@ assert.deepStrictEqual(new Boolean(false), test.toObject(false));
assert.deepStrictEqual(new Boolean(true), test.toObject(true));
assert.deepStrictEqual(new String(''), test.toObject(''));
assert.deepStrictEqual(new Number(0), test.toObject(0));
assert.deepStrictEqual(new Number(Number.NaN), test.toObject(Number.NaN));
assert.deepStrictEqual(new Object(testSym), test.toObject(testSym));
assert.notDeepStrictEqual(false, test.toObject(false));
assert.notDeepStrictEqual(true, test.toObject(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 @@ -439,4 +439,23 @@ assertOnlyDeepEqual([1, , , 3], [1, , , 3, , , ]);
assertOnlyDeepEqual(err1, {}, assert.AssertionError);
}

// 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 f6c65e6

Please sign in to comment.