Bug 1473841 - Transform property access on a dot-notation invalid name into an element access; r=bgrins.

This patch turns property access that would result in
Syntax error (e.g. `x.data-test`) into element access
(e.g. `x["data-test"]`) when accepting a completion
value in the console input.
In order to do that, we use Reflect to parse  a custom
expression where we try to define the property the user
is going to accept. If this throws, this means we need to
modify the input into an element access.
A test is added to make sure this works as expected.

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

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

devtools/client/webconsole/components/JSTerm.js

@@ -19,6 +19,7 @@ loader.lazyRequireGetter(this, "KeyCodes", "devtools/client/shared/keycodes", tr
 loader.lazyRequireGetter(this, "Editor", "devtools/client/sourceeditor/editor");
 loader.lazyRequireGetter(this, "Telemetry", "devtools/client/shared/telemetry");
 loader.lazyRequireGetter(this, "saveScreenshot", "devtools/shared/screenshot/save");
+loader.lazyRequireGetter(this, "Reflect", "resource://gre/modules/reflect.jsm", true);
 
 const l10n = require("devtools/client/webconsole/webconsole-l10n");
 
@@ -1399,6 +1400,38 @@ class JSTerm extends Component {
       }
     }
 
+    if (
+      this.autocompletePopup.selectedItem ||
+      (!this.autocompletePopup.isOpen && this.autocompletePopup.items.length === 1)
+    ) {
+      const isValidDotNotation = propertyName => {
+        // If the item is a command, we don't want to modify it.
+        if (propertyName.startsWith(":")
+          && propertyName.includes(this.getInputValue().trim())) {
+          return true;
+        }
+
+        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;
+      };
+
+      const selectedItem = this.autocompletePopup.selectedItem
+        || this.autocompletePopup.items[0];
+      const {label, preLabel, isElementAccess} = selectedItem;
+      if (!isElementAccess && !isValidDotNotation(label)) {
+        completionText = `["${label.replace(/"/gi, '\\"')}"]`;
+        numberOfCharsToReplaceCharsBeforeCursor = preLabel.length + 1;
+      }
+    }
+
     this.clearCompletion();
 
     if (completionText) {

devtools/client/webconsole/test/mochitest/browser.ini

@@ -212,6 +212,7 @@ skip-if = verify
 [browser_jsterm_completion_bracket_cached_results.js]
 [browser_jsterm_completion_bracket.js]
 [browser_jsterm_completion_case_sensitivity.js]
+[browser_jsterm_completion_invalid_dot_notation.js]
 [browser_jsterm_completion.js]
 [browser_jsterm_content_defined_helpers.js]
 [browser_jsterm_copy_command.js]

devtools/client/webconsole/test/mochitest/browser_jsterm_completion_invalid_dot_notation.js

@@ -0,0 +1,99 @@
+/* Any copyright is dedicated to the Public Domain.
+ * http://creativecommons.org/publicdomain/zero/1.0/ */
+
+// Tests that accepting a code completion with an invalid dot-notation property turns the
+// property access into an element access (`x.data-test` -> `x["data-test"]`).
+
+"use strict";
+
+const TEST_URI = `data:text/html;charset=utf8,<p>test code completion
+  <script>
+    x = Object.create(null);
+    x.dataTest = 1;
+    x["data-test"] = 2;
+    x['da"ta"test'] = 3;
+    x["DATA-TEST"] = 4;
+  </script>`;
+
+add_task(async function() {
+  // Run test with legacy JsTerm
+  await pushPref("devtools.webconsole.jsterm.codeMirror", false);
+  await performTests();
+  // And then run it with the CodeMirror-powered one.
+  await pushPref("devtools.webconsole.jsterm.codeMirror", true);
+  await performTests();
+});
+
+async function performTests() {
+  const {jsterm} = await openNewTabAndConsole(TEST_URI);
+  const {autocompletePopup} = jsterm;
+
+  info("Ensure we have the correct items in the autocomplete popup");
+  let onPopUpOpen = autocompletePopup.once("popup-opened");
+  EventUtils.sendString("x.da");
+  await onPopUpOpen;
+  is(getAutocompletePopupLabels(autocompletePopup).join("|"),
+    "da\"ta\"test|data-test|dataTest|DATA-TEST",
+    `popup has expected items`);
+
+  info("Test that accepting the completion of a property with quotes changes the " +
+    "property access into an element access");
+  checkJsTermCompletionValue(jsterm, `    "ta"test`, `completeNode has expected value`);
+  let onPopupClose = autocompletePopup.once("popup-closed");
+  EventUtils.synthesizeKey("KEY_Tab");
+  await onPopupClose;
+  checkJsTermValueAndCursor(jsterm, `x["da\\"ta\\"test"]|`,
+    "Property access was transformed into an element access, and quotes were properly " +
+    "escaped");
+  checkJsTermCompletionValue(jsterm, "", `completeNode is empty`);
+
+  jsterm.setInputValue("");
+
+  info("Test that accepting the completion of a property with a dash changes the " +
+    "property access into an element access");
+  onPopUpOpen = autocompletePopup.once("popup-opened");
+  EventUtils.sendString("x.data-");
+  await onPopUpOpen;
+
+  checkJsTermCompletionValue(jsterm, `       test`, `completeNode has expected value`);
+  onPopupClose = autocompletePopup.once("popup-closed");
+  EventUtils.synthesizeKey("KEY_Tab");
+  await onPopupClose;
+  checkJsTermValueAndCursor(jsterm, `x["data-test"]|`,
+   "Property access was transformed into an element access");
+  checkJsTermCompletionValue(jsterm, "", `completeNode is empty`);
+
+  jsterm.setInputValue("");
+
+  info("Test that accepting the completion of an uppercase property with a dash changes" +
+    " the property access into an element access with the correct casing");
+  onPopUpOpen = autocompletePopup.once("popup-opened");
+  EventUtils.sendString("x.data-");
+  await onPopUpOpen;
+
+  EventUtils.synthesizeKey("KEY_ArrowDown");
+  checkJsTermCompletionValue(jsterm, `       TEST`, `completeNode has expected value`);
+  onPopupClose = autocompletePopup.once("popup-closed");
+  EventUtils.synthesizeKey("KEY_Tab");
+  await onPopupClose;
+  checkJsTermValueAndCursor(jsterm, `x["DATA-TEST"]|`,
+   "Property access was transformed into an element access with the correct casing");
+  checkJsTermCompletionValue(jsterm, "", `completeNode is empty`);
+
+  jsterm.setInputValue("");
+
+  info("Test that accepting the completion of an property with a dash, when the popup " +
+    " is not displayed still changes the property access into an element access");
+  await setInputValueForAutocompletion(jsterm, "x.D");
+  checkJsTermCompletionValue(jsterm, `   ATA-TEST`, `completeNode has expected value`);
+
+  EventUtils.synthesizeKey("KEY_Tab");
+
+  checkJsTermValueAndCursor(jsterm, `x["DATA-TEST"]|`,
+   "Property access was transformed into an element access with the correct casing");
+  checkJsTermCompletionValue(jsterm, "", `completeNode is empty`);
+}
+
+function getAutocompletePopupLabels(autocompletePopup) {
+  return autocompletePopup.items.map(i => i.label);
+}