Bug 1472117 - Fix autocompletion with "." surrounded by spaces; r=bgrins.
authorNicolas Chevobbe <nchevobbe@mozilla.com>
Tue, 14 Aug 2018 13:40:08 +0000
changeset 431391 e477368edf31c7e2e01d18dbd92c4a9f498daf90
parent 431390 33d4952d34f9b6d6f1066a01aa162a2f4f5c2522
child 431392 5312e71473c402f520db9e24f5c7e5eb6795cbcf
push id67756
push usernchevobbe@mozilla.com
push dateTue, 14 Aug 2018 13:47:48 +0000
treeherderautoland@e477368edf31 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbgrins
bugs1472117
milestone63.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 1472117 - Fix autocompletion with "." surrounded by spaces; r=bgrins. Expressions like `window . addEventListener` are perfectly valid in JS, but our autocompletion provider wasn't working well in such case. We modify the JSPropertyProvider to make sure to handle this kind of syntax and add test cases to make sure it works as intended.` Differential Revision: https://phabricator.services.mozilla.com/D2988
devtools/server/actors/webconsole.js
devtools/shared/webconsole/js-property-provider.js
devtools/shared/webconsole/test/test_jsterm_autocomplete.html
--- a/devtools/server/actors/webconsole.js
+++ b/devtools/server/actors/webconsole.js
@@ -1175,19 +1175,20 @@ WebConsoleActor.prototype =
 
       if (!hadDebuggee && dbgObject) {
         this.dbg.removeDebuggee(this.evalWindow);
       }
 
       matches = result.matches || [];
       matchProp = result.matchProp;
 
-      // We consider '$' as alphanumerc because it is used in the names of some
-      // helper functions.
-      const lastNonAlphaIsDot = /[.][a-zA-Z0-9$]*$/.test(reqText);
+      // We consider '$' as alphanumeric because it is used in the names of some
+      // helper functions; we also consider whitespace as alphanum since it should not
+      // be seen as break in the evaled string.
+      const lastNonAlphaIsDot = /[.][a-zA-Z0-9$\s]*$/.test(reqText);
       if (!lastNonAlphaIsDot) {
         matches = matches.concat(this._getWebConsoleCommandsCache().filter(n =>
           // filter out `screenshot` command as it is inaccessible without
           // the `:` prefix
           n !== "screenshot" && n.startsWith(result.matchProp)
         ));
       }
     }
