util: refactor inspect code for constistency

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

PR-URL: #30225
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Rich Trott <[email protected]>

Author: BridgeAR

lib/internal/util/inspect.js

@@ -10,6 +10,7 @@ const {
   DatePrototypeToString,
   ErrorPrototypeToString,
   JSONStringify,
+  MapPrototype,
   MapPrototypeEntries,
   MathFloor,
   MathMax,
@@ -21,10 +22,8 @@ const {
   NumberPrototypeValueOf,
   ObjectAssign,
   ObjectCreate,
-  ObjectDefineProperties,
   ObjectDefineProperty,
   ObjectGetOwnPropertyDescriptor,
-  ObjectGetOwnPropertyDescriptors,
   ObjectGetOwnPropertyNames,
   ObjectGetOwnPropertySymbols,
   ObjectGetPrototypeOf,
@@ -34,6 +33,7 @@ const {
   ObjectPrototypePropertyIsEnumerable,
   ObjectSeal,
   RegExpPrototypeToString,
+  SetPrototype,
   SetPrototypeValues,
   StringPrototypeValueOf,
   SymbolPrototypeToString,
@@ -113,6 +113,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(
@@ -646,51 +651,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.
@@ -793,7 +753,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);
@@ -805,37 +767,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;
     }
@@ -913,36 +904,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)}{`;
     }
   }
 
@@ -959,7 +934,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));
@@ -1317,7 +1292,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 {
@@ -1419,7 +1394,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);
@@ -1450,7 +1425,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) {
@@ -1461,11 +1436,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) {
@@ -1475,7 +1450,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;
 }
 
@@ -1553,7 +1528,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.
@@ -1588,7 +1563,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) {

test/parallel/test-util-inspect.js

@@ -2219,6 +2219,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.