assert: use object argument in innerFail

Right now it is difficult to know what argument stands for what
property. By refactoring the arguments into a object it is clear
what stands for what.

PR-URL: nodejs#17582
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Jon Moss <me@jonathanmoss.me>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>

lib/assert.js

@@ -38,22 +38,26 @@ const assert = module.exports = ok;
 // both the actual and expected values to the assertion error for
 // display purposes.
 
-function innerFail(actual, expected, message, operator, stackStartFunction) {
-  throw new errors.AssertionError({
-    message,
-    actual,
-    expected,
-    operator,
-    stackStartFunction
-  });
+function innerFail(obj) {
+  throw new errors.AssertionError(obj);
 }
 
-function fail(actual, expected, message, operator, stackStartFunction) {
-  if (arguments.length === 1)
+function fail(actual, expected, message, operator, stackStartFn) {
+  const argsLen = arguments.length;
+
+  if (argsLen === 1) {
     message = actual;
-  if (arguments.length === 2)
+  } else if (argsLen === 2) {
     operator = '!=';
-  innerFail(actual, expected, message, operator, stackStartFunction || fail);
+  }
+
+  innerFail({
+    actual,
+    expected,
+    message,
+    operator,
+    stackStartFn: stackStartFn || fail
+  });
 }
 assert.fail = fail;
 
@@ -67,37 +71,71 @@ assert.AssertionError = errors.AssertionError;
 // Pure assertion tests whether a value is truthy, as determined
 // by !!value.
 function ok(value, message) {
-  if (!value) innerFail(value, true, message, '==', ok);
+  if (!value) {
+    innerFail({
+      actual: value,
+      expected: true,
+      message,
+      operator: '==',
+      stackStartFn: ok
+    });
+  }
 }
 assert.ok = ok;
 
 // The equality assertion tests shallow, coercive equality with ==.
 /* eslint-disable no-restricted-properties */
 assert.equal = function equal(actual, expected, message) {
   // eslint-disable-next-line eqeqeq
-  if (actual != expected) innerFail(actual, expected, message, '==', equal);
+  if (actual != expected) {
+    innerFail({
+      actual,
+      expected,
+      message,
+      operator: '==',
+      stackStartFn: equal
+    });
+  }
 };
 
 // The non-equality assertion tests for whether two objects are not
 // equal with !=.
 assert.notEqual = function notEqual(actual, expected, message) {
   // eslint-disable-next-line eqeqeq
   if (actual == expected) {
-    innerFail(actual, expected, message, '!=', notEqual);
+    innerFail({
+      actual,
+      expected,
+      message,
+      operator: '!=',
+      stackStartFn: notEqual
+    });
   }
 };
 
 // The equivalence assertion tests a deep equality relation.
 assert.deepEqual = function deepEqual(actual, expected, message) {
   if (!innerDeepEqual(actual, expected, false)) {
-    innerFail(actual, expected, message, 'deepEqual', deepEqual);
+    innerFail({
+      actual,
+      expected,
+      message,
+      operator: 'deepEqual',
+      stackStartFn: deepEqual
+    });
   }
 };
 /* eslint-enable */
 
 assert.deepStrictEqual = function deepStrictEqual(actual, expected, message) {
   if (!innerDeepEqual(actual, expected, true)) {
-    innerFail(actual, expected, message, 'deepStrictEqual', deepStrictEqual);
+    innerFail({
+      actual,
+      expected,
+      message,
+      operator: 'deepStrictEqual',
+      stackStartFn: deepStrictEqual
+    });
   }
 };
 
@@ -572,30 +610,53 @@ function objEquiv(a, b, strict, keys, memos) {
 // The non-equivalence assertion tests for any deep inequality.
 assert.notDeepEqual = function notDeepEqual(actual, expected, message) {
   if (innerDeepEqual(actual, expected, false)) {
-    innerFail(actual, expected, message, 'notDeepEqual', notDeepEqual);
+    innerFail({
+      actual,
+      expected,
+      message,
+      operator: 'notDeepEqual',
+      stackStartFn: notDeepEqual
+    });
   }
 };
 
 assert.notDeepStrictEqual = notDeepStrictEqual;
 function notDeepStrictEqual(actual, expected, message) {
   if (innerDeepEqual(actual, expected, true)) {
-    innerFail(actual, expected, message, 'notDeepStrictEqual',
-              notDeepStrictEqual);
+    innerFail({
+      actual,
+      expected,
+      message,
+      operator: 'notDeepStrictEqual',
+      stackStartFn: notDeepStrictEqual
+    });
   }
 }
 
 // The strict equality assertion tests strict equality, as determined by ===.
 assert.strictEqual = function strictEqual(actual, expected, message) {
   if (actual !== expected) {
-    innerFail(actual, expected, message, '===', strictEqual);
+    innerFail({
+      actual,
+      expected,
+      message,
+      operator: '===',
+      stackStartFn: strictEqual
+    });
   }
 };
 
 // The strict non-equality assertion tests for strict inequality, as
 // determined by !==.
 assert.notStrictEqual = function notStrictEqual(actual, expected, message) {
   if (actual === expected) {
-    innerFail(actual, expected, message, '!==', notStrictEqual);
+    innerFail({
+      actual,
+      expected,
+      message,
+      operator: '!==',
+      stackStartFn: notStrictEqual
+    });
   }
 };
 
@@ -643,18 +704,27 @@ function innerThrows(shouldThrow, block, expected, message) {
         details += ` (${expected.name})`;
       }
       details += message ? `: ${message}` : '.';
-      fail(actual, expected, `Missing expected exception${details}`, 'throws');
+      innerFail({
+        actual,
+        expected,
+        operator: 'throws',
+        message: `Missing expected exception${details}`,
+        stackStartFn: innerThrows
+      });
     }
     if (expected && expectedException(actual, expected) === false) {
       throw actual;
     }
   } else if (actual !== undefined) {
     if (!expected || expectedException(actual, expected)) {
       details = message ? `: ${message}` : '.';
-      fail(actual,
-           expected,
-           `Got unwanted exception${details}`,
-           'doesNotThrow');
+      innerFail({
+        actual,
+        expected,
+        operator: 'doesNotThrow',
+        message: `Got unwanted exception${details}`,
+        stackStartFn: innerThrows
+      });
     }
     throw actual;
   }

lib/internal/errors.js

@@ -80,7 +80,7 @@ class AssertionError extends Error {
     if (typeof options !== 'object' || options === null) {
       throw new exports.TypeError('ERR_INVALID_ARG_TYPE', 'options', 'object');
     }
-    var { actual, expected, message, operator, stackStartFunction } = options;
+    var { actual, expected, message, operator, stackStartFn } = options;
     if (message) {
       super(message);
     } else {
@@ -99,7 +99,7 @@ class AssertionError extends Error {
     this.actual = actual;
     this.expected = expected;
     this.operator = operator;
-    Error.captureStackTrace(this, stackStartFunction);
+    Error.captureStackTrace(this, stackStartFn);
   }
 }
 

test/message/error_exit.out

@@ -1,6 +1,6 @@
 Exiting with code=1
 assert.js:*
-  throw new errors.AssertionError({
+  throw new errors.AssertionError(obj);
   ^
 
 AssertionError [ERR_ASSERTION]: 1 === 2

test/parallel/test-assert.js

@@ -721,3 +721,12 @@ common.expectsError(
   /* eslint-enable no-restricted-properties */
   assert(7);
 }
+
+common.expectsError(
+  () => assert.ok(null),
+  {
+    code: 'ERR_ASSERTION',
+    type: assert.AssertionError,
+    message: 'null == true'
+  }
+);