--- a/devtools/shared/webconsole/js-property-provider.js
+++ b/devtools/shared/webconsole/js-property-provider.js
@@ -46,39 +46,68 @@ function hasArrayIndex(str) {
  *          If there was an error in the string detected, then a object like
  *
  *            { err: "ErrorMesssage" }
  *
  *          is returned, otherwise a object like
  *
  *            {
  *              state: STATE_NORMAL|STATE_QUOTE|STATE_DQUOTE,
- *              startPos: index of where the last statement begins
+ *              lastStatement: the last statement in the string
  *            }
  */
 function findCompletionBeginning(str) {
   const bodyStack = [];
 
   let state = STATE_NORMAL;
   let start = 0;
   let c;
-  for (let i = 0; i < str.length; i++) {
-    c = str[i];
+
+  // Use an array in order to handle character with a length > 2 (e.g. 😎).
+  const characters = Array.from(str);
+  for (let i = 0; i < characters.length; i++) {
+    c = characters[i];
 
     switch (state) {
       // Normal JS state.
       case STATE_NORMAL:
         if (c == '"') {
           state = STATE_DQUOTE;
         } else if (c == "'") {
           state = STATE_QUOTE;
         } else if (c == ";") {
           start = i + 1;
         } else if (c == " ") {
-          start = i + 1;
+          const before = characters.slice(0, i);
+          const after = characters.slice(i + 1);
+          const trimmedBefore = Array.from(before.join("").trimRight());
+          const trimmedAfter = Array.from(after.join("").trimLeft());
+
+          const nextNonSpaceChar = trimmedAfter[0];
+          const nextNonSpaceCharIndex = after.indexOf(nextNonSpaceChar);
+          const previousNonSpaceChar = trimmedBefore[trimmedBefore.length - 1];
+
+          // If the previous meaningful char was a dot and there is no meaningful char
+          // after, we can break out of the loop.
+          if (previousNonSpaceChar === "." && !nextNonSpaceChar) {
+            break;
+          }
+
+          if (nextNonSpaceChar) {
+            // If the previous char wasn't a dot, and the next one isn't a dot either,
+            // update the start pos.
+            if (previousNonSpaceChar !== "." && nextNonSpaceChar !== ".") {
+              start = i + nextNonSpaceCharIndex;
+            }
+            // Let's jump to handle the next non-space char.
+            i = i + nextNonSpaceCharIndex;
+          } else {
+            // There's only spaces after that, so we can break out of the loop.
+            break;
+          }
         } else if (OPEN_BODY.includes(c)) {
           bodyStack.push({
             token: c,
             start: start
           });
           start = i + 1;
         } else if (CLOSE_BODY.includes(c)) {
           const last = bodyStack.pop();
@@ -120,17 +149,17 @@ function findCompletionBeginning(str) {
           state = STATE_NORMAL;
         }
         break;
     }
   }
 
   return {
     state: state,
-    startPos: start
+    lastStatement: characters.slice(start).join("")
   };
 }
 
 /**
  * Provides a list of properties, that are possible matches based on the passed
  * Debugger.Environment/Debugger.Object and inputValue.
  *
  * @param object dbgObject
@@ -160,30 +189,30 @@ function JSPropertyProvider(dbgObject, a
   if (cursor === undefined) {
     cursor = inputValue.length;
   }
 
   inputValue = inputValue.substring(0, cursor);
 
   // Analyse the inputValue and find the beginning of the last part that
   // should be completed.
-  const beginning = findCompletionBeginning(inputValue);
+  const {err, state, lastStatement} = findCompletionBeginning(inputValue);
 
   // There was an error analysing the string.
-  if (beginning.err) {
+  if (err) {
     return null;
   }
 
   // If the current state is not STATE_NORMAL, then we are inside of an string
   // which means that no completion is possible.
-  if (beginning.state != STATE_NORMAL) {
+  if (state != STATE_NORMAL) {
     return null;
   }
 
-  const completionPart = inputValue.substring(beginning.startPos);
+  const completionPart = lastStatement;
   const lastDot = completionPart.lastIndexOf(".");
 
   // Don't complete on just an empty string.
   if (completionPart.trim() == "") {
     return null;
   }
 
   // Catch literals like [1,2,3] or "foo" and return the matches from
@@ -196,17 +225,17 @@ function JSPropertyProvider(dbgObject, a
     const syntaxTree = parser.get(completionPart.slice(0, lastDot));
     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;
-      const matchProp = completionPart.slice(lastDot + 1);
+      const matchProp = completionPart.slice(lastDot + 1).trimLeft();
       if (expression.type === "ArrayExpression") {
         return getMatchedProps(Array.prototype, matchProp);
       } else if (expression.type === "Literal" &&
                  (typeof expression.value === "string")) {
         return getMatchedProps(String.prototype, matchProp);
       }
     }
   }
--- a/devtools/shared/webconsole/test/test_jsterm_autocomplete.html
+++ b/devtools/shared/webconsole/test/test_jsterm_autocomplete.html
@@ -7,219 +7,241 @@
   <script type="text/javascript" src="common.js"></script>
   <!-- Any copyright is dedicated to the Public Domain.
      - http://creativecommons.org/publicdomain/zero/1.0/ -->
 </head>
 <body>
 <p>Test for JavaScript terminal autocomplete functionality</p>
 
 <script class="testbody" type="text/javascript">
-SimpleTest.waitForExplicitFinish();
+  SimpleTest.waitForExplicitFinish();
+  const {
+    MAX_AUTOCOMPLETE_ATTEMPTS,
+    MAX_AUTOCOMPLETIONS
+  } = require("devtools/shared/webconsole/js-property-provider");
 
-let gState;
-let {MAX_AUTOCOMPLETE_ATTEMPTS,MAX_AUTOCOMPLETIONS} = require("devtools/shared/webconsole/js-property-provider");
+  addEventListener("load", startTest);
+
+  async function startTest() {
+    // First run the tests with a tab as a target.
+    let state = await new Promise(resolve => attachConsoleToTab(["PageError"], resolve));
+    await performTests({state, isWorker: false});
 
-function evaluateJS(input, options = {}) {
-  return new Promise((resolve, reject) => {
-    gState.client.evaluateJSAsync(input, resolve, options);
-  });
-}
+    // Then run the tests with a worker as a target.
+    state = await new Promise(resolve => attachConsoleToWorker(["PageError"], resolve));
+    await performTests({state, isWorker: true});
+
+    SimpleTest.finish();
+  }
 
-function autocompletePromise(str, cursor = str.length, frameActor) {
-  return gState.client.autocomplete(str, cursor, frameActor);
-}
+  async function performTests({state, isWorker}) {
+    // Set up the global variables needed to test autocompletion in the target.
+    const script = `
+      // This is for workers so autocomplete acts the same
+      if (!this.window) {
+        window = this;
+      }
 
-// This test runs all of its assertions twice - once with
-// the tab as a target and once with a worker
-let runningInTab = true;
-function startTest({worker}) {
-  if (worker) {
-    attachConsoleToWorker(["PageError"], onAttach.bind(null, true));
-  } else {
-    attachConsoleToTab(["PageError"], onAttach.bind(null, false));
-  }
-};
+      window.foobarObject = Object.create(null);
+      window.foobarObject.foo = 1;
+      window.foobarObject.foobar = 2;
+      window.foobarObject.foobaz = 3;
+      window.foobarObject.omg = 4;
+      window.foobarObject.omgfoo = 5;
+      window.foobarObject.strfoo = "foobarz";
+      window.foobarObject.omgstr = "foobarz" +
+        (new Array(${DebuggerServer.LONG_STRING_LENGTH})).join("abb");
+      window.largeObject1 = Object.create(null);
+      for (let i = 0; i < ${MAX_AUTOCOMPLETE_ATTEMPTS + 1}; i++) {
+        window.largeObject1['a' + i] = i;
+      }
+
+      window.largeObject2 = Object.create(null);
+      for (let i = 0; i < ${MAX_AUTOCOMPLETIONS * 2}; i++) {
+        window.largeObject2['a' + i] = i;
+      }
 
-let onAttach = async function (isWorker, aState, response) {
-  gState = aState;
-
-  let longStrLength = DebuggerServer.LONG_STRING_LENGTH;
+      window.proxy1 = new Proxy({foo: 1}, {
+        getPrototypeOf() { throw new Error() }
+      });
+      window.proxy2 = new Proxy(Object.create(Object.create(null, {foo:{}})), {
+        ownKeys() { throw new Error() }
+      });
+      window.emojiObject = Object.create(null);
+      window.emojiObject["😎"] = "😎";
+    `;
+    await state.client.evaluateJSAsync(script);
 
-  // Set up the global variables needed to test autocompletion
-  // in the target.
-  let script = `
-    // This is for workers so autocomplete acts the same
-    if (!this.window) {
-      window = this;
+    const tests = [
+      doAutocomplete1,
+      doAutocomplete2,
+      doAutocomplete3,
+      doAutocomplete4,
+      doAutocompleteLarge1,
+      doAutocompleteLarge2,
+      doAutocompleteProxyThrowsPrototype,
+      doAutocompleteProxyThrowsOwnKeys,
+      doAutocompleteDotSurroundedBySpaces,
+      doAutocompleteAfterOr,
+    ];
+
+    if (!isWorker) {
+      // `Cu` is not defined in workers, then we can't test `Cu.Sandbox`
+      tests.push(doAutocompleteSandbox);
+      // Array literal completion isn't handled in Workers yet.
+      tests.push(doAutocompleteArray);
     }
 
-    window.foobarObject = Object.create(null);
-    window.foobarObject.foo = 1;
-    window.foobarObject.foobar = 2;
-    window.foobarObject.foobaz = 3;
-    window.foobarObject.omg = 4;
-    window.foobarObject.omgfoo = 5;
-    window.foobarObject.strfoo = "foobarz";
-    window.foobarObject.omgstr = "foobarz" +
-      (new Array(${longStrLength})).join("abb");
-    window.largeObject1 = Object.create(null);
-    for (let i = 0; i < ${MAX_AUTOCOMPLETE_ATTEMPTS + 1}; i++) {
-      window.largeObject1['a' + i] = i;
+    for (const test of tests) {
+      await test(state.client);
     }
 
-    window.largeObject2 = Object.create(null);
-    for (let i = 0; i < ${MAX_AUTOCOMPLETIONS * 2}; i++) {
-      window.largeObject2['a' + i] = i;
-    }
+    await closeDebugger(state);
+  }
+
+  async function doAutocomplete1(client) {
+    info("test autocomplete for 'window.foo'");
+    let response = await client.autocomplete("window.foo");
+    let matches = response.matches;
 
-    window.proxy1 = new Proxy({foo: 1}, {
-      getPrototypeOf() { throw new Error() }
-    });
-    window.proxy2 = new Proxy(Object.create(Object.create(null, {foo:{}})), {
-      ownKeys() { throw new Error() }
-    });
-  `;
+    is(response.matchProp, "foo", "matchProp");
+    is(matches.length, 1, "matches.length");
+    is(matches[0], "foobarObject", "matches[0]");
+  }
 
-  await evaluateJS(script);
+  async function doAutocomplete2(client) {
+    info("test autocomplete for 'window.foobarObject.'");
+    let response = await client.autocomplete("window.foobarObject.");
+    let matches = response.matches;
 
-  let tests = [doAutocomplete1, doAutocomplete2, doAutocomplete3,
-               doAutocomplete4, doAutocompleteLarge1,
-               doAutocompleteLarge2, doAutocompleteProxyThrowsPrototype,
-               doAutocompleteProxyThrowsOwnKeys];
-  if (!isWorker) {
-    // `Cu` is not defined in workers, then we can't test `Cu.Sandbox`
-    tests.push(doAutocompleteSandbox);
+    ok(!response.matchProp, "matchProp");
+    is(matches.length, 7, "matches.length");
+    checkObject(matches,
+      ["foo", "foobar", "foobaz", "omg", "omgfoo", "omgstr", "strfoo"]);
   }
 
-  runTests(tests, testEnd);
-};
-
-async function doAutocomplete1() {
-  info("test autocomplete for 'window.foo'");
-  let response = await autocompletePromise("window.foo");
-  let matches = response.matches;
-
-  is(response.matchProp, "foo", "matchProp");
-  is(matches.length, 1, "matches.length");
-  is(matches[0], "foobarObject", "matches[0]");
+  async function doAutocomplete3(client) {
+    // Check that completion suggestions are offered inside the string.
+    info("test autocomplete for 'dump(window.foobarObject.)'");
+    let response = await client.autocomplete("dump(window.foobarObject.)", 25);
+    let matches = response.matches;
 
-  nextTest();
-}
-
-async function doAutocomplete2() {
-  info("test autocomplete for 'window.foobarObject.'");
-  let response = await autocompletePromise("window.foobarObject.");
-  let matches = response.matches;
+    ok(!response.matchProp, "matchProp");
+    is(matches.length, 7, "matches.length");
+    checkObject(matches,
+      ["foo", "foobar", "foobaz", "omg", "omgfoo", "omgstr", "strfoo"]);
+  }
 
-  ok(!response.matchProp, "matchProp");
-  is(matches.length, 7, "matches.length");
-  checkObject(matches,
-    ["foo", "foobar", "foobaz", "omg", "omgfoo", "omgstr", "strfoo"]);
-
-  nextTest();
-}
+  async function doAutocomplete4(client) {
+    // Check that completion requests can have no suggestions.
+    info("test autocomplete for 'dump(window.foobarObject.)'");
+    let response = await client.autocomplete("dump(window.foobarObject.)");
+    ok(!response.matchProp, "matchProp");
+    is(response.matches.length, 0, "matches.length");
+  }
 
-async function doAutocomplete3() {
-  // Check that completion suggestions are offered inside the string.
-  info("test autocomplete for 'dump(window.foobarObject.)'");
-  let response = await autocompletePromise("dump(window.foobarObject.)", 25);
-  let matches = response.matches;
-
-  ok(!response.matchProp, "matchProp");
-  is(matches.length, 7, "matches.length");
-  checkObject(matches,
-    ["foo", "foobar", "foobaz", "omg", "omgfoo", "omgstr", "strfoo"]);
-
-  nextTest();
-}
+  async function doAutocompleteLarge1(client) {
+    // Check that completion requests with too large objects will
+    // have no suggestions.
+    info("test autocomplete for 'window.largeObject1.'");
+    let response = await client.autocomplete("window.largeObject1.");
+    ok(!response.matchProp, "matchProp");
+    info (response.matches.join("|"));
+    is(response.matches.length, 0, "Bailed out with too many properties");
+  }
 
-async function doAutocomplete4() {
-  // Check that completion requests can have no suggestions.
-  info("test autocomplete for 'dump(window.foobarObject.)'");
-  let response = await autocompletePromise("dump(window.foobarObject.)");
-  ok(!response.matchProp, "matchProp");
-  is(response.matches.length, 0, "matches.length");
-
-  nextTest();
-}
+  async function doAutocompleteLarge2(client) {
+    // Check that completion requests with pretty large objects will
+    // have MAX_AUTOCOMPLETIONS suggestions
+    info("test autocomplete for 'window.largeObject2.'");
+    let response = await client.autocomplete("window.largeObject2.");
+    ok(!response.matchProp, "matchProp");
+    is(response.matches.length, MAX_AUTOCOMPLETIONS, "matches.length is MAX_AUTOCOMPLETIONS");
+  }
 
-async function doAutocompleteLarge1() {
-  // Check that completion requests with too large objects will
-  // have no suggestions.
-  info("test autocomplete for 'window.largeObject1.'");
-  let response = await autocompletePromise("window.largeObject1.");
-  ok(!response.matchProp, "matchProp");
-  info (response.matches.join("|"));
-  is(response.matches.length, 0, "Bailed out with too many properties");
+  async function doAutocompleteProxyThrowsPrototype(client) {
+    // Check that completion provides own properties even if [[GetPrototypeOf]] throws.
+    info("test autocomplete for 'window.proxy1.'");
+    let response = await client.autocomplete("window.proxy1.");
+    ok(!response.matchProp, "matchProp");
+    is(response.matches.length, 1, "matches.length");
+    checkObject(response.matches, ["foo"]);
+  }
 
-  nextTest();
-}
+  async function doAutocompleteProxyThrowsOwnKeys(client) {
+    // Check that completion provides inherited properties even if [[OwnPropertyKeys]] throws.
+    info("test autocomplete for 'window.proxy2.'");
+    let response = await client.autocomplete("window.proxy2.");
+    ok(!response.matchProp, "matchProp");
+    is(response.matches.length, 1, "matches.length");
+    checkObject(response.matches, ["foo"]);
+  }
 
-async function doAutocompleteLarge2() {
-  // Check that completion requests with pretty large objects will
-  // have MAX_AUTOCOMPLETIONS suggestions
-  info("test autocomplete for 'window.largeObject2.'");
-  let response = await autocompletePromise("window.largeObject2.");
-  ok(!response.matchProp, "matchProp");
-  is(response.matches.length, MAX_AUTOCOMPLETIONS, "matches.length is MAX_AUTOCOMPLETIONS");
+  async function doAutocompleteSandbox(client) {
+    // Check that completion provides inherited properties even if [[OwnPropertyKeys]] throws.
+    info("test autocomplete for 'Cu.Sandbox.'");
+    let response = await client.autocomplete("Cu.Sandbox.");
+    ok(!response.matchProp, "matchProp");
+    let keys = Object.getOwnPropertyNames(Object.prototype).sort();
+    is(response.matches.length, keys.length, "matches.length");
+    checkObject(response.matches, keys);
+  }
 
-  nextTest();
-}
+  async function doAutocompleteArray(client) {
+    info("test autocomplete for [1,2,3]");
+    let response = await client.autocomplete("[1,2,3].");
+    let {matches} = response;
 
-async function doAutocompleteProxyThrowsPrototype() {
-  // Check that completion provides own properties even if [[GetPrototypeOf]] throws.
-  info("test autocomplete for 'window.proxy1.'");
-  let response = await autocompletePromise("window.proxy1.");
-  ok(!response.matchProp, "matchProp");
-  is(response.matches.length, 1, "matches.length");
-  checkObject(response.matches, ["foo"]);
-
-  nextTest();
-}
+    ok(matches.length > 0, "There are completion results for the array");
+    ok(matches.includes("length") && matches.includes("filter"),
+      "Array autocomplete contains expected results");
 
-async function doAutocompleteProxyThrowsOwnKeys() {
-  // Check that completion provides inherited properties even if [[OwnPropertyKeys]] throws.
-  info("test autocomplete for 'window.proxy2.'");
-  let response = await autocompletePromise("window.proxy2.");
-  ok(!response.matchProp, "matchProp");
-  is(response.matches.length, 1, "matches.length");
-  checkObject(response.matches, ["foo"]);
+    info("test autocomplete for '[] . '");
+    matches = (await client.autocomplete("[] . ")).matches;
+    ok(matches.length > 1);
+    ok(matches.includes("length") && matches.includes("filter"),
+      "Array autocomplete contains expected results");
+    ok(!matches.includes("copy"), "Array autocomplete does not contain helpers");
+  }
 
-  nextTest();
-}
+  async function doAutocompleteDotSurroundedBySpaces(client) {
+    info("test autocomplete for 'window.foobarObject\n  .'");
+    let {matches} = await client.autocomplete("window.foobarObject\n  .");
+    is(matches.length, 7);
+    checkObject(matches,
+      ["foo", "foobar", "foobaz", "omg", "omgfoo", "omgstr", "strfoo"]);
 
-async function doAutocompleteSandbox() {
-  // Check that completion provides inherited properties even if [[OwnPropertyKeys]] throws.
-  info("test autocomplete for 'Cu.Sandbox.'");
-  let response = await autocompletePromise("Cu.Sandbox.");
-  ok(!response.matchProp, "matchProp");
-  let keys = Object.getOwnPropertyNames(Object.prototype).sort();
-  is(response.matches.length, keys.length, "matches.length");
-  checkObject(response.matches, keys);
+    info("test autocomplete for 'window.foobarObject\n  .o'");
+    matches = (await client.autocomplete("window.foobarObject\n  .o")).matches;
+    is(matches.length, 3);
+    checkObject(matches, ["omg", "omgfoo", "omgstr"]);
 
-  nextTest();
-}
+    info("test autocomplete for 'window.foobarObject\n  .\n  s'");
+    matches = (await client.autocomplete("window.foobarObject\n  .\n  s")).matches;
+    is(matches.length, 1);
+    checkObject(matches, ["strfoo"]);
+
+    info("test autocomplete for 'window.foobarObject\n  .  '");
+    matches = (await client.autocomplete("window.foobarObject\n  .  ")).matches;
+    is(matches.length, 7);
+    checkObject(matches,
+      ["foo", "foobar", "foobaz", "omg", "omgfoo", "omgstr", "strfoo"]);
 
-function testEnd()
-{
-  // If this is the first run, reload the page and do it again
-  // in a worker.  Otherwise, end the test.
-  closeDebugger(gState, function() {
-    gState = null;
-    if (runningInTab) {
-      runningInTab = false;
-      startTest({
-        worker: true
-      });
-    } else {
-      SimpleTest.finish();
-    }
-  });
-}
+    matches =
+      (await client.autocomplete("window.foobarObject.  foo ; window.foo")).matches;
+    is(matches.length, 1);
+    checkObject(matches, ["foobarObject"]);
 
-addEventListener("load", () => {
-  startTest({
-    worker: false
-  });
-});
+    matches =
+      (await client.autocomplete("window.emojiObject  .  ")).matches;
+    is(matches.length, 1);
+    checkObject(matches, ["😎"]);
+  }
+
+  async function doAutocompleteAfterOr(client) {
+    info("test autocomplete for 'true || foo'");
+    const {matches} = await client.autocomplete("true || foobar");
+    is(matches.length, 1, "autocomplete returns expected results");
+    is(matches.join("-"), "foobarObject");
+  }
 </script>
 </body>
 </html>