From 5360dd151da9d8b1c97785a0e394cc731af31f69 Mon Sep 17 00:00:00 2001
From: Ruben Bridgewater <ruben@bridgewater.de>
Date: Mon, 2 Dec 2019 13:12:52 +0100
Subject: assert: handle (deep) equal(NaN, NaN) as being identical

This aligns the `equal` and `deepEqual()` implementations with the
strict versions by accepting `NaN` as being identical in case both
sides are NaN.

Refs: https://github.com/nodejs/node/issues/30350#issuecomment-552191641

PR-URL: https://github.com/nodejs/node/pull/30766
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Denys Otrishko <shishugi@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: David Carlier <devnexen@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
---
 doc/api/assert.md                 | 29 ++++++++++++++++++++++++++---
 lib/assert.js                     |  5 +++--
 lib/internal/util/comparisons.js  |  2 +-
 test/parallel/test-assert-deep.js |  7 +++----
 test/parallel/test-assert.js      |  6 ++++++
 5 files changed, 39 insertions(+), 10 deletions(-)

diff --git a/doc/api/assert.md b/doc/api/assert.md
index 9d1a7fb7e1..05df0d4483 100644
--- a/doc/api/assert.md
+++ b/doc/api/assert.md
@@ -160,6 +160,10 @@ An alias of [`assert.ok()`][].
 <!-- YAML
 added: v0.1.21
 changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/30766
+    description: NaN is now treated as being identical in case both sides are
+                 NaN.
   - version: v12.0.0
     pr-url: https://github.com/nodejs/node/pull/25008
     description: The type tags are now properly compared and there are a couple
@@ -203,7 +207,8 @@ are also recursively evaluated by the following rules.
 ### Comparison details
 
 * Primitive values are compared with the [Abstract Equality Comparison][]
-  ( `==` ).
+  ( `==` ) with the exception of `NaN`. It is treated as being identical in case
+  both sides are `NaN`.
 * [Type tags][Object.prototype.toString()] of objects should be the same.
 * Only [enumerable "own" properties][] are considered.
 * [`Error`][] names and messages are always compared, even if these are not
@@ -554,6 +559,11 @@ assert.doesNotThrow(
 ## assert.equal(actual, expected\[, message\])
 <!-- YAML
 added: v0.1.21
+changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/30766
+    description: NaN is now treated as being identical in case both sides are
+                 NaN.
 -->
 
 * `actual` {any}
@@ -569,7 +579,8 @@ An alias of [`assert.strictEqual()`][].
 > Stability: 0 - Deprecated: Use [`assert.strictEqual()`][] instead.
 
 Tests shallow, coercive equality between the `actual` and `expected` parameters
-using the [Abstract Equality Comparison][] ( `==` ).
+using the [Abstract Equality Comparison][] ( `==` ). `NaN` is special handled
+and treated as being identical in case both sides are `NaN`.
 
 ```js
 const assert = require('assert');
@@ -578,6 +589,8 @@ assert.equal(1, 1);
 // OK, 1 == 1
 assert.equal(1, '1');
 // OK, 1 == '1'
+assert.equal(NaN, NaN);
+// OK
 
 assert.equal(1, 2);
 // AssertionError: 1 == 2
@@ -732,6 +745,10 @@ let err;
 <!-- YAML
 added: v0.1.21
 changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/30766
+    description: NaN is now treated as being identical in case both sides are
+                 NaN.
   - version: v9.0.0
     pr-url: https://github.com/nodejs/node/pull/15001
     description: The `Error` names and messages are now properly compared
@@ -853,6 +870,11 @@ instead of the [`AssertionError`][].
 ## assert.notEqual(actual, expected\[, message\])
 <!-- YAML
 added: v0.1.21
+changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/30766
+    description: NaN is now treated as being identical in case both sides are
+                 NaN.
 -->
 
 * `actual` {any}
@@ -868,7 +890,8 @@ An alias of [`assert.notStrictEqual()`][].
 > Stability: 0 - Deprecated: Use [`assert.notStrictEqual()`][] instead.
 
 Tests shallow, coercive inequality with the [Abstract Equality Comparison][]
-( `!=` ).
+(`!=` ). `NaN` is special handled and treated as being identical in case both
+sides are `NaN`.
 
 ```js
 const assert = require('assert');
diff --git a/lib/assert.js b/lib/assert.js
index c3faba0905..828a443ec9 100644
--- a/lib/assert.js
+++ b/lib/assert.js
@@ -25,6 +25,7 @@ const {
   ObjectIs,
   ObjectKeys,
   ObjectPrototypeIsPrototypeOf,
+  NumberIsNaN
 } = primordials;
 
 const { Buffer } = require('buffer');
@@ -398,7 +399,7 @@ assert.equal = function equal(actual, expected, message) {
     throw new ERR_MISSING_ARGS('actual', 'expected');
   }
   // eslint-disable-next-line eqeqeq
-  if (actual != expected) {
+  if (actual != expected && (!NumberIsNaN(actual) || !NumberIsNaN(expected))) {
     innerFail({
       actual,
       expected,
@@ -416,7 +417,7 @@ assert.notEqual = function notEqual(actual, expected, message) {
     throw new ERR_MISSING_ARGS('actual', 'expected');
   }
   // eslint-disable-next-line eqeqeq
-  if (actual == expected) {
+  if (actual == expected || (NumberIsNaN(actual) && NumberIsNaN(expected))) {
     innerFail({
       actual,
       expected,
diff --git a/lib/internal/util/comparisons.js b/lib/internal/util/comparisons.js
index 734ecf6224..b26012923c 100644
--- a/lib/internal/util/comparisons.js
+++ b/lib/internal/util/comparisons.js
@@ -180,7 +180,7 @@ function innerDeepEqual(val1, val2, strict, memos) {
     if (val1 === null || typeof val1 !== 'object') {
       if (val2 === null || typeof val2 !== 'object') {
         // eslint-disable-next-line eqeqeq
-        return val1 == val2;
+        return val1 == val2 || (NumberIsNaN(val1) && NumberIsNaN(val2));
       }
       return false;
     }
diff --git a/test/parallel/test-assert-deep.js b/test/parallel/test-assert-deep.js
index 9d30f466f3..f2a8349219 100644
--- a/test/parallel/test-assert-deep.js
+++ b/test/parallel/test-assert-deep.js
@@ -590,10 +590,9 @@ assertNotDeepOrStrict(
 }
 
 // Handle NaN
-assert.notDeepEqual(NaN, NaN);
-assert.deepStrictEqual(NaN, NaN);
-assert.deepStrictEqual({ a: NaN }, { a: NaN });
-assert.deepStrictEqual([ 1, 2, NaN, 4 ], [ 1, 2, NaN, 4 ]);
+assertDeepAndStrictEqual(NaN, NaN);
+assertDeepAndStrictEqual({ a: NaN }, { a: NaN });
+assertDeepAndStrictEqual([ 1, 2, NaN, 4 ], [ 1, 2, NaN, 4 ]);
 
 // Handle boxed primitives
 {
diff --git a/test/parallel/test-assert.js b/test/parallel/test-assert.js
index 48b44ce29b..3db63a816a 100644
--- a/test/parallel/test-assert.js
+++ b/test/parallel/test-assert.js
@@ -501,6 +501,12 @@ assert.throws(
   }
 );
 
+a.equal(NaN, NaN);
+a.throws(
+  () => a.notEqual(NaN, NaN),
+  a.AssertionError
+);
+
 // Test strict assert.
 {
   const a = require('assert');
-- 
cgit v1.2.3