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

lib/state.js

@@ -21,9 +21,16 @@ module.exports = {
   evaluate: evaluate
 };
 
+var ScopeType = require('vm').runInDebugContext('ScopeType');
 var assert = require('assert');
 var semver = require('semver');
 var util = require('util');
+var lodash = require('lodash');
+var find = lodash.find;
+var transform = lodash.transform;
+var remove = lodash.remove;
+var flatten = lodash.flatten;
+var isEmpty = lodash.isEmpty;
 
 var StatusMessage = require('./apiclasses.js').StatusMessage;
 
@@ -67,8 +74,8 @@ MESSAGE_TABLE[STRING_LIMIT_MESSAGE_INDEX] =
  * @return an object with stackFrames, variableTable, and
  *         evaluatedExpressions fields
  */
-function capture(execState, expressions, config) {
-  return (new StateResolver(execState, expressions, config)).capture_();
+function capture(execState, expressions, config, v8) {
+  return (new StateResolver(execState, expressions, config, v8)).capture_();
 }
 
 
@@ -112,10 +119,11 @@ function evaluate(expression, frame) {
  * @param {!Object} config
  * @constructor
  */
-function StateResolver(execState, expressions, config) {
+function StateResolver(execState, expressions, config, v8) {
   this.state_ = execState;
   this.expressions_ = expressions;
   this.config_ = config;
+  this.ctx_ = v8;
 
   this.evaluatedExpressions_ = [];
   this.totalSize_ = 0;
@@ -277,14 +285,32 @@ StateResolver.prototype.isPathInNodeModulesDirectory_ = function(path) {
 };
 
 StateResolver.prototype.resolveFrame_ = function(frame, resolveVars) {
-  var args = resolveVars ? this.resolveArgumentList_(frame) : [{
+  var noArgs = [{
     name: 'arguments_not_available',
     varTableIndex: ARG_LOCAL_LIMIT_MESSAGE_INDEX
   }];
-  var locals = resolveVars ? this.resolveLocalsList_(frame, args) : [{
+  var noLocals = [{
     name: 'locals_not_available',
     varTableIndex: ARG_LOCAL_LIMIT_MESSAGE_INDEX
   }];
+  var args = this.extractArgumentsList_(frame);
+  var locals = resolveVars ? this.resolveLocalsList_(frame, args) : noLocals;
+  if (isEmpty(locals)) {
+    locals = noLocals;
+  }
+  if (semver.satisfies(process.version, '<1.6')) {
+    // If the node version is over 1.6 we do not read the frame arguments since
+    // the values produced by the frame for the arguments may contain inaccurate
+    // values. If the version is lower than 1.6, though, the frame's argument
+    // list can be relied upon to produce accurate values for arguments.
+    args = resolveVars && (!isEmpty(args)) ? this.resolveArgumentList_(args) :
+      noArgs;
+  } else {
+    // Otherwise, if the version is 1.6 or higher than we will use the values
+    // aggregated from the ScopeMirror traversal stored in locals which will
+    // include any applicable arguments from the invocation.
+    args = noArgs;
+  }
   return {
     function: this.resolveFunctionName_(frame.func()),
     location: this.resolveLocation_(frame),
@@ -308,36 +334,108 @@ StateResolver.prototype.resolveLocation_ = function(frame) {
   };
 };
 
-StateResolver.prototype.resolveArgumentList_ = function(frame) {
+StateResolver.prototype.extractArgumentsList_ = function (frame) {
   var args = [];
   for (var i = 0; i < frame.argumentCount(); i++) {
     // Don't resolve unnamed arguments.
     if (!frame.argumentName(i)) {
       continue;
     }
-    args.push(this.resolveVariable_(
-        frame.argumentName(i), frame.argumentValue(i)));
+    args.push({name: frame.argumentName(i), value: frame.argumentValue(i)});
   }
   return args;
 };
 
-StateResolver.prototype.resolveLocalsList_ = function(frame,
-    resolvedArguments) {
-  var locals = [];
-  // Arguments may have been captured as locals in a nested closure.
-  // We filter them out here.
-  var predicate = function(localEntry, argEntry) {
-    return argEntry.varTableIndex === localEntry.varTableIndex;
-  };
-  for (var i = 0; i < frame.localCount(); i++) {
-    var localEntry = this.resolveVariable_(
-        frame.localName(i), frame.localValue(i));
-    if (!resolvedArguments.some(predicate.bind(null, localEntry))) {
-      locals.push(this.resolveVariable_(
-        frame.localName(i), frame.localValue(i)));
+StateResolver.prototype.resolveArgumentList_ = function(args) {
+  var resolveVariable = this.resolveVariable_.bind(this);
+  return args.map(function (arg){
+    return resolveVariable(arg.name, arg.value);
+  });
+};
+
+/**
+ * Iterates and returns variable information for all scopes (excluding global)
+ * in a given frame. FrameMirrors should return their scope object list with 
+ * most deeply nested scope first so variables initially encountered will take
+ * precedence over subsequent instance with the same name - this is tracked in
+ * the usedNames map. The argument list given to this function may be
+ * manipulated if variables with a deeper scope occur which have the same name.
+ * @function resolveLocalsList_
+ * @memberof StateResolver
+ * @param {FrameMirror} frame - A instance of FrameMirror
+ * @param {Array<Object>} args - An array of objects representing any function
+ *  arguments the frame may list
+ * @returns {Array<Object>} - returns an array containing data about selected
+ *  variables
+ */
+StateResolver.prototype.resolveLocalsList_ = function (frame, args) {
+  var self = this;
+  var usedNames = {};
+  var makeMirror = this.ctx_.MakeMirror;
+  return flatten(frame.allScopes().map(
+    function (scope) {
+      switch (scope.scopeType()) {
+        case ScopeType.Global:
+          // We do not capture globals in the debug client.
+        case ScopeType.Closure:
+          // The closure scope is contaminated by Node.JS's require IIFE pattern
+          // and, if traversed, will cause local variable pools to include what
+          // are considered node 'globals'. Therefore, avoid traversal.
+          return [];
+      }
+      return transform(
+        scope.details().object(),
+        function (locals, value, name) {
+          var trg = makeMirror(value);
+          var argMatch = find(args, {name: name});
+          if (argMatch && (semver.satisfies(process.version, '<1.6'))) {
+            // If the version is lower than 1.6 we will use the frame's argument
+            // list to source argument values, yet the ScopeMirror traversal for
+            // these Node versions will also return the arguments. Therefore, on
+            // these versions, compare the value sourced as the argument from 
+            // the FrameMirror to the argument found in the ScopeMirror locals 
+            // list with the same name and attempt to determine whether or not 
+            // they have the same value. If each of these items has the same
+            // name and value we may assume that the ScopeMirror variable 
+            // representation is merely a duplicate of the FrameMirror's 
+            // variable representation. Otherwise, the variable may have been
+            // redeclared or reassigned in the function and is therefore a local
+            // triggering removal from the arguments list and insertion into the
+            // locals list.
+            if (argMatch.value.value() === trg.value()) {
+              // Argument ref is the same ref as the local ref - this is an
+              // argument do not push this into the locals list
+              return locals;
+            }
+            // There is another local/scope var with the same name and it is not 
+            // the argument so this will take precedence. Remove the same-named 
+            // entry from the arguments list and push the local value onto the
+            // locals list.
+            remove(args, {name: name});
+            usedNames[name] = true;
+            locals.push(self.resolveVariable_(name, trg));
+          } else if (!usedNames[name]) {
+            // It's a valid variable that belongs in the locals list and wasn't
+            // discovered at a lower-scope
+            usedNames[name] = true;
+            locals.push(self.resolveVariable_(name, trg));
+          } // otherwise another same-named variable occured at a lower scope
+          return locals;
+        },
+        []
+      );
     }
-  }
-  return locals;
+  )).concat((function () {
+    // The frame receiver is the 'this' context that is present during 
+    // invocation. Check to see whether a receiver context is substantive,
+    // (invocations may be bound to null) if so: store in the locals list
+    // under the name 'context' which is used by the Chrome DevTools.
+    var ctx = frame.details().receiver();
+    if (ctx) {
+      return [self.resolveVariable_('context', makeMirror(ctx))];
+    }
+    return [];
+  }()));
 };
 
 /**

lib/v8debugapi.js

@@ -551,7 +551,7 @@ module.exports.create = function(logger_, config_, fileStats_) {
         breakpoint.evaluatedExpressions = evaluatedExpressions;
       }
     } else {
-      var captured = state.capture(execState, breakpoint.expressions, config);
+      var captured = state.capture(execState, breakpoint.expressions, config, v8);
       breakpoint.stackFrames = captured.stackFrames;
       breakpoint.variableTable = captured.variableTable;
       breakpoint.evaluatedExpressions =

test/fixtures/fat-arrow.js

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