From 10aaa3208407155cbee30451d09b4c6300efad7e Mon Sep 17 00:00:00 2001
From: Ruben Bridgewater <ruben@bridgewater.de>
Date: Sat, 19 Aug 2017 02:50:57 -0300
Subject: [PATCH 1/2] assert: use Same-value equality in deepStrictEqual

---
 doc/api/assert.md                             |  9 ++++++
 lib/assert.js                                 | 26 +++++++++--------
 test/parallel/test-assert-deep.js             |  3 ++
 .../test-assert-typedarray-deepequal.js       | 28 +++++++++++++++----
 4 files changed, 49 insertions(+), 17 deletions(-)

diff --git a/doc/api/assert.md b/doc/api/assert.md
index 111fd629f747a9..1174698659a0dc 100644
--- a/doc/api/assert.md
+++ b/doc/api/assert.md
@@ -109,6 +109,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: zero is now compared using the [Object.is][] comparison.
   - version: REPLACEME
     pr-url: https://github.com/nodejs/node/pull/15036
     description: NaN is now compared using the [SameValueZero][] comparison.
@@ -144,6 +147,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');
@@ -181,6 +185,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`
diff --git a/lib/assert.js b/lib/assert.js
index f1ab145c0f8c4f..fcfc88dc8dba71 100644
--- a/lib/assert.js
+++ b/lib/assert.js
@@ -118,12 +118,13 @@ 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) {
+// NOTE: Floats should only use binary comparison in non strict mode!
+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 +161,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 +209,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 +265,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 +280,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
index 1624b9d0167968..b84ceaff896688 100644
--- 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
index 7eed19cc539466..b648f07d3297f6 100644
--- 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
+  );
 });

From a6f20938d7c3275ce0c173b53414a8aa956b4abe Mon Sep 17 00:00:00 2001
From: Ruben Bridgewater <ruben@bridgewater.de>
Date: Fri, 15 Sep 2017 18:25:03 -0300
Subject: [PATCH 2/2] [squash] address comments

---
 doc/api/assert.md | 2 +-
 lib/assert.js     | 1 -
 2 files changed, 1 insertion(+), 2 deletions(-)

diff --git a/doc/api/assert.md b/doc/api/assert.md
index 1174698659a0dc..3a36aec4b1954b 100644
--- a/doc/api/assert.md
+++ b/doc/api/assert.md
@@ -111,7 +111,7 @@ added: v1.2.0
 changes:
   - version: REPLACEME
     pr-url: https://github.com/nodejs/node/pull/REPLACEME
-    description: zero is now compared using the [Object.is][] comparison.
+    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
index fcfc88dc8dba71..b3e66aa74fd9be 100644
--- a/lib/assert.js
+++ b/lib/assert.js
@@ -118,7 +118,6 @@ 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.
-// NOTE: Floats should only use binary comparison in non strict mode!
 function areSimilarTypedArrays(a, b, max) {
   const len = a.byteLength;
   if (len !== b.byteLength) {