Add ScopeMirror traversal to state.js

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.

lib/state.js

@@ -24,6 +24,11 @@ module.exports = {
 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 StatusMessage = require('./apiclasses.js').StatusMessage;
 
@@ -67,8 +72,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 +117,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 +283,23 @@ StateResolver.prototype.isPathInNodeModulesDirectory_ = function(path) {
 };
 
 StateResolver.prototype.resolveFrame_ = function(frame, resolveVars) {
-  var args = resolveVars ? this.resolveArgumentList_(frame) : [{
-    name: 'arguments_not_available',
-    varTableIndex: ARG_LOCAL_LIMIT_MESSAGE_INDEX
-  }];
+  var args = this.extractArgumentsList_(frame);
   var locals = resolveVars ? this.resolveLocalsList_(frame, args) : [{
     name: 'locals_not_available',
     varTableIndex: ARG_LOCAL_LIMIT_MESSAGE_INDEX
   }];
+  if (semver.satisfies(process.version, '<1.6')) {
+    args = resolveVars && (args.length > 0) ? this.resolveArgumentList_(args) : [{
+      name: 'arguments_not_available',
+      varTableIndex: ARG_LOCAL_LIMIT_MESSAGE_INDEX
+    }];
+  } else {
+    args = [{
+      name: 'arguments_not_available',
+      varTableIndex: ARG_LOCAL_LIMIT_MESSAGE_INDEX
+    }];
+  }
+
   return {
     function: this.resolveFunctionName_(frame.func()),
     location: this.resolveLocation_(frame),
@@ -308,36 +323,82 @@ 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 usedNames = {};
+  var makeMirror = this.ctx_.MakeMirror;
+  // resolveVariable_ accesses instance properties - bind to instance
+  var resolveVariable = this.resolveVariable_.bind(this);
+  return flatten(frame.allScopes().map(
+    function (scope) {
+      switch (scope.scopeType()) {
+        case 0: // GLOBAL
+        case 3: // CLOSURE
+          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 (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(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(resolveVariable(name, trg));
+          } // otherwise another same-named variable occured at a lower scope
+          return locals;
+        },
+        []
+      );
     }
-  }
-  return locals;
+  ));
 };
 
 /**

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/standalone/test-duplicate-nested-expressions.js

@@ -0,0 +1,198 @@
+/*1* KEEP THIS CODE AT THE TOP TO AVOID LINE NUMBER CHANGES */ /* jshint shadow:true */
+/*2*/'use strict';
+/*3*/function foo(a) {
+/*4*/ var a = 10;
+/*5*/ a += 1;
+/*6*/ return (function (b) {
+/*7*/   var a = 10;
+/*8*/   return a;
+/*9*/ }());
+/*10*/}
+/**
+ * Copyright 2015 Google Inc. All Rights Reserved.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+var assert = require('assert');
+var v8debugapi = require('../../lib/v8debugapi.js');
+var logModule = require('@google/cloud-diagnostics-common').logger;
+var config = require('../../config.js').debug;
+var scanner = require('../../lib/scanner.js');
+var path = require('path');
+var semver = require('semver');
+
+function stateIsClean(api) {
+  assert.equal(api.numBreakpoints_(), 0,
+    'there should be no breakpoints active');
+  assert.equal(api.numListeners_(), 0,
+    'there should be no listeners active');
+  return true;
+}
+
+describe('v8debugapi', function() {
+  config.workingDirectory = path.join(process.cwd(), 'test', 'standalone');
+  var logger = logModule.create(config.logLevel);
+  var api = null;
+
+  beforeEach(function(done) {
+    if (!api) {
+      scanner.scan(true, config.workingDirectory, function(err, fileStats, hash) {
+        assert(!err);
+        api = v8debugapi.create(logger, config, fileStats);
+        assert.ok(api, 'should be able to create the api');
+        done();
+      });
+    } else {
+      assert(stateIsClean(api));
+      done();
+    }
+  });
+  afterEach(function() { assert(stateIsClean(api)); });
+  it('Should read the argument before the name is confounded', function(done) {
+      var brk = {
+        id: 'fake-id-123',
+        location: { path: 'test-duplicate-nested-expressions.js', line: 4 }
+      };
+    api.set(brk, function(err) {
+      assert.ifError(err);
+      api.wait(brk, function(err) {
+        assert.ifError(err);
+        var frame = brk.stackFrames[0];
+        var args = frame.arguments;
+        var locals = frame.locals;
+        if (semver.satisfies(process.version, '<1.6')) {
+            assert.equal(args.length, 1, 'There should be one argument');
+            assert.deepEqual(
+              args[0],
+              {name: 'a', value: '11'}
+   	   	    );
+        } else {
+          assert.equal(args.length, 1, 'There should be one argument');
+          assert.equal(args[0].name, 'arguments_not_available');
+          assert.equal(locals.length, 1, 'There should be one locals');
+          assert.deepEqual(
+            locals[0],
+            {name: 'a', value: 'test'}
+   	      );
+        }
+        api.clear(brk);
+        done();
+      });
+      process.nextTick(foo.bind(null, 'test'));
+    });
+  });
+
+  it('Should read an argument after the name is confounded', function(done) {
+    var brk = {
+      id: 'fake-id-1234',
+      location: { path: 'test-duplicate-nested-expressions.js', line: 5 }
+    };
+    api.set(brk, function(err) {
+      assert.ifError(err);
+      api.wait(brk, function(err) {
+        assert.ifError(err);
+        var frame = brk.stackFrames[0];
+        var args = frame.arguments;
+        var locals = frame.locals;
+        if (semver.satisfies(process.version, '<1.6')) {
+            assert.equal(args.length, 1, 'There should be one argument');
+            assert.deepEqual(
+              args[0],
+              {name: 'a', value: '11'}
+   	   	    );
+        } else {
+          assert.equal(args.length, 1, 'There should be one argument');
+          assert.equal(args[0].name, 'arguments_not_available');
+          assert.equal(locals.length, 1, 'There should be one local');
+          assert.deepEqual(
+            locals[0],
+            {name: 'a', value: '10'}
+          );
+        }
+        api.clear(brk);
+        done();
+      });
+      process.nextTick(foo.bind(null, 'test'));
+    });
+  });
+
+  it('Should read an argument value after its value is modified', function(done) {
+    var brk = {
+      id: 'fake-id-1234',
+      location: { path: 'test-duplicate-nested-expressions.js', line: 6 }
+    };
+    api.set(brk, function(err) {
+      assert.ifError(err);
+      api.wait(brk, function(err) {
+        assert.ifError(err);
+        var frame = brk.stackFrames[0];
+        var args = frame.arguments;
+        var locals = frame.locals;
+        if (semver.satisfies(process.version, '<1.6')) {
+            assert.equal(args.length, 1, 'There should be one argument');
+            assert.deepEqual(
+              args[0],
+              {name: 'a', value: '11'}
+   	   	    );
+        } else {
+          assert.equal(args.length, 1, 'There should be one argument');
+          assert.equal(args[0].name, 'arguments_not_available');
+          assert.equal(locals.length, 1, 'There should be one local');
+          assert.deepEqual(
+            locals[0],
+            {name: 'a', value: '11'}
+          );
+        }
+        api.clear(brk);
+        done();
+      });
+      process.nextTick(foo.bind(null, 'test'));
+    });
+  });
+
+  it('Should represent a var name at its local-scope when clearly defined', function(done) {
+    var brk = {
+      id: 'fake-id-1234',
+      location: { path: 'test-duplicate-nested-expressions.js', line: 8 }
+    };
+    if (semver.satisfies(process.version, '<1.6')) {
+      // this IIFE test does not work on 0.12
+      done();
+      return;
+    }
+    api.set(brk, function(err) {
+      assert.ifError(err);
+      api.wait(brk, function(err) {
+        assert.ifError(err);
+        var frame = brk.stackFrames[0];
+        var args = frame.arguments;
+        var locals = frame.locals;
+        assert.equal(args.length, 1, 'There should be one argument');
+        assert.equal(args[0].name, 'arguments_not_available');
+        assert.equal(locals.length, 2, 'There should be two locals');
+        assert.deepEqual(
+          locals[0],
+          {name: 'b', value: 'undefined'}
+        );
+        assert.deepEqual(
+          locals[1],
+          {name: 'a', value: 10}
+        );
+        api.clear(brk);
+        done();
+      });
+      process.nextTick(foo.bind(null, 'test'));
+    });
+  });
+});