Bug 1473841 - Don't return dot-notation invalid properties; r=Honza.

In JsPropertyProvider, if doing a property access (with a dot),
check that the results are suited for a dot notation (e.g. `data`
is, while `data-test` is not).
In case, of an element access, we can return everything.

This implies making some changes to some tests which were using
invalid dot notation access in some case, which revealed a
bug with bracket autocomplete and spaces.

So the bracket autocomplete with spaces is now also fixed, and
a test case was added for that as well.

Differential Revision: https://phabricator.services.mozilla.com/D12726

--HG--
extra : moz-landing-system : lando

devtools/shared/webconsole/js-property-provider.js

@@ -11,6 +11,7 @@ const DevToolsUtils = require("devtools/shared/DevToolsUtils");
 if (!isWorker) {
   loader.lazyImporter(this, "Parser", "resource://devtools/shared/Parser.jsm");
 }
+loader.lazyRequireGetter(this, "Reflect", "resource://gre/modules/reflect.jsm", true);
 
 // Provide an easy way to bail out of even attempting an autocompletion
 // if an object has way too many properties. Protects against large objects
@@ -117,12 +118,13 @@ function analyzeInputString(str) {
             return buildReturnObject();
           }
 
-          // 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 the previous char isn't a dot or opening bracket, and the next one isn't
+          // one 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)
+            previousNonSpaceChar !== "." && nextNonSpaceChar !== "." &&
+            previousNonSpaceChar !== "[" && nextNonSpaceChar !== "[" &&
+            !NO_AUTOCOMPLETE_PREFIXES.includes(currentLastStatement)
           ) {
             start = i + nextNonSpaceCharIndex;
           }
@@ -479,7 +481,24 @@ function JSPropertyProvider({
       // 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);
+    } else if (!isWorker) {
+      // If we're not performing an element access, we need to check that the property
+      // are suited for a dot access. (Reflect.jsm is not available in worker context yet,
+      // see Bug 1507181).
+      matches = new Set([...matches].filter(propertyName => {
+        let valid = true;
+        try {
+          // In order to know if the property is suited for dot notation, we use Reflect
+          // to parse an expression where we try to access the property with a dot. If it
+          // throws, this means that we need to do an element access instead.
+          Reflect.parse(`({${propertyName}: true})`);
+        } catch (e) {
+          valid = false;
+        }
+        return valid;
+      }));
     }
+
     return {isElementAccess, matchProp, matches};
   };
 

devtools/shared/webconsole/test/test_jsterm_autocomplete.html

@@ -111,11 +111,13 @@
     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.
+      // Some cases are handled in worker context because we can't use Parser.jsm.
+      // See Bug 1507181.
       tests.push(
         doAutocompleteArray,
         doAutocompleteString,
         doAutocompleteCommands,
+        doAutocompleteBracketSurroundedBySpaces,
       );
     }
 
@@ -299,11 +301,31 @@
       (await client.autocomplete("window.foobarObject.  foo ; window.foo")).matches;
     is(matches.length, 1);
     checkObject(matches, ["foobarObject"]);
+  }
 
-    matches =
-      (await client.autocomplete("window.emojiObject  .  ")).matches;
+  async function doAutocompleteBracketSurroundedBySpaces(client) {
+    const wrap = (arr, quote = `"`) => arr.map(x => `${quote}${x}${quote}`);
+    let matches = await getAutocompleteMatches(client, "window.foobarObject\n  [")
+    is(matches.length, 7);
+    checkObject(matches,
+      wrap(["foo", "foobar", "foobaz", "omg", "omgfoo", "omgstr", "strfoo"]));
+
+    matches = await getAutocompleteMatches(client, "window.foobarObject\n  ['o")
+    is(matches.length, 3);
+    checkObject(matches, wrap(["omg", "omgfoo", "omgstr"], "'"));
+
+    matches = await getAutocompleteMatches(client, "window.foobarObject\n  [\n  s");
     is(matches.length, 1);
-    checkObject(matches, ["😎"]);
+    checkObject(matches, [`"strfoo"`]);
+
+    matches = await getAutocompleteMatches(client, "window.foobarObject\n  [  ");
+    is(matches.length, 7);
+    checkObject(matches,
+      wrap(["foo", "foobar", "foobaz", "omg", "omgfoo", "omgstr", "strfoo"]));
+
+    matches = await getAutocompleteMatches(client, "window.emojiObject  [   '");
+    is(matches.length, 1);
+    checkObject(matches, [`'😎'`]);
   }
 
   async function doAutocompleteAfterOr(client) {
@@ -542,6 +564,12 @@ async function doKeywordsAutocomplete(client) {
   ok(!matches.includes("function"),
     "'function' is not returned when doing an element access");
   }
+
+  async function getAutocompleteMatches(client, input) {
+    info(`test autocomplete for "${input}"`);
+    const res = (await client.autocomplete(input));
+    return res.matches;
+  }
 </script>
 </body>
 </html>

devtools/shared/webconsole/test/unit/test_js_property_provider.js

@@ -348,6 +348,14 @@ function runChecks(dbgObject, environment, sandbox) {
 
   results = propertyProvider("(1)['toFixed");
   test_has_exact_results(results, ["'toFixed'"]);
+
+  info("Test access on dot-notation invalid property name");
+  results = propertyProvider("testHyphenated.prop");
+  Assert.ok(!results.matches.has("prop-A"),
+    "Does not return invalid property name on dot access");
+
+  results = propertyProvider("testHyphenated['prop");
+  test_has_result(results, `'prop-A'`);
 }
 
 /**