assert: refactor the code

1. Rename private functions
2. Use destructuring
3. Remove obsolete comments

Backport-PR-URL: https://github.com/nodejs/node/pull/15516
PR-URL: https://github.com/nodejs/node/pull/13862
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>

lib/assert.js

@@ -20,8 +20,7 @@
 
 'use strict';
 
-// UTILITY
-const compare = process.binding('buffer').compare;
+const { compare } = process.binding('buffer');
 const util = require('util');
 const Buffer = require('buffer').Buffer;
 const pToString = (obj) => Object.prototype.toString.call(obj);
@@ -49,7 +48,7 @@ assert.AssertionError = function AssertionError(options) {
     this.message = getMessage(this);
     this.generatedMessage = true;
   }
-  var stackStartFunction = options.stackStartFunction || fail;
+  const stackStartFunction = options.stackStartFunction || fail;
   Error.captureStackTrace(this, stackStartFunction);
 };
 
@@ -73,7 +72,7 @@ function getMessage(self) {
 
 // All of the following functions must throw an AssertionError
 // when a corresponding condition is not met, with a message that
-// may be undefined if not provided.  All assertion methods provide
+// may be undefined if not provided. All assertion methods provide
 // both the actual and expected values to the assertion error for
 // display purposes.
 
@@ -86,25 +85,16 @@ function fail(actual, expected, message, operator, stackStartFunction) {
     stackStartFunction: stackStartFunction
   });
 }
-
-// EXTENSION! allows for well behaved errors defined elsewhere.
 assert.fail = fail;
 
 // Pure assertion tests whether a value is truthy, as determined
-// by !!guard.
-// assert.ok(guard, message_opt);
-// This statement is equivalent to assert.equal(true, !!guard,
-// message_opt);. To test strictly for the value true, use
-// assert.strictEqual(true, guard, message_opt);.
-
+// by !!value.
 function ok(value, message) {
-  if (!value) fail(value, true, message, '==', assert.ok);
+  if (!value) fail(value, true, message, '==', ok);
 }
 assert.ok = ok;
 
