Skip to content
This repository was archived by the owner on Apr 3, 2024. It is now read-only.

Commit edcfb04

Browse files
Add ScopeMirror traversal to state.js (#142)
Replace the `FrameMirror.localValue` code path with `ScopeMirror` traversal and rewrite the arguments collector to only serialize arguments which are not matched by name with a more deeply-scoped value. Add closure scope type to filter list because of confusion with node.JS requires. Add tests for nested, confounding variable names and try/catch. 03/08/2016-10:07 PST Update to use locals only becuase of 0.12 behaviour. 12/08/2016-12:40 PST Update with full-range of test-cases gathered from bug-bash. Add more testing around locals for node versions less than 1.6. Add more documentation around new ScopeMirror traversal. Add 'this' context extraction and tests. 15/08/2016-10:54 PST Respond to PR feedback
1 parent ab7273d commit edcfb04

File tree

8 files changed

+771
-38
lines changed

8 files changed

+771
-38
lines changed

lib/state.js

Lines changed: 122 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -21,9 +21,16 @@ module.exports = {
2121
evaluate: evaluate
2222
};
2323

24+
var ScopeType = require('vm').runInDebugContext('ScopeType');
2425
var assert = require('assert');
2526
var semver = require('semver');
2627
var util = require('util');
28+
var lodash = require('lodash');
29+
var find = lodash.find;
30+
var transform = lodash.transform;
31+
var remove = lodash.remove;
32+
var flatten = lodash.flatten;
33+
var isEmpty = lodash.isEmpty;
2734

2835
var StatusMessage = require('./apiclasses.js').StatusMessage;
2936

@@ -67,8 +74,8 @@ MESSAGE_TABLE[STRING_LIMIT_MESSAGE_INDEX] =
6774
* @return an object with stackFrames, variableTable, and
6875
* evaluatedExpressions fields
6976
*/
70-
function capture(execState, expressions, config) {
71-
return (new StateResolver(execState, expressions, config)).capture_();
77+
function capture(execState, expressions, config, v8) {
78+
return (new StateResolver(execState, expressions, config, v8)).capture_();
7279
}
7380

7481

@@ -112,10 +119,11 @@ function evaluate(expression, frame) {
112119
* @param {!Object} config
113120
* @constructor
114121
*/
115-
function StateResolver(execState, expressions, config) {
122+
function StateResolver(execState, expressions, config, v8) {
116123
this.state_ = execState;
117124
this.expressions_ = expressions;
118125
this.config_ = config;
126+
this.ctx_ = v8;
119127

120128
this.evaluatedExpressions_ = [];
121129
this.totalSize_ = 0;
@@ -277,14 +285,32 @@ StateResolver.prototype.isPathInNodeModulesDirectory_ = function(path) {
277285
};
278286

279287
StateResolver.prototype.resolveFrame_ = function(frame, resolveVars) {
280-
var args = resolveVars ? this.resolveArgumentList_(frame) : [{
288+
var noArgs = [{
281289
name: 'arguments_not_available',
282290
varTableIndex: ARG_LOCAL_LIMIT_MESSAGE_INDEX
283291
}];
284-
var locals = resolveVars ? this.resolveLocalsList_(frame, args) : [{
292+
var noLocals = [{
285293
name: 'locals_not_available',
286294
varTableIndex: ARG_LOCAL_LIMIT_MESSAGE_INDEX
287295
}];
296+
var args = this.extractArgumentsList_(frame);
297+
var locals = resolveVars ? this.resolveLocalsList_(frame, args) : noLocals;
298+
if (isEmpty(locals)) {
299+
locals = noLocals;
300+
}
301+
if (semver.satisfies(process.version, '<1.6')) {
302+
// If the node version is over 1.6 we do not read the frame arguments since
303+
// the values produced by the frame for the arguments may contain inaccurate
304+
// values. If the version is lower than 1.6, though, the frame's argument
305+
// list can be relied upon to produce accurate values for arguments.
306+
args = resolveVars && (!isEmpty(args)) ? this.resolveArgumentList_(args) :
307+
noArgs;
308+
} else {
309+
// Otherwise, if the version is 1.6 or higher than we will use the values
310+
// aggregated from the ScopeMirror traversal stored in locals which will
311+
// include any applicable arguments from the invocation.
312+
args = noArgs;
313+
}
288314
return {
289315
function: this.resolveFunctionName_(frame.func()),
290316
location: this.resolveLocation_(frame),
@@ -308,36 +334,108 @@ StateResolver.prototype.resolveLocation_ = function(frame) {
308334
};
309335
};
310336

311-
StateResolver.prototype.resolveArgumentList_ = function(frame) {
337+
StateResolver.prototype.extractArgumentsList_ = function (frame) {
312338
var args = [];
313339
for (var i = 0; i < frame.argumentCount(); i++) {
314340
// Don't resolve unnamed arguments.
315341
if (!frame.argumentName(i)) {
316342
continue;
317343
}
318-
args.push(this.resolveVariable_(
319-
frame.argumentName(i), frame.argumentValue(i)));
344+
args.push({name: frame.argumentName(i), value: frame.argumentValue(i)});
320345
}
321346
return args;
322347
};
323348

324-
StateResolver.prototype.resolveLocalsList_ = function(frame,
325-
resolvedArguments) {
326-
var locals = [];
327-
// Arguments may have been captured as locals in a nested closure.
328-
// We filter them out here.
329-
var predicate = function(localEntry, argEntry) {
330-
return argEntry.varTableIndex === localEntry.varTableIndex;
331-
};
332-
for (var i = 0; i < frame.localCount(); i++) {
333-
var localEntry = this.resolveVariable_(
334-
frame.localName(i), frame.localValue(i));
335-
if (!resolvedArguments.some(predicate.bind(null, localEntry))) {
336-
locals.push(this.resolveVariable_(
337-
frame.localName(i), frame.localValue(i)));
349+
StateResolver.prototype.resolveArgumentList_ = function(args) {
350+
var resolveVariable = this.resolveVariable_.bind(this);
351+
return args.map(function (arg){
352+
return resolveVariable(arg.name, arg.value);
353+
});
354+
};
355+
356+
/**
357+
* Iterates and returns variable information for all scopes (excluding global)
358+
* in a given frame. FrameMirrors should return their scope object list with
359+
* most deeply nested scope first so variables initially encountered will take
360+
* precedence over subsequent instance with the same name - this is tracked in
361+
* the usedNames map. The argument list given to this function may be
362+
* manipulated if variables with a deeper scope occur which have the same name.
363+
* @function resolveLocalsList_
364+
* @memberof StateResolver
365+
* @param {FrameMirror} frame - A instance of FrameMirror
366+
* @param {Array<Object>} args - An array of objects representing any function
367+
* arguments the frame may list
368+
* @returns {Array<Object>} - returns an array containing data about selected
369+
* variables
370+
*/
371+
StateResolver.prototype.resolveLocalsList_ = function (frame, args) {
372+
var self = this;
373+
var usedNames = {};
374+
var makeMirror = this.ctx_.MakeMirror;
375+
return flatten(frame.allScopes().map(
376+
function (scope) {
377+
switch (scope.scopeType()) {
378+
case ScopeType.Global:
379+
// We do not capture globals in the debug client.
380+
case ScopeType.Closure:
381+
// The closure scope is contaminated by Node.JS's require IIFE pattern
382+
// and, if traversed, will cause local variable pools to include what
383+
// are considered node 'globals'. Therefore, avoid traversal.
384+
return [];
385+
}
386+
return transform(
387+
scope.details().object(),
388+
function (locals, value, name) {
389+
var trg = makeMirror(value);
390+
var argMatch = find(args, {name: name});
391+
if (argMatch && (semver.satisfies(process.version, '<1.6'))) {
392+
// If the version is lower than 1.6 we will use the frame's argument
393+
// list to source argument values, yet the ScopeMirror traversal for
394+
// these Node versions will also return the arguments. Therefore, on
395+
// these versions, compare the value sourced as the argument from
396+
// the FrameMirror to the argument found in the ScopeMirror locals
397+
// list with the same name and attempt to determine whether or not
398+
// they have the same value. If each of these items has the same
399+
// name and value we may assume that the ScopeMirror variable
400+
// representation is merely a duplicate of the FrameMirror's
401+
// variable representation. Otherwise, the variable may have been
402+
// redeclared or reassigned in the function and is therefore a local
403+
// triggering removal from the arguments list and insertion into the
404+
// locals list.
405+
if (argMatch.value.value() === trg.value()) {
406+
// Argument ref is the same ref as the local ref - this is an
407+
// argument do not push this into the locals list
408+
return locals;
409+
}
410+
// There is another local/scope var with the same name and it is not
411+
// the argument so this will take precedence. Remove the same-named
412+
// entry from the arguments list and push the local value onto the
413+
// locals list.
414+
remove(args, {name: name});
415+
usedNames[name] = true;
416+
locals.push(self.resolveVariable_(name, trg));
417+
} else if (!usedNames[name]) {
418+
// It's a valid variable that belongs in the locals list and wasn't
419+
// discovered at a lower-scope
420+
usedNames[name] = true;
421+
locals.push(self.resolveVariable_(name, trg));
422+
} // otherwise another same-named variable occured at a lower scope
423+
return locals;
424+
},
425+
[]
426+
);
338427
}
339-
}
340-
return locals;
428+
)).concat((function () {
429+
// The frame receiver is the 'this' context that is present during
430+
// invocation. Check to see whether a receiver context is substantive,
431+
// (invocations may be bound to null) if so: store in the locals list
432+
// under the name 'context' which is used by the Chrome DevTools.
433+
var ctx = frame.details().receiver();
434+
if (ctx) {
435+
return [self.resolveVariable_('context', makeMirror(ctx))];
436+
}
437+
return [];
438+
}()));
341439
};
342440

343441
/**

lib/v8debugapi.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -551,7 +551,7 @@ module.exports.create = function(logger_, config_, fileStats_) {
551551
breakpoint.evaluatedExpressions = evaluatedExpressions;
552552
}
553553
} else {
554-
var captured = state.capture(execState, breakpoint.expressions, config);
554+
var captured = state.capture(execState, breakpoint.expressions, config, v8);
555555
breakpoint.stackFrames = captured.stackFrames;
556556
breakpoint.variableTable = captured.variableTable;
557557
breakpoint.evaluatedExpressions =

test/fixtures/fat-arrow.js

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
/*1* KEEP THIS CODE AT THE TOP TO AVOID LINE NUMBER CHANGES */ /* jshint shadow:true */
2+
/*2*/'use strict';
3+
/*3*/function foo() {
4+
/*4*/ var a = (b) => {
5+
/*5*/ b += 1;
6+
/*6*/ return b;
7+
/*7*/ };
8+
/*8*/ return a(1);
9+
/*9*/}
10+
/*10*/module.exports = foo;
11+
/*11*/

0 commit comments

Comments
 (0)