From ea8b932f30665436796c7e5abd485f048a0f41c8 Mon Sep 17 00:00:00 2001 From: Ruben Bridgewater Date: Tue, 21 Aug 2018 17:39:01 +0200 Subject: [PATCH] util: restore all information in inspect The former implementation lacked symbols on the iterator objects without prototype. This is now fixed. The special handling for overriding `Symbol.iterator` was removed as it's very difficult to deal with this properly. Manipulating the symbols is just not supported. PR-URL: https://github.com/nodejs/node/pull/22437 Reviewed-By: James M Snell Reviewed-By: Matteo Collina --- lib/util.js | 83 +++++++++++++++--------------- test/parallel/test-util-inspect.js | 21 ++------ 2 files changed, 45 insertions(+), 59 deletions(-) diff --git a/lib/util.js b/lib/util.js index fa96ade26a5954..e8fb41a2198a99 100644 --- a/lib/util.js +++ b/lib/util.js @@ -513,13 +513,6 @@ function getPrefix(constructor, tag, fallback) { return ''; } -function addExtraKeys(source, target, keys) { - for (const key of keys) { - target[key] = source[key]; - } - return target; -} - function findTypedConstructor(value) { for (const [check, clazz] of [ [isUint8Array, Uint8Array], @@ -535,14 +528,36 @@ function findTypedConstructor(value) { [isBigUint64Array, BigUint64Array] ]) { if (check(value)) { - return new clazz(value); + return clazz; } } - return value; } const getBoxedValue = formatPrimitive.bind(null, stylizeNoColor); +function noPrototypeIterator(ctx, value, recurseTimes) { + let newVal; + // TODO: Create a Subclass in case there's no prototype and show + // `null-prototype`. + if (isSet(value)) { + const clazz = Object.getPrototypeOf(value) || Set; + newVal = new clazz(setValues(value)); + } else if (isMap(value)) { + const clazz = Object.getPrototypeOf(value) || Map; + newVal = new clazz(mapEntries(value)); + } else if (Array.isArray(value)) { + const clazz = Object.getPrototypeOf(value) || Array; + newVal = new clazz(value.length || 0); + } else if (isTypedArray(value)) { + const clazz = findTypedConstructor(value) || Uint8Array; + newVal = new clazz(value); + } + if (newVal) { + Object.defineProperties(newVal, Object.getOwnPropertyDescriptors(value)); + return formatValue(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. @@ -798,39 +813,25 @@ function formatValue(ctx, value, recurseTimes) { braces = ['{', '}']; // The input prototype got manipulated. Special handle these. // We have to rebuild the information so we are able to display everything. - } else if (isSet(value)) { - const newVal = addExtraKeys(value, new Set(setValues(value)), keys); - return formatValue(ctx, newVal, recurseTimes); - } else if (isMap(value)) { - const newVal = addExtraKeys(value, new Map(mapEntries(value)), keys); - return formatValue(ctx, newVal, recurseTimes); - } else if (Array.isArray(value)) { - // The prefix is not always possible to fully reconstruct. - const prefix = getPrefix(constructor, tag); - braces = [`${prefix === 'Array ' ? '' : prefix}[`, ']']; - formatter = formatArray; - const newValue = []; - newValue.length = value.length; - value = addExtraKeys(value, newValue, keys); - } else if (isTypedArray(value)) { - const newValue = findTypedConstructor(value); - value = addExtraKeys(value, newValue, keys.slice(newValue.length)); - // The prefix is not always possible to fully reconstruct. - braces = [`${getPrefix(getConstructorName(value), tag)}[`, ']']; - formatter = formatTypedArray; - } else if (isMapIterator(value)) { - braces = [`[${tag || 'Map Iterator'}] {`, '}']; - formatter = formatMapIterator; - } else if (isSetIterator(value)) { - braces = [`[${tag || 'Set Iterator'}] {`, '}']; - formatter = formatSetIterator; - // Handle other regular objects again. - } else if (keyLength === 0) { - if (isExternal(value)) - return ctx.stylize('[External]', 'special'); - return `${getPrefix(constructor, tag)}{}`; } else { - braces[0] = `${getPrefix(constructor, tag)}{`; + const specialIterator = noPrototypeIterator(ctx, value, recurseTimes); + if (specialIterator) { + return specialIterator; + } + if (isMapIterator(value)) { + braces = [`[${tag || 'Map Iterator'}] {`, '}']; + formatter = formatMapIterator; + } else if (isSetIterator(value)) { + braces = [`[${tag || 'Set Iterator'}] {`, '}']; + formatter = formatSetIterator; + // Handle other regular objects again. + } else if (keyLength === 0) { + if (isExternal(value)) + return ctx.stylize('[External]', 'special'); + return `${getPrefix(constructor, tag)}{}`; + } else { + braces[0] = `${getPrefix(constructor, tag)}{`; + } } } diff --git a/test/parallel/test-util-inspect.js b/test/parallel/test-util-inspect.js index 4240ff6d923fdd..8e78430c01204d 100644 --- a/test/parallel/test-util-inspect.js +++ b/test/parallel/test-util-inspect.js @@ -1557,24 +1557,6 @@ assert.strictEqual(util.inspect('"\''), '`"\'`'); // eslint-disable-next-line no-template-curly-in-string assert.strictEqual(util.inspect('"\'${a}'), "'\"\\'${a}'"); -// Manipulating the Symbol.iterator should still produce nice results. -[ - [[1, 2], '[ 1, 2 ]'], - [[, , 5, , , , ], '[ <2 empty items>, 5, <3 empty items> ]'], - [new Set([1, 2]), 'Set { 1, 2 }'], - [new Map([[1, 2]]), 'Map { 1 => 2 }'], - [new Uint8Array(2), 'Uint8Array [ 0, 0 ]'], - // It seems like the following can not be fully restored :( - [new Set([1, 2]).entries(), 'Object [Set Iterator] {}'], - [new Map([[1, 2]]).keys(), 'Object [Map Iterator] {}'], -].forEach(([value, expected]) => { - // "Remove the Symbol.iterator" - Object.defineProperty(value, Symbol.iterator, { - value: false - }); - assert.strictEqual(util.inspect(value), expected); -}); - // Verify the output in case the value has no prototype. // Sadly, these cases can not be fully inspected :( [ @@ -1630,6 +1612,9 @@ assert.strictEqual(util.inspect('"\'${a}'), "'\"\\'${a}'"); ); value.foo = 'bar'; assert.notStrictEqual(util.inspect(value), expected); + delete value.foo; + value[Symbol('foo')] = 'yeah'; + assert.notStrictEqual(util.inspect(value), expected); }); assert.strictEqual(inspect(1n), '1n');