[ts] Fix restrictions for optional parameters (#15414)

Fixes #15396
babel · Feb 9, 2023 · d3bd1a3 · d3bd1a3
1 parent 9112b8f
commit d3bd1a3
Show file tree

Hide file tree

Showing 23 changed files with 249 additions and 89 deletions.
diff --git a/packages/babel-parser/src/parser/expression.ts b/packages/babel-parser/src/parser/expression.ts
@@ -95,10 +95,7 @@ export default abstract class ExpressionParser extends LValParser {
     allowExpressionBody?: boolean,
     isAsync?: boolean,
   ): T;
-  abstract parseFunctionParams(
-    node: N.Function,
-    allowModifiers?: boolean,
-  ): void;
+  abstract parseFunctionParams(node: N.Function, isConstructor?: boolean): void;
   abstract parseBlockOrModuleBlockBody(
     body: N.Statement[],
     directives: N.Directive[] | null | undefined,
@@ -2427,15 +2424,14 @@ export default abstract class ExpressionParser extends LValParser {
   ): T {
     this.initFunction(node, isAsync);
     node.generator = isGenerator;
-    const allowModifiers = isConstructor; // For TypeScript parameter properties
     this.scope.enter(
       SCOPE_FUNCTION |
         SCOPE_SUPER |
         (inClassScope ? SCOPE_CLASS : 0) |
         (allowDirectSuper ? SCOPE_DIRECT_SUPER : 0),
     );
     this.prodParam.enter(functionFlags(isAsync, node.generator));
-    this.parseFunctionParams(node, allowModifiers);
+    this.parseFunctionParams(node, isConstructor);
     const finishedNode = this.parseFunctionBodyAndFinish(node, type, true);
     this.prodParam.exit();
     this.scope.exit();

diff --git a/packages/babel-parser/src/parser/lval.ts b/packages/babel-parser/src/parser/lval.ts
@@ -44,6 +44,12 @@ const unwrapParenthesizedExpression = (node: Node): Node => {
     : node;
 };
 
+export const enum ParseBindingListFlags {
+  ALLOW_EMPTY = 1 << 0,
+  IS_FUNCTION_PARAMS = 1 << 1,
+  IS_CONSTRUCTOR_PARAMS = 1 << 2,
+}
+
 export default abstract class LValParser extends NodeUtils {
   // Forward-declaration: defined in expression.js
   abstract parseIdentifier(liberal?: boolean): Identifier;
@@ -366,7 +372,7 @@ export default abstract class LValParser extends NodeUtils {
         node.elements = this.parseBindingList(
           tt.bracketR,
           charCodes.rightSquareBracket,
-          true,
+          ParseBindingListFlags.ALLOW_EMPTY,
         );
         return this.finishNode(node, "ArrayPattern");
       }
@@ -384,9 +390,10 @@ export default abstract class LValParser extends NodeUtils {
     this: Parser,
     close: TokenType,
     closeCharCode: typeof charCodes[keyof typeof charCodes],
-    allowEmpty?: boolean,
-    allowModifiers?: boolean,
+    flags: ParseBindingListFlags,
   ): Array<Pattern | TSParameterProperty> {
+    const allowEmpty = flags & ParseBindingListFlags.ALLOW_EMPTY;
+
     const elts: Array<Pattern | TSParameterProperty> = [];
     let first = true;
     while (!this.eat(close)) {
@@ -400,7 +407,9 @@ export default abstract class LValParser extends NodeUtils {
       } else if (this.eat(close)) {
         break;
       } else if (this.match(tt.ellipsis)) {
-        elts.push(this.parseAssignableListItemTypes(this.parseRestBinding()));
+        elts.push(
+          this.parseAssignableListItemTypes(this.parseRestBinding(), flags),
+        );
         if (!this.checkCommaAfterRest(closeCharCode)) {
           this.expect(close);
           break;
@@ -416,7 +425,7 @@ export default abstract class LValParser extends NodeUtils {
         while (this.match(tt.at)) {
           decorators.push(this.parseDecorator());
         }
-        elts.push(this.parseAssignableListItem(allowModifiers, decorators));
+        elts.push(this.parseAssignableListItem(flags, decorators));
       }
     }
     return elts;
@@ -460,11 +469,11 @@ export default abstract class LValParser extends NodeUtils {
 
   parseAssignableListItem(
     this: Parser,
-    allowModifiers: boolean | undefined | null,
+    flags: ParseBindingListFlags,
     decorators: Decorator[],
   ): Pattern | TSParameterProperty {
     const left = this.parseMaybeDefault();
-    this.parseAssignableListItemTypes(left);
+    this.parseAssignableListItemTypes(left, flags);
     const elt = this.parseMaybeDefault(left.loc.start, left);
     if (decorators.length) {
       left.decorators = decorators;
@@ -473,7 +482,11 @@ export default abstract class LValParser extends NodeUtils {
   }
 
   // Used by flow/typescript plugin to add type annotations to binding elements
-  parseAssignableListItemTypes(param: Pattern): Pattern {
+  parseAssignableListItemTypes(
+    param: Pattern,
+    // eslint-disable-next-line @typescript-eslint/no-unused-vars
+    flags: ParseBindingListFlags,
+  ): Pattern {
     return param;
   }
 

diff --git a/packages/babel-parser/src/parser/statement.ts b/packages/babel-parser/src/parser/statement.ts
@@ -43,6 +43,7 @@ import type { Position } from "../util/location";
 import { createPositionWithColumnOffset } from "../util/location";
 import { cloneStringLiteral, cloneIdentifier, type Undone } from "./node";
 import type Parser from "./index";
+import { ParseBindingListFlags } from "./lval";
 
 const loopLabel = { kind: "loop" } as const,
   switchLabel = { kind: "switch" } as const;
@@ -1566,7 +1567,7 @@ export default abstract class StatementParser extends ExpressionParser {
       node.id = this.parseFunctionId();
     }
 
-    this.parseFunctionParams(node, /* allowModifiers */ false);
+    this.parseFunctionParams(node, /* isConstructor */ false);
 
     // For the smartPipelines plugin: Disable topic references from outer
     // contexts within the function body. They are permitted in function
@@ -1602,15 +1603,15 @@ export default abstract class StatementParser extends ExpressionParser {
   parseFunctionParams(
     this: Parser,
     node: Undone<N.Function>,
-    allowModifiers?: boolean,
+    isConstructor?: boolean,
   ): void {
     this.expect(tt.parenL);
     this.expressionScope.enter(newParameterDeclarationScope());
     node.params = this.parseBindingList(
       tt.parenR,
       charCodes.rightParenthesis,
-      /* allowEmpty */ false,
-      allowModifiers,
+      ParseBindingListFlags.IS_FUNCTION_PARAMS |
+        (isConstructor ? ParseBindingListFlags.IS_CONSTRUCTOR_PARAMS : 0),
     );
 
     this.expressionScope.exit();

diff --git a/packages/babel-parser/src/plugins/flow/index.ts b/packages/babel-parser/src/plugins/flow/index.ts
@@ -2887,14 +2887,14 @@ export default (superClass: typeof Parser) =>
     // parse function type parameters - function foo<T>() {}
     parseFunctionParams(
       node: Undone<N.Function>,
-      allowModifiers?: boolean,
+      isConstructor: boolean,
     ): void {
       // @ts-expect-error kind may not index node
       const kind = node.kind;
       if (kind !== "get" && kind !== "set" && this.match(tt.lt)) {
         node.typeParameters = this.flowParseTypeParameterDeclaration();
       }
-      super.parseFunctionParams(node, allowModifiers);
+      super.parseFunctionParams(node, isConstructor);
     }
 
     // parse flow type annotations on variable declarator heads - let foo: string = bar
@@ -3291,7 +3291,7 @@ export default (superClass: typeof Parser) =>
       startLoc: Position,
     ): N.ArrowFunctionExpression | undefined | null {
       const node = this.startNodeAt<N.ArrowFunctionExpression>(startLoc);
-      this.parseFunctionParams(node);
+      this.parseFunctionParams(node, false);
       if (!this.parseArrow(node)) return;
       return super.parseArrowExpression(
         node,

diff --git a/packages/babel-parser/src/plugins/typescript/index.ts b/packages/babel-parser/src/plugins/typescript/index.ts
@@ -42,6 +42,7 @@ import { cloneIdentifier, type Undone } from "../../parser/node";
 import type { Pattern } from "../../types";
 import type { Expression } from "../../types";
 import type { IJSXParserMixin } from "../jsx";
+import { ParseBindingListFlags } from "../../parser/lval";
 
 const getOwn = <T extends {}>(object: T, key: keyof T) =>
   Object.hasOwnProperty.call(object, key) && object[key];
@@ -755,7 +756,11 @@ export default (superClass: ClassWithMixin<typeof Parser, IJSXParserMixin>) =>
       N.Identifier | N.RestElement | N.ObjectPattern | N.ArrayPattern
     > {
       return super
-        .parseBindingList(tt.parenR, charCodes.rightParenthesis)
+        .parseBindingList(
+          tt.parenR,
+          charCodes.rightParenthesis,
+          ParseBindingListFlags.IS_FUNCTION_PARAMS,
+        )
         .map(pattern => {
           if (
             pattern.type !== "Identifier" &&
@@ -1422,7 +1427,7 @@ export default (superClass: ClassWithMixin<typeof Parser, IJSXParserMixin>) =>
           super.parseBindingList(
             tt.bracketR,
             charCodes.rightSquareBracket,
-            true,
+            ParseBindingListFlags.ALLOW_EMPTY,
           );
           return errors.length === previousErrorCount;
         } catch {
@@ -2249,40 +2254,35 @@ export default (superClass: ClassWithMixin<typeof Parser, IJSXParserMixin>) =>
     }
 
     parseAssignableListItem(
-      allowModifiers: boolean | undefined | null,
+      flags: ParseBindingListFlags,
       decorators: N.Decorator[],
     ): N.Pattern | N.TSParameterProperty {
       // Store original location to include modifiers in range
       const startLoc = this.state.startLoc;
 
-      let accessibility: N.Accessibility | undefined | null;
-      let readonly = false;
-      let override = false;
-      if (allowModifiers !== undefined) {
-        const modified: ModifierBase = {};
-        this.tsParseModifiers({
-          modified,
-          allowedModifiers: [
-            "public",
-            "private",
-            "protected",
-            "override",
-            "readonly",
-          ],
-        });
-        accessibility = modified.accessibility;
-        override = modified.override;
-        readonly = modified.readonly;
-        if (
-          allowModifiers === false &&
-          (accessibility || readonly || override)
-        ) {
-          this.raise(TSErrors.UnexpectedParameterModifier, { at: startLoc });
-        }
+      const modified: ModifierBase = {};
+      this.tsParseModifiers({
+        modified,
+        allowedModifiers: [
+          "public",
+          "private",
+          "protected",
+          "override",
+          "readonly",
+        ],
+      });
+      const accessibility = modified.accessibility;
+      const override = modified.override;
+      const readonly = modified.readonly;
+      if (
+        !(flags & ParseBindingListFlags.IS_CONSTRUCTOR_PARAMS) &&
+        (accessibility || readonly || override)
+      ) {
+        this.raise(TSErrors.UnexpectedParameterModifier, { at: startLoc });
       }
 
       const left = this.parseMaybeDefault();
-      this.parseAssignableListItemTypes(left);
+      this.parseAssignableListItemTypes(left, flags);
       const elt = this.parseMaybeDefault(left.loc.start, left);
       if (accessibility || readonly || override) {
         const pp = this.startNodeAt<N.TSParameterProperty>(startLoc);
@@ -2314,6 +2314,27 @@ export default (superClass: ClassWithMixin<typeof Parser, IJSXParserMixin>) =>
       );
     }
 
+    tsDisallowOptionalPattern(node: Undone<N.Function>) {
+      for (const param of node.params) {
+        if (
+          param.type !== "Identifier" &&
+          (param as any).optional &&
+          !this.state.isAmbientContext
+        ) {
+          this.raise(TSErrors.PatternIsOptional, { at: param });
+        }
+      }
+    }
+
+    setArrowFunctionParameters(
+      node: Undone<N.ArrowFunctionExpression>,
+      params: N.Expression[],
+      trailingCommaLoc?: Position | null,
+    ): void {
+      super.setArrowFunctionParameters(node, params, trailingCommaLoc);
+      this.tsDisallowOptionalPattern(node);
+    }
+
     parseFunctionBodyAndFinish<
       T extends
         | N.Function
@@ -2340,6 +2361,7 @@ export default (superClass: ClassWithMixin<typeof Parser, IJSXParserMixin>) =>
           return super.parseFunctionBodyAndFinish(node, bodilessType, isMethod);
         }
       }
+      this.tsDisallowOptionalPattern(node);
 
       return super.parseFunctionBodyAndFinish(node, type, isMethod);
     }
@@ -3271,10 +3293,10 @@ export default (superClass: ClassWithMixin<typeof Parser, IJSXParserMixin>) =>
       );
     }
 
-    parseFunctionParams(node: N.Function, allowModifiers?: boolean): void {
+    parseFunctionParams(node: N.Function, isConstructor: boolean): void {
       const typeParameters = this.tsTryParseTypeParameters();
       if (typeParameters) node.typeParameters = typeParameters;
-      super.parseFunctionParams(node, allowModifiers);
+      super.parseFunctionParams(node, isConstructor);
     }
 
     // `let x: number;`
@@ -3504,16 +3526,13 @@ export default (superClass: ClassWithMixin<typeof Parser, IJSXParserMixin>) =>
     }
 
     // Allow type annotations inside of a parameter list.
-    parseAssignableListItemTypes(param: N.Pattern) {
-      if (this.eat(tt.question)) {
-        if (
-          param.type !== "Identifier" &&
-          !this.state.isAmbientContext &&
-          !this.state.inType
-        ) {
-          this.raise(TSErrors.PatternIsOptional, { at: param });
-        }
+    parseAssignableListItemTypes(
+      param: N.Pattern,
+      flags: ParseBindingListFlags,
+    ) {
+      if (!(flags & ParseBindingListFlags.IS_FUNCTION_PARAMS)) return param;
 
+      if (this.eat(tt.question)) {
         (param as any as N.Identifier).optional = true;
       }
       const type = this.tsTryParseTypeAnnotation();

diff --git a/...-parser/test/fixtures/typescript/arrow-function/async-rest-optional-parameter/output.json b/...-parser/test/fixtures/typescript/arrow-function/async-rest-optional-parameter/output.json
@@ -1,6 +1,9 @@
 {
   "type": "File",
   "start":0,"end":34,"loc":{"start":{"line":1,"column":0,"index":0},"end":{"line":1,"column":34,"index":34}},
+  "errors": [
+    "SyntaxError: A binding pattern parameter cannot be optional in an implementation signature. (1:6)"
+  ],
   "program": {
     "type": "Program",
     "start":0,"end":34,"loc":{"start":{"line":1,"column":0,"index":0},"end":{"line":1,"column":34,"index":34}},

diff --git a/...babel-parser/test/fixtures/typescript/function/pattern-optional-parameters-arrow/input.ts b/...babel-parser/test/fixtures/typescript/function/pattern-optional-parameters-arrow/input.ts
@@ -0,0 +1 @@
+([]?, {}) => {}
diff --git a/...el-parser/test/fixtures/typescript/function/pattern-optional-parameters-arrow/output.json b/...el-parser/test/fixtures/typescript/function/pattern-optional-parameters-arrow/output.json
@@ -0,0 +1,46 @@
+{
+  "type": "File",
+  "start":0,"end":15,"loc":{"start":{"line":1,"column":0,"index":0},"end":{"line":1,"column":15,"index":15}},
+  "errors": [
+    "SyntaxError: A binding pattern parameter cannot be optional in an implementation signature. (1:1)"
+  ],
+  "program": {
+    "type": "Program",
+    "start":0,"end":15,"loc":{"start":{"line":1,"column":0,"index":0},"end":{"line":1,"column":15,"index":15}},
+    "sourceType": "module",
+    "interpreter": null,
+    "body": [
+      {
+        "type": "ExpressionStatement",
+        "start":0,"end":15,"loc":{"start":{"line":1,"column":0,"index":0},"end":{"line":1,"column":15,"index":15}},
+        "expression": {
+          "type": "ArrowFunctionExpression",
+          "start":0,"end":15,"loc":{"start":{"line":1,"column":0,"index":0},"end":{"line":1,"column":15,"index":15}},
+          "id": null,
+          "generator": false,
+          "async": false,
+          "params": [
+            {
+              "type": "ArrayPattern",
+              "start":1,"end":4,"loc":{"start":{"line":1,"column":1,"index":1},"end":{"line":1,"column":4,"index":4}},
+              "elements": [],
+              "optional": true
+            },
+            {
+              "type": "ObjectPattern",
+              "start":6,"end":8,"loc":{"start":{"line":1,"column":6,"index":6},"end":{"line":1,"column":8,"index":8}},
+              "properties": []
+            }
+          ],
+          "body": {
+            "type": "BlockStatement",
+            "start":13,"end":15,"loc":{"start":{"line":1,"column":13,"index":13},"end":{"line":1,"column":15,"index":15}},
+            "body": [],
+            "directives": []
+          }
+        }
+      }
+    ],
+    "directives": []
+  }
+}
diff --git a/...ript/function/pattern-parameters/input.ts → ...tion/pattern-optional-parameters/input.ts b/...ript/function/pattern-parameters/input.ts → ...tion/pattern-optional-parameters/input.ts
diff --git a/...t/function/pattern-parameters/output.json → ...n/pattern-optional-parameters/output.json b/...t/function/pattern-parameters/output.json → ...n/pattern-optional-parameters/output.json