From 9d82b7659c23669b948acf3e2ba0e4e6f631a4b2 Mon Sep 17 00:00:00 2001
From: Ruben Bridgewater <ruben@bridgewater.de>
Date: Sat, 9 Nov 2019 16:14:24 +0300
Subject: [PATCH 01/15] util: fix .format() not always calling toString when it
 should be

This makes sure that `util.format('%s', object)` will always call
a user defined `toString` function. It was formerly not the case
when the object had the function declared on the super class.

At the same time this also makes sure that getters won't be
triggered accessing the `constructor` property.

PR-URL: https://github.com/nodejs/node/pull/30343
Fixes: https://github.com/nodejs/node/issues/30333
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Denys Otrishko <shishugi@gmail.com>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
---
 lib/internal/util/inspect.js      | 56 +++++++++++++++++------------
 test/parallel/test-util-format.js | 60 +++++++++++++++++++++++++++++++
 2 files changed, 94 insertions(+), 22 deletions(-)

diff --git a/lib/internal/util/inspect.js b/lib/internal/util/inspect.js
index abf440be779dac..da6f9426ec7b25 100644
--- a/lib/internal/util/inspect.js
+++ b/lib/internal/util/inspect.js
@@ -1645,6 +1645,30 @@ function format(...args) {
   return formatWithOptions(undefined, ...args);
 }
 
+function hasBuiltInToString(value) {
+  // Count objects that have no `toString` function as built-in.
+  if (typeof value.toString !== 'function') {
+    return true;
+  }
+
+  // The object has a own `toString` property. Thus it's not not a built-in one.
+  if (ObjectPrototypeHasOwnProperty(value, 'toString')) {
+    return false;
+  }
+
+  // Find the object that has the `toString` property as own property in the
+  // prototype chain.
+  let pointer = value;
+  do {
+    pointer = ObjectGetPrototypeOf(pointer);
+  } while (!ObjectPrototypeHasOwnProperty(pointer, 'toString'));
+
+  // Check closer if the object is a built-in.
+  const descriptor = ObjectGetOwnPropertyDescriptor(pointer, 'constructor');
+  return descriptor !== undefined &&
+    typeof descriptor.value === 'function' &&
+    builtInObjects.has(descriptor.value.name);
+}
 
 const firstErrorLine = (error) => error.message.split('\n')[0];
 let CIRCULAR_ERROR_MESSAGE;
@@ -1692,29 +1716,17 @@ function formatWithOptions(inspectOptions, ...args) {
                 tempStr = formatNumber(stylizeNoColor, tempArg);
               } else if (typeof tempArg === 'bigint') {
                 tempStr = `${tempArg}n`;
+              } else if (typeof tempArg !== 'object' ||
+                         tempArg === null ||
+                         !hasBuiltInToString(tempArg)) {
+                tempStr = String(tempArg);
               } else {
-                let constr;
-                if (typeof tempArg !== 'object' ||
-                    tempArg === null ||
-                    (typeof tempArg.toString === 'function' &&
-                     // A direct own property.
-                     (ObjectPrototypeHasOwnProperty(tempArg, 'toString') ||
-                      // A direct own property on the constructor prototype in
-                      // case the constructor is not an built-in object.
-                      ((constr = tempArg.constructor) &&
-                      !builtInObjects.has(constr.name) &&
-                      constr.prototype &&
-                      ObjectPrototypeHasOwnProperty(constr.prototype,
-                                                    'toString'))))) {
-                  tempStr = String(tempArg);
-                } else {
-                  tempStr = inspect(tempArg, {
-                    ...inspectOptions,
-                    compact: 3,
-                    colors: false,
-                    depth: 0
-                  });
-                }
+                tempStr = inspect(tempArg, {
+                  ...inspectOptions,
+                  compact: 3,
+                  colors: false,
+                  depth: 0
+                });
               }
               break;
             case 106: // 'j'
diff --git a/test/parallel/test-util-format.js b/test/parallel/test-util-format.js
index cebc666728b246..9b30ca72a07816 100644
--- a/test/parallel/test-util-format.js
+++ b/test/parallel/test-util-format.js
@@ -160,6 +160,66 @@ assert.strictEqual(util.format('%s', () => 5), '() => 5');
     util.format('%s', new Foobar(5)),
     'Foobar [ <5 empty items>, aaa: true ]'
   );
+
+  // Subclassing:
+  class B extends Foo {}
+
+  function C() {}
+  C.prototype.toString = function() {
+    return 'Custom';
+  };
+
+  function D() {
+    C.call(this);
+  }
+  D.prototype = Object.create(C.prototype);
+
+  assert.strictEqual(
+    util.format('%s', new B()),
+    'Bar'
+  );
+  assert.strictEqual(
+    util.format('%s', new C()),
+    'Custom'
+  );
+  assert.strictEqual(
+    util.format('%s', new D()),
+    'Custom'
+  );
+
+  D.prototype.constructor = D;
+  assert.strictEqual(
+    util.format('%s', new D()),
+    'Custom'
+  );
+
+  D.prototype.constructor = null;
+  assert.strictEqual(
+    util.format('%s', new D()),
+    'Custom'
+  );
+
+  D.prototype.constructor = { name: 'Foobar' };
+  assert.strictEqual(
+    util.format('%s', new D()),
+    'Custom'
+  );
+
+  Object.defineProperty(D.prototype, 'constructor', {
+    get() {
+      throw new Error();
+    },
+    configurable: true
+  });
+  assert.strictEqual(
+    util.format('%s', new D()),
+    'Custom'
+  );
+
+  assert.strictEqual(
+    util.format('%s', Object.create(null)),
+    '[Object: null prototype] {}'
+  );
 }
 
 // JSON format specifier

From 2859667b1bcca2238085c6a3ab9ebb4ae9e70224 Mon Sep 17 00:00:00 2001
From: Ruben Bridgewater <ruben@bridgewater.de>
Date: Sat, 30 Nov 2019 10:56:11 +0100
Subject: [PATCH 02/15] benchmark: add more util inspect and format benchmarks

This adds a couple of benchmarks to check different options and code
paths.

PR-URL: https://github.com/nodejs/node/pull/30767
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Denys Otrishko <shishugi@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
---
 benchmark/util/format.js              |  2 ++
 benchmark/util/inspect-proxy.js       | 18 +++++++++++++-----
 test/benchmark/test-benchmark-util.js |  4 +++-
 3 files changed, 18 insertions(+), 6 deletions(-)

