Bug 1498598 - Make js-property-provider better; r=bgrins.
authorNicolas Chevobbe <nchevobbe@mozilla.com>
Wed, 17 Oct 2018 15:02:41 +0000
changeset 500153 a8e1eaf6d370c7861f6e3b56f3f2ffe93cf3626d
parent 500152 fd7038210e70cbd56468c5577edfb6fd5c84b986
child 500154 b684eb7a73c05a4e3f3d6cd4b7de9d0664dbc1ee
push id1864
push userffxbld-merge
push dateMon, 03 Dec 2018 15:51:40 +0000
treeherdermozilla-release@f040763d99ad [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbgrins
bugs1498598
milestone64.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 1498598 - Make js-property-provider better; r=bgrins. This patches solves 2 issues: - it doesn't return any result when the user is trying to perform a variable, function or class declaration (e.g. var d). - js-property-provider used to compute the last statement by only looking for space or ; chars. But there are a lot of cases (basically each time using an operator), where we should return results and we weren't. Test cases are added to cover those fixes. Differential Revision: https://phabricator.services.mozilla.com/D8968
devtools/shared/webconsole/js-property-provider.js
devtools/shared/webconsole/test/test_jsterm_autocomplete.html
--- a/devtools/shared/webconsole/js-property-provider.js
+++ b/devtools/shared/webconsole/js-property-provider.js
@@ -27,16 +27,18 @@ const STATE_TEMPLATE_LITERAL = Symbol("S
 const OPEN_BODY = "{[(".split("");
 const CLOSE_BODY = "}])".split("");
 const OPEN_CLOSE_BODY = {
   "{": "}",
   "[": "]",
   "(": ")",
 };
 
+const NO_AUTOCOMPLETE_PREFIXES = ["var", "const", "let", "function", "class"];
+
 function hasArrayIndex(str) {
   return /\[\d+\]$/.test(str);
 }
 
 /**
  * Analyses a given string to find the last statement that is interesting for
  * later completion.
  *
@@ -61,58 +63,77 @@ function analyzeInputString(str) {
   const bodyStack = [];
 
   let state = STATE_NORMAL;
   let start = 0;
   let c;
 
   // Use an array in order to handle character with a length > 2 (e.g. 😎).
   const characters = Array.from(str);
+
+  const buildReturnObject = () => {
+    let isElementAccess = false;
+    if (bodyStack.length === 1 && bodyStack[0].token === "[") {
+      start = bodyStack[0].start;
+      isElementAccess = true;
+      if ([STATE_DQUOTE, STATE_QUOTE, STATE_TEMPLATE_LITERAL].includes(state)) {
+        state = STATE_NORMAL;
+      }
+    }
+
+    return {
+      state,
+      lastStatement: characters.slice(start).join(""),
+      isElementAccess,
+    };
+  };
+
   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 == "`") {
           state = STATE_TEMPLATE_LITERAL;
-        } else if (c == ";") {
+        } else if (";,:=<>+-*/%|&^~?!".split("").includes(c)) {
+          // If the character is an operator, we need to update the start position.
           start = i + 1;
         } else if (c == " ") {
+          const currentLastStatement = characters.slice(start, i).join("");
           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;
+          // There's only spaces after that, so we can return.
+          if (!nextNonSpaceChar) {
+            return buildReturnObject();
           }
 
-          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;
+          // If the previous char in't a dot, and the next one isn't a dot either,
+          // and the current computed statement is not a variable/function/class
+          // declaration, update the start position.
+          if (
+            previousNonSpaceChar !== "." && nextNonSpaceChar !== "."
+            && !NO_AUTOCOMPLETE_PREFIXES.includes(currentLastStatement)
+          ) {
+            start = i + nextNonSpaceCharIndex;
           }
+
+          // Let's jump to handle the next non-space char.
+          i = i + nextNonSpaceCharIndex;
         } else if (OPEN_BODY.includes(c)) {
           bodyStack.push({
             token: c,
             start
           });
           start = i + 1;
         } else if (CLOSE_BODY.includes(c)) {
           const last = bodyStack.pop();
@@ -161,30 +182,17 @@ function analyzeInputString(str) {
           };
         } else if (c == "'") {
           state = STATE_NORMAL;
         }
         break;
     }
   }
 
