util: improve display of iterators and weak entries

This adds the number of not visible elements when inspecting iterators
while exceeding `maxArrayLength`.
It also fixes a edge case with `maxArrayLength` and the map.entries()
iterator. Now the whole entry will be visible instead of only the key
but not the value of the first entry.
Besides that it uses a slighly better algorithm that improves the
performance by skipping unnecessary steps.
PR-URL: #20961
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>

Author: BridgeAR

lib/console.js

@@ -360,8 +360,11 @@ Console.prototype.table = function(tabularData, properties) {
 constmapIter=isMapIterator(tabularData);
 letisKeyValue=false;
 leti=0;
-if(mapIter)
-[tabularData,isKeyValue]=previewEntries(tabularData);
+if(mapIter){
+constres=previewEntries(tabularData,true);
+tabularData=res[0];
+isKeyValue=res[1];
+}
 
 if(isKeyValue||isMap(tabularData)){
 constkeys=[];
@@ -391,7 +394,7 @@ Console.prototype.table = function(tabularData, properties) {
 
 constsetIter=isSetIterator(tabularData);
 if(setIter)
-[tabularData]=previewEntries(tabularData);
+tabularData=previewEntries(tabularData);
 
 constsetlike=setIter||(mapIter&&!isKeyValue)||isSet(tabularData);
 if(setlike){

lib/util.js

@@ -115,6 +115,10 @@ const meta = [
 '','','','','','','','','','',
 '','','','','','','','\\\\'
 ];
+// Constants to map the iterator state.
+constkWeak=0;
+constkIterator=1;
+constkMapEntries=2;
 
 constescapeFn=(str)=>meta[str.charCodeAt(0)];
 
@@ -982,77 +986,83 @@ function formatMap(ctx, value, recurseTimes, keys) {
 returnoutput;
 }
 
-functionformatWeakSet(ctx,value,recurseTimes,keys){
+functionformatSetIterInner(ctx,value,recurseTimes,keys,entries,state){
 constmaxArrayLength=Math.max(ctx.maxArrayLength,0);
-const[entries]=previewEntries(value).slice(0,maxArrayLength+1);
 constmaxLength=Math.min(maxArrayLength,entries.length);
 letoutput=newArray(maxLength);
 for(vari=0;i<maxLength;++i)
 output[i]=formatValue(ctx,entries[i],recurseTimes);
-// Sort all entries to have a halfway reliable output (if more entries than
-// retrieved ones exist, we can not reliably return the same output).
-output=output.sort();
-if(entries.length>maxArrayLength)
-output.push('... more items');
+if(state===kWeak){
+// Sort all entries to have a halfway reliable output (if more entries than
+// retrieved ones exist, we can not reliably return the same output).
+output=output.sort();
+}
+constremaining=entries.length-maxLength;
+if(remaining>0){
+output.push(`... ${remaining} more item${remaining>1 ? 's' : ''}`);
+}
 for(i=0;i<keys.length;i++)
 output.push(formatProperty(ctx,value,recurseTimes,keys[i],0));
 returnoutput;
 }
 
-functionformatWeakMap(ctx,value,recurseTimes,keys){
+functionformatMapIterInner(ctx,value,recurseTimes,keys,entries,state){
 constmaxArrayLength=Math.max(ctx.maxArrayLength,0);
-const[entries]=previewEntries(value).slice(0,(maxArrayLength+1)*2);
 // Entries exist as [key1, val1, key2, val2, ...]
-constremainder=entries.length/2>maxArrayLength;
-constlen=entries.length/2-(remainder ? 1 : 0);
+constlen=entries.length/2;
+constremaining=len-maxArrayLength;
 constmaxLength=Math.min(maxArrayLength,len);
 letoutput=newArray(maxLength);
-for(vari=0;i<maxLength;i++){
+letstart='';
+letend='';
+letmiddle=' => ';
+leti=0;
+if(state===kMapEntries){
+start='[ ';
+end=' ]';
+middle=', ';
+}
+for(;i<maxLength;i++){
 constpos=i*2;
-output[i]=`${formatValue(ctx,entries[pos],recurseTimes)} => `+
-formatValue(ctx,entries[pos+1],recurseTimes);
+output[i]=`${start}${formatValue(ctx,entries[pos],recurseTimes)}`+
+`${middle}${formatValue(ctx,entries[pos+1],recurseTimes)}${end}`;
+}
+if(state===kWeak){
+// Sort all entries to have a halfway reliable output (if more entries
+// than retrieved ones exist, we can not reliably return the same output).
+output=output.sort();
+}
+if(remaining>0){
+output.push(`... ${remaining} more item${remaining>1 ? 's' : ''}`);
 }
-// Sort all entries to have a halfway reliable output (if more entries than
-// retrieved ones exist, we can not reliably return the same output).
-output=output.sort();
-if(remainder>0)
-output.push('... more items');
 for(i=0;i<keys.length;i++)
 output.push(formatProperty(ctx,value,recurseTimes,keys[i],0));
 returnoutput;
 }
 
-functionzip2(list){
-constret=Array(list.length/2);
-for(vari=0;i<ret.length;++i)
-ret[i]=[list[2*i],list[2*i+1]];
-returnret;
+functionformatWeakSet(ctx,value,recurseTimes,keys){
+constentries=previewEntries(value);
+returnformatSetIterInner(ctx,value,recurseTimes,keys,entries,kWeak);
 }
 
-functionformatCollectionIterator(ctx,value,recurseTimes,keys){
-constoutput=[];
-var[entries,isKeyValue]=previewEntries(value);
-if(isKeyValue)
-entries=zip2(entries);
-for(constentryofentries){
-if(ctx.maxArrayLength===output.length){
-output.push('... more items');
-break;
-}
-output.push(formatValue(ctx,entry,recurseTimes));
-}
-for(varn=0;n<keys.length;n++){
-output.push(formatProperty(ctx,value,recurseTimes,keys[n],0));
-}
-returnoutput;
+functionformatWeakMap(ctx,value,recurseTimes,keys){
+constentries=previewEntries(value);
+returnformatMapIterInner(ctx,value,recurseTimes,keys,entries,kWeak);
 }
 
-functionformatMapIterator(ctx,value,recurseTimes,keys){
-returnformatCollectionIterator(ctx,value,recurseTimes,keys);
+functionformatSetIterator(ctx,value,recurseTimes,keys){
+constentries=previewEntries(value);
+returnformatSetIterInner(ctx,value,recurseTimes,keys,entries,kIterator);
 }
 
-functionformatSetIterator(ctx,value,recurseTimes,keys){
-returnformatCollectionIterator(ctx,value,recurseTimes,keys);
+functionformatMapIterator(ctx,value,recurseTimes,keys){
+const[entries,isKeyValue]=previewEntries(value,true);
+if(isKeyValue){
+returnformatMapIterInner(
+ctx,value,recurseTimes,keys,entries,kMapEntries);
+}
+
+returnformatSetIterInner(ctx,value,recurseTimes,keys,entries,kIterator);
 }
 
 functionformatPromise(ctx,value,recurseTimes,keys){

src/node_util.cc

@@ -57,6 +57,9 @@ static void PreviewEntries(const FunctionCallbackInfo<Value>& args) {
  Local<Array> entries;
 if (!args[0].As<Object>()->PreviewEntries(&is_key_value).ToLocal(&entries))
 return;
+// Fast path for WeakMap, WeakSet and Set iterators.
+if (args.Length() == 1)
+return args.GetReturnValue().Set(entries);
  Local<Array> ret = Array::New(env->isolate(), 2);
  ret->Set(env->context(), 0, entries).FromJust();
  ret->Set(env->context(), 1, v8::Boolean::New(env->isolate(), is_key_value))

test/parallel/test-util-inspect.js

@@ -447,13 +447,15 @@ assert.strictEqual(util.inspect(-5e-324), '-5e-324');
 {
 constmap=newMap();
 map.set(1,2);
-const[vals]=previewEntries(map.entries());
-constvalsOutput=[];
-for(constoofvals){
-valsOutput.push(o);
-}
-
-assert.strictEqual(util.inspect(valsOutput),'[ 1, 2 ]');
+// Passing only a single argument to indicate a set iterator.
+constvalsSetIterator=previewEntries(map.entries());
+// Passing through true to indicate a map iterator.
+constvalsMapIterEntries=previewEntries(map.entries(),true);
+constvalsMapIterKeys=previewEntries(map.keys(),true);
+
+assert.strictEqual(util.inspect(valsSetIterator),'[ 1, 2 ]');
+assert.strictEqual(util.inspect(valsMapIterEntries),'[ [ 1, 2 ], true ]');
+assert.strictEqual(util.inspect(valsMapIterKeys),'[ [ 1 ], false ]');
 }
 
 // Test for other constructors in different context.
@@ -965,18 +967,19 @@ if (typeof Symbol !== 'undefined') {
 // Test Map iterators.
 {
 constmap=newMap([['foo','bar']]);
-assert.strictEqual(util.inspect(map.keys()),"[Map Iterator] { 'foo' }");
-assert.strictEqual(util.inspect(map.values()),"[Map Iterator] { 'bar' }");
-assert.strictEqual(util.inspect(map.entries()),
-"[Map Iterator] { [ 'foo', 'bar' ] }");
+assert.strictEqual(util.inspect(map.keys()),'[Map Iterator] { \'foo\' }');
+assert.strictEqual(util.inspect(map.values()),'[Map Iterator] { \'bar\' }');
+map.set('A','B!');
+assert.strictEqual(util.inspect(map.entries(),{maxArrayLength: 1}),
+"[Map Iterator] { [ 'foo', 'bar' ], ... 1 more item }");
 // Make sure the iterator doesn't get consumed.
 constkeys=map.keys();
-assert.strictEqual(util.inspect(keys),"[Map Iterator] { 'foo' }");
-assert.strictEqual(util.inspect(keys),"[Map Iterator] { 'foo' }");
+assert.strictEqual(util.inspect(keys),"[Map Iterator] { 'foo', 'A' }");
+assert.strictEqual(util.inspect(keys),"[Map Iterator] { 'foo', 'A' }");
 keys.extra=true;
 assert.strictEqual(
 util.inspect(keys,{maxArrayLength: 0}),
-'[Map Iterator] { ... more items, extra: true }');
+'[Map Iterator] { ... 2 more items, extra: true }');
 }
 
 // Test Set iterators.
@@ -992,7 +995,7 @@ if (typeof Symbol !== 'undefined') {
 keys.extra=true;
 assert.strictEqual(
 util.inspect(keys,{maxArrayLength: 1}),
-'[Set Iterator] { 1, ... more items, extra: true }');
+'[Set Iterator] { 1, ... 1 more item, extra: true }');
 }
 
 // Test alignment of items in container.
@@ -1413,17 +1416,17 @@ util.inspect(process);
 assert.strictEqual(out,expect);
 
 out=util.inspect(weakMap,{maxArrayLength: 0,showHidden: true});
-expect='WeakMap { ... more items }';
+expect='WeakMap { ... 2 more items }';
 assert.strictEqual(out,expect);
 
 weakMap.extra=true;
 out=util.inspect(weakMap,{maxArrayLength: 1,showHidden: true});
 // It is not possible to determine the output reliable.
-expect='WeakMap { [ [length]: 0 ] => {}, ... more items, extra: true }';
-constexpectAlt='WeakMap { {} => [ [length]: 0 ], ... more items, '+
+expect='WeakMap { [ [length]: 0 ] => {}, ... 1 more item, extra: true }';
+constexpectAlt='WeakMap { {} => [ [length]: 0 ], ... 1 more item, '+
 'extra: true }';
 assert(out===expect||out===expectAlt,
-`Found "${out}" rather than "${expect}" or "${expectAlt}"`);
+`Found: "${out}"\nrather than: "${expect}"\nor: "${expectAlt}"`);
 }
 
 {// Test WeakSet
@@ -1437,17 +1440,17 @@ util.inspect(process);
 assert.strictEqual(out,expect);
 
 out=util.inspect(weakSet,{maxArrayLength: -2,showHidden: true});
-expect='WeakSet { ... more items }';
+expect='WeakSet { ... 2 more items }';
 assert.strictEqual(out,expect);
 
 weakSet.extra=true;
 out=util.inspect(weakSet,{maxArrayLength: 1,showHidden: true});
 // It is not possible to determine the output reliable.
-expect='WeakSet { {}, ... more items, extra: true }';
-constexpectAlt='WeakSet { [ 1, [length]: 1 ], ... more items, '+
+expect='WeakSet { {}, ... 1 more item, extra: true }';
+constexpectAlt='WeakSet { [ 1, [length]: 1 ], ... 1 more item, '+
 'extra: true }';
 assert(out===expect||out===expectAlt,
-`Found "${out}" rather than "${expect}" or "${expectAlt}"`);
+`Found: "${out}"\nrather than: "${expect}"\nor: "${expectAlt}"`);
 }
 
 {// Test argument objects.