From 3b9360da11bb46ca00c42bc2cfad39e6b7bda2a5 Mon Sep 17 00:00:00 2001 From: Ruben Bridgewater Date: Sat, 2 Nov 2019 18:50:09 +0100 Subject: [PATCH] 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. Backport-PR-URL: https://github.com/nodejs/node/pull/31431 PR-URL: https://github.com/nodejs/node/pull/30225 Reviewed-By: Michaël Zasso Reviewed-By: Rich Trott --- 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.