Skip to content

Commit

Permalink
util: correctly inspect Map/Set Iterators
Browse files Browse the repository at this point in the history
Previously, a MapIterator or SetIterator would
not be inspected properly. This change makes it possible
to inspect them by creating a Debug Mirror and previewing
the iterators to not consume the actual iterator that
we are trying to inspect.

This change also adds a node_util binding that uses
v8's Value::IsSetIterator and Value::IsMapIterator
to verify that the values passed in are actual iterators.

Fixes: #3107
PR-URL: #3119
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
  • Loading branch information
evanlucas committed Oct 6, 2015
1 parent 089d688 commit 8853388
Show file tree
Hide file tree
Showing 4 changed files with 89 additions and 5 deletions.
35 changes: 30 additions & 5 deletions lib/util.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
const uv = process.binding('uv');
const Buffer = require('buffer').Buffer;
const internalUtil = require('internal/util');
const binding = process.binding('util');

var Debug;
var ObjectIsPromise;
Expand Down Expand Up @@ -323,11 +324,23 @@ function formatValue(ctx, value, recurseTimes) {
braces = ['{', '}'];
formatter = formatPromise;
} else {
if (constructor === Object)
constructor = null;
braces = ['{', '}'];
empty = true; // No other data than keys.
formatter = formatObject;
if (binding.isMapIterator(value)) {
constructor = { name: 'MapIterator' };
braces = ['{', '}'];
empty = false;
formatter = formatCollectionIterator;
} else if (binding.isSetIterator(value)) {
constructor = { name: 'SetIterator' };
braces = ['{', '}'];
empty = false;
formatter = formatCollectionIterator;
} else {
if (constructor === Object)
constructor = null;
braces = ['{', '}'];
empty = true; // No other data than keys.
formatter = formatObject;
}
}
}

Expand Down Expand Up @@ -501,6 +514,18 @@ function formatMap(ctx, value, recurseTimes, visibleKeys, keys) {
return output;
}

function formatCollectionIterator(ctx, value, recurseTimes, visibleKeys, keys) {
ensureDebugIsInitialized();
const mirror = Debug.MakeMirror(value, true);
var nextRecurseTimes = recurseTimes === null ? null : recurseTimes - 1;
var vals = mirror.preview();
var output = [];
for (let o of vals) {
output.push(formatValue(ctx, o, nextRecurseTimes));
}
return output;
}

function formatPromise(ctx, value, recurseTimes, visibleKeys, keys) {
var output = [];
var internals = inspectPromise(value);
Expand Down
1 change: 1 addition & 0 deletions node.gyp
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,7 @@
'src/node_javascript.cc',
'src/node_main.cc',
'src/node_os.cc',
'src/node_util.cc',
'src/node_v8.cc',
'src/node_stat_watcher.cc',
'src/node_watchdog.cc',
Expand Down
38 changes: 38 additions & 0 deletions src/node_util.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
#include "node.h"
#include "v8.h"
#include "env.h"
#include "env-inl.h"

namespace node {
namespace util {

using v8::Context;
using v8::FunctionCallbackInfo;
using v8::Local;
using v8::Object;
using v8::Value;

static void IsMapIterator(const FunctionCallbackInfo<Value>& args) {
CHECK_EQ(1, args.Length());
args.GetReturnValue().Set(args[0]->IsMapIterator());
}


static void IsSetIterator(const FunctionCallbackInfo<Value>& args) {
CHECK_EQ(1, args.Length());
args.GetReturnValue().Set(args[0]->IsSetIterator());
}


void Initialize(Local<Object> target,
Local<Value> unused,
Local<Context> context) {
Environment* env = Environment::GetCurrent(context);
env->SetMethod(target, "isMapIterator", IsMapIterator);
env->SetMethod(target, "isSetIterator", IsSetIterator);
}

} // namespace util
} // namespace node

NODE_MODULE_CONTEXT_AWARE_BUILTIN(util, node::util::Initialize)
20 changes: 20 additions & 0 deletions test/parallel/test-util-inspect.js
Original file line number Diff line number Diff line change
Expand Up @@ -287,6 +287,26 @@ global.Promise = function() { this.bar = 42; };
assert.equal(util.inspect(new Promise()), '{ bar: 42 }');
global.Promise = oldPromise;

// Map/Set Iterators
var m = new Map([['foo', 'bar']]);
assert.strictEqual(util.inspect(m.keys()), 'MapIterator { \'foo\' }');
assert.strictEqual(util.inspect(m.values()), 'MapIterator { \'bar\' }');
assert.strictEqual(util.inspect(m.entries()),
'MapIterator { [ \'foo\', \'bar\' ] }');
// make sure the iterator doesn't get consumed
var keys = m.keys();
assert.strictEqual(util.inspect(keys), 'MapIterator { \'foo\' }');
assert.strictEqual(util.inspect(keys), 'MapIterator { \'foo\' }');

var s = new Set([1, 3]);
assert.strictEqual(util.inspect(s.keys()), 'SetIterator { 1, 3 }');
assert.strictEqual(util.inspect(s.values()), 'SetIterator { 1, 3 }');
assert.strictEqual(util.inspect(s.entries()),
'SetIterator { [ 1, 1 ], [ 3, 3 ] }');
// make sure the iterator doesn't get consumed
keys = s.keys();
assert.strictEqual(util.inspect(keys), 'SetIterator { 1, 3 }');
assert.strictEqual(util.inspect(keys), 'SetIterator { 1, 3 }');

// Test alignment of items in container
// Assumes that the first numeric character is the start of an item.
Expand Down

0 comments on commit 8853388

Please sign in to comment.