Bug 1520957 - [release 119] Fix unhelpful error in top-level await malformed expression. (#7695). r=dwalsh
☠☠ backed out by 5b1c54cbac38 ☠ ☠
authorNicolas Chevobbe <nchevobbe@users.noreply.github.com>
Fri, 18 Jan 2019 12:18:06 -0500
changeset 454577 07d417fb91d28f5b9566999ba672660e13f54a77
parent 454576 b74cd7c3ae9b23815cc5b863f1db2cc0d8c4a6fc
child 454578 2a0d5bc0699151aa7d4d66b4476090dc17540391
push id35400
push usercsabou@mozilla.com
push dateSat, 19 Jan 2019 09:59:33 +0000
treeherdermozilla-central@f90bab5af97e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdwalsh
bugs1520957
milestone66.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 1520957 - [release 119] Fix unhelpful error in top-level await malformed expression. (#7695). r=dwalsh
devtools/client/debugger/new/dist/parser-worker.js
devtools/client/debugger/new/src/workers/parser/mapAwaitExpression.js
devtools/client/debugger/new/src/workers/parser/mapBindings.js
devtools/client/debugger/new/src/workers/parser/mapExpression.js
devtools/client/debugger/new/src/workers/parser/mapOriginalExpression.js
devtools/client/debugger/new/src/workers/parser/tests/mapBindings.spec.js
devtools/client/debugger/new/src/workers/parser/tests/mapExpression.spec.js
devtools/client/debugger/new/src/workers/parser/tests/mapOriginalExpression.spec.js
devtools/client/debugger/new/src/workers/parser/utils/ast.js
--- a/devtools/client/debugger/new/dist/parser-worker.js
+++ b/devtools/client/debugger/new/dist/parser-worker.js
@@ -875,21 +875,25 @@ function parseVueScript(code) {
     }
   } else {
     ast = parse(code, sourceOptions.original);
   }
   return ast;
 }
 
 function parseConsoleScript(text, opts) {
-  return _parse(text, {
-    plugins: ["objectRestSpread"],
-    ...opts,
-    allowAwaitOutsideFunction: true
-  });
+  try {
+    return _parse(text, {
+      plugins: ["objectRestSpread"],
+      ...opts,
+      allowAwaitOutsideFunction: true
+    });
+  } catch (e) {
+    return null;
+  }
 }
 
 function parseScript(text, opts) {
   return _parse(text, opts);
 }
 
 function getAst(sourceId) {
   if (ASTs.has(sourceId)) {
@@ -23028,18 +23032,17 @@ function getFirstExpression(ast) {
 
   return statements[0].expression;
 }
 
 function locationKey(start) {
   return `${start.line}:${start.column}`;
 }
 
-function mapOriginalExpression(expression, mappings) {
-  const ast = (0, _ast.parseConsoleScript)(expression);
+function mapOriginalExpression(expression, ast, mappings) {
   const scopes = (0, _getScopes.buildScopeList)(ast, "");
   let shouldUpdate = false;
 
   const nodes = new Map();
   const replacements = new Map();
 
   // The ref-only global bindings are the ones that are accessed, but not
   // declared anywhere in the parsed code, meaning they are either global,
@@ -23493,100 +23496,99 @@ module.exports = {
 "use strict";
 
 
 Object.defineProperty(exports, "__esModule", {
   value: true
 });
 exports.default = mapExpression;
 
+var _ast = __webpack_require__(1375);
+
 var _mapOriginalExpression = __webpack_require__(3613);
 
 var _mapOriginalExpression2 = _interopRequireDefault(_mapOriginalExpression);
 
 var _mapBindings = __webpack_require__(3756);
 
 var _mapBindings2 = _interopRequireDefault(_mapBindings);
 
 var _mapAwaitExpression = __webpack_require__(3778);
 
 var _mapAwaitExpression2 = _interopRequireDefault(_mapAwaitExpression);
 
 function _interopRequireDefault(obj) { return obj && obj.__esModule ? obj : { default: obj }; }
 
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at <http://mozilla.org/MPL/2.0/>. */
+
 function mapExpression(expression, mappings, bindings, shouldMapBindings = true, shouldMapAwait = true) {
   const mapped = {
     await: false,
     bindings: false,
     originalExpression: false
   };
 
+  const ast = (0, _ast.parseConsoleScript)(expression);
   try {
-    if (mappings) {
+    if (mappings && ast) {
       const beforeOriginalExpression = expression;
-      expression = (0, _mapOriginalExpression2.default)(expression, mappings);
+      expression = (0, _mapOriginalExpression2.default)(expression, ast, mappings);
       mapped.originalExpression = beforeOriginalExpression !== expression;
     }
 
-    if (shouldMapBindings) {
+    if (shouldMapBindings && ast) {
       const beforeBindings = expression;
-      expression = (0, _mapBindings2.default)(expression, bindings);
+      expression = (0, _mapBindings2.default)(expression, ast, bindings);
       mapped.bindings = beforeBindings !== expression;
     }
 
     if (shouldMapAwait) {
       const beforeAwait = expression;
-      expression = (0, _mapAwaitExpression2.default)(expression);
+      expression = (0, _mapAwaitExpression2.default)(expression, ast);
       mapped.await = beforeAwait !== expression;
     }
   } catch (e) {
     console.warn(`Error when mapping ${expression} expression:`, e);
   }
 
   return {
     expression,
     mapped
   };
-} /* This Source Code Form is subject to the terms of the Mozilla Public
-   * License, v. 2.0. If a copy of the MPL was not distributed with this
-   * file, You can obtain one at <http://mozilla.org/MPL/2.0/>. */
+}
 
 /***/ }),
 
 /***/ 3756:
 /***/ (function(module, exports, __webpack_require__) {
 
 "use strict";
 
 
 Object.defineProperty(exports, "__esModule", {
   value: true
 });
 exports.default = mapExpressionBindings;
 
-var _ast = __webpack_require__(1375);
-
 var _helpers = __webpack_require__(1411);
 
 var _generator = __webpack_require__(2365);
 
 var _generator2 = _interopRequireDefault(_generator);
 
 var _types = __webpack_require__(2268);
 
 var t = _interopRequireWildcard(_types);
 
 function _interopRequireWildcard(obj) { if (obj && obj.__esModule) { return obj; } else { var newObj = {}; if (obj != null) { for (var key in obj) { if (Object.prototype.hasOwnProperty.call(obj, key)) newObj[key] = obj[key]; } } newObj.default = obj; return newObj; } }
 
 function _interopRequireDefault(obj) { return obj && obj.__esModule ? obj : { default: obj }; }
 
-/* This Source Code Form is subject to the terms of the Mozilla Public
- * License, v. 2.0. If a copy of the MPL was not distributed with this
- * file, You can obtain one at <http://mozilla.org/MPL/2.0/>. */
-
 function getAssignmentTarget(node, bindings) {
   if (t.isObjectPattern(node)) {
     for (const property of node.properties) {
       if (t.isRestElement(property)) {
         property.argument = getAssignmentTarget(property.argument, bindings);
       } else {
         property.value = getAssignmentTarget(property.value, bindings);
       }
@@ -23619,16 +23621,20 @@ function getAssignmentTarget(node, bindi
     return bindings.includes(node.name) ? node : t.memberExpression(t.identifier("self"), node);
   }
 
   return node;
 }
 
 // translates new bindings `var a = 3` into `self.a = 3`
 // and existing bindings `var a = 3` into `a = 3` for re-assignments
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at <http://mozilla.org/MPL/2.0/>. */
+
 function globalizeDeclaration(node, bindings) {
   return node.declarations.map(declaration => t.expressionStatement(t.assignmentExpression("=", getAssignmentTarget(declaration.id, bindings), declaration.init)));
 }
 
 // translates new bindings `a = 3` into `self.a = 3`
 // and keeps assignments the same for existing bindings.
 function globalizeAssignment(node, bindings) {
   return t.assignmentExpression(node.operator, getAssignmentTarget(node.left, bindings), node.right);
@@ -23643,19 +23649,17 @@ function replaceNode(ancestors, node) {
     } else {
       parent.node[parent.key][parent.index] = node;
     }
   } else {
     parent.node[parent.key] = node;
   }
 }
 
-function mapExpressionBindings(expression, bindings = []) {
-  const ast = (0, _ast.parseConsoleScript)(expression);
-
+function mapExpressionBindings(expression, ast, bindings = []) {
   let isMapped = false;
   let shouldUpdate = true;
 
   t.traverse(ast, (node, ancestors) => {
     const parent = ancestors[ancestors.length - 1];
 
     if (t.isWithStatement(node)) {
       shouldUpdate = false;
@@ -44366,40 +44370,51 @@ var _helpers = __webpack_require__(1411)
 function _interopRequireWildcard(obj) { if (obj && obj.__esModule) { return obj; } else { var newObj = {}; if (obj != null) { for (var key in obj) { if (Object.prototype.hasOwnProperty.call(obj, key)) newObj[key] = obj[key]; } } newObj.default = obj; return newObj; } }
 
 function _interopRequireDefault(obj) { return obj && obj.__esModule ? obj : { default: obj }; }
 
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at <http://mozilla.org/MPL/2.0/>. */
 
-function hasTopLevelAwait(expression) {
-  const ast = (0, _ast.parseConsoleScript)(expression);
+function hasTopLevelAwait(ast) {
   const hasAwait = (0, _ast.hasNode)(ast, (node, ancestors, b) => t.isAwaitExpression(node) && (0, _helpers.isTopLevel)(ancestors));
 
-  return hasAwait && ast;
-}
-
-function wrapExpression(ast) {
+  return hasAwait;
+}
+
+function wrapExpressionFromAst(ast) {
   const statements = ast.program.body;
   const lastStatement = statements[statements.length - 1];
   const body = statements.slice(0, -1).concat(t.returnStatement(lastStatement.expression));
 
   const newAst = t.expressionStatement(t.callExpression(t.arrowFunctionExpression([], t.blockStatement(body), true), []));
 
   return (0, _generator2.default)(newAst).code;
 }
 
-function mapTopLevelAwait(expression) {
-  const ast = hasTopLevelAwait(expression);
-  if (ast) {
-    return wrapExpression(ast);
-  }
-
-  return expression;
+function mapTopLevelAwait(expression, ast) {
+  if (!ast) {
+    // If there's no ast this means the expression is malformed. And if the
+    // expression contains the await keyword, we still want to wrap it in an
+    // async iife in order to get a meaningful message (without this, the
+    // engine will throw an Error stating that await keywords are only valid
+    // in async functions and generators).
+    if (expression.includes("await ")) {
+      return `(async () => { ${expression} })();`;
+    }
+
+    return expression;
+  }
+
+  if (!hasTopLevelAwait(ast)) {
+    return expression;
+  }
+
+  return wrapExpressionFromAst(ast);
 }
 
 /***/ }),
 
 /***/ 398:
 /***/ (function(module, exports, __webpack_require__) {
 
 /* WEBPACK VAR INJECTION */(function(module) {var root = __webpack_require__(8);
--- a/devtools/client/debugger/new/src/workers/parser/mapAwaitExpression.js
+++ b/devtools/client/debugger/new/src/workers/parser/mapAwaitExpression.js
@@ -2,46 +2,60 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at <http://mozilla.org/MPL/2.0/>. */
 
 // @flow
 
 import generate from "@babel/generator";
 import * as t from "@babel/types";
 
-import { parseConsoleScript, hasNode } from "./utils/ast";
+import { hasNode } from "./utils/ast";
 import { isTopLevel } from "./utils/helpers";
 
-function hasTopLevelAwait(expression: string) {
-  const ast = parseConsoleScript(expression);
+function hasTopLevelAwait(ast: Object): boolean {
   const hasAwait = hasNode(
     ast,
     (node, ancestors, b) => t.isAwaitExpression(node) && isTopLevel(ancestors)
   );
 
-  return hasAwait && ast;
+  return hasAwait;
 }
 
-function wrapExpression(ast) {
+function wrapExpressionFromAst(ast): string {
   const statements = ast.program.body;
   const lastStatement = statements[statements.length - 1];
   const body = statements
     .slice(0, -1)
     .concat(t.returnStatement(lastStatement.expression));
 
   const newAst = t.expressionStatement(
     t.callExpression(
       t.arrowFunctionExpression([], t.blockStatement(body), true),
       []
     )
   );
 
   return generate(newAst).code;
 }
 
-export default function mapTopLevelAwait(expression: string) {
-  const ast = hasTopLevelAwait(expression);
-  if (ast) {
-    return wrapExpression(ast);
+export default function mapTopLevelAwait(
+  expression: string,
+  ast?: Object
+): string {
+  if (!ast) {
+    // If there's no ast this means the expression is malformed. And if the
+    // expression contains the await keyword, we still want to wrap it in an
+    // async iife in order to get a meaningful message (without this, the
+    // engine will throw an Error stating that await keywords are only valid
+    // in async functions and generators).
+    if (expression.includes("await ")) {
+      return `(async () => { ${expression} })();`;
+    }
+
+    return expression;
   }
 
-  return expression;
+  if (!hasTopLevelAwait(ast)) {
+    return expression;
+  }
+
+  return wrapExpressionFromAst(ast);
 }
--- a/devtools/client/debugger/new/src/workers/parser/mapBindings.js
+++ b/devtools/client/debugger/new/src/workers/parser/mapBindings.js
@@ -1,15 +1,14 @@
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at <http://mozilla.org/MPL/2.0/>. */
 
 // @flow
 
-import { parseConsoleScript } from "./utils/ast";
 import { isTopLevel } from "./utils/helpers";
 
 import generate from "@babel/generator";
 import * as t from "@babel/types";
 
 function getAssignmentTarget(node, bindings) {
   if (t.isObjectPattern(node)) {
     for (const property of node.properties) {
@@ -87,20 +86,19 @@ function replaceNode(ancestors, node) {
     }
   } else {
     parent.node[parent.key] = node;
   }
 }
 
 export default function mapExpressionBindings(
   expression: string,
+  ast?: Object,
   bindings: string[] = []
 ): string {
-  const ast = parseConsoleScript(expression);
-
   let isMapped = false;
   let shouldUpdate = true;
 
   t.traverse(ast, (node, ancestors) => {
     const parent = ancestors[ancestors.length - 1];
 
     if (t.isWithStatement(node)) {
       shouldUpdate = false;
--- a/devtools/client/debugger/new/src/workers/parser/mapExpression.js
+++ b/devtools/client/debugger/new/src/workers/parser/mapExpression.js
@@ -1,14 +1,15 @@
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at <http://mozilla.org/MPL/2.0/>. */
 
 // @flow
 
+import { parseConsoleScript } from "./utils/ast";
 import mapOriginalExpression from "./mapOriginalExpression";
 import mapExpressionBindings from "./mapBindings";
 import mapTopLevelAwait from "./mapAwaitExpression";
 
 export default function mapExpression(
   expression: string,
   mappings: {
     [string]: string | null
@@ -25,32 +26,33 @@ export default function mapExpression(
   }
 } {
   const mapped = {
     await: false,
     bindings: false,
     originalExpression: false
   };
 
+  const ast = parseConsoleScript(expression);
   try {
-    if (mappings) {
+    if (mappings && ast) {
       const beforeOriginalExpression = expression;
-      expression = mapOriginalExpression(expression, mappings);
+      expression = mapOriginalExpression(expression, ast, mappings);
       mapped.originalExpression = beforeOriginalExpression !== expression;
     }
 
-    if (shouldMapBindings) {
+    if (shouldMapBindings && ast) {
       const beforeBindings = expression;
-      expression = mapExpressionBindings(expression, bindings);
+      expression = mapExpressionBindings(expression, ast, bindings);
       mapped.bindings = beforeBindings !== expression;
     }
 
     if (shouldMapAwait) {
       const beforeAwait = expression;
-      expression = mapTopLevelAwait(expression);
+      expression = mapTopLevelAwait(expression, ast);
       mapped.await = beforeAwait !== expression;
     }
   } catch (e) {
     console.warn(`Error when mapping ${expression} expression:`, e);
   }
 
   return {
     expression,
--- a/devtools/client/debugger/new/src/workers/parser/mapOriginalExpression.js
+++ b/devtools/client/debugger/new/src/workers/parser/mapOriginalExpression.js
@@ -1,16 +1,16 @@
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at <http://mozilla.org/MPL/2.0/>. */
 
 // @flow
 
 import type { Position } from "../../types";
-import { parseScript, parseConsoleScript } from "./utils/ast";
+import { parseScript } from "./utils/ast";
 import { buildScopeList } from "./getScopes";
 import generate from "@babel/generator";
 import * as t from "@babel/types";
 
 // NOTE: this will only work if we are replacing an original identifier
 function replaceNode(ancestors, node) {
   const ancestor = ancestors[ancestors.length - 1];
 
@@ -31,21 +31,21 @@ function getFirstExpression(ast) {
 }
 
 function locationKey(start: Position): string {
   return `${start.line}:${start.column}`;
 }
 
 export default function mapOriginalExpression(
   expression: string,
+  ast: ?Object,
   mappings: {
     [string]: string | null
   }
 ): string {
-  const ast = parseConsoleScript(expression);
   const scopes = buildScopeList(ast, "");
   let shouldUpdate = false;
 
   const nodes = new Map();
   const replacements = new Map();
 
   // The ref-only global bindings are the ones that are accessed, but not
   // declared anywhere in the parsed code, meaning they are either global,
--- a/devtools/client/debugger/new/src/workers/parser/tests/mapBindings.spec.js
+++ b/devtools/client/debugger/new/src/workers/parser/tests/mapBindings.spec.js
@@ -1,28 +1,37 @@
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at <http://mozilla.org/MPL/2.0/>. */
 
 import mapExpressionBindings from "../mapBindings";
+import { parseConsoleScript } from "../utils/ast";
 import cases from "jest-in-case";
 
 const prettier = require("prettier");
 
 function format(code) {
   return prettier.format(code, { semi: false, parser: "babylon" });
 }
 
 function excludedTest({ name, expression, bindings = [] }) {
-  const safeExpression = mapExpressionBindings(expression, bindings);
+  const safeExpression = mapExpressionBindings(
+    expression,
+    parseConsoleScript(expression),
+    bindings
+  );
   expect(format(safeExpression)).toEqual(format(expression));
 }
 
 function includedTest({ name, expression, newExpression, bindings }) {
-  const safeExpression = mapExpressionBindings(expression, bindings);
+  const safeExpression = mapExpressionBindings(
+    expression,
+    parseConsoleScript(expression),
+    bindings
+  );
   expect(format(safeExpression)).toEqual(format(newExpression));
 }
 
 describe("mapExpressionBindings", () => {
   cases("included cases", includedTest, [
     {
       name: "single declaration",
       expression: "const a = 2; let b = 3; var c = 4;",
--- a/devtools/client/debugger/new/src/workers/parser/tests/mapExpression.spec.js
+++ b/devtools/client/debugger/new/src/workers/parser/tests/mapExpression.spec.js
@@ -7,29 +7,36 @@ import { format } from "prettier";
 import cases from "jest-in-case";
 
 function test({
   expression,
   newExpression,
   bindings,
   mappings,
   shouldMapExpression,
-  expectedMapped
+  expectedMapped,
+  parseExpression = true
 }) {
   const res = mapExpression(
     expression,
     mappings,
     bindings,
     shouldMapExpression
   );
-  expect(
-    format(res.expression, {
-      parser: "babylon"
-    })
-  ).toEqual(format(newExpression, { parser: "babylon" }));
+
+  if (parseExpression) {
+    expect(
+      format(res.expression, {
+        parser: "babylon"
+      })
+    ).toEqual(format(newExpression, { parser: "babylon" }));
+  } else {
+    expect(res.expression).toEqual(newExpression);
+  }
+
   expect(res.mapped).toEqual(expectedMapped);
 }
 
 function formatAwait(body) {
   return `(async () => { ${body} })();`;
 }
 
 describe("mapExpression", () => {
@@ -290,16 +297,30 @@ describe("mapExpression", () => {
       shouldMapExpression: true,
       expectedMapped: {
         await: true,
         bindings: true,
         originalExpression: false
       }
     },
     {
+      name: "await (with SyntaxError)",
+      expression: "await new Promise())",
+      newExpression: formatAwait("await new Promise())"),
+      parseExpression: false,
+      bindings: [],
+      mappings: {},
+      shouldMapExpression: true,
+      expectedMapped: {
+        await: true,
+        bindings: false,
+        originalExpression: false
+      }
+    },
+    {
       name: "simple",
       expression: "a",
       newExpression: "a",
       bindings: [],
       mappings: {},
       shouldMapExpression: true,
       expectedMapped: {
         await: false,
--- a/devtools/client/debugger/new/src/workers/parser/tests/mapOriginalExpression.spec.js
+++ b/devtools/client/debugger/new/src/workers/parser/tests/mapOriginalExpression.spec.js
@@ -1,20 +1,23 @@
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at <http://mozilla.org/MPL/2.0/>. */
 
-import mapOriginalExpression from "../mapOriginalExpression";
+import mapExpression from "../mapExpression";
 import { format } from "prettier";
 
 const formatOutput = output =>
   format(output, {
     parser: "babylon"
   });
 
+const mapOriginalExpression = (expression, mappings) =>
+  mapExpression(expression, mappings, null, false, false).expression;
+
 describe("mapOriginalExpression", () => {
   it("simple", () => {
     const generatedExpression = mapOriginalExpression("a + b;", {
       a: "foo",
       b: "bar"
     });
     expect(generatedExpression).toEqual("foo + bar;");
   });
--- a/devtools/client/debugger/new/src/workers/parser/utils/ast.js
+++ b/devtools/client/debugger/new/src/workers/parser/utils/ast.js
@@ -93,22 +93,26 @@ function parseVueScript(code) {
       ast.program.sourceType = "module";
     }
   } else {
     ast = parse(code, sourceOptions.original);
   }
   return ast;
 }
 
-export function parseConsoleScript(text: string, opts?: Object) {
-  return _parse(text, {
-    plugins: ["objectRestSpread"],
-    ...opts,
-    allowAwaitOutsideFunction: true
-  });
+export function parseConsoleScript(text: string, opts?: Object): Object | null {
+  try {
+    return _parse(text, {
+      plugins: ["objectRestSpread"],
+      ...opts,
+      allowAwaitOutsideFunction: true
+    });
+  } catch (e) {
+    return null;
+  }
 }
 
 export function parseScript(text: string, opts?: Object) {
   return _parse(text, opts);
 }
 
 export function getAst(sourceId: string) {
   if (ASTs.has(sourceId)) {