diff --git a/benchmark/util/format.js b/benchmark/util/format.js
index 44e950a6ca6522..976e0f4e655486 100644
--- a/benchmark/util/format.js
+++ b/benchmark/util/format.js
@@ -13,6 +13,8 @@ const inputs = {
   'no-replace-2': ['foobar', 'yeah', 'mensch', 5],
   'only-objects': [{ msg: 'This is an error' }, { msg: 'This is an error' }],
   'many-%': ['replace%%%%s%%%%many%s%s%s', 'percent'],
+  'object-to-string': ['foo %s bar', { toString() { return 'bla'; } }],
+  'object-%s': ['foo %s bar', { a: true, b: false }],
 };
 
 const bench = common.createBenchmark(main, {
diff --git a/benchmark/util/inspect-proxy.js b/benchmark/util/inspect-proxy.js
index dde86941ff42a3..fd89d568aba35f 100644
--- a/benchmark/util/inspect-proxy.js
+++ b/benchmark/util/inspect-proxy.js
@@ -3,13 +3,21 @@
 const util = require('util');
 const common = require('../common.js');
 
-const bench = common.createBenchmark(main, { n: [2e4] });
+const bench = common.createBenchmark(main, {
+  n: [2e4],
+  showProxy: [0, 1],
+  isProxy: [0, 1]
+});
 
-function main({ n }) {
-  const proxyA = new Proxy({}, { get: () => {} });
-  const proxyB = new Proxy(() => {}, {});
+function main({ n, showProxy, isProxy }) {
+  let proxyA = {};
+  let proxyB = () => {};
+  if (isProxy) {
+    proxyA = new Proxy(proxyA, { get: () => {} });
+    proxyB = new Proxy(proxyB, {});
+  }
   bench.start();
   for (let i = 0; i < n; i += 1)
-    util.inspect({ a: proxyA, b: proxyB }, { showProxy: true });
+    util.inspect({ a: proxyA, b: proxyB }, { showProxy });
   bench.end(n);
 }
diff --git a/test/benchmark/test-benchmark-util.js b/test/benchmark/test-benchmark-util.js
index 97b02bbdeed5cd..b66d4fdb9b4cf6 100644
--- a/test/benchmark/test-benchmark-util.js
+++ b/test/benchmark/test-benchmark-util.js
@@ -14,5 +14,7 @@ runBenchmark('util',
               'size=1',
               'type=',
               'len=1',
-              'version=native'],
+              'version=native',
+              'isProxy=1',
+              'showProxy=1'],
              { NODEJS_BENCHMARK_ZERO_ALLOWED: 1 });

From e110449d4e0e725d2c2eaad78a621ad1a1783446 Mon Sep 17 00:00:00 2001
From: Ruben Bridgewater <ruben@bridgewater.de>
Date: Sat, 30 Nov 2019 11:04:34 +0100
Subject: [PATCH 03/15] util: improve performance inspecting proxies

This makes sure we do not retrieve the handler in case it's not
required. This improves the performance a tiny bit for these cases.

PR-URL: https://github.com/nodejs/node/pull/30767
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Denys Otrishko <shishugi@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
---
 benchmark/util/inspect-proxy.js          |  2 +-
 lib/internal/util/inspect.js             |  4 ++--
 src/node_util.cc                         | 20 ++++++++++++++------
 test/parallel/test-util-inspect-proxy.js |  7 +++++--
 4 files changed, 22 insertions(+), 11 deletions(-)

diff --git a/benchmark/util/inspect-proxy.js b/benchmark/util/inspect-proxy.js
index fd89d568aba35f..02379cdc770f3b 100644
--- a/benchmark/util/inspect-proxy.js
+++ b/benchmark/util/inspect-proxy.js
@@ -4,7 +4,7 @@ const util = require('util');
 const common = require('../common.js');
 
 const bench = common.createBenchmark(main, {
-  n: [2e4],
+  n: [1e5],
   showProxy: [0, 1],
   isProxy: [0, 1]
 });
diff --git a/lib/internal/util/inspect.js b/lib/internal/util/inspect.js
index da6f9426ec7b25..09f755ac00649f 100644
--- a/lib/internal/util/inspect.js
+++ b/lib/internal/util/inspect.js
@@ -640,12 +640,12 @@ function formatValue(ctx, value, recurseTimes, typedArray) {
   const context = value;
   // Always check for proxies to prevent side effects and to prevent triggering
   // any proxy handlers.
-  const proxy = getProxyDetails(value);
+  const proxy = getProxyDetails(value, !!ctx.showProxy);
   if (proxy !== undefined) {
     if (ctx.showProxy) {
       return formatProxy(ctx, proxy, recurseTimes);
     }
-    value = proxy[0];
+    value = proxy;
   }
 
   // Provide a hook for user-specified inspect functions.
diff --git a/src/node_util.cc b/src/node_util.cc
index 4d0b109371fb60..b9553eaaa6763d 100644
--- a/src/node_util.cc
+++ b/src/node_util.cc
@@ -91,15 +91,23 @@ static void GetProxyDetails(const FunctionCallbackInfo<Value>& args) {
   if (!args[0]->IsProxy())
     return;
 
+  CHECK(args[1]->IsBoolean());
+
   Local<Proxy> proxy = args[0].As<Proxy>();
 
-  Local<Value> ret[] = {
-    proxy->GetTarget(),
-    proxy->GetHandler()
-  };
+  if (args[1]->IsTrue()) {
+    Local<Value> ret[] = {
+      proxy->GetTarget(),
+      proxy->GetHandler()
+    };
 
-  args.GetReturnValue().Set(
-      Array::New(args.GetIsolate(), ret, arraysize(ret)));
+    args.GetReturnValue().Set(
+        Array::New(args.GetIsolate(), ret, arraysize(ret)));
+  } else {
+    Local<Value> ret = proxy->GetTarget();
+
+    args.GetReturnValue().Set(ret);
+  }
 }
 
 static void PreviewEntries(const FunctionCallbackInfo<Value>& args) {
diff --git a/test/parallel/test-util-inspect-proxy.js b/test/parallel/test-util-inspect-proxy.js
index c20af7450a426e..5e85b92f713175 100644
--- a/test/parallel/test-util-inspect-proxy.js
+++ b/test/parallel/test-util-inspect-proxy.js
@@ -43,10 +43,13 @@ util.inspect(proxyObj, opts);
 
 // getProxyDetails is an internal method, not intended for public use.
 // This is here to test that the internals are working correctly.
-const details = processUtil.getProxyDetails(proxyObj);
+let details = processUtil.getProxyDetails(proxyObj, true);
 assert.strictEqual(target, details[0]);
 assert.strictEqual(handler, details[1]);
 
+details = processUtil.getProxyDetails(proxyObj, false);
+assert.strictEqual(target, details);
+
 assert.strictEqual(
   util.inspect(proxyObj, opts),
   'Proxy [\n' +
@@ -105,7 +108,7 @@ const expected6 = 'Proxy [\n' +
                   '  ]\n' +
                   ']';
 assert.strictEqual(
-  util.inspect(proxy1, { showProxy: true, depth: null }),
+  util.inspect(proxy1, { showProxy: 1, depth: null }),
   expected1);
 assert.strictEqual(util.inspect(proxy2, opts), expected2);
 assert.strictEqual(util.inspect(proxy3, opts), expected3);

From 62ed6a20373b834956f6703015167cb75f809a3c Mon Sep 17 00:00:00 2001
From: Ruben Bridgewater <ruben@bridgewater.de>
Date: Sun, 1 Dec 2019 01:08:37 +0100
Subject: [PATCH 04/15] util: never trigger any proxy traps using `format()`

PR-URL: https://github.com/nodejs/node/pull/30767
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Denys Otrishko <shishugi@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
---
 lib/internal/util/inspect.js             | 7 +++++++
 test/parallel/test-util-inspect-proxy.js | 3 +++
 2 files changed, 10 insertions(+)

diff --git a/lib/internal/util/inspect.js b/lib/internal/util/inspect.js
index 09f755ac00649f..38665d1bc2a515 100644
--- a/lib/internal/util/inspect.js
+++ b/lib/internal/util/inspect.js
@@ -1646,6 +1646,13 @@ function format(...args) {
 }
 
 function hasBuiltInToString(value) {
+  // Prevent triggering proxy traps.
+  const getFullProxy = false;
+  const proxyTarget = getProxyDetails(value, getFullProxy);
+  if (proxyTarget !== undefined) {
+    value = proxyTarget;
+  }
+
   // Count objects that have no `toString` function as built-in.
   if (typeof value.toString !== 'function') {
     return true;
diff --git a/test/parallel/test-util-inspect-proxy.js b/test/parallel/test-util-inspect-proxy.js
index 5e85b92f713175..5789a6ffbcf6c5 100644
--- a/test/parallel/test-util-inspect-proxy.js
+++ b/test/parallel/test-util-inspect-proxy.js
@@ -41,6 +41,9 @@ proxyObj = new Proxy(target, handler);
 // Inspecting the proxy should not actually walk it's properties
 util.inspect(proxyObj, opts);
 
+// Make sure inspecting object does not trigger any proxy traps.
+util.format('%s', proxyObj);
+
 // getProxyDetails is an internal method, not intended for public use.
 // This is here to test that the internals are working correctly.
 let details = processUtil.getProxyDetails(proxyObj, true);

From 7d19abacec2677a4939efa58f8194a4f823cc7e2 Mon Sep 17 00:00:00 2001
From: Ruben Bridgewater <ruben@bridgewater.de>
Date: Sat, 30 Nov 2019 13:33:18 +0100
Subject: [PATCH 05/15] util: fix built-in detection
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

This makes sure that the regular expression matches all built-in
objects properly. So far a couple where missed.

PR-URL: https://github.com/nodejs/node/pull/30768
Fixes: https://github.com/nodejs/node/issues/30183
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
---
 lib/internal/util/inspect.js | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/internal/util/inspect.js b/lib/internal/util/inspect.js
index 38665d1bc2a515..dd8a863a81083c 100644
--- a/lib/internal/util/inspect.js
+++ b/lib/internal/util/inspect.js
@@ -118,7 +118,7 @@ const { NativeModule } = require('internal/bootstrap/loaders');
 let hexSlice;
 
 const builtInObjects = new Set(
-  ObjectGetOwnPropertyNames(global).filter((e) => /^([A-Z][a-z]+)+$/.test(e))
+  ObjectGetOwnPropertyNames(global).filter((e) => /^[A-Z][a-zA-Z0-9]+$/.test(e))
 );
 
 // These options must stay in sync with `getUserOptions`. So if any option will

From a9d41b71fbc8db21c1ad90cf377503f30bc6ec0b Mon Sep 17 00:00:00 2001
From: Ruben Bridgewater <ruben@bridgewater.de>
Date: Sat, 30 Nov 2019 11:07:12 +0100
Subject: [PATCH 06/15] util: inspect (user defined) prototype properties
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

This is only active if the `showHidden` option is truthy.

The implementation is a trade-off between accuracy and performance.
This will miss properties such as properties added to built-in data
types.

The goal is mainly to visualize prototype getters and setters such as:

class Foo {
  ownProperty = true
  get bar() {
    return 'Hello world!'
  }
}

const a = new Foo()

The `bar` property is a non-enumerable property on the prototype while
`ownProperty` will be set directly on the created instance.

The output is similar to the one of Chromium when inspecting objects
closer. The output from Firefox is difficult to compare, since it's
always a structured interactive output and was therefore not taken
into account.

PR-URL: https://github.com/nodejs/node/pull/30768
Fixes: https://github.com/nodejs/node/issues/30183
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
---
 doc/api/util.md                               |   7 +-
 lib/internal/util/inspect.js                  | 116 +++++++++++++++---
 test/parallel/test-util-inspect.js            |  80 +++++++++++-
 ...test-whatwg-encoding-custom-textdecoder.js |  17 ++-
 4 files changed, 198 insertions(+), 22 deletions(-)

diff --git a/doc/api/util.md b/doc/api/util.md
index b26d2a9558c987..e2024764676653 100644
--- a/doc/api/util.md
+++ b/doc/api/util.md
@@ -398,6 +398,10 @@ stream.write('With ES6');
 <!-- YAML
 added: v0.3.0
 changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/30768
+    description: User defined prototype properties are inspected in case
+                 `showHidden` is `true`.
   - version: v12.0.0
     pr-url: https://github.com/nodejs/node/pull/27109
     description: The `compact` options default is changed to `3` and the
@@ -458,7 +462,8 @@ changes:
 * `options` {Object}
   * `showHidden` {boolean} If `true`, `object`'s non-enumerable symbols and
     properties are included in the formatted result. [`WeakMap`][] and
-    [`WeakSet`][] entries are also included. **Default:** `false`.
+    [`WeakSet`][] entries are also included as well as user defined prototype
+    properties (excluding method properties). **Default:** `false`.
   * `depth` {number} Specifies the number of times to recurse while formatting
     `object`. This is useful for inspecting large objects. To recurse up to
     the maximum call stack size pass `Infinity` or `null`.
diff --git a/lib/internal/util/inspect.js b/lib/internal/util/inspect.js
index dd8a863a81083c..0b773adadaf58d 100644
--- a/lib/internal/util/inspect.js
+++ b/lib/internal/util/inspect.js
@@ -450,7 +450,7 @@ function getEmptyFormatArray() {
   return [];
 }
 
-function getConstructorName(obj, ctx, recurseTimes) {
+function getConstructorName(obj, ctx, recurseTimes, protoProps) {
   let firstProto;
   const tmp = obj;
   while (obj) {
@@ -458,6 +458,12 @@ function getConstructorName(obj, ctx, recurseTimes) {
     if (descriptor !== undefined &&
         typeof descriptor.value === 'function' &&
         descriptor.value.name !== '') {
+      if (protoProps !== undefined &&
+          !builtInObjects.has(descriptor.value.name)) {
+        const isProto = firstProto !== undefined;
+        addPrototypeProperties(
+          ctx, tmp, obj, recurseTimes, isProto, protoProps);
+      }
       return descriptor.value.name;
     }
 
@@ -477,7 +483,8 @@ function getConstructorName(obj, ctx, recurseTimes) {
     return `${res} <Complex prototype>`;
   }
 
-  const protoConstr = getConstructorName(firstProto, ctx, recurseTimes + 1);
+  const protoConstr = getConstructorName(
+    firstProto, ctx, recurseTimes + 1, protoProps);
 
   if (protoConstr === null) {
     return `${res} <${inspect(firstProto, {
@@ -490,6 +497,68 @@ function getConstructorName(obj, ctx, recurseTimes) {
   return `${res} <${protoConstr}>`;
 }
 
+// This function has the side effect of adding prototype properties to the
+// `output` argument (which is an array). This is intended to highlight user
+// defined prototype properties.
+function addPrototypeProperties(ctx, main, obj, recurseTimes, isProto, output) {
+  let depth = 0;
+  let keys;
+  let keySet;
+  do {
+    if (!isProto) {
+      obj = ObjectGetPrototypeOf(obj);
+      // Stop as soon as a null prototype is encountered.
+      if (obj === null) {
+        return;
+      }
+      // Stop as soon as a built-in object type is detected.
+      const descriptor = ObjectGetOwnPropertyDescriptor(obj, 'constructor');
+      if (descriptor !== undefined &&
+          typeof descriptor.value === 'function' &&
+          builtInObjects.has(descriptor.value.name)) {
+        return;
+      }
+    } else {
+      isProto = false;
+    }
+
+    if (depth === 0) {
+      keySet = new Set();
+    } else {
+      keys.forEach((key) => keySet.add(key));
+    }
+    // Get all own property names and symbols.
+    keys = ObjectGetOwnPropertyNames(obj);
+    const symbols = ObjectGetOwnPropertySymbols(obj);
+    if (symbols.length !== 0) {
+      keys.push(...symbols);
+    }
+    for (const key of keys) {
+      // Ignore the `constructor` property and keys that exist on layers above.
+      if (key === 'constructor' ||
+          ObjectPrototypeHasOwnProperty(main, key) ||
+          (depth !== 0 && keySet.has(key))) {
+        continue;
+      }
+      const desc = ObjectGetOwnPropertyDescriptor(obj, key);
+      if (typeof desc.value === 'function') {
+        continue;
+      }
+      const value = formatProperty(
+        ctx, obj, recurseTimes, key, kObjectType, desc);
+      if (ctx.colors) {
+        // Faint!
+        output.push(`\u001b[2m${value}\u001b[22m`);
+      } else {
+        output.push(value);
+      }
+    }
+  // Limit the inspection to up to three prototype layers. Using `recurseTimes`
+  // is not a good choice here, because it's as if the properties are declared
+  // on the current object from the users perspective.
+  } while (++depth !== 3);
+}
+
 function getPrefix(constructor, tag, fallback) {
   if (constructor === null) {
     if (tag !== '') {
@@ -693,8 +762,17 @@ function formatValue(ctx, value, recurseTimes, typedArray) {
 
 function formatRaw(ctx, value, recurseTimes, typedArray) {
   let keys;
+  let protoProps;
+  if (ctx.showHidden && (recurseTimes <= ctx.depth || ctx.depth === null)) {
+    protoProps = [];
+  }
+
+  const constructor = getConstructorName(value, ctx, recurseTimes, protoProps);
+  // Reset the variable to check for this later on.
+  if (protoProps !== undefined && protoProps.length === 0) {
+    protoProps = undefined;
+  }
 
-  const constructor = getConstructorName(value, ctx, recurseTimes);
   let tag = value[SymbolToStringTag];
   // Only list the tag in case it's non-enumerable / not an own property.
   // Otherwise we'd print this twice.
@@ -724,21 +802,21 @@ function formatRaw(ctx, value, recurseTimes, typedArray) {
       // Only set the constructor for non ordinary ("Array [...]") arrays.
       const prefix = getPrefix(constructor, tag, 'Array');
       braces = [`${prefix === 'Array ' ? '' : prefix}[`, ']'];
-      if (value.length === 0 && keys.length === 0)
+      if (value.length === 0 && keys.length === 0 && protoProps === undefined)
         return `${braces[0]}]`;
       extrasType = kArrayExtrasType;
       formatter = formatArray;
     } else if (isSet(value)) {
       keys = getKeys(value, ctx.showHidden);
       const prefix = getPrefix(constructor, tag, 'Set');
-      if (value.size === 0 && keys.length === 0)
+      if (value.size === 0 && keys.length === 0 && protoProps === undefined)
         return `${prefix}{}`;
       braces = [`${prefix}{`, '}'];
       formatter = formatSet;
     } else if (isMap(value)) {
       keys = getKeys(value, ctx.showHidden);
       const prefix = getPrefix(constructor, tag, 'Map');
-      if (value.size === 0 && keys.length === 0)
+      if (value.size === 0 && keys.length === 0 && protoProps === undefined)
         return `${prefix}{}`;
       braces = [`${prefix}{`, '}'];
       formatter = formatMap;
@@ -773,12 +851,12 @@ function formatRaw(ctx, value, recurseTimes, typedArray) {
       } else if (tag !== '') {
         braces[0] = `${getPrefix(constructor, tag, 'Object')}{`;
       }
-      if (keys.length === 0) {
+      if (keys.length === 0 && protoProps === undefined) {
         return `${braces[0]}}`;
       }
     } else if (typeof value === 'function') {
       base = getFunctionBase(value, constructor, tag);
-      if (keys.length === 0)
+      if (keys.length === 0 && protoProps === undefined)
         return ctx.stylize(base, 'special');
     } else if (isRegExp(value)) {
       // Make RegExps say that they are RegExps
@@ -788,8 +866,10 @@ function formatRaw(ctx, value, recurseTimes, typedArray) {
       const prefix = getPrefix(constructor, tag, 'RegExp');
       if (prefix !== 'RegExp ')
         base = `${prefix}${base}`;
-      if (keys.length === 0 || (recurseTimes > ctx.depth && ctx.depth !== null))
+      if ((keys.length === 0 && protoProps === undefined) ||
+          (recurseTimes > ctx.depth && ctx.depth !== null)) {
         return ctx.stylize(base, 'regexp');
+      }
     } else if (isDate(value)) {
       // Make dates with properties first say the date
       base = NumberIsNaN(DatePrototypeGetTime(value)) ?
@@ -798,12 +878,12 @@ function formatRaw(ctx, value, recurseTimes, typedArray) {
       const prefix = getPrefix(constructor, tag, 'Date');
       if (prefix !== 'Date ')
         base = `${prefix}${base}`;
-      if (keys.length === 0) {
+      if (keys.length === 0 && protoProps === undefined) {
         return ctx.stylize(base, 'date');
       }
     } else if (isError(value)) {
       base = formatError(value, constructor, tag, ctx);
-      if (keys.length === 0)
+      if (keys.length === 0 && protoProps === undefined)
         return base;
     } else if (isAnyArrayBuffer(value)) {
       // Fast path for ArrayBuffer and SharedArrayBuffer.
@@ -814,7 +894,7 @@ function formatRaw(ctx, value, recurseTimes, typedArray) {
       const prefix = getPrefix(constructor, tag, arrayType);
       if (typedArray === undefined) {
         formatter = formatArrayBuffer;
-      } else if (keys.length === 0) {
+      } else if (keys.length === 0 && protoProps === undefined) {
         return prefix +
               `{ byteLength: ${formatNumber(ctx.stylize, value.byteLength)} }`;
       }
@@ -838,7 +918,7 @@ function formatRaw(ctx, value, recurseTimes, typedArray) {
       formatter = formatNamespaceObject;
     } else if (isBoxedPrimitive(value)) {
       base = getBoxedBase(value, ctx, keys, constructor, tag);
-      if (keys.length === 0) {
+      if (keys.length === 0 && protoProps === undefined) {
         return base;
       }
     } else {
@@ -858,7 +938,7 @@ function formatRaw(ctx, value, recurseTimes, typedArray) {
         formatter = formatIterator;
       // Handle other regular objects again.
       } else {
-        if (keys.length === 0) {
+        if (keys.length === 0 && protoProps === undefined) {
           if (isExternal(value))
             return ctx.stylize('[External]', 'special');
           return `${getCtxStyle(value, constructor, tag)}{}`;
@@ -886,6 +966,9 @@ function formatRaw(ctx, value, recurseTimes, typedArray) {
       output.push(
         formatProperty(ctx, value, recurseTimes, keys[i], extrasType));
     }
+    if (protoProps !== undefined) {
+      output.push(...protoProps);
+    }
   } catch (err) {
     const constructorName = getCtxStyle(value, constructor, tag).slice(0, -1);
     return handleMaxCallStackSize(ctx, err, constructorName, indentationLvl);
@@ -1349,6 +1432,7 @@ function formatTypedArray(ctx, value, recurseTimes) {
   }
   if (ctx.showHidden) {
     // .buffer goes last, it's not a primitive like the others.
+    // All besides `BYTES_PER_ELEMENT` are actually getters.
     ctx.indentationLvl += 2;
     for (const key of [
       'BYTES_PER_ELEMENT',
@@ -1497,10 +1581,10 @@ function formatPromise(ctx, value, recurseTimes) {
   return output;
 }
 
-function formatProperty(ctx, value, recurseTimes, key, type) {
+function formatProperty(ctx, value, recurseTimes, key, type, desc) {
   let name, str;
   let extra = ' ';
-  const desc = ObjectGetOwnPropertyDescriptor(value, key) ||
+  desc = desc || ObjectGetOwnPropertyDescriptor(value, key) ||
     { value: value[key], enumerable: true };
   if (desc.value !== undefined) {
     const diff = (type !== kObjectType || ctx.compact !== true) ? 2 : 3;
diff --git a/test/parallel/test-util-inspect.js b/test/parallel/test-util-inspect.js
index eef34ae3143d8f..5c1ee2b8d50150 100644
--- a/test/parallel/test-util-inspect.js
+++ b/test/parallel/test-util-inspect.js
@@ -391,8 +391,24 @@ assert.strictEqual(
 {
   class CustomArray extends Array {}
   CustomArray.prototype[5] = 'foo';
+  CustomArray.prototype[49] = 'bar';
+  CustomArray.prototype.foo = true;
   const arr = new CustomArray(50);
-  assert.strictEqual(util.inspect(arr), 'CustomArray [ <50 empty items> ]');
+  arr[49] = 'I win';
+  assert.strictEqual(
+    util.inspect(arr),
+    "CustomArray [ <49 empty items>, 'I win' ]"
+  );
+  assert.strictEqual(
+    util.inspect(arr, { showHidden: true }),
+    'CustomArray [\n' +
+    '  <49 empty items>,\n' +
+    "  'I win',\n" +
+    '  [length]: 50,\n' +
+    "  '5': 'foo',\n" +
+    '  foo: true\n' +
+    ']'
+  );
 }
 
 // Array with extra properties.
@@ -2585,3 +2601,65 @@ assert.strictEqual(
       throw err;
   }
 }
+
+// Inspect prototype properties.
+{
+  class Foo extends Map {
+    prop = false;
+    prop2 = true;
+    get abc() {
+      return true;
+    }
+    get def() {
+      return false;
+    }
+    set def(v) {}
+    get xyz() {
+      return 'Should be ignored';
+    }
+    func(a) {}
+    [util.inspect.custom]() {
+      return this;
+    }
+  }
+
+  class Bar extends Foo {
+    abc = true;
+    prop = true;
+    get xyz() {
+      return 'YES!';
+    }
+    [util.inspect.custom]() {
+      return this;
+    }
+  }
+
+  const bar = new Bar();
+
+  assert.strictEqual(
+    inspect(bar),
+    'Bar [Map] { prop: true, prop2: true, abc: true }'
+  );
+  assert.strictEqual(
+    inspect(bar, { showHidden: true, getters: true, colors: false }),
+    'Bar [Map] {\n' +
+    '  [size]: 0,\n' +
+    '  prop: true,\n' +
+    '  prop2: true,\n' +
+    '  abc: true,\n' +
+    "  [xyz]: [Getter: 'YES!'],\n" +
+    '  [def]: [Getter/Setter: false]\n' +
+    '}'
+  );
+  assert.strictEqual(
+    inspect(bar, { showHidden: true, getters: false, colors: true }),
+    'Bar [Map] {\n' +
+    '  [size]: \x1B[33m0\x1B[39m,\n' +
+    '  prop: \x1B[33mtrue\x1B[39m,\n' +
+    '  prop2: \x1B[33mtrue\x1B[39m,\n' +
+    '  abc: \x1B[33mtrue\x1B[39m,\n' +
+    '  \x1B[2m[xyz]: \x1B[36m[Getter]\x1B[39m\x1B[22m,\n' +
+    '  \x1B[2m[def]: \x1B[36m[Getter/Setter]\x1B[39m\x1B[22m\n' +
+    '}'
+  );
+}
diff --git a/test/parallel/test-whatwg-encoding-custom-textdecoder.js b/test/parallel/test-whatwg-encoding-custom-textdecoder.js
index e3779a945de4a9..c475d94dc7acd1 100644
--- a/test/parallel/test-whatwg-encoding-custom-textdecoder.js
+++ b/test/parallel/test-whatwg-encoding-custom-textdecoder.js
@@ -113,10 +113,19 @@ if (common.hasIntl) {
   } else {
     assert.strictEqual(
       util.inspect(dec, { showHidden: true }),
-      "TextDecoder {\n  encoding: 'utf-8',\n  fatal: false,\n  " +
-      'ignoreBOM: true,\n  [Symbol(flags)]: 4,\n  [Symbol(handle)]: ' +
-      "StringDecoder {\n    encoding: 'utf8',\n    " +
-      '[Symbol(kNativeDecoder)]: <Buffer 00 00 00 00 00 00 01>\n  }\n}'
+      'TextDecoder {\n' +
+      "  encoding: 'utf-8',\n" +
+      '  fatal: false,\n' +
+      '  ignoreBOM: true,\n' +
+      '  [Symbol(flags)]: 4,\n' +
+      '  [Symbol(handle)]: StringDecoder {\n' +
+      "    encoding: 'utf8',\n" +
+      '    [Symbol(kNativeDecoder)]: <Buffer 00 00 00 00 00 00 01>,\n' +
+      '    lastChar: [Getter],\n' +
+      '    lastNeed: [Getter],\n' +
+      '    lastTotal: [Getter]\n' +
+      '  }\n' +
+      '}'
     );
   }
 }

From 0b08d329afd73d734b7ad4923d5f4705234399d3 Mon Sep 17 00:00:00 2001
From: Ruben Bridgewater <ruben@bridgewater.de>
Date: Sat, 2 Nov 2019 18:50:09 +0100
Subject: [PATCH 07/15] util: refactor inspect code for constistency
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

This removes the special handling to inspect iterable objects with
a null prototype. It is now handled together with the regular
prototype.

PR-URL: https://github.com/nodejs/node/pull/30225
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
---
 lib/internal/util/inspect.js       | 159 ++++++++++++-----------------
 test/parallel/test-util-inspect.js |  13 +++
 2 files changed, 80 insertions(+), 92 deletions(-)

diff --git a/lib/internal/util/inspect.js b/lib/internal/util/inspect.js
index 0b773adadaf58d..4133eb5341ea11 100644
--- a/lib/internal/util/inspect.js
+++ b/lib/internal/util/inspect.js
@@ -11,6 +11,7 @@ const {
   ErrorPrototypeToString,
   JSONStringify,
   Map,
+  MapPrototype,
   MapPrototypeEntries,
   MathFloor,
   MathMax,
@@ -22,10 +23,8 @@ const {
   NumberPrototypeValueOf,
   ObjectAssign,
   ObjectCreate,
-  ObjectDefineProperties,
   ObjectDefineProperty,
   ObjectGetOwnPropertyDescriptor,
-  ObjectGetOwnPropertyDescriptors,
   ObjectGetOwnPropertyNames,
   ObjectGetOwnPropertySymbols,
   ObjectGetPrototypeOf,
@@ -36,6 +35,7 @@ const {
   ObjectSeal,
   RegExpPrototypeToString,
   Set,
+  SetPrototype,
   SetPrototypeValues,
   StringPrototypeValueOf,
   SymbolPrototypeToString,
@@ -115,6 +115,11 @@ const assert = require('internal/assert');
 
 const { NativeModule } = require('internal/bootstrap/loaders');
 
+const setSizeGetter = uncurryThis(
+  ObjectGetOwnPropertyDescriptor(SetPrototype, 'size').get);
+const mapSizeGetter = uncurryThis(
+  ObjectGetOwnPropertyDescriptor(MapPrototype, 'size').get);
+
 let hexSlice;
 
 const builtInObjects = new Set(
@@ -648,51 +653,6 @@ function findTypedConstructor(value) {
   }
 }
 
-let lazyNullPrototypeCache;
-// Creates a subclass and name
-// the constructor as `${clazz} : null prototype`
-function clazzWithNullPrototype(clazz, name) {
-  if (lazyNullPrototypeCache === undefined) {
-    lazyNullPrototypeCache = new Map();
-  } else {
-    const cachedClass = lazyNullPrototypeCache.get(clazz);
-    if (cachedClass !== undefined) {
-      return cachedClass;
-    }
-  }
-  class NullPrototype extends clazz {
-    get [SymbolToStringTag]() {
-      return '';
-    }
-  }
-  ObjectDefineProperty(NullPrototype.prototype.constructor, 'name',
-                       { value: `[${name}: null prototype]` });
-  lazyNullPrototypeCache.set(clazz, NullPrototype);
-  return NullPrototype;
-}
-
-function noPrototypeIterator(ctx, value, recurseTimes) {
-  let newVal;
-  if (isSet(value)) {
-    const clazz = clazzWithNullPrototype(Set, 'Set');
-    newVal = new clazz(SetPrototypeValues(value));
-  } else if (isMap(value)) {
-    const clazz = clazzWithNullPrototype(Map, 'Map');
-    newVal = new clazz(MapPrototypeEntries(value));
-  } else if (ArrayIsArray(value)) {
-    const clazz = clazzWithNullPrototype(Array, 'Array');
-    newVal = new clazz(value.length);
-  } else if (isTypedArray(value)) {
-    const constructor = findTypedConstructor(value);
-    const clazz = clazzWithNullPrototype(constructor, constructor.name);
-    newVal = new clazz(value);
-  }
-  if (newVal !== undefined) {
-    ObjectDefineProperties(newVal, ObjectGetOwnPropertyDescriptors(value));
-    return formatRaw(ctx, newVal, recurseTimes);
-  }
-}
-
 // Note: using `formatValue` directly requires the indentation level to be
 // corrected by setting `ctx.indentationLvL += diff` and then to decrease the
 // value afterwards again.
@@ -795,7 +755,9 @@ function formatRaw(ctx, value, recurseTimes, typedArray) {
   let extrasType = kObjectType;
 
   // Iterators and the rest are split to reduce checks.
-  if (value[SymbolIterator]) {
+  // We have to check all values in case the constructor is set to null.
+  // Otherwise it would not possible to identify all types properly.
+  if (value[SymbolIterator] || constructor === null) {
     noIterator = false;
     if (ArrayIsArray(value)) {
       keys = getOwnNonIndexProperties(value, filter);
@@ -807,37 +769,66 @@ function formatRaw(ctx, value, recurseTimes, typedArray) {
       extrasType = kArrayExtrasType;
       formatter = formatArray;
     } else if (isSet(value)) {
+      const size = setSizeGetter(value);
       keys = getKeys(value, ctx.showHidden);
-      const prefix = getPrefix(constructor, tag, 'Set');
-      if (value.size === 0 && keys.length === 0 && protoProps === undefined)
+      let prefix = '';
+      if (constructor !== null) {
+        if (constructor === tag)
+          tag = '';
+        prefix = getPrefix(`${constructor}`, tag, '');
+        formatter = formatSet.bind(null, value, size);
+      } else {
+        prefix = getPrefix(constructor, tag, 'Set');
+        formatter = formatSet.bind(null, SetPrototypeValues(value), size);
+      }
+      if (size === 0 && keys.length === 0 && protoProps === undefined)
         return `${prefix}{}`;
       braces = [`${prefix}{`, '}'];
-      formatter = formatSet;
     } else if (isMap(value)) {
+      const size = mapSizeGetter(value);
       keys = getKeys(value, ctx.showHidden);
-      const prefix = getPrefix(constructor, tag, 'Map');
-      if (value.size === 0 && keys.length === 0 && protoProps === undefined)
+      let prefix = '';
+      if (constructor !== null) {
+        if (constructor === tag)
+          tag = '';
+        prefix = getPrefix(`${constructor}`, tag, '');
+        formatter = formatMap.bind(null, value, size);
+      } else {
+        prefix = getPrefix(constructor, tag, 'Map');
+        formatter = formatMap.bind(null, MapPrototypeEntries(value), size);
+      }
+      if (size === 0 && keys.length === 0 && protoProps === undefined)
         return `${prefix}{}`;
       braces = [`${prefix}{`, '}'];
-      formatter = formatMap;
     } else if (isTypedArray(value)) {
       keys = getOwnNonIndexProperties(value, filter);
-      const prefix = constructor !== null ?
-        getPrefix(constructor, tag) :
-        getPrefix(constructor, tag, findTypedConstructor(value).name);
+      let bound = value;
+      let prefix = '';
+      if (constructor === null) {
+        const constr = findTypedConstructor(value);
+        prefix = getPrefix(constructor, tag, constr.name);
+        // Reconstruct the array information.
+        bound = new constr(value);
+      } else {
+        prefix = getPrefix(constructor, tag);
+      }
       braces = [`${prefix}[`, ']'];
       if (value.length === 0 && keys.length === 0 && !ctx.showHidden)
         return `${braces[0]}]`;
-      formatter = formatTypedArray;
+      // Special handle the value. The original value is required below. The
+      // bound function is required to reconstruct missing information.
+      formatter = formatTypedArray.bind(null, bound);
       extrasType = kArrayExtrasType;
     } else if (isMapIterator(value)) {
       keys = getKeys(value, ctx.showHidden);
       braces = getIteratorBraces('Map', tag);
-      formatter = formatIterator;
+      // Add braces to the formatter parameters.
+      formatter = formatIterator.bind(null, braces);
     } else if (isSetIterator(value)) {
       keys = getKeys(value, ctx.showHidden);
       braces = getIteratorBraces('Set', tag);
-      formatter = formatIterator;
+      // Add braces to the formatter parameters.
+      formatter = formatIterator.bind(null, braces);
     } else {
       noIterator = true;
     }
@@ -915,36 +906,20 @@ function formatRaw(ctx, value, recurseTimes, typedArray) {
       formatter = ctx.showHidden ? formatWeakMap : formatWeakCollection;
     } else if (isModuleNamespaceObject(value)) {
       braces[0] = `[${tag}] {`;
-      formatter = formatNamespaceObject;
+      // Special handle keys for namespace objects.
+      formatter = formatNamespaceObject.bind(null, keys);
     } else if (isBoxedPrimitive(value)) {
       base = getBoxedBase(value, ctx, keys, constructor, tag);
       if (keys.length === 0 && protoProps === undefined) {
         return base;
       }
     } else {
-      // The input prototype got manipulated. Special handle these. We have to
-      // rebuild the information so we are able to display everything.
-      if (constructor === null) {
-        const specialIterator = noPrototypeIterator(ctx, value, recurseTimes);
-        if (specialIterator) {
-          return specialIterator;
-        }
-      }
-      if (isMapIterator(value)) {
-        braces = getIteratorBraces('Map', tag);
-        formatter = formatIterator;
-      } else if (isSetIterator(value)) {
-        braces = getIteratorBraces('Set', tag);
-        formatter = formatIterator;
-      // Handle other regular objects again.
-      } else {
-        if (keys.length === 0 && protoProps === undefined) {
-          if (isExternal(value))
-            return ctx.stylize('[External]', 'special');
-          return `${getCtxStyle(value, constructor, tag)}{}`;
-        }
-        braces[0] = `${getCtxStyle(value, constructor, tag)}{`;
+      if (keys.length === 0 && protoProps === undefined) {
+        if (isExternal(value))
+          return ctx.stylize('[External]', 'special');
+        return `${getCtxStyle(value, constructor, tag)}{}`;
       }
+      braces[0] = `${getCtxStyle(value, constructor, tag)}{`;
     }
   }
 
@@ -961,7 +936,7 @@ function formatRaw(ctx, value, recurseTimes, typedArray) {
   let output;
   const indentationLvl = ctx.indentationLvl;
   try {
-    output = formatter(ctx, value, recurseTimes, keys, braces);
+    output = formatter(ctx, value, recurseTimes);
     for (i = 0; i < keys.length; i++) {
       output.push(
         formatProperty(ctx, value, recurseTimes, keys[i], extrasType));
@@ -1316,7 +1291,7 @@ function formatPrimitive(fn, value, ctx) {
   return fn(SymbolPrototypeToString(value), 'symbol');
 }
 
-function formatNamespaceObject(ctx, value, recurseTimes, keys) {
+function formatNamespaceObject(keys, ctx, value, recurseTimes) {
   const output = new Array(keys.length);
   for (let i = 0; i < keys.length; i++) {
     try {
@@ -1418,7 +1393,7 @@ function formatArray(ctx, value, recurseTimes) {
   return output;
 }
 
-function formatTypedArray(ctx, value, recurseTimes) {
+function formatTypedArray(value, ctx, ignored, recurseTimes) {
   const maxLength = MathMin(MathMax(0, ctx.maxArrayLength), value.length);
   const remaining = value.length - maxLength;
   const output = new Array(maxLength);
@@ -1449,7 +1424,7 @@ function formatTypedArray(ctx, value, recurseTimes) {
   return output;
 }
 
-function formatSet(ctx, value, recurseTimes) {
+function formatSet(value, size, ctx, ignored, recurseTimes) {
   const output = [];
   ctx.indentationLvl += 2;
   for (const v of value) {
@@ -1460,11 +1435,11 @@ function formatSet(ctx, value, recurseTimes) {
   // arrays. For consistency's sake, do the same for `size`, even though this
   // property isn't selected by ObjectGetOwnPropertyNames().
   if (ctx.showHidden)
-    output.push(`[size]: ${ctx.stylize(`${value.size}`, 'number')}`);
+    output.push(`[size]: ${ctx.stylize(`${size}`, 'number')}`);
   return output;
 }
 
-function formatMap(ctx, value, recurseTimes) {
+function formatMap(value, size, ctx, ignored, recurseTimes) {
   const output = [];
   ctx.indentationLvl += 2;
   for (const [k, v] of value) {
@@ -1474,7 +1449,7 @@ function formatMap(ctx, value, recurseTimes) {
   ctx.indentationLvl -= 2;
   // See comment in formatSet
   if (ctx.showHidden)
-    output.push(`[size]: ${ctx.stylize(`${value.size}`, 'number')}`);
+    output.push(`[size]: ${ctx.stylize(`${size}`, 'number')}`);
   return output;
 }
 
@@ -1552,7 +1527,7 @@ function formatWeakMap(ctx, value, recurseTimes) {
   return formatMapIterInner(ctx, recurseTimes, entries, kWeak);
 }
 
-function formatIterator(ctx, value, recurseTimes, keys, braces) {
+function formatIterator(braces, ctx, value, recurseTimes) {
   const [entries, isKeyValue] = previewEntries(value, true);
   if (isKeyValue) {
     // Mark entry iterators as such.
@@ -1587,7 +1562,7 @@ function formatProperty(ctx, value, recurseTimes, key, type, desc) {
   desc = desc || ObjectGetOwnPropertyDescriptor(value, key) ||
     { value: value[key], enumerable: true };
   if (desc.value !== undefined) {
-    const diff = (type !== kObjectType || ctx.compact !== true) ? 2 : 3;
+    const diff = (ctx.compact !== true || type !== kObjectType) ? 2 : 3;
     ctx.indentationLvl += diff;
     str = formatValue(ctx, desc.value, recurseTimes);
     if (diff === 3) {
diff --git a/test/parallel/test-util-inspect.js b/test/parallel/test-util-inspect.js
index 5c1ee2b8d50150..d307d0f2dd35b8 100644
--- a/test/parallel/test-util-inspect.js
+++ b/test/parallel/test-util-inspect.js
@@ -2216,6 +2216,19 @@ assert.strictEqual(
     configurable: true
   });
   assert.strictEqual(util.inspect(obj), '[Set: null prototype] { 1, 2 }');
+  Object.defineProperty(obj, Symbol.iterator, {
+    value: true,
+    configurable: true
+  });
+  Object.defineProperty(obj, 'size', {
+    value: NaN,
+    configurable: true,
+    enumerable: true
+  });
+  assert.strictEqual(
+    util.inspect(obj),
+    '[Set: null prototype] { 1, 2, size: NaN }'
+  );
 }
 
 // Check the getter option.

From 1e2c53559f6d602467faf843c6ef8662f432e043 Mon Sep 17 00:00:00 2001
From: Ruben Bridgewater <ruben@bridgewater.de>
Date: Thu, 19 Dec 2019 16:12:30 +0100
Subject: [PATCH 08/15] util: add (typed) array length to the default output
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Align the inspect output with the one used in the Chrome dev tools.
A recent survey outlined that most users prefer to see the number
of set and map entries. This should count as well for array sizes.
The size is only added to regular arrays in case the constructor is
not the default constructor.
Typed arrays always indicate their size.

PR-URL: https://github.com/nodejs/node/pull/31027
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Anto Aravinth <anto.aravinth.cse@gmail.com>
---
 lib/internal/util/inspect.js               | 57 ++++++++----------
 test/parallel/test-assert-deep.js          |  8 +--
 test/parallel/test-buffer-inspect.js       |  2 +-
 test/parallel/test-fs-read-empty-buffer.js |  6 +-
 test/parallel/test-util-format.js          |  2 +-
 test/parallel/test-util-inspect.js         | 67 +++++++++++-----------
 6 files changed, 69 insertions(+), 73 deletions(-)

diff --git a/lib/internal/util/inspect.js b/lib/internal/util/inspect.js
index 4133eb5341ea11..04b76077bba313 100644
--- a/lib/internal/util/inspect.js
+++ b/lib/internal/util/inspect.js
@@ -119,6 +119,9 @@ const setSizeGetter = uncurryThis(
   ObjectGetOwnPropertyDescriptor(SetPrototype, 'size').get);
 const mapSizeGetter = uncurryThis(
   ObjectGetOwnPropertyDescriptor(MapPrototype, 'size').get);
+const typedArraySizeGetter = uncurryThis(
+  ObjectGetOwnPropertyDescriptor(
+    ObjectGetPrototypeOf(Uint8Array.prototype), 'length').get);
 
 let hexSlice;
 
@@ -564,18 +567,18 @@ function addPrototypeProperties(ctx, main, obj, recurseTimes, isProto, output) {
   } while (++depth !== 3);
 }
 
-function getPrefix(constructor, tag, fallback) {
+function getPrefix(constructor, tag, fallback, size = '') {
   if (constructor === null) {
     if (tag !== '') {
-      return `[${fallback}: null prototype] [${tag}] `;
+      return `[${fallback}${size}: null prototype] [${tag}] `;
     }
-    return `[${fallback}: null prototype] `;
+    return `[${fallback}${size}: null prototype] `;
   }
 
   if (tag !== '' && constructor !== tag) {
-    return `${constructor} [${tag}] `;
+    return `${constructor}${size} [${tag}] `;
   }
-  return `${constructor} `;
+  return `${constructor}${size} `;
 }
 
 // Look up the keys of the object.
@@ -760,58 +763,48 @@ function formatRaw(ctx, value, recurseTimes, typedArray) {
   if (value[SymbolIterator] || constructor === null) {
     noIterator = false;
     if (ArrayIsArray(value)) {
-      keys = getOwnNonIndexProperties(value, filter);
       // Only set the constructor for non ordinary ("Array [...]") arrays.
-      const prefix = getPrefix(constructor, tag, 'Array');
-      braces = [`${prefix === 'Array ' ? '' : prefix}[`, ']'];
+      const prefix = (constructor !== 'Array' || tag !== '') ?
+        getPrefix(constructor, tag, 'Array', `(${value.length})`) :
+        '';
+      keys = getOwnNonIndexProperties(value, filter);
+      braces = [`${prefix}[`, ']'];
       if (value.length === 0 && keys.length === 0 && protoProps === undefined)
         return `${braces[0]}]`;
       extrasType = kArrayExtrasType;
       formatter = formatArray;
     } else if (isSet(value)) {
       const size = setSizeGetter(value);
+      const prefix = getPrefix(constructor, tag, 'Set');
       keys = getKeys(value, ctx.showHidden);
-      let prefix = '';
-      if (constructor !== null) {
-        if (constructor === tag)
-          tag = '';
-        prefix = getPrefix(`${constructor}`, tag, '');
-        formatter = formatSet.bind(null, value, size);
-      } else {
-        prefix = getPrefix(constructor, tag, 'Set');
-        formatter = formatSet.bind(null, SetPrototypeValues(value), size);
-      }
+      formatter = constructor !== null ?
+        formatSet.bind(null, value, size) :
+        formatSet.bind(null, SetPrototypeValues(value), size);
       if (size === 0 && keys.length === 0 && protoProps === undefined)
         return `${prefix}{}`;
       braces = [`${prefix}{`, '}'];
     } else if (isMap(value)) {
       const size = mapSizeGetter(value);
+      const prefix = getPrefix(constructor, tag, 'Map');
       keys = getKeys(value, ctx.showHidden);
-      let prefix = '';
-      if (constructor !== null) {
-        if (constructor === tag)
-          tag = '';
-        prefix = getPrefix(`${constructor}`, tag, '');
-        formatter = formatMap.bind(null, value, size);
-      } else {
-        prefix = getPrefix(constructor, tag, 'Map');
-        formatter = formatMap.bind(null, MapPrototypeEntries(value), size);
-      }
+      formatter = constructor !== null ?
+        formatMap.bind(null, value, size) :
+        formatMap.bind(null, MapPrototypeEntries(value), size);
       if (size === 0 && keys.length === 0 && protoProps === undefined)
         return `${prefix}{}`;
       braces = [`${prefix}{`, '}'];
     } else if (isTypedArray(value)) {
       keys = getOwnNonIndexProperties(value, filter);
       let bound = value;
-      let prefix = '';
+      let fallback = '';
       if (constructor === null) {
         const constr = findTypedConstructor(value);
-        prefix = getPrefix(constructor, tag, constr.name);
+        fallback = constr.name;
         // Reconstruct the array information.
         bound = new constr(value);
-      } else {
-        prefix = getPrefix(constructor, tag);
       }
+      const size = typedArraySizeGetter(value);
+      const prefix = getPrefix(constructor, tag, fallback, `(${size})`);
       braces = [`${prefix}[`, ']'];
       if (value.length === 0 && keys.length === 0 && !ctx.showHidden)
         return `${braces[0]}]`;
diff --git a/test/parallel/test-assert-deep.js b/test/parallel/test-assert-deep.js
index fcc3765105f57c..914f9ecd1a3823 100644
--- a/test/parallel/test-assert-deep.js
+++ b/test/parallel/test-assert-deep.js
@@ -51,8 +51,8 @@ assert.throws(
   {
     code: 'ERR_ASSERTION',
     message: `${defaultMsgStartFull} ... Lines skipped\n\n` +
-             '+ Uint8Array [\n' +
-             '- Buffer [Uint8Array] [\n    120,\n...\n    122,\n    10\n  ]'
+             '+ Uint8Array(4) [\n' +
+             '- Buffer(4) [Uint8Array] [\n    120,\n...\n    122,\n    10\n  ]'
   }
 );
 assert.deepEqual(arr, buf);
@@ -66,7 +66,7 @@ assert.deepEqual(arr, buf);
     {
       code: 'ERR_ASSERTION',
       message: `${defaultMsgStartFull}\n\n` +
-               '  Buffer [Uint8Array] [\n' +
+               '  Buffer(4) [Uint8Array] [\n' +
                '    120,\n' +
                '    121,\n' +
                '    122,\n' +
@@ -86,7 +86,7 @@ assert.deepEqual(arr, buf);
     {
       code: 'ERR_ASSERTION',
       message: `${defaultMsgStartFull}\n\n` +
-               '  Uint8Array [\n' +
+               '  Uint8Array(4) [\n' +
                '    120,\n' +
                '    121,\n' +
                '    122,\n' +
diff --git a/test/parallel/test-buffer-inspect.js b/test/parallel/test-buffer-inspect.js
index d6ecf6b7fc5c38..1e8212e876344a 100644
--- a/test/parallel/test-buffer-inspect.js
+++ b/test/parallel/test-buffer-inspect.js
@@ -58,7 +58,7 @@ b.inspect = undefined;
 b.prop = new Uint8Array(0);
 assert.strictEqual(
   util.inspect(b),
-  '<Buffer 31 32, inspect: undefined, prop: Uint8Array []>'
+  '<Buffer 31 32, inspect: undefined, prop: Uint8Array(0) []>'
 );
 
 b = Buffer.alloc(0);
diff --git a/test/parallel/test-fs-read-empty-buffer.js b/test/parallel/test-fs-read-empty-buffer.js
index 2f44b9a1843a6b..8ca08448182d4a 100644
--- a/test/parallel/test-fs-read-empty-buffer.js
+++ b/test/parallel/test-fs-read-empty-buffer.js
@@ -15,7 +15,7 @@ assert.throws(
   {
     code: 'ERR_INVALID_ARG_VALUE',
     message: 'The argument \'buffer\' is empty and cannot be written. ' +
-    'Received Uint8Array []'
+    'Received Uint8Array(0) []'
   }
 );
 
@@ -24,7 +24,7 @@ assert.throws(
   {
     code: 'ERR_INVALID_ARG_VALUE',
     message: 'The argument \'buffer\' is empty and cannot be written. ' +
-    'Received Uint8Array []'
+    'Received Uint8Array(0) []'
   }
 );
 
@@ -35,7 +35,7 @@ assert.throws(
     {
       code: 'ERR_INVALID_ARG_VALUE',
       message: 'The argument \'buffer\' is empty and cannot be written. ' +
-               'Received Uint8Array []'
+               'Received Uint8Array(0) []'
     }
   );
 })();
diff --git a/test/parallel/test-util-format.js b/test/parallel/test-util-format.js
index 9b30ca72a07816..d39df436146820 100644
--- a/test/parallel/test-util-format.js
+++ b/test/parallel/test-util-format.js
@@ -158,7 +158,7 @@ assert.strictEqual(util.format('%s', () => 5), '() => 5');
   class Foobar extends Array { aaa = true; }
   assert.strictEqual(
     util.format('%s', new Foobar(5)),
-    'Foobar [ <5 empty items>, aaa: true ]'
+    'Foobar(5) [ <5 empty items>, aaa: true ]'
   );
 
   // Subclassing:
diff --git a/test/parallel/test-util-inspect.js b/test/parallel/test-util-inspect.js
index d307d0f2dd35b8..bba497c89683c9 100644
--- a/test/parallel/test-util-inspect.js
+++ b/test/parallel/test-util-inspect.js
@@ -126,7 +126,7 @@ assert.strictEqual(util.inspect({ 'a': { 'b': { 'c': 2 } } }, false, 1),
                    '{ a: { b: [Object] } }');
 assert.strictEqual(util.inspect({ 'a': { 'b': ['c'] } }, false, 1),
                    '{ a: { b: [Array] } }');
-assert.strictEqual(util.inspect(new Uint8Array(0)), 'Uint8Array []');
+assert.strictEqual(util.inspect(new Uint8Array(0)), 'Uint8Array(0) []');
 assert(inspect(new Uint8Array(0), { showHidden: true }).includes('[buffer]'));
 assert.strictEqual(
   util.inspect(
@@ -263,7 +263,7 @@ assert(!/Object/.test(
   array[1] = 97;
   assert.strictEqual(
     util.inspect(array, { showHidden: true }),
-    `${constructor.name} [\n` +
+    `${constructor.name}(${length}) [\n` +
       '  65,\n' +
       '  97,\n' +
       `  [BYTES_PER_ELEMENT]: ${constructor.BYTES_PER_ELEMENT},\n` +
@@ -273,7 +273,7 @@ assert(!/Object/.test(
       `  [buffer]: ArrayBuffer { byteLength: ${byteLength} }\n]`);
   assert.strictEqual(
     util.inspect(array, false),
-    `${constructor.name} [ 65, 97 ]`
+    `${constructor.name}(${length}) [ 65, 97 ]`
   );
 });
 
@@ -297,7 +297,7 @@ assert(!/Object/.test(
   array[1] = 97;
   assert.strictEqual(
     util.inspect(array, true),
-    `${constructor.name} [\n` +
+    `${constructor.name}(${length}) [\n` +
       '  65,\n' +
       '  97,\n' +
       `  [BYTES_PER_ELEMENT]: ${constructor.BYTES_PER_ELEMENT},\n` +
@@ -307,7 +307,7 @@ assert(!/Object/.test(
       `  [buffer]: ArrayBuffer { byteLength: ${byteLength} }\n]`);
   assert.strictEqual(
     util.inspect(array, false),
-    `${constructor.name} [ 65, 97 ]`
+    `${constructor.name}(${length}) [ 65, 97 ]`
   );
 });
 
@@ -397,11 +397,11 @@ assert.strictEqual(
   arr[49] = 'I win';
   assert.strictEqual(
     util.inspect(arr),
-    "CustomArray [ <49 empty items>, 'I win' ]"
+    "CustomArray(50) [ <49 empty items>, 'I win' ]"
   );
   assert.strictEqual(
     util.inspect(arr, { showHidden: true }),
-    'CustomArray [\n' +
+    'CustomArray(50) [\n' +
     '  <49 empty items>,\n' +
     "  'I win',\n" +
     '  [length]: 50,\n' +
@@ -1291,7 +1291,7 @@ if (typeof Symbol !== 'undefined') {
   assert.strictEqual(util.inspect(x),
                      'ObjectSubclass { foo: 42 }');
   assert.strictEqual(util.inspect(new ArraySubclass(1, 2, 3)),
-                     'ArraySubclass [ 1, 2, 3 ]');
+                     'ArraySubclass(3) [ 1, 2, 3 ]');
   assert.strictEqual(util.inspect(new SetSubclass([1, 2, 3])),
                      'SetSubclass [Set] { 1, 2, 3 }');
   assert.strictEqual(util.inspect(new MapSubclass([['foo', 42]])),
@@ -1387,7 +1387,7 @@ if (typeof Symbol !== 'undefined') {
   assert(util.inspect(x).endsWith('1 more item\n]'));
   assert(!util.inspect(x, { maxArrayLength: 101 }).includes('1 more item'));
   assert.strictEqual(util.inspect(x, { maxArrayLength: 0 }),
-                     'Uint8Array [ ... 101 more items ]');
+                     'Uint8Array(101) [ ... 101 more items ]');
   assert(!util.inspect(x, { maxArrayLength: null }).includes('1 more item'));
   assert(util.inspect(x, { maxArrayLength: Infinity }).endsWith(' 0, 0\n]'));
 }
@@ -1672,7 +1672,7 @@ util.inspect(process);
     '      ],',
     '      [length]: 1',
     '    ]',
-    '  } => Uint8Array [',
+    '  } => Uint8Array(0) [',
     '    [BYTES_PER_ELEMENT]: 1,',
     '    [length]: 0,',
     '    [byteLength]: 0,',
@@ -1689,7 +1689,7 @@ util.inspect(process);
     '      [length]: 2',
     '    ]',
     '  } => [Map Iterator] {',
-    '    Uint8Array [',
+    '    Uint8Array(0) [',
     '      [BYTES_PER_ELEMENT]: 1,',
     '      [length]: 0,',
     '      [byteLength]: 0,',
@@ -1720,7 +1720,7 @@ util.inspect(process);
     '      ],',
     '      [length]: 1',
     '    ]',
-    '  } => Uint8Array [',
+    '  } => Uint8Array(0) [',
     '    [BYTES_PER_ELEMENT]: 1,',
     '    [length]: 0,',
     '    [byteLength]: 0,',
@@ -1728,7 +1728,7 @@ util.inspect(process);
     '    [buffer]: ArrayBuffer { byteLength: 0, foo: true }',
     '  ],',
     '  [Set Iterator] { [ 1, 2, [length]: 2 ] } => [Map Iterator] {',
-    '    Uint8Array [',
+    '    Uint8Array(0) [',
     '      [BYTES_PER_ELEMENT]: 1,',
     '      [length]: 0,',
     '      [byteLength]: 0,',
@@ -1756,7 +1756,7 @@ util.inspect(process);
     '            [length]: 2 ],',
     '          [size]: 1 },',
     '        [length]: 2 ],',
-    '      [length]: 1 ] } => Uint8Array [',
+    '      [length]: 1 ] } => Uint8Array(0) [',
     '    [BYTES_PER_ELEMENT]: 1,',
     '    [length]: 0,',
     '    [byteLength]: 0,',
@@ -1768,7 +1768,7 @@ util.inspect(process);
     '    [ 1,',
     '      2,',
     '      [length]: 2 ] } => [Map Iterator] {',
-    '    Uint8Array [',
+    '    Uint8Array(0) [',
     '      [BYTES_PER_ELEMENT]: 1,',
     '      [length]: 0,',
     '      [byteLength]: 0,',
@@ -1946,7 +1946,7 @@ assert.strictEqual(util.inspect('"\'${a}'), "'\"\\'${a}'");
   [new Set([1, 2]).entries(), '[Set Entries] { [ 1, 1 ], [ 2, 2 ] }'],
   [new Map([[1, 2]]).keys(), '[Map Iterator] { 1 }'],
   [new Date(2000), '1970-01-01T00:00:02.000Z'],
-  [new Uint8Array(2), 'Uint8Array [ 0, 0 ]'],
+  [new Uint8Array(2), 'Uint8Array(2) [ 0, 0 ]'],
   [new Promise((resolve) => setTimeout(resolve, 10)), 'Promise { <pending> }'],
   [new WeakSet(), 'WeakSet { <items unknown> }'],
   [new WeakMap(), 'WeakMap { <items unknown> }'],
@@ -1972,23 +1972,23 @@ assert.strictEqual(util.inspect('"\'${a}'), "'\"\\'${a}'");
 
 // Verify that having no prototype still produces nice results.
 [
-  [[1, 3, 4], '[Array: null prototype] [ 1, 3, 4 ]'],
+  [[1, 3, 4], '[Array(3): null prototype] [ 1, 3, 4 ]'],
   [new Set([1, 2]), '[Set: null prototype] { 1, 2 }'],
   [new Map([[1, 2]]), '[Map: null prototype] { 1 => 2 }'],
   [new Promise((resolve) => setTimeout(resolve, 10)),
    '[Promise: null prototype] { <pending> }'],
   [new WeakSet(), '[WeakSet: null prototype] { <items unknown> }'],
   [new WeakMap(), '[WeakMap: null prototype] { <items unknown> }'],
-  [new Uint8Array(2), '[Uint8Array: null prototype] [ 0, 0 ]'],
-  [new Uint16Array(2), '[Uint16Array: null prototype] [ 0, 0 ]'],
-  [new Uint32Array(2), '[Uint32Array: null prototype] [ 0, 0 ]'],
-  [new Int8Array(2), '[Int8Array: null prototype] [ 0, 0 ]'],
-  [new Int16Array(2), '[Int16Array: null prototype] [ 0, 0 ]'],
-  [new Int32Array(2), '[Int32Array: null prototype] [ 0, 0 ]'],
-  [new Float32Array(2), '[Float32Array: null prototype] [ 0, 0 ]'],
-  [new Float64Array(2), '[Float64Array: null prototype] [ 0, 0 ]'],
-  [new BigInt64Array(2), '[BigInt64Array: null prototype] [ 0n, 0n ]'],
-  [new BigUint64Array(2), '[BigUint64Array: null prototype] [ 0n, 0n ]'],
+  [new Uint8Array(2), '[Uint8Array(2): null prototype] [ 0, 0 ]'],
+  [new Uint16Array(2), '[Uint16Array(2): null prototype] [ 0, 0 ]'],
+  [new Uint32Array(2), '[Uint32Array(2): null prototype] [ 0, 0 ]'],
+  [new Int8Array(2), '[Int8Array(2): null prototype] [ 0, 0 ]'],
+  [new Int16Array(2), '[Int16Array(2): null prototype] [ 0, 0 ]'],
+  [new Int32Array(2), '[Int32Array(2): null prototype] [ 0, 0 ]'],
+  [new Float32Array(2), '[Float32Array(2): null prototype] [ 0, 0 ]'],
+  [new Float64Array(2), '[Float64Array(2): null prototype] [ 0, 0 ]'],
+  [new BigInt64Array(2), '[BigInt64Array(2): null prototype] [ 0n, 0n ]'],
+  [new BigUint64Array(2), '[BigUint64Array(2): null prototype] [ 0n, 0n ]'],
   [new ArrayBuffer(16), '[ArrayBuffer: null prototype] {\n' +
      '  [Uint8Contents]: <00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00>,\n' +
      '  byteLength: undefined\n}'],
@@ -2026,8 +2026,10 @@ assert.strictEqual(util.inspect('"\'${a}'), "'\"\\'${a}'");
   class Foo extends base {}
   const value = new Foo(...input);
   const symbol = value[Symbol.toStringTag];
-  const expected = `Foo ${symbol ? `[${symbol}] ` : ''}${rawExpected}`;
-  const expectedWithoutProto = `[${base.name}: null prototype] ${rawExpected}`;
+  const size = base.name.includes('Array') ? `(${input[0]})` : '';
+  const expected = `Foo${size} ${symbol ? `[${symbol}] ` : ''}${rawExpected}`;
+  const expectedWithoutProto =
+    `[${base.name}${size}: null prototype] ${rawExpected}`;
   assert.strictEqual(util.inspect(value), expected);
   value.foo = 'bar';
   assert.notStrictEqual(util.inspect(value), expected);
@@ -2050,8 +2052,9 @@ assert.strictEqual(util.inspect('"\'${a}'), "'\"\\'${a}'");
 assert.strictEqual(inspect(1n), '1n');
 assert.strictEqual(inspect(Object(-1n)), '[BigInt: -1n]');
 assert.strictEqual(inspect(Object(13n)), '[BigInt: 13n]');
-assert.strictEqual(inspect(new BigInt64Array([0n])), 'BigInt64Array [ 0n ]');
-assert.strictEqual(inspect(new BigUint64Array([0n])), 'BigUint64Array [ 0n ]');
+assert.strictEqual(inspect(new BigInt64Array([0n])), 'BigInt64Array(1) [ 0n ]');
+assert.strictEqual(
+  inspect(new BigUint64Array([0n])), 'BigUint64Array(1) [ 0n ]');
 
 // Verify non-enumerable keys get escaped.
 {
@@ -2170,7 +2173,7 @@ assert.strictEqual(
   Object.setPrototypeOf(obj, value);
   assert.strictEqual(
     util.inspect(obj),
-    'Object <[Array: null prototype] []> { a: true }'
+    'Object <[Array(0): null prototype] []> { a: true }'
   );
 
   function StorageObject() {}

From a67653149eb452845af0d87443d028108a2790b0 Mon Sep 17 00:00:00 2001
From: Ruben Bridgewater <ruben@bridgewater.de>
Date: Sun, 8 Dec 2019 21:38:25 +0100
Subject: [PATCH 09/15] src: accept single argument in getProxyDetails
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

This makes sure this function stays backwards compatible in case
it's accessed through the binding directly.

Refs: https://github.com/nodejs/node/pull/29947#issuecomment-562987888

PR-URL: https://github.com/nodejs/node/pull/30858
Refs: https://github.com/nodejs/node/pull/30767
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: David Carlier <devnexen@gmail.com>
Reviewed-By: Yongsheng Zhang <zyszys98@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
---
 src/node_util.cc                         | 7 ++++---
 test/parallel/test-util-inspect-proxy.js | 4 ++++
 2 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/src/node_util.cc b/src/node_util.cc
index b9553eaaa6763d..e0ef7d421ff24a 100644
--- a/src/node_util.cc
+++ b/src/node_util.cc
@@ -91,11 +91,12 @@ static void GetProxyDetails(const FunctionCallbackInfo<Value>& args) {
   if (!args[0]->IsProxy())
     return;
 
-  CHECK(args[1]->IsBoolean());
-
   Local<Proxy> proxy = args[0].As<Proxy>();
 
-  if (args[1]->IsTrue()) {
+  // TODO(BridgeAR): Remove the length check as soon as we prohibit access to
+  // the util binding layer. It's accessed in the wild and `esm` would break in
+  // case the check is removed.
+  if (args.Length() == 1 || args[1]->IsTrue()) {
     Local<Value> ret[] = {
       proxy->GetTarget(),
       proxy->GetHandler()
diff --git a/test/parallel/test-util-inspect-proxy.js b/test/parallel/test-util-inspect-proxy.js
index 5789a6ffbcf6c5..f87ebecb8e7e7c 100644
--- a/test/parallel/test-util-inspect-proxy.js
+++ b/test/parallel/test-util-inspect-proxy.js
@@ -50,6 +50,10 @@ let details = processUtil.getProxyDetails(proxyObj, true);
 assert.strictEqual(target, details[0]);
 assert.strictEqual(handler, details[1]);
 
+details = processUtil.getProxyDetails(proxyObj);
+assert.strictEqual(target, details[0]);
+assert.strictEqual(handler, details[1]);
+
 details = processUtil.getProxyDetails(proxyObj, false);
 assert.strictEqual(target, details);
 

From 9c9b338cf976128413544165453f6705a4414f36 Mon Sep 17 00:00:00 2001
From: Ruben Bridgewater <ruben@bridgewater.de>
Date: Thu, 26 Dec 2019 20:57:53 +0100
Subject: [PATCH 10/15] util: improve prototype inspection using `inspect()`
 and `showHidden`

The fast path for the prototype inspection had a bug that caused some
prototype properties to be skipped that should in fact be inspected.

PR-URL: https://github.com/nodejs/node/pull/31113
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
---
 lib/internal/util/inspect.js       | 12 +++++-------
 test/parallel/test-util-inspect.js | 29 +++++++++++++++++++++++------
 2 files changed, 28 insertions(+), 13 deletions(-)

diff --git a/lib/internal/util/inspect.js b/lib/internal/util/inspect.js
index 04b76077bba313..4eb493dfe1fdae 100644
--- a/lib/internal/util/inspect.js
+++ b/lib/internal/util/inspect.js
@@ -467,10 +467,10 @@ function getConstructorName(obj, ctx, recurseTimes, protoProps) {
         typeof descriptor.value === 'function' &&
         descriptor.value.name !== '') {
       if (protoProps !== undefined &&
-          !builtInObjects.has(descriptor.value.name)) {
-        const isProto = firstProto !== undefined;
+         (firstProto !== obj ||
+         !builtInObjects.has(descriptor.value.name))) {
         addPrototypeProperties(
-          ctx, tmp, obj, recurseTimes, isProto, protoProps);
+          ctx, tmp, firstProto || tmp, recurseTimes, protoProps);
       }
       return descriptor.value.name;
     }
@@ -508,12 +508,12 @@ function getConstructorName(obj, ctx, recurseTimes, protoProps) {
 // This function has the side effect of adding prototype properties to the
 // `output` argument (which is an array). This is intended to highlight user
 // defined prototype properties.
-function addPrototypeProperties(ctx, main, obj, recurseTimes, isProto, output) {
+function addPrototypeProperties(ctx, main, obj, recurseTimes, output) {
   let depth = 0;
   let keys;
   let keySet;
   do {
-    if (!isProto) {
+    if (depth !== 0 || main === obj) {
       obj = ObjectGetPrototypeOf(obj);
       // Stop as soon as a null prototype is encountered.
       if (obj === null) {
@@ -526,8 +526,6 @@ function addPrototypeProperties(ctx, main, obj, recurseTimes, isProto, output) {
           builtInObjects.has(descriptor.value.name)) {
         return;
       }
-    } else {
-      isProto = false;
     }
 
     if (depth === 0) {
diff --git a/test/parallel/test-util-inspect.js b/test/parallel/test-util-inspect.js
index bba497c89683c9..b1cd48a6a83c03 100644
--- a/test/parallel/test-util-inspect.js
+++ b/test/parallel/test-util-inspect.js
@@ -1687,7 +1687,8 @@ util.inspect(process);
     '      1,',
     '      2,',
     '      [length]: 2',
-    '    ]',
+    '    ],',
+    "    [Symbol(Symbol.toStringTag)]: 'Set Iterator'",
     '  } => [Map Iterator] {',
     '    Uint8Array(0) [',
     '      [BYTES_PER_ELEMENT]: 1,',
@@ -1699,7 +1700,8 @@ util.inspect(process);
     '        foo: true',
     '      }',
     '    ],',
-    '    [Circular]',
+    '    [Circular],',
+    "    [Symbol(Symbol.toStringTag)]: 'Map Iterator'",
     '  },',
     '  [size]: 2',
     '}'
@@ -1727,7 +1729,10 @@ util.inspect(process);
     '    [byteOffset]: 0,',
     '    [buffer]: ArrayBuffer { byteLength: 0, foo: true }',
     '  ],',
-    '  [Set Iterator] { [ 1, 2, [length]: 2 ] } => [Map Iterator] {',
+    '  [Set Iterator] {',
+    '    [ 1, 2, [length]: 2 ],',
+    "    [Symbol(Symbol.toStringTag)]: 'Set Iterator'",
+    '  } => [Map Iterator] {',
     '    Uint8Array(0) [',
     '      [BYTES_PER_ELEMENT]: 1,',
     '      [length]: 0,',
@@ -1735,7 +1740,8 @@ util.inspect(process);
     '      [byteOffset]: 0,',
     '      [buffer]: ArrayBuffer { byteLength: 0, foo: true }',
     '    ],',
-    '    [Circular]',
+    '    [Circular],',
+    "    [Symbol(Symbol.toStringTag)]: 'Map Iterator'",
     '  },',
     '  [size]: 2',
     '}'
@@ -1767,7 +1773,9 @@ util.inspect(process);
     '  [Set Iterator] {',
     '    [ 1,',
     '      2,',
-    '      [length]: 2 ] } => [Map Iterator] {',
+    '      [length]: 2 ],',
+    '    [Symbol(Symbol.toStringTag)]:',
+    "     'Set Iterator' } => [Map Iterator] {",
     '    Uint8Array(0) [',
     '      [BYTES_PER_ELEMENT]: 1,',
     '      [length]: 0,',
@@ -1776,7 +1784,9 @@ util.inspect(process);
     '      [buffer]: ArrayBuffer {',
     '        byteLength: 0,',
     '        foo: true } ],',
-    '    [Circular] },',
+    '    [Circular],',
+    '    [Symbol(Symbol.toStringTag)]:',
+    "     'Map Iterator' },",
     '  [size]: 2 }'
   ].join('\n');
 
@@ -2678,4 +2688,11 @@ assert.strictEqual(
     '  \x1B[2m[def]: \x1B[36m[Getter/Setter]\x1B[39m\x1B[22m\n' +
     '}'
   );
+
+  const obj = Object.create({ abc: true, def: 5, toString() {} });
+  assert.strictEqual(
+    inspect(obj, { showHidden: true, colors: true }),
+    '{ \x1B[2mabc: \x1B[33mtrue\x1B[39m\x1B[22m, ' +
+      '\x1B[2mdef: \x1B[33m5\x1B[39m\x1B[22m }'
+  );
 }

From 32dfdb8f98a92cb01d5cef56d767acb7e8124ace Mon Sep 17 00:00:00 2001
From: Ruben Bridgewater <ruben@bridgewater.de>
Date: Fri, 14 Jun 2019 10:49:20 +0200
Subject: [PATCH 11/15] assert: fix generatedMessage property

This makes sure the `generatedMessage` property is always set as
expected. This was not the case some `assert.throws` and
`assert.rejects` calls.

PR-URL: #28263
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
---
 lib/assert.js | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/lib/assert.js b/lib/assert.js
index 025e5bedc02e27..eb74493c443902 100644
--- a/lib/assert.js
+++ b/lib/assert.js
@@ -578,14 +578,21 @@ function expectedException(actual, expected, message, fn) {
       if (expected.test(str))
         return;
 
-      throw new AssertionError({
+      if (!message) {
+        generatedMessage = true;
+        message = 'The input did not match the regular expression ' +
+                  `${inspect(expected)}. Input:\n\n${inspect(str)}\n`;
+      }
+
+      const err = new AssertionError({
         actual,
         expected,
-        message: message || 'The input did not match the regular expression ' +
-                            `${inspect(expected)}. Input:\n\n${inspect(str)}\n`,
+        message,
         operator: fn.name,
         stackStartFn: fn
       });
+      err.generatedMessage = generatedMessage;
+      throw err;
     }
 
     // Handle primitives properly.

From ad12514e9738391ca97655b82d5296d1c0312686 Mon Sep 17 00:00:00 2001
From: Ruben Bridgewater <ruben@bridgewater.de>
Date: Fri, 14 Jun 2019 16:29:25 +0200
Subject: [PATCH 12/15] assert: DRY .throws code

This refactors some code for less duplication.

PR-URL: #28263
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
---
 lib/assert.js | 88 ++++++++++++++++++++++++++-------------------------
 1 file changed, 45 insertions(+), 43 deletions(-)

diff --git a/lib/assert.js b/lib/assert.js
index eb74493c443902..52523db3948ca7 100644
--- a/lib/assert.js
+++ b/lib/assert.js
@@ -25,6 +25,7 @@ const {
   ObjectAssign,
   ObjectIs,
   ObjectKeys,
+  ObjectPrototypeIsPrototypeOf,
   Map,
 } = primordials;
 
@@ -571,6 +572,9 @@ function compareExceptionKey(actual, expected, key, message, keys, fn) {
 }
 
 function expectedException(actual, expected, message, fn) {
+  let generatedMessage = false;
+  let throwError = false;
+
   if (typeof expected !== 'function') {
     // Handle regular expressions.
     if (isRegExp(expected)) {
@@ -583,20 +587,9 @@ function expectedException(actual, expected, message, fn) {
         message = 'The input did not match the regular expression ' +
                   `${inspect(expected)}. Input:\n\n${inspect(str)}\n`;
       }
-
-      const err = new AssertionError({
-        actual,
-        expected,
-        message,
-        operator: fn.name,
-        stackStartFn: fn
-      });
-      err.generatedMessage = generatedMessage;
-      throw err;
-    }
-
-    // Handle primitives properly.
-    if (typeof actual !== 'object' || actual === null) {
+      throwError = true;
+      // Handle primitives properly.
+    } else if (typeof actual !== 'object' || actual === null) {
       const err = new AssertionError({
         actual,
         expected,
@@ -606,43 +599,52 @@ function expectedException(actual, expected, message, fn) {
       });
       err.operator = fn.name;
       throw err;
-    }
-
-    // Handle validation objects.
-    const keys = ObjectKeys(expected);
-    // Special handle errors to make sure the name and the message are compared
-    // as well.
-    if (expected instanceof Error) {
-      keys.push('name', 'message');
-    } else if (keys.length === 0) {
-      throw new ERR_INVALID_ARG_VALUE('error',
-                                      expected, 'may not be an empty object');
-    }
-    if (isDeepEqual === undefined) lazyLoadComparison();
-    for (const key of keys) {
-      if (typeof actual[key] === 'string' &&
-          isRegExp(expected[key]) &&
-          expected[key].test(actual[key])) {
-        continue;
+    } else {
+      // Handle validation objects.
+      const keys = ObjectKeys(expected);
+      // Special handle errors to make sure the name and the message are
+      // compared as well.
+      if (expected instanceof Error) {
+        keys.push('name', 'message');
+      } else if (keys.length === 0) {
+        throw new ERR_INVALID_ARG_VALUE('error',
+                                        expected, 'may not be an empty object');
+      }
+      if (isDeepEqual === undefined) lazyLoadComparison();
+      for (const key of keys) {
+        if (typeof actual[key] === 'string' &&
+            isRegExp(expected[key]) &&
+            expected[key].test(actual[key])) {
+          continue;
+        }
+        compareExceptionKey(actual, expected, key, message, keys, fn);
       }
-      compareExceptionKey(actual, expected, key, message, keys, fn);
+      return;
     }
-    return;
-  }
-
   // Guard instanceof against arrow functions as they don't have a prototype.
   // Check for matching Error classes.
-  if (expected.prototype !== undefined && actual instanceof expected) {
+  } else if (expected.prototype !== undefined && actual instanceof expected) {
     return;
-  }
-  if (Error.isPrototypeOf(expected)) {
+  } else if (ObjectPrototypeIsPrototypeOf(Error, expected)) {
     throw actual;
+  } else {
+    // Check validation functions return value.
+    const res = expected.call({}, actual);
+    if (res !== true) {
+      throw actual;
+    }
   }
 
-  // Check validation functions return value.
-  const res = expected.call({}, actual);
-  if (res !== true) {
-    throw actual;
+  if (throwError) {
+    const err = new AssertionError({
+      actual,
+      expected,
+      message,
+      operator: fn.name,
+      stackStartFn: fn
+    });
+    err.generatedMessage = generatedMessage;
+    throw err;
   }
 }
 

From 8b82c856f926ac90e473ae1e0c445fec772b0df6 Mon Sep 17 00:00:00 2001
From: Ruben Bridgewater <ruben@bridgewater.de>
Date: Mon, 17 Jun 2019 16:35:10 +0200
Subject: [PATCH 13/15] doc: update assert.throws() examples

This updates two outdated examples to the current implementation.

PR-URL: https://github.com/nodejs/node/pull/28263
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
---
 doc/api/assert.md | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/doc/api/assert.md b/doc/api/assert.md
index eaf5c5447da6db..c96516855cee8b 100644
--- a/doc/api/assert.md
+++ b/doc/api/assert.md
@@ -1192,10 +1192,15 @@ assert.throws(
 assert.throws(
   () => {
     const otherErr = new Error('Not found');
-    otherErr.code = 404;
+    // Copy all enumerable properties from `err` to `otherErr`.
+    for (const [key, value] of Object.entries(err)) {
+      otherErr[key] = value;
+    }
     throw otherErr;
   },
-  err // This tests for `message`, `name` and `code`.
+  // The error's `message` and `name` properties will also be checked when using
+  // an error as validation object.
+  err
 );
 ```
 
@@ -1282,11 +1287,9 @@ assert.throws(notThrowing, 'Second');
 // It does not throw because the error messages match.
 assert.throws(throwingSecond, /Second$/);
 
-// If the error message does not match, the error from within the function is
-// not caught.
+// If the error message does not match, an AssertionError is thrown.
 assert.throws(throwingFirst, /Second$/);
-// Error: First
-//     at throwingFirst (repl:2:9)
+// AssertionError [ERR_ASSERTION]
 ```
 
 Due to the confusing error-prone notation, avoid a string as the second

From ed2d864421eb40d3a20bdbce644dfeb03313db31 Mon Sep 17 00:00:00 2001
From: Ruben Bridgewater <ruben@bridgewater.de>
Date: Thu, 20 Jun 2019 17:13:31 +0200
Subject: [PATCH 14/15] doc: make `AssertionError` a link

This makes sure that `AssertionError` links to the correct place in
the assert documentation.

PR-URL: https://github.com/nodejs/node/pull/28263
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
---
 doc/api/assert.md | 85 +++++++++++++++++++++++++----------------------
 1 file changed, 45 insertions(+), 40 deletions(-)

diff --git a/doc/api/assert.md b/doc/api/assert.md
index c96516855cee8b..585a9dff4b3e48 100644
--- a/doc/api/assert.md
+++ b/doc/api/assert.md
@@ -218,8 +218,9 @@ are also recursively evaluated by the following rules.
 * [`Symbol`][] properties are not compared.
 * [`WeakMap`][] and [`WeakSet`][] comparison does not rely on their values.
 
-The following example does not throw an `AssertionError` because the primitives
-are considered equal by the [Abstract Equality Comparison][] ( `==` ).
+The following example does not throw an [`AssertionError`][] because the
+primitives are considered equal by the [Abstract Equality Comparison][]
+( `==` ).
 
 ```js
 // WARNING: This does not throw an AssertionError!
@@ -264,11 +265,11 @@ assert.deepEqual(obj1, obj4);
 // AssertionError: { a: { b: 1 } } deepEqual {}
 ```
 
-If the values are not equal, an `AssertionError` is thrown with a `message`
+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. If the `message`
 parameter is an instance of an [`Error`][] then it will be thrown instead of the
-`AssertionError`.
+[`AssertionError`][].
 
 ## `assert.deepStrictEqual(actual, expected[, message])`
 <!-- YAML
@@ -418,7 +419,7 @@ assert.deepStrictEqual(weakMap1, weakMap3);
 //   }
 ```
 
-If the values are not equal, an `AssertionError` is thrown with a `message`
+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. If the `message`
 parameter is an instance of an [`Error`][] then it will be thrown instead of the
@@ -501,9 +502,9 @@ When `assert.doesNotThrow()` is called, it will immediately call the `fn`
 function.
 
 If an error is thrown and it is the same type as that specified by the `error`
-parameter, then an `AssertionError` is thrown. If the error is of a different
-type, or if the `error` parameter is undefined, the error is propagated back
-to the caller.
+parameter, then an [`AssertionError`][] is thrown. If the error is of a
+different type, or if the `error` parameter is undefined, the error is
+propagated back to the caller.
 
 If specified, `error` can be a [`Class`][], [`RegExp`][] or a validation
 function. See [`assert.throws()`][] for more details.
@@ -521,7 +522,7 @@ assert.doesNotThrow(
 );
 ```
 
-However, the following will result in an `AssertionError` with the message
+However, the following will result in an [`AssertionError`][] with the message
 'Got unwanted exception...':
 
 <!-- eslint-disable no-restricted-syntax -->
@@ -534,8 +535,8 @@ assert.doesNotThrow(
 );
 ```
 
-If an `AssertionError` is thrown and a value is provided for the `message`
-parameter, the value of `message` will be appended to the `AssertionError`
+If an [`AssertionError`][] is thrown and a value is provided for the `message`
+parameter, the value of `message` will be appended to the [`AssertionError`][]
 message:
 
 <!-- eslint-disable no-restricted-syntax -->
@@ -584,7 +585,7 @@ assert.equal({ a: { b: 1 } }, { a: { b: 1 } });
 // AssertionError: { a: { b: 1 } } == { a: { b: 1 } }
 ```
 
-If the values are not equal, an `AssertionError` is thrown with a `message`
+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. If the `message`
 parameter is an instance of an [`Error`][] then it will be thrown instead of the
@@ -597,9 +598,9 @@ added: v0.1.21
 
 * `message` {string|Error} **Default:** `'Failed'`
 
-Throws an `AssertionError` with the provided error message or a default error
-message. If the `message` parameter is an instance of an [`Error`][] then it
-will be thrown instead of the `AssertionError`.
+Throws an [`AssertionError`][] with the provided error message or a default
+error message. If the `message` parameter is an instance of an [`Error`][] then
+it will be thrown instead of the [`AssertionError`][].
 
 ```js
 const assert = require('assert').strict;
@@ -687,7 +688,7 @@ changes:
   - version: v10.0.0
     pr-url: https://github.com/nodejs/node/pull/18247
     description: Instead of throwing the original error it is now wrapped into
-                 an `AssertionError` that contains the full stack trace.
+                 an [`AssertionError`][] that contains the full stack trace.
   - version: v10.0.0
     pr-url: https://github.com/nodejs/node/pull/18247
     description: Value may now only be `undefined` or `null`. Before all falsy
@@ -795,11 +796,11 @@ assert.notDeepEqual(obj1, obj4);
 // OK
 ```
 
-If the values are deeply 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. If the `message`
-parameter is an instance of an [`Error`][] then it will be thrown instead of the
-`AssertionError`.
+If the values are deeply 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. If the
+`message` parameter is an instance of an [`Error`][] then it will be thrown
+instead of the `AssertionError`.
 
 ## `assert.notDeepStrictEqual(actual, expected[, message])`
 <!-- YAML
@@ -843,11 +844,11 @@ assert.notDeepStrictEqual({ a: 1 }, { a: '1' });
 // OK
 ```
 
-If the values are deeply and strictly 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. If the
-`message` parameter is an instance of an [`Error`][] then it will be thrown
-instead of the `AssertionError`.
+If the values are deeply and strictly 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. If
+the `message` parameter is an instance of an [`Error`][] then it will be thrown
+instead of the [`AssertionError`][].
 
 ## `assert.notEqual(actual, expected[, message])`
 <!-- YAML
@@ -882,10 +883,10 @@ assert.notEqual(1, '1');
 // AssertionError: 1 != '1'
 ```
 
-If the values are 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. If the `message` parameter is an
-instance of an [`Error`][] then it will be thrown instead of the
+If the values are 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. If the `message`
+parameter is an instance of an [`Error`][] then it will be thrown instead of the
 `AssertionError`.
 
 ## `assert.notStrictEqual(actual, expected[, message])`
@@ -919,11 +920,11 @@ assert.notStrictEqual(1, '1');
 // OK
 ```
 
-If the values are strictly 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. If the `message`
-parameter is an instance of an [`Error`][] then it will be thrown instead of the
-`AssertionError`.
+If the values are strictly 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. If the
+`message` parameter is an instance of an [`Error`][] then it will be thrown
+instead of the `AssertionError`.
 
 ## `assert.ok(value[, message])`
 <!-- YAML
@@ -941,7 +942,7 @@ changes:
 Tests if `value` is truthy. It is equivalent to
 `assert.equal(!!value, true, message)`.
 
-If `value` is not truthy, an `AssertionError` is thrown with a `message`
+If `value` is not truthy, 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. If the `message`
 parameter is an instance of an [`Error`][] then it will be thrown instead of the
@@ -1020,8 +1021,8 @@ an object where each property will be tested for, or an instance of error where
 each property will be tested for including the non-enumerable `message` and
 `name` properties.
 
-If specified, `message` will be the message provided by the `AssertionError` if
-the `asyncFn` fails to reject.
+If specified, `message` will be the message provided by the [`AssertionError`][]
+if the `asyncFn` fails to reject.
 
 ```js
 (async () => {
@@ -1096,11 +1097,11 @@ assert.strictEqual(1, '1', new TypeError('Inputs are not identical'));
 // TypeError: Inputs are not identical
 ```
 
-If the values are not strictly equal, an `AssertionError` is thrown with a
+If the values are not strictly 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. If the
 `message` parameter is an instance of an [`Error`][] then it will be thrown
-instead of the `AssertionError`.
+instead of the [`AssertionError`][].
 
 ## `assert.throws(fn[, error][, message])`
 <!-- YAML
@@ -1231,6 +1232,9 @@ assert.throws(
 
 Custom error validation:
 
+The function must return `true` to indicate all internal validations passed.
+It will otherwise fail with an [`AssertionError`][].
+
 ```js
 assert.throws(
   () => {
@@ -1295,6 +1299,7 @@ assert.throws(throwingFirst, /Second$/);
 Due to the confusing error-prone notation, avoid a string as the second
 argument.
 
+[`AssertionError`]: #assert_class_assert_assertionerror
 [`Class`]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Classes
 [`ERR_INVALID_RETURN_VALUE`]: errors.html#errors_err_invalid_return_value
 [`Error.captureStackTrace`]: errors.html#errors_error_capturestacktrace_targetobject_constructoropt

From e2970ab766c1caf98135ad1f3a24d5d34d7a71dd Mon Sep 17 00:00:00 2001
From: Ruben Bridgewater <ruben@bridgewater.de>
Date: Thu, 12 Dec 2019 22:51:43 +0100
Subject: [PATCH 15/15] assert: implement `assert.match()` and
 `assert.doesNotMatch()`
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

This adds a new functionality to the assertion module: a dedicated
check for regular expressions. So far it's possible to use
`assert.ok(regexp.test(string))`. This is not ideal though when it
comes to the error message, since it's not possible to know how
either of the input values look like. It's just known that the
assertion failed.
This allows to pass through the regular expression and the input
string. The string is then matched against the regular expression
and reports a expressive error message in case of a failure.

PR-URL: https://github.com/nodejs/node/pull/30929
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Stephen Belanger <admin@stephenbelanger.com>
---
 doc/api/assert.md            |  72 +++++++++++++++++++++++++
 lib/assert.js                |  52 ++++++++++++++++--
 test/parallel/test-assert.js | 100 +++++++++++++++++++++++++++++++++++
 3 files changed, 220 insertions(+), 4 deletions(-)

diff --git a/doc/api/assert.md b/doc/api/assert.md
index 585a9dff4b3e48..277f29880d5b24 100644
--- a/doc/api/assert.md
+++ b/doc/api/assert.md
@@ -425,6 +425,42 @@ parameter is undefined, a default error message is assigned. If the `message`
 parameter is an instance of an [`Error`][] then it will be thrown instead of the
 `AssertionError`.
 
+## `assert.doesNotMatch(string, regexp[, message])`
+<!-- YAML
+added: REPLACEME
+-->
+
+* `string` {string}
+* `regexp` {RegExp}
+* `message` {string|Error}
+
+> Stability: 1 - Experimental
+
+Expects the `string` input not to match the regular expression.
+
+This feature is currently experimental and the name might change or it might be
+completely removed again.
+
+```js
+const assert = require('assert').strict;
+
+assert.doesNotMatch('I will fail', /fail/);
+// AssertionError [ERR_ASSERTION]: The input was expected to not match the ...
+
+assert.doesNotMatch(123, /pass/);
+// AssertionError [ERR_ASSERTION]: The "string" argument must be of type string.
+
+assert.doesNotMatch('I will pass', /different/);
+// OK
+```
+
+If the values do match, or if the `string` argument is of another type than
+`string`, 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. If the `message` parameter is an
+instance of an [`Error`][] then it will be thrown instead of the
+[`AssertionError`][].
+
 ## `assert.doesNotReject(asyncFn[, error][, message])`
 <!-- YAML
 added: v10.0.0
@@ -728,6 +764,42 @@ let err;
 //     at errorFrame
 ```
 
+## `assert.match(string, regexp[, message])`
+<!-- YAML
+added: REPLACEME
+-->
+
+* `string` {string}
+* `regexp` {RegExp}
+* `message` {string|Error}
+
+> Stability: 1 - Experimental
+
+Expects the `string` input to match the regular expression.
+
+This feature is currently experimental and the name might change or it might be
+completely removed again.
+
+```js
+const assert = require('assert').strict;
+
+assert.match('I will fail', /pass/);
+// AssertionError [ERR_ASSERTION]: The input did not match the regular ...
+
+assert.match(123, /pass/);
+// AssertionError [ERR_ASSERTION]: The "string" argument must be of type string.
+
+assert.match('I will pass', /pass/);
+// OK
+```
+
+If the values do not match, or if the `string` argument is of another type than
+`string`, 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. If the `message` parameter is an
+instance of an [`Error`][] then it will be thrown instead of the
+[`AssertionError`][].
+
 ## `assert.notDeepEqual(actual, expected[, message])`
 <!-- YAML
 added: v0.1.21
diff --git a/lib/assert.js b/lib/assert.js
index 52523db3948ca7..f37166be9c6798 100644
--- a/lib/assert.js
+++ b/lib/assert.js
@@ -27,6 +27,7 @@ const {
   ObjectKeys,
   ObjectPrototypeIsPrototypeOf,
   Map,
+  RegExpPrototypeTest,
 } = primordials;
 
 const { Buffer } = require('buffer');
@@ -533,7 +534,7 @@ class Comparison {
         if (actual !== undefined &&
             typeof actual[key] === 'string' &&
             isRegExp(obj[key]) &&
-            obj[key].test(actual[key])) {
+            RegExpPrototypeTest(obj[key], actual[key])) {
           this[key] = actual[key];
         } else {
           this[key] = obj[key];
@@ -579,7 +580,7 @@ function expectedException(actual, expected, message, fn) {
     // Handle regular expressions.
     if (isRegExp(expected)) {
       const str = String(actual);
-      if (expected.test(str))
+      if (RegExpPrototypeTest(expected, str))
         return;
 
       if (!message) {
@@ -614,7 +615,7 @@ function expectedException(actual, expected, message, fn) {
       for (const key of keys) {
         if (typeof actual[key] === 'string' &&
             isRegExp(expected[key]) &&
-            expected[key].test(actual[key])) {
+            RegExpPrototypeTest(expected[key], actual[key])) {
           continue;
         }
         compareExceptionKey(actual, expected, key, message, keys, fn);
@@ -751,7 +752,7 @@ function hasMatchingError(actual, expected) {
   if (typeof expected !== 'function') {
     if (isRegExp(expected)) {
       const str = String(actual);
-      return expected.test(str);
+      return RegExpPrototypeTest(expected, str);
     }
     throw new ERR_INVALID_ARG_TYPE(
       'expected', ['Function', 'RegExp'], expected
@@ -856,6 +857,49 @@ assert.ifError = function ifError(err) {
   }
 };
 
+function internalMatch(string, regexp, message, fn) {
+  if (!isRegExp(regexp)) {
+    throw new ERR_INVALID_ARG_TYPE(
+      'regexp', 'RegExp', regexp
+    );
+  }
+  const match = fn.name === 'match';
+  if (typeof string !== 'string' ||
+      RegExpPrototypeTest(regexp, string) !== match) {
+    if (message instanceof Error) {
+      throw message;
+    }
+
+    const generatedMessage = !message;
+
+    // 'The input was expected to not match the regular expression ' +
+    message = message || (typeof string !== 'string' ?
+      'The "string" argument must be of type string. Received type ' +
+        `${typeof string} (${inspect(string)})` :
+      (match ?
+        'The input did not match the regular expression ' :
+        'The input was expected to not match the regular expression ') +
+          `${inspect(regexp)}. Input:\n\n${inspect(string)}\n`);
+    const err = new AssertionError({
+      actual: string,
+      expected: regexp,
+      message,
+      operator: fn.name,
+      stackStartFn: fn
+    });
+    err.generatedMessage = generatedMessage;
+    throw err;
+  }
+}
+
+assert.match = function match(string, regexp, message) {
+  internalMatch(string, regexp, message, match);
+};
+
+assert.doesNotMatch = function doesNotMatch(string, regexp, message) {
+  internalMatch(string, regexp, message, doesNotMatch);
+};
+
 // Expose a strict only variant of assert
 function strict(...args) {
   innerOk(strict, args.length, ...args);
diff --git a/test/parallel/test-assert.js b/test/parallel/test-assert.js
index f0204d52f51b5b..27421d2373eab1 100644
--- a/test/parallel/test-assert.js
+++ b/test/parallel/test-assert.js
@@ -1301,3 +1301,103 @@ assert.throws(
     assert(!err2.stack.includes('hidden'));
   })();
 }
+
+// Multiple assert.match() tests.
+{
+  assert.throws(
+    () => assert.match(/abc/, 'string'),
+    {
+      code: 'ERR_INVALID_ARG_TYPE',
+      message: 'The "regexp" argument must be an instance of RegExp. ' +
+               "Received type string ('string')"
+    }
+  );
+  assert.throws(
+    () => assert.match('string', /abc/),
+    {
+      actual: 'string',
+      expected: /abc/,
+      operator: 'match',
+      message: 'The input did not match the regular expression /abc/. ' +
+               "Input:\n\n'string'\n",
+      generatedMessage: true
+    }
+  );
+  assert.throws(
+    () => assert.match('string', /abc/, 'foobar'),
+    {
+      actual: 'string',
+      expected: /abc/,
+      operator: 'match',
+      message: 'foobar',
+      generatedMessage: false
+    }
+  );
+  const errorMessage = new RangeError('foobar');
+  assert.throws(
+    () => assert.match('string', /abc/, errorMessage),
+    errorMessage
+  );
+  assert.throws(
+    () => assert.match({ abc: 123 }, /abc/),
+    {
+      actual: { abc: 123 },
+      expected: /abc/,
+      operator: 'match',
+      message: 'The "string" argument must be of type string. ' +
+               'Received type object ({ abc: 123 })',
+      generatedMessage: true
+    }
+  );
+  assert.match('I will pass', /pass$/);
+}
+
+// Multiple assert.doesNotMatch() tests.
+{
+  assert.throws(
+    () => assert.doesNotMatch(/abc/, 'string'),
+    {
+      code: 'ERR_INVALID_ARG_TYPE',
+      message: 'The "regexp" argument must be an instance of RegExp. ' +
+               "Received type string ('string')"
+    }
+  );
+  assert.throws(
+    () => assert.doesNotMatch('string', /string/),
+    {
+      actual: 'string',
+      expected: /string/,
+      operator: 'doesNotMatch',
+      message: 'The input was expected to not match the regular expression ' +
+               "/string/. Input:\n\n'string'\n",
+      generatedMessage: true
+    }
+  );
+  assert.throws(
+    () => assert.doesNotMatch('string', /string/, 'foobar'),
+    {
+      actual: 'string',
+      expected: /string/,
+      operator: 'doesNotMatch',
+      message: 'foobar',
+      generatedMessage: false
+    }
+  );
+  const errorMessage = new RangeError('foobar');
+  assert.throws(
+    () => assert.doesNotMatch('string', /string/, errorMessage),
+    errorMessage
+  );
+  assert.throws(
+    () => assert.doesNotMatch({ abc: 123 }, /abc/),
+    {
+      actual: { abc: 123 },
+      expected: /abc/,
+      operator: 'doesNotMatch',
+      message: 'The "string" argument must be of type string. ' +
+               'Received type object ({ abc: 123 })',
+      generatedMessage: true
+    }
+  );
+  assert.doesNotMatch('I will pass', /different$/);
+}