Bug 1506867 - Use AST to get the properties-access chain; r=bgrins.
authorNicolas Chevobbe <nchevobbe@mozilla.com>
Tue, 20 Nov 2018 06:43:03 +0000
changeset 503621 c54258be2dd7137d03345df4c907c4f98af521fb
parent 503620 eeddcefcdad847bf8a5737153079e9619ee5aa66
child 503622 492c7f2cb6d0802c150544f2e3f186d90bb6de31
push id10290
push userffxbld-merge
push dateMon, 03 Dec 2018 16:23:23 +0000
treeherdermozilla-beta@700bed2445e6 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbgrins
bugs1506867
milestone65.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 1506867 - Use AST to get the properties-access chain; r=bgrins. Differential Revision: https://phabricator.services.mozilla.com/D11893
devtools/shared/webconsole/js-property-provider.js
devtools/shared/webconsole/test/unit/test_js_property_provider.js
--- a/devtools/shared/webconsole/js-property-provider.js
+++ b/devtools/shared/webconsole/js-property-provider.js
@@ -285,54 +285,65 @@ function JSPropertyProvider({
   }
 
   const completionPart = lastStatement;
   const lastDotIndex = completionPart.lastIndexOf(".");
   const lastOpeningBracketIndex = isElementAccess ? completionPart.lastIndexOf("[") : -1;
   const lastCompletionCharIndex = Math.max(lastDotIndex, lastOpeningBracketIndex);
   const startQuoteRegex = /^('|"|`)/;
 
+  // AST representation of the expression before the last access char (`.` or `[`).
+  let astExpression;
+  let matchProp = completionPart.slice(lastCompletionCharIndex + 1).trimLeft();
+
   // Catch literals like [1,2,3] or "foo" and return the matches from
   // their prototypes.
   // Don't run this is a worker, migrating to acorn should allow this
   // to run in a worker - Bug 1217198.
   if (!isWorker && lastCompletionCharIndex > 0) {
     const parser = new Parser();
     parser.logExceptions = false;
     const parsedExpression = completionPart.slice(0, lastCompletionCharIndex);
     const syntaxTree = parser.get(parsedExpression);
     const lastTree = syntaxTree.getLastSyntaxTree();
     const lastBody = lastTree && lastTree.AST.body[lastTree.AST.body.length - 1];
 
     // Finding the last expression since we've sliced up until the dot.
     // If there were parse errors this won't exist.
     if (lastBody) {
-      const expression = lastBody.expression;
+      astExpression = lastBody.expression;
       let matchingObject;
 
-      if (expression.type === "ArrayExpression") {
+      if (astExpression.type === "ArrayExpression") {
         matchingObject = Array.prototype;
-      } else if (expression.type === "Literal" && typeof expression.value === "string") {
+      } else if (
+        astExpression.type === "Literal" &&
+        typeof astExpression.value === "string"
+      ) {
         matchingObject = String.prototype;
-      } else if (expression.type === "Literal" && Number.isFinite(expression.value)) {
+      } else if (
+        astExpression.type === "Literal" &&
+        Number.isFinite(astExpression.value)
+      ) {
         // The parser rightfuly indicates that we have a number in some cases (e.g. `1.`),
         // but we don't want to return Number proto properties in that case since
         // the result would be invalid (i.e. `1.toFixed()` throws).
         // So if the expression value is an integer, it should not end with `{Number}.`
         // (but the following are fine: `1..`, `(1.).`).
         if (
-          !Number.isInteger(expression.value) ||
+          !Number.isInteger(astExpression.value) ||
           /\d[^\.]{0}\.$/.test(completionPart) === false
         ) {
           matchingObject = Number.prototype;
+        } else {
+          return null;
         }
       }
 
       if (matchingObject) {
-        const matchProp = completionPart.slice(lastCompletionCharIndex + 1).trimLeft();
         let search = matchProp;
 
         let elementAccessQuote;
         if (isElementAccess && startQuoteRegex.test(matchProp)) {
           elementAccessQuote = matchProp[0];
           search = matchProp.replace(startQuoteRegex, "");
         }
 
@@ -346,25 +357,36 @@ function JSPropertyProvider({
           matchProp,
           matches: props,
         };
       }
     }
   }
 
   // We are completing a variable / a property lookup.
-  const properties = completionPart.split(".");
-  let matchProp;
-  if (isElementAccess) {
-    const lastPart = properties[properties.length - 1];
-    const openBracketIndex = lastPart.lastIndexOf("[");
-    matchProp = lastPart.substr(openBracketIndex + 1);
-    properties[properties.length - 1] = lastPart.substring(0, openBracketIndex);
+  let properties = [];
+
+  if (astExpression) {
+    if (lastCompletionCharIndex > -1) {
+      properties = getPropertiesFromAstExpression(astExpression);
+
+      if (properties === null) {
+        return null;
+      }
+    }
   } else {
-    matchProp = properties.pop().trimLeft();
+    properties = completionPart.split(".");
+    if (isElementAccess) {
+      const lastPart = properties[properties.length - 1];
+      const openBracketIndex = lastPart.lastIndexOf("[");
+      matchProp = lastPart.substr(openBracketIndex + 1);
+      properties[properties.length - 1] = lastPart.substring(0, openBracketIndex);
+    } else {
+      matchProp = properties.pop().trimLeft();
+    }
   }
 
   let search = matchProp;
   let elementAccessQuote;
   if (isElementAccess && startQuoteRegex.test(search)) {
     elementAccessQuote = search[0];
     search = search.replace(startQuoteRegex, "");
   }
@@ -411,18 +433,21 @@ function JSPropertyProvider({
 
   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 [index, prop] of properties.entries()) {
-    prop = prop.trim();
-    if (!prop) {
+    if (typeof prop === "string") {
+      prop = prop.trim();
+    }
+
+    if (prop === undefined || prop === null || prop === "") {
       return null;
     }
 
     if (!invokeUnsafeGetter && DevToolsUtils.isUnsafeGetter(obj, prop)) {
       // If the unsafe getter is not the last property access of the input, bail out as
       // things might get complex.
       if (index !== properties.length - 1) {
         return null;
@@ -444,32 +469,63 @@ function JSPropertyProvider({
       obj = DevToolsUtils.getProperty(obj, prop, invokeUnsafeGetter);
     }
 
     if (!isObjectUsable(obj)) {
       return null;
     }
   }
 
+  const prepareReturnedObject = matches => {
+    if (isElementAccess) {
+      // If it's an element access, we need to wrap properties in quotes (either the one
+      // the user already typed, or `"`).
+      matches = wrapMatchesInQuotes(matches, elementAccessQuote);
+    }
+    return {isElementAccess, matchProp, matches};
+  };
+
   // If the final property is a primitive
   if (typeof obj != "object") {
-    return {
-      isElementAccess,
-      matchProp,
-      matches: getMatchedProps(obj, search),
-    };
+    return prepareReturnedObject(getMatchedProps(obj, search));
   }
 
-  let matches = getMatchedPropsInDbgObject(obj, search);
-  if (isElementAccess) {
-    // If it's an element access, we need to wrap properties in quotes (either the one
-    // the user already typed, or `"`).
-    matches = wrapMatchesInQuotes(matches, elementAccessQuote);
+  return prepareReturnedObject(getMatchedPropsInDbgObject(obj, search));
+}
+
+/**
+ * @param {Object} ast: An AST representing a property access (e.g. `foo.bar["baz"].x`)
+ * @returns {Array|null} An array representing the property access
+ *                       (e.g. ["foo", "bar", "baz", "x"]).
+ */
+function getPropertiesFromAstExpression(ast) {
+  let result = [];
+  if (!ast) {
+    return result;
   }
-  return {isElementAccess, matchProp, matches};
+  const {type, property, object, name} = ast;
+  if (type === "ThisExpression") {
+    result.unshift("this");
+  } else if (type === "Identifier" && name) {
+    result.unshift(name);
+  } else if (type === "MemberExpression") {
+    if (property) {
+      if (property.type === "Identifier" && property.name) {
+        result.unshift(property.name);
+      } else if (property.type === "Literal") {
+        result.unshift(property.value);
+      }
+    }
+    if (object) {
+      result = (getPropertiesFromAstExpression(object) || []).concat(result);
+    }
+  } else {
+    return null;
+  }
+  return result;
 }
 
 function wrapMatchesInQuotes(matches, quote = `"`) {
   return new Set([...matches].map(p =>
     `${quote}${p.replace(new RegExp(`${quote}`, "g"), `\\${quote}`)}${quote}`));
 }
 
 /**
--- a/devtools/shared/webconsole/test/unit/test_js_property_provider.js
+++ b/devtools/shared/webconsole/test/unit/test_js_property_provider.js
@@ -93,16 +93,33 @@ function runChecks(dbgObject, environmen
   let results = propertyProvider("t");
   test_has_result(results, "this");
 
   if (dbgObject != null) {
     info("Test that suggestions are given for 'this.'");
     results = propertyProvider("this.");
     test_has_result(results, "testObject");
 
+    info("Test that suggestions are given for '(this).'");
+    results = propertyProvider("(this).");
+    test_has_result(results, "testObject");
+
+    info("Test that suggestions are given for deep 'this' properties access");
+    results = propertyProvider("(this).testObject.propA.");
+    test_has_result(results, "shift");
+
+    results = propertyProvider("(this).testObject.propA[");
+    test_has_result(results, `"shift"`);
+
+    results = propertyProvider("(this)['testObject']['propA'][");
+    test_has_result(results, `"shift"`);
+
+    results = propertyProvider("(this).testObject['propA'].");
+    test_has_result(results, "shift");
+
     info("Test that no suggestions are given for 'this.this'");
     results = propertyProvider("this.this");
     test_has_no_results(results);
   }
 
   info("Testing lexical scope issues (Bug 1207868)");
   results = propertyProvider("foobar");
   test_has_result(results, "foobar");
@@ -173,41 +190,53 @@ function runChecks(dbgObject, environmen
   test_has_no_results(results);
   results = propertyProvider("`foo`");
   test_has_no_results(results);
   results = propertyProvider("[1,2,3]");
   test_has_no_results(results);
   results = propertyProvider("[1,2,3].\n'foo'");
   test_has_no_results(results);
 
-  info("Test that suggestions are not given for numeric literals.");
-  results = propertyProvider("1.");
-  Assert.equal(null, results);
-
   info("Test that suggestions are not given for index that's out of bounds.");
   results = propertyProvider("testArray[10].");
   Assert.equal(null, results);
 
-  info("Test that no suggestions are given if an index is not numerical "
-       + "somewhere in the chain.");
-  results = propertyProvider("testArray[0]['propC'][0].");
-  Assert.equal(null, results);
-
-  results = propertyProvider("testObject['propA'][0].");
-  Assert.equal(null, results);
-
-  results = propertyProvider("testArray[0]['propC'].");
-  Assert.equal(null, results);
-
+  info("Test that invalid element access syntax does not return anything");
   results = propertyProvider("testArray[][1].");
   Assert.equal(null, results);
 
-  info("Test that suggestions are not given if there is an hyphen in the chain.");
+  info("Test that deep element access works.");
+  results = propertyProvider("testObject['propA'][0].");
+  test_has_result(results, "propB");
+
+  results = propertyProvider("testArray[1]['propC'].");
+  test_has_result(results, "shift");
+
+  results = propertyProvider("testArray[1].propC[0][");
+  test_has_result(results, `"trim"`);
+
+  results = propertyProvider("testArray[1].propC[0].");
+  test_has_result(results, "trim");
+
+  info("Test that suggestions are displayed when variable is wrapped in parens");
+  results = propertyProvider("(testObject)['propA'][0].");
+  test_has_result(results, "propB");
+
+  results = propertyProvider("(testArray)[1]['propC'].");
+  test_has_result(results, "shift");
+
+  results = propertyProvider("(testArray)[1].propC[0][");
+  test_has_result(results, `"trim"`);
+
+  results = propertyProvider("(testArray)[1].propC[0].");
+  test_has_result(results, "trim");
+
+  info("Test that suggestions are given if there is an hyphen in the chain.");
   results = propertyProvider("testHyphenated['prop-A'].");
-  Assert.equal(null, results);
+  test_has_result(results, "trim");
 
   info("Test that we have suggestions for generators.");
   const gen1Result = Cu.evalInSandbox("gen1.next().value", sandbox);
   results = propertyProvider("gen1.");
   test_has_result(results, "next");
   info("Test that the generator next() was not executed");
   const gen1NextResult = Cu.evalInSandbox("gen1.next().value", sandbox);
   Assert.equal(gen1Result + 1, gen1NextResult);