assert: use Same-value equality in deepStrictEqual

PR-URL: #15398 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Rich Trott <[email protected]>
nodejs · Sep 16, 2017 · b0d3bec · b0d3bec
1 parent ca2c73c
commit b0d3bec
Show file tree

Hide file tree

Showing 4 changed files with 48 additions and 17 deletions.
diff --git a/doc/api/assert.md b/doc/api/assert.md
@@ -142,6 +142,7 @@ Generally identical to `assert.deepEqual()` with a few exceptions:
   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.
+5. `0` and `-0` are not considered equal.
 
 ```js
 const assert = require('assert');
@@ -179,6 +180,11 @@ 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.
+
+assert.deepStrictEqual(-0, -0);
+// OK
+assert.deepStrictEqual(0, -0);
+// AssertionError: 0 deepStrictEqual -0
 ```
 
 If the values are not equal, an `AssertionError` is thrown with a `message`
@@ -438,6 +444,9 @@ parameter is an instance of an `Error` then it will be thrown instead of the
 <!-- YAML
 added: v1.2.0
 changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/REPLACEME
+    description: -0 and +0 are not considered equal anymore.
   - version: REPLACEME
     pr-url: https://github.com/nodejs/node/pull/15036
     description: NaN is now compared using the [SameValueZero][] comparison.

