util: change sparse arrays inspection format

Missing elements in sparse arrays used to be serialized to empty
placeholders delimited with commas by util.inspect() and in some cases
the result was a syntactically correct representation of a JavaScript
array with shorter length than the original one. This commit implements
@TimothyGu's suggestion to change the way util.inspect() formats sparse
arrays to something similar to how Firefox shows them.

Fixes: https://github.com/nodejs/node/issues/11570
PR-URL: https://github.com/nodejs/node/pull/11576
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Timothy Gu <timothygu99@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>

lib/util.js

@@ -626,16 +626,26 @@ function formatObject(ctx, value, recurseTimes, visibleKeys, keys) {
 
 function formatArray(ctx, value, recurseTimes, visibleKeys, keys) {
   var output = [];
-  const maxLength = Math.min(Math.max(0, ctx.maxArrayLength), value.length);
-  const remaining = value.length - maxLength;
-  for (var i = 0; i < maxLength; ++i) {
-    if (hasOwnProperty(value, String(i))) {
-      output.push(formatProperty(ctx, value, recurseTimes, visibleKeys,
-                                 String(i), true));
+  let visibleLength = 0;
+  let index = 0;
+  while (index < value.length && visibleLength < ctx.maxArrayLength) {
+    let emptyItems = 0;
+    while (index < value.length && !hasOwnProperty(value, String(index))) {
+      emptyItems++;
+      index++;
+    }
+    if (emptyItems > 0) {
+      const ending = emptyItems > 1 ? 's' : '';
+      const message = `<${emptyItems} empty item${ending}>`;
+      output.push(ctx.stylize(message, 'undefined'));
     } else {
-      output.push('');
+      output.push(formatProperty(ctx, value, recurseTimes, visibleKeys,
+                                 String(index), true));
+      index++;
     }
+    visibleLength++;
   }
+  const remaining = value.length - index;
   if (remaining > 0) {
     output.push(`... ${remaining} more item${remaining > 1 ? 's' : ''}`);
   }

test/parallel/test-util-inspect.js

@@ -295,12 +295,18 @@ assert.strictEqual(util.inspect(-0), '-0');
 const a = ['foo', 'bar', 'baz'];
 assert.strictEqual(util.inspect(a), '[ \'foo\', \'bar\', \'baz\' ]');
 delete a[1];
-assert.strictEqual(util.inspect(a), '[ \'foo\', , \'baz\' ]');
+assert.strictEqual(util.inspect(a), '[ \'foo\', <1 empty item>, \'baz\' ]');
 assert.strictEqual(
   util.inspect(a, true),
-  '[ \'foo\', , \'baz\', [length]: 3 ]'
+  '[ \'foo\', <1 empty item>, \'baz\', [length]: 3 ]'
+);
+assert.strictEqual(util.inspect(new Array(5)), '[ <5 empty items> ]');
+a[3] = 'bar';
+a[100] = 'qux';
+assert.strictEqual(
+  util.inspect(a, { breakLength: Infinity }),
+  '[ \'foo\', <1 empty item>, \'baz\', \'bar\', <96 empty items>, \'qux\' ]'
 );
-assert.strictEqual(util.inspect(new Array(5)), '[ , , , ,  ]');
 
 // test for Array constructor in different context
 {
@@ -835,15 +841,21 @@ checkAlignment(new Map(big_array.map(function(y) { return [y, null]; })));
 // Do not backport to v5/v4 unless all of
 // https://github.com/nodejs/node/pull/6334 is backported.
 {
-  const x = Array(101);
+  const x = new Array(101).fill();
   assert(/1 more item/.test(util.inspect(x)));
 }
 
 {
-  const x = Array(101);
+  const x = new Array(101).fill();
   assert(!/1 more item/.test(util.inspect(x, {maxArrayLength: 101})));
 }
 
+{
+  const x = new Array(101).fill();
+  assert(/^\[ ... 101 more items ]$/.test(
+      util.inspect(x, {maxArrayLength: 0})));
+}
+
 {
   const x = Array(101);
   assert(/^\[ ... 101 more items ]$/.test(
@@ -901,7 +913,7 @@ checkAlignment(new Map(big_array.map(function(y) { return [y, null]; })));
 
 // util.inspect.defaultOptions tests
 {
-  const arr = Array(101);
+  const arr = new Array(101).fill();
   const obj = {a: {a: {a: {a: 1}}}};
 
   const oldOptions = Object.assign({}, util.inspect.defaultOptions);