fix: disallow all parenthesized pattern except parsing LHS (#12327)

* fix: disallow all parenthesized pattern except parsing LHS * fix: forStatement requires LHS * simplify toAssignable * add more test cases * fix: pass through isLHS on object property and assignment expression * fix: record parenthesized identifier error for LHS * remove duplicated skipped tests * fix: do not record errors on ancestry arrow head * Update packages/babel-parser/src/util/expression-scope.js Co-authored-by: Brian Ng <bng412@gmail.com> Co-authored-by: Brian Ng <bng412@gmail.com>
2020-11-10 14:42:37 -05:00
parent 08c7280167
commit 5bbad8936b
93 changed files with 2273 additions and 115 deletions
--- a/packages/babel-parser/src/parser/expression.js
+++ b/packages/babel-parser/src/parser/expression.js
@@ -278,7 +278,7 @@ export default class ExpressionParser extends LValParser {
      node.operator = operator;

      if (this.match(tt.eq)) {
-        node.left = this.toAssignable(left);
+        node.left = this.toAssignable(left, /* isLHS */ true);
        refExpressionErrors.doubleProto = -1; // reset because double __proto__ is valid in assignment expression
      } else {
        node.left = left;
@@ -842,7 +842,6 @@ export default class ExpressionParser extends LValParser {
    nodeForExtra?: ?N.Node,
  ): $ReadOnlyArray<?N.Expression> {
    const elts = [];
-    let innerParenStart;
    let first = true;
    const oldInFSharpPipelineDirectBody = this.state.inFSharpPipelineDirectBody;
    this.state.inFSharpPipelineDirectBody = false;
@@ -875,12 +874,6 @@ export default class ExpressionParser extends LValParser {
        }
      }

-      // we need to make sure that if this is an async arrow functions,
-      // that we don't allow inner parens inside the params
-      if (this.match(tt.parenL) && !innerParenStart) {
-        innerParenStart = this.state.start;
-      }
-
      elts.push(
        this.parseExprListItem(
          false,
@@ -891,11 +884,6 @@ export default class ExpressionParser extends LValParser {
      );
    }

-    // we found an async arrow function so let's not allow any inner parens
-    if (possibleAsyncArrow && innerParenStart && this.shouldParseAsyncArrow()) {
-      this.unexpected();
-    }
-
    this.state.inFSharpPipelineDirectBody = oldInFSharpPipelineDirectBody;

    return elts;
@@ -1421,12 +1409,6 @@ export default class ExpressionParser extends LValParser {
    ) {
      this.expressionScope.validateAsPattern();
      this.expressionScope.exit();
-      for (const param of exprList) {
-        if (param.extra && param.extra.parenthesized) {
-          this.unexpected(param.extra.parenStart);
-        }
-      }
-
      this.parseArrowExpression(arrowNode, exprList, false);
      return arrowNode;
    }
@@ -2056,7 +2038,7 @@ export default class ExpressionParser extends LValParser {
    params: N.Expression[],
    trailingCommaPos: ?number,
  ): void {
-    node.params = this.toAssignableList(params, trailingCommaPos);
+    node.params = this.toAssignableList(params, trailingCommaPos, false);
  }

  parseFunctionBodyAndFinish(
--- a/packages/babel-parser/src/parser/lval.js
+++ b/packages/babel-parser/src/parser/lval.js
@@ -1,5 +1,6 @@
 // @flow

+/*:: declare var invariant; */
 import * as charCodes from "charcodes";
 import { types as tt, type TokenType } from "../tokenizer/types";
 import type {
@@ -24,7 +25,7 @@ import { type BindingTypes, BIND_NONE } from "../util/scopeflags";
 import { ExpressionErrors } from "./util";
 import { Errors } from "./error";

-const unwrapParenthesizedExpression = (node: Node) => {
+const unwrapParenthesizedExpression = (node: Node): Node => {
  return node.type === "ParenthesizedExpression"
    ? unwrapParenthesizedExpression(node.expression)
    : node;
@@ -51,19 +52,45 @@ export default class LValParser extends NodeUtils {
  +parseDecorator: () => Decorator;
  */

-  // Convert existing expression atom to assignable pattern
-  // if possible.
-  // NOTE: There is a corresponding "isAssignable" method in flow.js.
-  // When this one is updated, please check if also that one needs to be updated.
+  /**
+   * Convert existing expression atom to assignable pattern
+   * if possible. Also checks invalid destructuring targets:

-  toAssignable(node: Node): Node {
+   - Parenthesized Destructuring patterns
+   - RestElement is not the last element
+   - Missing `=` in assignment pattern
+
+   NOTE: There is a corresponding "isAssignable" method in flow.js.
+   When this one is updated, please check if also that one needs to be updated.
+
+   * @param {Node} node The expression atom
+   * @param {boolean} [isLHS=false] Whether we are parsing a LeftHandSideExpression. If isLHS is `true`, the following cases are allowed:
+                                    `[(a)] = [0]`, `[(a.b)] = [0]`
+
+   * @returns {Node} The converted assignable pattern
+   * @memberof LValParser
+   */
+  toAssignable(node: Node, isLHS: boolean = false): Node {
    let parenthesized = undefined;
    if (node.type === "ParenthesizedExpression" || node.extra?.parenthesized) {
      parenthesized = unwrapParenthesizedExpression(node);
-      if (
-        parenthesized.type !== "Identifier" &&
-        parenthesized.type !== "MemberExpression"
-      ) {
+      if (isLHS) {
+        // an LHS can be reinterpreted to a binding pattern but not vice versa.
+        // therefore a parenthesized identifier is ambiguous until we are sure it is an assignment expression
+        // i.e. `([(a) = []] = []) => {}`
+        // see also `recordParenthesizedIdentifierError` signature in packages/babel-parser/src/util/expression-scope.js
+        if (parenthesized.type === "Identifier") {
+          this.expressionScope.recordParenthesizedIdentifierError(
+            node.start,
+            Errors.InvalidParenthesizedAssignment,
+          );
+        } else if (parenthesized.type !== "MemberExpression") {
+          // A parenthesized member expression can be in LHS but not in pattern.
+          // If the LHS is later interpreted as a pattern, `checkLVal` will throw for member expression binding
+          // i.e. `([(a.b) = []] = []) => {}`
+          this.raise(node.start, Errors.InvalidParenthesizedAssignment);
+        }
+      } else {
        this.raise(node.start, Errors.InvalidParenthesizedAssignment);
      }
    }
@@ -84,7 +111,7 @@ export default class LValParser extends NodeUtils {
        ) {
          const prop = node.properties[i];
          const isLast = i === last;
-          this.toAssignableObjectExpressionProp(prop, isLast);
+          this.toAssignableObjectExpressionProp(prop, isLast, isLHS);

          if (
            isLast &&
@@ -97,7 +124,7 @@ export default class LValParser extends NodeUtils {
        break;

      case "ObjectProperty":
-        this.toAssignable(node.value);
+        this.toAssignable(node.value, isLHS);
        break;

      case "SpreadElement": {
@@ -105,13 +132,13 @@ export default class LValParser extends NodeUtils {

        node.type = "RestElement";
        const arg = node.argument;
-        this.toAssignable(arg);
+        this.toAssignable(arg, isLHS);
        break;
      }

      case "ArrayExpression":
        node.type = "ArrayPattern";
-        this.toAssignableList(node.elements, node.extra?.trailingComma);
+        this.toAssignableList(node.elements, node.extra?.trailingComma, isLHS);
        break;

      case "AssignmentExpression":
@@ -121,11 +148,12 @@ export default class LValParser extends NodeUtils {

        node.type = "AssignmentPattern";
        delete node.operator;
-        this.toAssignable(node.left);
+        this.toAssignable(node.left, isLHS);
        break;

      case "ParenthesizedExpression":
-        this.toAssignable(((parenthesized: any): Expression));
+        /*::invariant (parenthesized !== undefined) */
+        this.toAssignable(parenthesized, isLHS);
        break;

      default:
@@ -135,7 +163,11 @@ export default class LValParser extends NodeUtils {
    return node;
  }

-  toAssignableObjectExpressionProp(prop: Node, isLast: boolean) {
+  toAssignableObjectExpressionProp(
+    prop: Node,
+    isLast: boolean,
+    isLHS: boolean,
+  ) {
    if (prop.type === "ObjectMethod") {
      const error =
        prop.kind === "get" || prop.kind === "set"
@@ -148,7 +180,7 @@ export default class LValParser extends NodeUtils {
    } else if (prop.type === "SpreadElement" && !isLast) {
      this.raiseRestNotLast(prop.start);
    } else {
-      this.toAssignable(prop);
+      this.toAssignable(prop, isLHS);
    }
  }

@@ -157,6 +189,7 @@ export default class LValParser extends NodeUtils {
  toAssignableList(
    exprList: Expression[],
    trailingCommaPos?: ?number,
+    isLHS: boolean,
  ): $ReadOnlyArray<Pattern> {
    let end = exprList.length;
    if (end) {
@@ -165,8 +198,9 @@ export default class LValParser extends NodeUtils {
        --end;
      } else if (last?.type === "SpreadElement") {
        last.type = "RestElement";
-        const arg = last.argument;
-        this.toAssignable(arg);
+        let arg = last.argument;
+        this.toAssignable(arg, isLHS);
+        arg = unwrapParenthesizedExpression(arg);
        if (
          arg.type !== "Identifier" &&
          arg.type !== "MemberExpression" &&
@@ -186,7 +220,7 @@ export default class LValParser extends NodeUtils {
    for (let i = 0; i < end; i++) {
      const elt = exprList[i];
      if (elt) {
-        this.toAssignable(elt);
+        this.toAssignable(elt, isLHS);
        if (elt.type === "RestElement") {
          this.raiseRestNotLast(elt.start);
        }
--- a/packages/babel-parser/src/parser/statement.js
+++ b/packages/babel-parser/src/parser/statement.js
@@ -528,7 +528,7 @@ export default class StatementParser extends ExpressionParser {
    const refExpressionErrors = new ExpressionErrors();
    const init = this.parseExpression(true, refExpressionErrors);
    if (this.match(tt._in) || this.isContextual("of")) {
-      this.toAssignable(init);
+      this.toAssignable(init, /* isLHS */ true);
      const description = this.isContextual("of")
        ? "for-of statement"
        : "for-in statement";
--- a/packages/babel-parser/src/plugins/estree.js
+++ b/packages/babel-parser/src/plugins/estree.js
@@ -341,23 +341,23 @@ export default (superClass: Class<Parser>): Class<Parser> =>
      return (node: any);
    }

-    toAssignable(node: N.Node): N.Node {
+    toAssignable(node: N.Node, isLHS: boolean = false): N.Node {
      if (isSimpleProperty(node)) {
        this.toAssignable(node.value);

        return node;
      }

-      return super.toAssignable(node);
+      return super.toAssignable(node, isLHS);
    }

-    toAssignableObjectExpressionProp(prop: N.Node, isLast: boolean) {
+    toAssignableObjectExpressionProp(prop: N.Node, ...args) {
      if (prop.kind === "get" || prop.kind === "set") {
        throw this.raise(prop.key.start, Errors.PatternHasAccessor);
      } else if (prop.method) {
        throw this.raise(prop.key.start, Errors.PatternHasMethod);
      } else {
-        super.toAssignableObjectExpressionProp(prop, isLast);
+        super.toAssignableObjectExpressionProp(prop, ...args);
      }
    }

--- a/packages/babel-parser/src/plugins/flow.js
+++ b/packages/babel-parser/src/plugins/flow.js
@@ -1955,6 +1955,7 @@ export default (superClass: Class<Parser>): Class<Parser> =>
        // has not been converted yet.
        ((node.params: any): N.Expression[]),
        node.extra?.trailingComma,
+        /* isLHS */ false,
      );
      // Enter scope, as checkParams defines bindings
      this.scope.enter(SCOPE_FUNCTION | SCOPE_ARROW);
@@ -2192,11 +2193,11 @@ export default (superClass: Class<Parser>): Class<Parser> =>
      }
    }

-    toAssignable(node: N.Node): N.Node {
+    toAssignable(node: N.Node, isLHS: boolean = false): N.Node {
      if (node.type === "TypeCastExpression") {
-        return super.toAssignable(this.typeCastToParameter(node));
+        return super.toAssignable(this.typeCastToParameter(node), isLHS);
      } else {
-        return super.toAssignable(node);
+        return super.toAssignable(node, isLHS);
      }
    }

@@ -2204,6 +2205,7 @@ export default (superClass: Class<Parser>): Class<Parser> =>
    toAssignableList(
      exprList: N.Expression[],
      trailingCommaPos?: ?number,
+      isLHS: boolean,
    ): $ReadOnlyArray<N.Pattern> {
      for (let i = 0; i < exprList.length; i++) {
        const expr = exprList[i];
@@ -2211,7 +2213,7 @@ export default (superClass: Class<Parser>): Class<Parser> =>
          exprList[i] = this.typeCastToParameter(expr);
        }
      }
-      return super.toAssignableList(exprList, trailingCommaPos);
+      return super.toAssignableList(exprList, trailingCommaPos, isLHS);
    }

    // this is a list of nodes, from something like a call expression, we need to filter the
--- a/packages/babel-parser/src/plugins/typescript/index.js
+++ b/packages/babel-parser/src/plugins/typescript/index.js
@@ -2596,19 +2596,19 @@ export default (superClass: Class<Parser>): Class<Parser> =>
      return param;
    }

-    toAssignable(node: N.Node): N.Node {
+    toAssignable(node: N.Node, isLHS: boolean = false): N.Node {
      switch (node.type) {
        case "TSTypeCastExpression":
-          return super.toAssignable(this.typeCastToParameter(node));
+          return super.toAssignable(this.typeCastToParameter(node), isLHS);
        case "TSParameterProperty":
-          return super.toAssignable(node);
+          return super.toAssignable(node, isLHS);
        case "TSAsExpression":
        case "TSNonNullExpression":
        case "TSTypeAssertion":
-          node.expression = this.toAssignable(node.expression);
+          node.expression = this.toAssignable(node.expression, isLHS);
          return node;
        default:
-          return super.toAssignable(node);
+          return super.toAssignable(node, isLHS);
      }
    }

--- a/packages/babel-parser/src/util/expression-scope.js
+++ b/packages/babel-parser/src/util/expression-scope.js
@@ -20,6 +20,7 @@ some expression scopes and thrown later when we know what the ambigous pattern i
 - AwaitBindingIdentifier
 - AwaitExpressionFormalParameter
 - YieldInParameter
+- InvalidParenthesizedAssignment when parenthesized is an identifier

 There are four different expression scope
 - Expression
@@ -130,6 +131,41 @@ export default class ExpressionScopeHandler {
    /* eslint-disable @babel/development-internal/dry-error-messages */
    this.raise(pos, message);
  }
+
+  /**
+   * Record parenthesized identifier errors
+   *
+   * A parenthesized identifier in LHS can be ambiguous because the assignment
+   * can be transformed to an assignable later, but not vice versa:
+   * For example, in `([(a) = []] = []) => {}`, we think `(a) = []` is an LHS in `[(a) = []]`,
+   * an LHS within `[(a) = []] = []`. However the LHS chain is then transformed by toAssignable,
+   * and we should throw assignment `(a)`, which is only valid in LHS. Hence we record the
+   * location of parenthesized `(a)` to current scope if it is one of MaybeArrowParameterDeclaration
+   * and MaybeAsyncArrowParameterDeclaration
+   *
+   * Unlike `recordParameterInitializerError`, we don't record to ancestry scope because we
+   * validate arrow head parsing scope before exit, and then the LHS will be unambiguous:
+   * For example, in `( x = ( [(a) = []] = [] ) ) => {}`, we should not record `(a)` in `( x = ... ) =>`
+   * arrow scope because when we finish parsing `( [(a) = []] = [] )`, it is an unambiguous assignment
+   * expression and can not be cast to pattern
+   * @param {number} pos
+   * @param {string} message
+   * @returns {void}
+   * @memberof ExpressionScopeHandler
+   */
+  recordParenthesizedIdentifierError(pos: number, message: string): void {
+    const { stack } = this;
+    const scope: ExpressionScope = stack[stack.length - 1];
+    if (scope.isCertainlyParameterDeclaration()) {
+      this.raise(pos, message);
+    } else if (scope.canBeArrowParameterDeclaration()) {
+      /*:: invariant(scope instanceof ArrowHeadParsingScope) */
+      scope.recordDeclarationError(pos, message);
+    } else {
+      return;
+    }
+  }
+
  /**
   * Record likely async arrow parameter errors
   *