diff --git a/lib/assert.js b/lib/assert.js
@@ -118,12 +118,12 @@ function areSimilarRegExps(a, b) {
 // For small buffers it's faster to compare the buffer in a loop. The c++
 // barrier including the Uint8Array operation takes the advantage of the faster
 // binary compare otherwise. The break even point was at about 300 characters.
-function areSimilarTypedArrays(a, b) {
+function areSimilarTypedArrays(a, b, max) {
   const len = a.byteLength;
   if (len !== b.byteLength) {
     return false;
   }
-  if (len < 300) {
+  if (len < max) {
     for (var offset = 0; offset < len; offset++) {
       if (a[offset] !== b[offset]) {
         return false;
@@ -160,10 +160,7 @@ function isObjectOrArrayTag(tag) {
 // reasonable to interpret their underlying memory in the same way,
 // which is checked by comparing their type tags.
 // (e.g. a Uint8Array and a Uint16Array with the same memory content
-// could still be different because they will be interpreted differently)
-// Never perform binary comparisons for Float*Arrays, though,
-// since e.g. +0 === -0 is true despite the two values' bit patterns
-// not being identical.
+// could still be different because they will be interpreted differently).
 //
 // For strict comparison, objects should have
 // a) The same built-in type tags
@@ -211,8 +208,9 @@ function strictDeepEqual(actual, expected) {
     if (actual.message !== expected.message) {
       return false;
     }
-  } else if (!isFloatTypedArrayTag(actualTag) && ArrayBuffer.isView(actual)) {
-    if (!areSimilarTypedArrays(actual, expected)) {
+  } else if (ArrayBuffer.isView(actual)) {
+    if (!areSimilarTypedArrays(actual, expected,
+                               isFloatTypedArrayTag(actualTag) ? 0 : 300)) {
       return false;
     }
     // Buffer.compare returns true, so actual.length === expected.length
@@ -266,9 +264,10 @@ function looseDeepEqual(actual, expected) {
   const actualTag = objectToString(actual);
   const expectedTag = objectToString(expected);
   if (actualTag === expectedTag) {
-    if (!isObjectOrArrayTag(actualTag) && !isFloatTypedArrayTag(actualTag) &&
-      ArrayBuffer.isView(actual)) {
-      return areSimilarTypedArrays(actual, expected);
+    if (!isObjectOrArrayTag(actualTag) && ArrayBuffer.isView(actual)) {
+      return areSimilarTypedArrays(actual, expected,
+                                   isFloatTypedArrayTag(actualTag) ?
+                                     Infinity : 300);
     }
   // Ensure reflexivity of deepEqual with `arguments` objects.
   // See https://github.com/nodejs/node-v0.x-archive/pull/7178
@@ -280,7 +279,9 @@ function looseDeepEqual(actual, expected) {
 function innerDeepEqual(actual, expected, strict, memos) {
   // All identical values are equivalent, as determined by ===.
   if (actual === expected) {
-    return true;
+    if (actual !== 0)
+      return true;
+    return strict ? Object.is(actual, expected) : true;
   }
 
   // Returns a boolean if (not) equal and undefined in case we have to check

diff --git a/test/parallel/test-assert-deep.js b/test/parallel/test-assert-deep.js
@@ -507,5 +507,8 @@ assert.doesNotThrow(
   boxedSymbol.slow = true;
   assertNotDeepOrStrict(boxedSymbol, {});
 }
+// Minus zero
+assertOnlyDeepEqual(0, -0);
+assertDeepAndStrictEqual(-0, -0);
 
 /* eslint-enable */
diff --git a/test/parallel/test-assert-typedarray-deepequal.js b/test/parallel/test-assert-typedarray-deepequal.js
@@ -20,16 +20,20 @@ const equalArrayPairs = [
   [new Int32Array(1e5), new Int32Array(1e5)],
   [new Float32Array(1e5), new Float32Array(1e5)],
   [new Float64Array(1e5), new Float64Array(1e5)],
-  [new Int16Array(256), new Uint16Array(256)],
-  [new Int16Array([256]), new Uint16Array([256])],
-  [new Float32Array([+0.0]), new Float32Array([-0.0])],
-  [new Float64Array([+0.0]), new Float32Array([-0.0])],
-  [new Float64Array([+0.0]), new Float64Array([-0.0])],
+  [new Float32Array([+0.0]), new Float32Array([+0.0])],
   [new Uint8Array([1, 2, 3, 4]).subarray(1), new Uint8Array([2, 3, 4])],
   [new Uint16Array([1, 2, 3, 4]).subarray(1), new Uint16Array([2, 3, 4])],
   [new Uint32Array([1, 2, 3, 4]).subarray(1, 3), new Uint32Array([2, 3])]
 ];
 
+const looseEqualArrayPairs = [
+  [new Float64Array([+0.0]), new Float32Array([-0.0])],
+  [new Int16Array(256), new Uint16Array(256)],
+  [new Int16Array([256]), new Uint16Array([256])],
+  [new Float32Array([+0.0]), new Float32Array([-0.0])],
+  [new Float64Array([+0.0]), new Float64Array([-0.0])]
+];
+
 const notEqualArrayPairs = [
   [new Uint8Array(2), new Uint8Array(3)],
   [new Uint8Array([1, 2, 3]), new Uint8Array([4, 5, 6])],
@@ -46,6 +50,16 @@ const notEqualArrayPairs = [
 equalArrayPairs.forEach((arrayPair) => {
   // eslint-disable-next-line no-restricted-properties
   assert.deepEqual(arrayPair[0], arrayPair[1]);
+  assert.deepStrictEqual(arrayPair[0], arrayPair[1]);
+});
+
+looseEqualArrayPairs.forEach((arrayPair) => {
+  // eslint-disable-next-line no-restricted-properties
+  assert.deepEqual(arrayPair[0], arrayPair[1]);
+  assert.throws(
+    makeBlock(assert.deepStrictEqual, arrayPair[0], arrayPair[1]),
+    assert.AssertionError
+  );
 });
 
 notEqualArrayPairs.forEach((arrayPair) => {
@@ -54,4 +68,8 @@ notEqualArrayPairs.forEach((arrayPair) => {
     makeBlock(assert.deepEqual, arrayPair[0], arrayPair[1]),
     assert.AssertionError
   );
+  assert.throws(
+    makeBlock(assert.deepStrictEqual, arrayPair[0], arrayPair[1]),
+    assert.AssertionError
+  );
 });