Bug 1207868 - Fix lexical scope autocomplete for global 'let' and 'const';r=past
authorBrian Grinstead <bgrinstead@mozilla.com>
Thu, 03 Dec 2015 11:35:33 -0800
changeset 309649 ca30d9070e53f75b90b9710d2533bb5bf0a74240
parent 309648 b8249000cec9855cf25054abe8870a25bf0bbc3c
child 309650 74d91f50bad6807a8bfc7d9f89ee724e2502ec35
push id5513
push userraliiev@mozilla.com
push dateMon, 25 Jan 2016 13:55:34 +0000
treeherdermozilla-beta@5ee97dd05b5c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerspast
bugs1207868
milestone45.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1207868 - Fix lexical scope autocomplete for global 'let' and 'const';r=past
devtools/client/webconsole/test/browser_webconsole_autocomplete-properties-with-non-alphanumeric-names.js
devtools/client/webconsole/test/browser_webconsole_bug_632347_iterators_generators.js
devtools/client/webconsole/test/browser_webconsole_property_provider.js
devtools/server/actors/webconsole.js
devtools/shared/webconsole/js-property-provider.js
devtools/shared/webconsole/test/unit/test_js_property_provider.js
--- a/devtools/client/webconsole/test/browser_webconsole_autocomplete-properties-with-non-alphanumeric-names.js
+++ b/devtools/client/webconsole/test/browser_webconsole_autocomplete-properties-with-non-alphanumeric-names.js
@@ -17,28 +17,32 @@ var test = asyncTest(function*() {
   function* autocomplete(term) {
     let deferred = promise.defer();
 
     jsterm.setInputValue(term);
     jsterm.complete(jsterm.COMPLETE_HINT_ONLY, deferred.resolve);
 
     yield deferred.promise;
 
-    ok(popup.itemCount > 0, "There's suggestions for '" + term + "'");
+    ok(popup.itemCount > 0,
+       "There's " + popup.itemCount + " suggestions for '" + term + "'");
   }
 
   let { jsterm } = yield openConsole();
   let popup = jsterm.autocompletePopup;
 
   yield jsterm.execute("var testObject = {$$aaab: '', $$aaac: ''}");
 
-  // FIXMEshu: global lexicals can't be autocompleted without extra platform
-  // support. See bug 1207868.
-  //yield jsterm.execute("let testObject = {$$aaab: '', $$aaac: ''}");
-
   // Should work with bug 967468.
   yield autocomplete("Object.__d");
   yield autocomplete("testObject.$$a");
 
   // Here's when things go wrong in bug 967468.
   yield autocomplete("Object.__de");
   yield autocomplete("testObject.$$aa");
+
+  // Should work with bug 1207868.
+  yield jsterm.execute("let foobar = {a: ''}; const blargh = {a: 1};");
+  yield autocomplete("foobar");
+  yield autocomplete("blargh");
+  yield autocomplete("foobar.a");
+  yield autocomplete("blargh.a");
 });
