doc,assert: document stackStartFunction in fail

* refactor the code
  1. Rename private functions
  2. Use destructuring
  3. Remove obsolete comments

* remove eslint rule

PR-URL: #13862
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>

doc/api/assert.md

@@ -194,22 +194,25 @@ If the values are not equal, an `AssertionError` is thrown with a `message`
 property set equal to the value of the `message` parameter. If the `message`
 parameter is undefined, a default error message is assigned.
 
-## assert.fail(message)
+## assert.fail([message])
 ## assert.fail(actual, expected[, message[, operator[, stackStartFunction]]])
 <!-- YAML
 added: v0.1.21
 -->
 * `actual` {any}
 * `expected` {any}
-* `message` {any}
+* `message` {any} (default: 'Failed')
 * `operator` {string} (default: '!=')
 * `stackStartFunction` {function} (default: `assert.fail`)
 
 Throws an `AssertionError`. If `message` is falsy, the error message is set as
 the values of `actual` and `expected` separated by the provided `operator`.
-Otherwise, the error message is the value of `message`.
+If just the two `actual` and `expected` arguments are provided, `operator` will
+default to `'!='`. If `message` is provided only it will be used as the error
+message, the other arguments will be stored as properties on the thrown object.
 If `stackStartFunction` is provided, all stack frames above that function will