-// The equality assertion tests shallow, coercive equality with
-// ==.
-// assert.equal(actual, expected, message_opt);
+// 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);
@@ -112,31 +102,27 @@ assert.equal = function equal(actual, expected, message) {
 
 // The non-equality assertion tests for whether two objects are not
 // equal with !=.
-// assert.notEqual(actual, expected, message_opt);
-
 assert.notEqual = function notEqual(actual, expected, message) {
   if (actual == expected) {
-    fail(actual, expected, message, '!=', assert.notEqual);
+    fail(actual, expected, message, '!=', notEqual);
   }
 };
 
 // The equivalence assertion tests a deep equality relation.
-// assert.deepEqual(actual, expected, message_opt);
-
 assert.deepEqual = function deepEqual(actual, expected, message) {
-  if (!_deepEqual(actual, expected, false)) {
-    fail(actual, expected, message, 'deepEqual', assert.deepEqual);
+  if (!innerDeepEqual(actual, expected, false)) {
+    fail(actual, expected, message, 'deepEqual', deepEqual);
   }
 };
 /* eslint-enable */
 
 assert.deepStrictEqual = function deepStrictEqual(actual, expected, message) {
-  if (!_deepEqual(actual, expected, true)) {
-    fail(actual, expected, message, 'deepStrictEqual', assert.deepStrictEqual);
+  if (!innerDeepEqual(actual, expected, true)) {
+    fail(actual, expected, message, 'deepStrictEqual', deepStrictEqual);
   }
 };
 
-function _deepEqual(actual, expected, strict, memos) {
+function innerDeepEqual(actual, expected, strict, memos) {
   // All identical values are equivalent, as determined by ===.
   if (actual === expected) {
     return true;
@@ -247,45 +233,40 @@ function objEquiv(a, b, strict, actualVisitedObjects) {
   // Possibly expensive deep test:
   for (i = ka.length - 1; i >= 0; i--) {
     key = ka[i];
-    if (!_deepEqual(a[key], b[key], strict, actualVisitedObjects))
+    if (!innerDeepEqual(a[key], b[key], strict, actualVisitedObjects))
       return false;
   }
   return true;
 }
 
 // The non-equivalence assertion tests for any deep inequality.
-// assert.notDeepEqual(actual, expected, message_opt);
-
 assert.notDeepEqual = function notDeepEqual(actual, expected, message) {
-  if (_deepEqual(actual, expected, false)) {
-    fail(actual, expected, message, 'notDeepEqual', assert.notDeepEqual);
+  if (innerDeepEqual(actual, expected, false)) {
+    fail(actual, expected, message, 'notDeepEqual', notDeepEqual);
   }
 };
 
 assert.notDeepStrictEqual = notDeepStrictEqual;
 function notDeepStrictEqual(actual, expected, message) {
-  if (_deepEqual(actual, expected, true)) {
-    fail(actual, expected, message, 'notDeepStrictEqual', notDeepStrictEqual);
+  if (innerDeepEqual(actual, expected, true)) {
+    fail(actual, expected, message, 'notDeepStrictEqual',
+         notDeepStrictEqual);
   }
 }
 
 
 // The strict equality assertion tests strict equality, as determined by ===.
-// assert.strictEqual(actual, expected, message_opt);
-
 assert.strictEqual = function strictEqual(actual, expected, message) {
   if (actual !== expected) {
-    fail(actual, expected, message, '===', assert.strictEqual);
+    fail(actual, expected, message, '===', strictEqual);
   }
 };
 
 // The strict non-equality assertion tests for strict inequality, as
 // determined by !==.
-// assert.notStrictEqual(actual, expected, message_opt);
-
 assert.notStrictEqual = function notStrictEqual(actual, expected, message) {
   if (actual === expected) {
-    fail(actual, expected, message, '!==', assert.notStrictEqual);
+    fail(actual, expected, message, '!==', notStrictEqual);
   }
 };
 
@@ -314,7 +295,7 @@ function expectedException(actual, expected) {
   return expected.call({}, actual) === true;
 }
 
-function _tryBlock(block) {
+function tryBlock(block) {
   var error;
   try {
     block();
@@ -324,9 +305,7 @@ function _tryBlock(block) {
   return error;
 }
 
-function _throws(shouldThrow, block, expected, message) {
-  var actual;
-
+function innerThrows(shouldThrow, block, expected, message) {
   if (typeof block !== 'function') {
     throw new TypeError('"block" argument must be a function');
   }
@@ -336,13 +315,13 @@ function _throws(shouldThrow, block, expected, message) {
     expected = null;
   }
 
-  actual = _tryBlock(block);
+  const actual = tryBlock(block);
 
   message = (expected && expected.name ? ' (' + expected.name + ').' : '.') +
             (message ? ' ' + message : '.');
 
   if (shouldThrow && !actual) {
-    fail(actual, expected, 'Missing expected exception' + message);
+    fail(actual, expected, 'Missing expected exception' + message, fail);
   }
 
   const userProvidedMessage = typeof message === 'string';
@@ -353,7 +332,7 @@ function _throws(shouldThrow, block, expected, message) {
       userProvidedMessage &&
       expectedException(actual, expected)) ||
       isUnexpectedException) {
-    fail(actual, expected, 'Got unwanted exception' + message);
+    fail(actual, expected, 'Got unwanted exception' + message, fail);
   }
 
   if ((shouldThrow && actual && expected &&
@@ -363,15 +342,12 @@ function _throws(shouldThrow, block, expected, message) {
 }
 
 // Expected to throw an error.
-// assert.throws(block, Error_opt, message_opt);
-
-assert.throws = function(block, /*optional*/error, /*optional*/message) {
-  _throws(true, block, error, message);
+assert.throws = function throws(block, error, message) {
+  innerThrows(true, block, error, message);
 };
 
-// EXTENSION! This is annoying to write outside this module.
-assert.doesNotThrow = function(block, /*optional*/error, /*optional*/message) {
-  _throws(false, block, error, message);
+assert.doesNotThrow = function doesNotThrow(block, error, message) {
+  innerThrows(false, block, error, message);
 };
 
-assert.ifError = function(err) { if (err) throw err; };
+assert.ifError = function ifError(err) { if (err) throw err; };

test/parallel/test-assert-fail.js

@@ -0,0 +1,10 @@
+'use strict';
+
+require('../common');
+const assert = require('assert');
+
+// The stackFrameFunction should exclude the foo frame
+assert.throws(
+  function foo() { assert.fail('first', 'second', 'message', '!==', foo); },
+  (err) => !/foo/m.test(err.stack)
+);