-  let isElementAccess = false;
-  if (bodyStack.length === 1 && bodyStack[0].token === "[") {
-    start = bodyStack[0].start;
-    isElementAccess = true;
-    if ([STATE_DQUOTE, STATE_QUOTE, STATE_TEMPLATE_LITERAL].includes(state)) {
-      state = STATE_NORMAL;
-    }
-  }
-
-  return {
-    state,
-    lastStatement: characters.slice(start).join(""),
-    isElementAccess,
-  };
+  return buildReturnObject();
 }
 
 /**
  * Provides a list of properties, that are possible matches based on the passed
  * Debugger.Environment/Debugger.Object and inputValue.
  *
  * @param object dbgObject
  *        When the debugger is not paused this Debugger.Object wraps
@@ -232,26 +240,32 @@ function JSPropertyProvider(dbgObject, a
     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 (state != STATE_NORMAL) {
     return null;
   }
+
+  // Don't complete on just an empty string.
+  if (lastStatement.trim() == "") {
+    return null;
+  }
+
+  if (NO_AUTOCOMPLETE_PREFIXES.some(prefix => lastStatement.startsWith(prefix + " "))) {
+    return null;
+  }
+
   const completionPart = lastStatement;
   const lastDotIndex = completionPart.lastIndexOf(".");
   const lastOpeningBracketIndex = isElementAccess ? completionPart.lastIndexOf("[") : -1;
   const lastCompletionCharIndex = Math.max(lastDotIndex, lastOpeningBracketIndex);
   const startQuoteRegex = /^('|"|`)/;
 
-  // 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
   // 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 syntaxTree = parser.get(completionPart.slice(0, lastCompletionCharIndex));
--- a/devtools/shared/webconsole/test/test_jsterm_autocomplete.html
+++ b/devtools/shared/webconsole/test/test_jsterm_autocomplete.html
@@ -80,32 +80,36 @@
         bar: "",
         BAR: "",
         dataTest: "",
         "data-test": "",
         'da"ta"test': "",
         'da\`ta\`test': "",
         "da'ta'test": "",
       }));
+
+      window.varify = true;
     `;
     await state.client.evaluateJSAsync(script);
 
     const tests = [
       doAutocomplete1,
       doAutocomplete2,
       doAutocomplete3,
       doAutocomplete4,
       doAutocompleteLarge1,
       doAutocompleteLarge2,
       doAutocompleteProxyThrowsPrototype,
       doAutocompleteProxyThrowsOwnKeys,
       doAutocompleteDotSurroundedBySpaces,
       doAutocompleteAfterOr,
       doInsensitiveAutocomplete,
       doElementAccessAutocomplete,
+      doAutocompleteAfterOperator,
+      dontAutocompleteAfterDeclaration,
     ];
 
     if (!isWorker) {
       // `Cu` is not defined in workers, then we can't test `Cu.Sandbox`
       tests.push(doAutocompleteSandbox);
       // Array literal, string and commands completion aren't handled in Workers yet.
       tests.push(
         doAutocompleteArray,
@@ -443,11 +447,79 @@
     info(`test autocomplete for 'window[";c'`);
     matches = (await client.autocomplete(`window[";c`)).matches;
     ok(!matches.includes("cd") && !matches.includes("clear"), "commands are not returned");
 
     info(`test autocomplete for 'window[;c'`);
     matches = (await client.autocomplete(`window[;c`)).matches;
     ok(!matches.includes("cd") && !matches.includes("clear"), "commands are not returned");
   }
+
+  async function doAutocompleteAfterOperator(client) {
+    const inputs = [
+      "true;foob",
+      "true,foob",
+      "({key:foob",
+      "a=foob",
+      "if(a<foob",
+      "if(a>foob",
+      "1+foob",
+      "1-foob",
+      "++foob",
+      "--foob",
+      "1*foob",
+      "2**foob",
+      "1/foob",
+      "1%foob",
+      "1|foob",
+      "1&foob",
+      "1^foob",
+      "~foob",
+      "1<<foob",
+      "1>>foob",
+      "1>>>foob",
+      "false||foob",
+      "false&&foob",
+      "x=true?foob",
+      "x=false?1:foob",
+      "!foob",
+    ];
+
+    for (const input of inputs) {
+      info(`test autocomplete for "${input}"`);
+      let matches = (await client.autocomplete(input)).matches;
+      ok(matches.includes("foobarObject"), `Expected autocomplete result for ${input}"`);
+    }
+  }
+
+  async function dontAutocompleteAfterDeclaration(client) {
+    info("test autocomplete for 'var win'");
+    let matches = (await client.autocomplete("var win")).matches;
+    is(matches.length, 0, "no autocompletion on a var declaration");
+
+    info("test autocomplete for 'const win'");
+    matches = (await client.autocomplete("const win")).matches;
+    is(matches.length, 0, "no autocompletion on a const declaration");
+
+    info("test autocomplete for 'let win'");
+    matches = (await client.autocomplete("let win")).matches;
+    is(matches.length, 0, "no autocompletion on a let declaration");
+
+    info("test autocomplete for 'function win'");
+    matches = (await client.autocomplete("function win")).matches;
+    is(matches.length, 0, "no autocompletion on a function declaration");
+
+    info("test autocomplete for 'class win'");
+    matches = (await client.autocomplete("class win")).matches;
+    is(matches.length, 0, "no autocompletion on a class declaration");
+
+    info("test autocomplete for 'const win = win'");
+    matches = (await client.autocomplete("const win = win")).matches;
+    ok(matches.includes("window"), "autocompletion still happens after the `=` sign");
+
+    info("test autocomplete for 'in var'");
+    matches = (await client.autocomplete("in var")).matches;
+    ok(matches.includes("varify"),
+      "autocompletion still happens with a property name starting with 'var'");
+  }
 </script>
 </body>
 </html>