-be removed from stacktrace (see [`Error.captureStackTrace`]).
+be removed from stacktrace (see [`Error.captureStackTrace`]). If no arguments
+are given, the default message `Failed` will be used.
 
 ```js
 const assert = require('assert');
@@ -222,6 +225,14 @@ assert.fail(1, 2, 'fail');
 
 assert.fail(1, 2, 'whoops', '>');
 // AssertionError: whoops
+```
+
+*Note*: Is the last two cases `actual`, `expected`, and `operator` have no
+influence on the error message.
+
+```js
+assert.fail();
+// AssertionError: Failed
 
 assert.fail('boom');
 // AssertionError: boom

lib/assert.js

@@ -22,7 +22,7 @@
 
 const { compare } = process.binding('buffer');
 const util = require('util');
-const Buffer = require('buffer').Buffer;
+const { Buffer } = require('buffer');
 const pToString = (obj) => Object.prototype.toString.call(obj);
 
 // The assert module provides functions that throw
@@ -76,53 +76,65 @@ function getMessage(self) {
 // both the actual and expected values to the assertion error for
 // display purposes.
 
+function innerFail(actual, expected, message, operator, stackStartFunction) {
+  throw new assert.AssertionError({
+    message,
+    actual,
+    expected,
+    operator,
+    stackStartFunction
+  });
+}
+
 function fail(actual, expected, message, operator, stackStartFunction) {
-  if (arguments.length === 1)
+  if (arguments.length === 0) {
+    message = 'Failed';
+  }
+  if (arguments.length === 1) {
     message = actual;
-  if (arguments.length === 2)
+    actual = undefined;
+  }
+  if (arguments.length === 2) {
     operator = '!=';
-  throw new assert.AssertionError({
-    message: message,
-    actual: actual,
-    expected: expected,
-    operator: operator,
-    stackStartFunction: stackStartFunction
-  });
+  }
+  innerFail(actual, expected, message, operator, stackStartFunction || fail);
 }
 assert.fail = fail;
 
 // Pure assertion tests whether a value is truthy, as determined
 // by !!value.
 function ok(value, message) {
-  if (!value) fail(value, true, message, '==', ok);
+  if (!value) innerFail(value, true, message, '==', ok);
 }
 assert.ok = ok;
 
 // The equality assertion tests shallow, coercive equality with ==.
 /* eslint-disable no-restricted-properties */
 assert.equal = function equal(actual, expected, message) {
-  if (actual != expected) fail(actual, expected, message, '==', assert.equal);
+  // eslint-disable-next-line eqeqeq
+  if (actual != expected) innerFail(actual, expected, message, '==', 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) {
-    fail(actual, expected, message, '!=', notEqual);
+    innerFail(actual, expected, message, '!=', notEqual);
   }
 };
 
 // The equivalence assertion tests a deep equality relation.
 assert.deepEqual = function deepEqual(actual, expected, message) {
   if (!innerDeepEqual(actual, expected, false)) {
-    fail(actual, expected, message, 'deepEqual', deepEqual);
+    innerFail(actual, expected, message, 'deepEqual', deepEqual);
   }
 };
 /* eslint-enable */
 
 assert.deepStrictEqual = function deepStrictEqual(actual, expected, message) {
   if (!innerDeepEqual(actual, expected, true)) {
-    fail(actual, expected, message, 'deepStrictEqual', deepStrictEqual);
+    innerFail(actual, expected, message, 'deepStrictEqual', deepStrictEqual);
   }
 };
 
@@ -246,31 +258,30 @@ function objEquiv(a, b, strict, actualVisitedObjects) {
 // The non-equivalence assertion tests for any deep inequality.
 assert.notDeepEqual = function notDeepEqual(actual, expected, message) {
   if (innerDeepEqual(actual, expected, false)) {
-    fail(actual, expected, message, 'notDeepEqual', notDeepEqual);
+    innerFail(actual, expected, message, 'notDeepEqual', notDeepEqual);
   }
 };
 
 assert.notDeepStrictEqual = notDeepStrictEqual;
 function notDeepStrictEqual(actual, expected, message) {
   if (innerDeepEqual(actual, expected, true)) {
-    fail(actual, expected, message, 'notDeepStrictEqual',
-         notDeepStrictEqual);
+    innerFail(actual, expected, message, 'notDeepStrictEqual',
+              notDeepStrictEqual);
   }
 }
 
-
 // The strict equality assertion tests strict equality, as determined by ===.
 assert.strictEqual = function strictEqual(actual, expected, message) {
   if (actual !== expected) {
-    fail(actual, expected, message, '===', strictEqual);
+    innerFail(actual, expected, message, '===', strictEqual);
   }
 };
 
 // The strict non-equality assertion tests for strict inequality, as
 // determined by !==.
 assert.notStrictEqual = function notStrictEqual(actual, expected, message) {
   if (actual === expected) {
-    fail(actual, expected, message, '!==', notStrictEqual);
+    innerFail(actual, expected, message, '!==', notStrictEqual);
   }
 };
 
@@ -325,7 +336,7 @@ function innerThrows(shouldThrow, block, expected, message) {
             (message ? ' ' + message : '.');
 
   if (shouldThrow && !actual) {
-    fail(actual, expected, 'Missing expected exception' + message, fail);
+    innerFail(actual, expected, 'Missing expected exception' + message, fail);
   }
 
   const userProvidedMessage = typeof message === 'string';
@@ -336,7 +347,7 @@ function innerThrows(shouldThrow, block, expected, message) {
       userProvidedMessage &&
       expectedException(actual, expected)) ||
       isUnexpectedException) {
-    fail(actual, expected, 'Got unwanted exception' + message, fail);
+    innerFail(actual, expected, 'Got unwanted exception' + message, fail);
   }
 
   if ((shouldThrow && actual && expected &&
@@ -354,4 +365,4 @@ assert.doesNotThrow = function doesNotThrow(block, error, message) {
   innerThrows(false, block, error, message);
 };
 
-assert.ifError = function ifError(err) { if (err) throw err; };
+assert.ifError = function(err) { if (err) throw err; };

test/parallel/test-assert-fail.js

@@ -2,35 +2,41 @@
 
 require('../common');
 const assert = require('assert');
+const { AssertionError } = assert;
 
-// no args
+// No args
 assert.throws(
   () => { assert.fail(); },
-  /^AssertionError: undefined undefined undefined$/
+  AssertionError,
+  'Failed'
 );
 
-// one arg = message
+// One arg = message
 assert.throws(
   () => { assert.fail('custom message'); },
-  /^AssertionError: custom message$/
+  AssertionError,
+  'custom message'
 );
 
-// two args only, operator defaults to '!='
+// Two args only, operator defaults to '!='
 assert.throws(
   () => { assert.fail('first', 'second'); },
-  /^AssertionError: 'first' != 'second'$/
+  AssertionError,
+  '\'first\' != \'second\''
 );
 
-// three args
+// Three args
 assert.throws(
   () => { assert.fail('ignored', 'ignored', 'another custom message'); },
-  /^AssertionError: another custom message$/
+  AssertionError,
+  'another custom message'
 );
 
-// no third arg (but a fourth arg)
+// No third arg (but a fourth arg)
 assert.throws(
   () => { assert.fail('first', 'second', undefined, 'operator'); },
-  /^AssertionError: 'first' operator 'second'$/
+  AssertionError,
+  '\'first\' operator \'second\''
 );
 
 // The stackFrameFunction should exclude the foo frame