Skip to content

Commit

Permalink
util: restore all information in inspect
Browse files Browse the repository at this point in the history
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: #22437
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
  • Loading branch information
BridgeAR authored and targos committed Sep 3, 2018
1 parent fcf059a commit 5b14066
Show file tree
Hide file tree
Showing 2 changed files with 45 additions and 59 deletions.
83 changes: 42 additions & 41 deletions lib/util.js
Original file line number Diff line number Diff line change
Expand Up @@ -462,13 +462,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],
Expand All @@ -484,14 +477,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.
Expand Down Expand Up @@ -757,39 +772,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)}{`;
}
}
}

Expand Down
21 changes: 3 additions & 18 deletions test/parallel/test-util-inspect.js
Original file line number Diff line number Diff line change
Expand Up @@ -1600,24 +1600,6 @@ util.inspect(process);
'prematurely. Maximum call stack size exceeded.]'));
}

// 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 :(
[
Expand Down Expand Up @@ -1673,6 +1655,9 @@ util.inspect(process);
);
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');
Expand Down

0 comments on commit 5b14066

Please sign in to comment.