--- a/devtools/client/webconsole/test/browser_webconsole_bug_632347_iterators_generators.js
+++ b/devtools/client/webconsole/test/browser_webconsole_bug_632347_iterators_generators.js
@@ -20,17 +20,17 @@ function consoleOpened(HUD) {
   let {JSPropertyProvider} = require("devtools/shared/webconsole/js-property-provider");
 
   let tmp = Cu.import("resource://gre/modules/jsdebugger.jsm", {});
   tmp.addDebuggerToGlobal(tmp);
   let dbg = new tmp.Debugger();
 
   let jsterm = HUD.jsterm;
   let win = content.wrappedJSObject;
-  let dbgWindow = dbg.makeGlobalObjectReference(win);
+  let dbgWindow = dbg.addDebuggee(content);
   let container = win._container;
 
   // Make sure autocomplete does not walk through iterators and generators.
   let result = container.gen1.next();
   let completion = JSPropertyProvider(dbgWindow, null, "_container.gen1.");
   isnot(completion.matches.length, 0, "Got matches for gen1");
 
   is(result + 1, container.gen1.next(), "gen1.next() did not execute");
@@ -57,16 +57,17 @@ function consoleOpened(HUD) {
   let dbgContent = dbg.makeGlobalObjectReference(content);
   completion = JSPropertyProvider(dbgContent, null, "_container.iter2.");
   isnot(completion.matches.length, 0, "Got matches for iter2");
 
   completion = JSPropertyProvider(dbgWindow, null, "window._container.");
   ok(completion, "matches available for window._container");
   ok(completion.matches.length, "matches available for window (length)");
 
+  dbg.removeDebuggee(content);
   jsterm.clearOutput();
 
   jsterm.execute("window._container", (msg) => {
     jsterm.once("variablesview-fetched", testVariablesView.bind(null, HUD));
     let anchor = msg.querySelector(".message-body a");
     EventUtils.synthesizeMouse(anchor, 2, 2, {}, HUD.iframeWindow);
   });
 }
--- a/devtools/client/webconsole/test/browser_webconsole_property_provider.js
+++ b/devtools/client/webconsole/test/browser_webconsole_property_provider.js
@@ -16,17 +16,17 @@ function test() {
 
 function testPropertyProvider({browser}) {
   browser.removeEventListener("load", testPropertyProvider, true);
   let {JSPropertyProvider} = require("devtools/shared/webconsole/js-property-provider");
 
   let tmp = Cu.import("resource://gre/modules/jsdebugger.jsm", {});
   tmp.addDebuggerToGlobal(tmp);
   let dbg = new tmp.Debugger();
-  let dbgWindow = dbg.makeGlobalObjectReference(content);
+  let dbgWindow = dbg.addDebuggee(content);
 
   let completion = JSPropertyProvider(dbgWindow, null, "thisIsNotDefined");
   is(completion.matches.length, 0, "no match for 'thisIsNotDefined");
 
   // This is a case the PropertyProvider can't handle. Should return null.
   completion = JSPropertyProvider(dbgWindow, null, "window[1].acb");
   is(completion, null, "no match for 'window[1].acb");
 
@@ -36,10 +36,11 @@ function testPropertyProvider({browser})
   completion = JSPropertyProvider(dbgWindow, null, strComplete);
   ok(completion.matches.length == 2, "two matches found");
   ok(completion.matchProp == "locatio", "matching part is 'test'");
   let matches = completion.matches;
   matches.sort();
   ok(matches[0] == "location", "the first match is 'location'");
   ok(matches[1] == "locationbar", "the second match is 'locationbar'");
 
+  dbg.removeDebuggee(content);
   finishTest();
 }
--- a/devtools/server/actors/webconsole.js
+++ b/devtools/server/actors/webconsole.js
@@ -900,36 +900,43 @@ WebConsoleActor.prototype =
    * @return object
    *         The response message - matched properties.
    */
   onAutocomplete: function WCA_onAutocomplete(aRequest)
   {
     let frameActorId = aRequest.frameActor;
     let dbgObject = null;
     let environment = null;
+    let hadDebuggee = false;
 
     // This is the case of the paused debugger
     if (frameActorId) {
       let frameActor = this.conn.getActor(frameActorId);
       if (frameActor) {
         let frame = frameActor.frame;
         environment = frame.environment;
       }
       else {
         DevToolsUtils.reportException("onAutocomplete",
           Error("The frame actor was not found: " + frameActorId));
       }
     }
     // This is the general case (non-paused debugger)
     else {
-      dbgObject = this.dbg.makeGlobalObjectReference(this.evalWindow);
+      hadDebuggee = this.dbg.hasDebuggee(this.evalWindow);
+      dbgObject = this.dbg.addDebuggee(this.evalWindow);
     }
 
     let result = JSPropertyProvider(dbgObject, environment, aRequest.text,
                                     aRequest.cursor, frameActorId) || {};
+
+    if (!hadDebuggee && dbgObject) {
+      this.dbg.removeDebuggee(this.evalWindow);
+    }
+
     let matches = result.matches || [];
     let reqText = aRequest.text.substr(0, aRequest.cursor);
 
     // We consider '$' as alphanumerc because it is used in the names of some
     // helper functions.
     let lastNonAlphaIsDot = /[.][a-zA-Z0-9$]*$/.test(reqText);
     if (!lastNonAlphaIsDot) {
       if (!this._webConsoleCommandsCache) {
--- a/devtools/shared/webconsole/js-property-provider.js
+++ b/devtools/shared/webconsole/js-property-provider.js
@@ -27,16 +27,20 @@ const STATE_DQUOTE = 3;
 const OPEN_BODY = "{[(".split("");
 const CLOSE_BODY = "}])".split("");
 const OPEN_CLOSE_BODY = {
   "{": "}",
   "[": "]",
   "(": ")",
 };
 
+function hasArrayIndex(str) {
+  return /\[\d+\]$/.test(str);
+}
+
 /**
  * Analyses a given string to find the last statement that is interesting for
  * later completion.
  *
  * @param   string aStr
  *          A string to analyse.
  *
  * @returns object
@@ -218,91 +222,98 @@ function JSPropertyProvider(aDbgObject, 
     }
   }
 
   // We are completing a variable / a property lookup.
   let properties = completionPart.split(".");
   let matchProp = properties.pop().trimLeft();
   let obj = aDbgObject;
 
-  // The first property must be found in the environment if the debugger is
-  // paused.
-  if (anEnvironment) {
-    if (properties.length == 0) {
-      return getMatchedPropsInEnvironment(anEnvironment, matchProp);
-    }
-    obj = getVariableInEnvironment(anEnvironment, properties.shift());
+  // The first property must be found in the environment of the paused debugger
+  // or of the global lexical scope.
+  let env = anEnvironment || obj.asEnvironment();
+
+  if (properties.length === 0) {
+    return getMatchedPropsInEnvironment(env, matchProp);
+  }
+
+  let firstProp = properties.shift().trim();
+  if (firstProp === "this") {
+    // Special case for 'this' - try to get the Object from the Environment.
+    // No problem if it throws, we will just not autocomplete.
+    try {
+      obj = env.object;
+    } catch(e) { }
+  }
+  else if (hasArrayIndex(firstProp)) {
+    obj = getArrayMemberProperty(null, env, firstProp);
+  } else {
+    obj = getVariableInEnvironment(env, firstProp);
   }
 
   if (!isObjectUsable(obj)) {
     return null;
   }
 
   // We get the rest of the properties recursively starting from the Debugger.Object
   // that wraps the first property
   for (let i = 0; i < properties.length; i++) {
     let prop = properties[i].trim();
     if (!prop) {
       return null;
     }
 
-    // Special case for 'this' since it's not part of the global's properties
-    // but we want autocompletion to work properly for it
-    if (prop === "this" && obj === aDbgObject && i === 0) {
-      continue;
-    }
-
-    if (/\[\d+\]$/.test(prop)) {
+    if (hasArrayIndex(prop)) {
       // The property to autocomplete is a member of array. For example
       // list[i][j]..[n]. Traverse the array to get the actual element.
-      obj = getArrayMemberProperty(obj, prop);
+      obj = getArrayMemberProperty(obj, null, prop);
     }
     else {
       obj = DevToolsUtils.getProperty(obj, prop);
     }
 
     if (!isObjectUsable(obj)) {
       return null;
     }
   }
 
   // If the final property is a primitive
   if (typeof obj != "object") {
     return getMatchedProps(obj, matchProp);
   }
 
-  let matchedProps = getMatchedPropsInDbgObject(obj, matchProp);
-  if (properties.length !== 0 || obj !== aDbgObject) {
-    let thisInd = matchedProps.matches.indexOf("this");
-    if (thisInd > -1) {
-      matchedProps.matches.splice(thisInd, 1)
-    }
-  }
-
-  return matchedProps;
+  return getMatchedPropsInDbgObject(obj, matchProp);
 }
 
 /**
  * Get the array member of aObj for the given aProp. For example, given
  * aProp='list[0][1]' the element at [0][1] of aObj.list is returned.
  *
  * @param object aObj
- *        The object to operate on.
+ *        The object to operate on. Should be null if aEnv is passed.
+ * @param object aEnv
+ *        The Environment to operate in. Should be null if aObj is passed.
  * @param string aProp
  *        The property to return.
  * @return null or Object
  *         Returns null if the property couldn't be located. Otherwise the array
  *         member identified by aProp.
  */
-function getArrayMemberProperty(aObj, aProp)
+function getArrayMemberProperty(aObj, aEnv, aProp)
 {
   // First get the array.
   let obj = aObj;
   let propWithoutIndices = aProp.substr(0, aProp.indexOf("["));
-  obj = DevToolsUtils.getProperty(obj, propWithoutIndices);
+
+  if (aEnv) {
+    obj = getVariableInEnvironment(aEnv, propWithoutIndices);
+  } else {
+    obj = DevToolsUtils.getProperty(obj, propWithoutIndices);
+  }
+
   if (!isObjectUsable(obj)) {
     return null;
   }
 
   // Then traverse the list of indices to get the actual element.
   let result;
   let arrayIndicesRegex = /\[[^\]]*\]/g;
   while ((result = arrayIndicesRegex.exec(aProp)) !== null) {
@@ -491,27 +502,17 @@ var DebuggerObjectSupport = {
     while (aObj) {
       yield aObj;
       aObj = aObj.proto;
     }
   },
 
   getProperties: function(aObj)
   {
-    let names = aObj.getOwnPropertyNames();
-    // Include 'this' in results (in sorted order).  It will be removed
-    // in all cases except for the first property request on the global, but
-    // it needs to be added now so it can be filtered based on string input.
-    for (let i = 0; i < names.length; i++) {
-      if (i === names.length - 1 || names[i+1] > "this") {
-        names.splice(i+1, 0, "this");
-        break;
-      }
-    }
-    return names;
+    return aObj.getOwnPropertyNames();
   },
 
   getProperty: function(aObj, aName, aRootObj)
   {
     // This is left unimplemented in favor to DevToolsUtils.getProperty().
     throw "Unimplemented!";
   },
 };
@@ -522,26 +523,44 @@ var DebuggerEnvironmentSupport = {
     while (aObj) {
       yield aObj;
       aObj = aObj.parent;
     }
   },
 
   getProperties: function(aObj)
   {
-    return aObj.names();
+    let names = aObj.names();
+
+    // Include 'this' in results (in sorted order)
+    for (let i = 0; i < names.length; i++) {
+      if (i === names.length - 1 || names[i+1] > "this") {
+        names.splice(i+1, 0, "this");
+        break;
+      }
+    }
+
+    return names;
   },
 
   getProperty: function(aObj, aName)
   {
-    // TODO: we should use getVariableDescriptor() here - bug 725815.
-    let result = aObj.getVariable(aName);
+    let result;
+    // Try/catch since aName can be anything, and getVariable throws if
+    // it's not a valid ECMAScript identifier name
+    try {
+      // TODO: we should use getVariableDescriptor() here - bug 725815.
+      result = aObj.getVariable(aName);
+    } catch(e) { }
+
     // FIXME: Need actual UI, bug 941287.
     if (result === undefined || result.optimizedOut || result.missingArguments) {
       return null;
     }
     return { value: result };
   },
 };
 
 
 exports.JSPropertyProvider = DevToolsUtils.makeInfallible(JSPropertyProvider);
 
+// Export a version that will throw (for tests)
+exports.FallibleJSPropertyProvider = JSPropertyProvider;
--- a/devtools/shared/webconsole/test/unit/test_js_property_provider.js
+++ b/devtools/shared/webconsole/test/unit/test_js_property_provider.js
@@ -1,122 +1,154 @@
 /* -*- js-indent-level: 2; indent-tabs-mode: nil -*- */
 // Any copyright is dedicated to the Public Domain.
 // http://creativecommons.org/publicdomain/zero/1.0/
 
 "use strict";
 const { require } = Components.utils.import("resource://devtools/shared/Loader.jsm", {});
-const { JSPropertyProvider } = require("devtools/shared/webconsole/js-property-provider");
+const { FallibleJSPropertyProvider: JSPropertyProvider } =
+  require("devtools/shared/webconsole/js-property-provider");
 
 Components.utils.import("resource://gre/modules/jsdebugger.jsm");
 addDebuggerToGlobal(this);
 
 function run_test() {
-  const testArray = 'var testArray = [\
-    {propA: "A"},\
-    {\
-      propB: "B", \
-      propC: [\
-        {propD: "D"}\
-      ]\
-    },\
-    [\
-      {propE: "E"}\
-    ]\
-  ];'
+  const testArray = `var testArray = [
+    {propA: "A"},
+    {
+      propB: "B",
+      propC: [
+        "D"
+      ]
+    },
+    [
+      {propE: "E"}
+    ]
+  ]`;
 
   const testObject = 'var testObject = {"propA": [{"propB": "B"}]}';
   const testHyphenated = 'var testHyphenated = {"prop-A": "res-A"}';
+  const testLet = "let foobar = {a: ''}; const blargh = {a: 1};";
 
   let sandbox = Components.utils.Sandbox("http://example.com");
   let dbg = new Debugger;
   let dbgObject = dbg.addDebuggee(sandbox);
+  let dbgEnv = dbgObject.asEnvironment();
   Components.utils.evalInSandbox(testArray, sandbox);
   Components.utils.evalInSandbox(testObject, sandbox);
   Components.utils.evalInSandbox(testHyphenated, sandbox);
+  Components.utils.evalInSandbox(testLet, sandbox);
+
+  do_print("Running tests with dbgObject");
+  runChecks(dbgObject, null);
+
+  do_print("Running tests with dbgEnv");
+  runChecks(null, dbgEnv);
+
+}
+
+function runChecks(dbgObject, dbgEnv) {
+  do_print("Test that suggestions are given for 'this'");
+  let results = JSPropertyProvider(dbgObject, dbgEnv, "t");
+  test_has_result(results, "this");
+
+  if (dbgObject != null) {
+    do_print("Test that suggestions are given for 'this.'");
+    results = JSPropertyProvider(dbgObject, dbgEnv, "this.");
+    test_has_result(results, "testObject");
+
+    do_print("Test that no suggestions are given for 'this.this'");
+    results = JSPropertyProvider(dbgObject, dbgEnv, "this.this");
+    test_has_no_results(results);
+  }
+
+  do_print("Testing lexical scope issues (Bug 1207868)");
+  results = JSPropertyProvider(dbgObject, dbgEnv, "foobar");
+  test_has_result(results, "foobar");
+
+  results = JSPropertyProvider(dbgObject, dbgEnv, "foobar.");
+  test_has_result(results, "a");
+
+  results = JSPropertyProvider(dbgObject, dbgEnv, "blargh");
+  test_has_result(results, "blargh");
+
+  results = JSPropertyProvider(dbgObject, dbgEnv, "blargh.");
+  test_has_result(results, "a");
 
   do_print("Test that suggestions are given for 'foo[n]' where n is an integer.");
-  let results = JSPropertyProvider(dbgObject, null, "testArray[0].");
+  results = JSPropertyProvider(dbgObject, dbgEnv, "testArray[0].");
   test_has_result(results, "propA");
 
   do_print("Test that suggestions are given for multidimensional arrays.");
-  results = JSPropertyProvider(dbgObject, null, "testArray[2][0].");
+  results = JSPropertyProvider(dbgObject, dbgEnv, "testArray[2][0].");
   test_has_result(results, "propE");
 
+  do_print("Test that suggestions are given for nested arrays.");
+  results = JSPropertyProvider(dbgObject, dbgEnv, "testArray[1].propC[0].");
+  test_has_result(results, "indexOf");
+
   do_print("Test that suggestions are given for literal arrays.");
-  results = JSPropertyProvider(dbgObject, null, "[1,2,3].");
+  results = JSPropertyProvider(dbgObject, dbgEnv, "[1,2,3].");
   test_has_result(results, "indexOf");
 
   do_print("Test that suggestions are given for literal arrays with newlines.");
-  results = JSPropertyProvider(dbgObject, null, "[1,2,3,\n4\n].");
+  results = JSPropertyProvider(dbgObject, dbgEnv, "[1,2,3,\n4\n].");
   test_has_result(results, "indexOf");
 
-  do_print("Test that suggestions are given for 'this'");
-  results = JSPropertyProvider(dbgObject, null, "t");
-  test_has_result(results, "this");
-
-  do_print("Test that suggestions are given for 'this.'");
-  results = JSPropertyProvider(dbgObject, null, "this.");
-  test_has_result(results, "testObject");
-
-  do_print("Test that no suggestions are given for 'this.this'");
-  results = JSPropertyProvider(dbgObject, null, "this.this");
-  test_has_no_results(results);
-
   do_print("Test that suggestions are given for literal strings.");
-  results = JSPropertyProvider(dbgObject, null, "'foo'.");
+  results = JSPropertyProvider(dbgObject, dbgEnv, "'foo'.");
   test_has_result(results, "charAt");
-  results = JSPropertyProvider(dbgObject, null, '"foo".');
+  results = JSPropertyProvider(dbgObject, dbgEnv, '"foo".');
   test_has_result(results, "charAt");
-  results = JSPropertyProvider(dbgObject, null, "`foo`.");
+  results = JSPropertyProvider(dbgObject, dbgEnv, "`foo`.");
   test_has_result(results, "charAt");
-  results = JSPropertyProvider(dbgObject, null, "'[1,2,3]'.");
+  results = JSPropertyProvider(dbgObject, dbgEnv, "'[1,2,3]'.");
   test_has_result(results, "charAt");
 
   do_print("Test that suggestions are not given for syntax errors.");
-  results = JSPropertyProvider(dbgObject, null, "'foo\"");
+  results = JSPropertyProvider(dbgObject, dbgEnv, "'foo\"");
   do_check_null(results);
-  results = JSPropertyProvider(dbgObject, null, "[1,',2]");
+  results = JSPropertyProvider(dbgObject, dbgEnv, "[1,',2]");
   do_check_null(results);
-  results = JSPropertyProvider(dbgObject, null, "'[1,2].");
+  results = JSPropertyProvider(dbgObject, dbgEnv, "'[1,2].");
   do_check_null(results);
-  results = JSPropertyProvider(dbgObject, null, "'foo'..");
+  results = JSPropertyProvider(dbgObject, dbgEnv, "'foo'..");
   do_check_null(results);
 
   do_print("Test that suggestions are not given without a dot.");
-  results = JSPropertyProvider(dbgObject, null, "'foo'");
+  results = JSPropertyProvider(dbgObject, dbgEnv, "'foo'");
   test_has_no_results(results);
-  results = JSPropertyProvider(dbgObject, null, "[1,2,3]");
+  results = JSPropertyProvider(dbgObject, dbgEnv, "[1,2,3]");
   test_has_no_results(results);
-  results = JSPropertyProvider(dbgObject, null, "[1,2,3].\n'foo'");
+  results = JSPropertyProvider(dbgObject, dbgEnv, "[1,2,3].\n'foo'");
   test_has_no_results(results);
 
   do_print("Test that suggestions are not given for numeric literals.");
-  results = JSPropertyProvider(dbgObject, null, "1.");
+  results = JSPropertyProvider(dbgObject, dbgEnv, "1.");
   do_check_null(results);
 
   do_print("Test that suggestions are not given for index that's out of bounds.");
-  results = JSPropertyProvider(dbgObject, null, "testArray[10].");
+  results = JSPropertyProvider(dbgObject, dbgEnv, "testArray[10].");
   do_check_null(results);
 
   do_print("Test that no suggestions are given if an index is not numerical somewhere in the chain.");
-  results = JSPropertyProvider(dbgObject, null, "testArray[0]['propC'][0].");
+  results = JSPropertyProvider(dbgObject, dbgEnv, "testArray[0]['propC'][0].");
   do_check_null(results);
 
-  results = JSPropertyProvider(dbgObject, null, "testObject['propA'][0].");
+  results = JSPropertyProvider(dbgObject, dbgEnv, "testObject['propA'][0].");
   do_check_null(results);
 
-  results = JSPropertyProvider(dbgObject, null, "testArray[0]['propC'].");
+  results = JSPropertyProvider(dbgObject, dbgEnv, "testArray[0]['propC'].");
   do_check_null(results);
 
-  results = JSPropertyProvider(dbgObject, null, "testArray[][1].");
+  results = JSPropertyProvider(dbgObject, dbgEnv, "testArray[][1].");
   do_check_null(results);
 
   do_print("Test that suggestions are not given if there is an hyphen in the chain.");
-  results = JSPropertyProvider(dbgObject, null, "testHyphenated['prop-A'].");
+  results = JSPropertyProvider(dbgObject, dbgEnv, "testHyphenated['prop-A'].");
   do_check_null(results);
 }
 
 /**
  * A helper that ensures an empty array of results were found.
  * @param Object aResults
  *        The results returned by JSPropertyProvider.
  */