Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ts] Fix restrictions for optional parameters #15414

Merged
merged 5 commits into from Feb 9, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
8 changes: 2 additions & 6 deletions packages/babel-parser/src/parser/expression.ts
Expand Up @@ -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,
Expand Down Expand Up @@ -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();
Expand Down
29 changes: 21 additions & 8 deletions packages/babel-parser/src/parser/lval.ts
Expand Up @@ -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;
Expand Down Expand Up @@ -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");
}
Expand All @@ -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)) {
Expand All @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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;
Expand All @@ -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;
}

Expand Down
9 changes: 5 additions & 4 deletions packages/babel-parser/src/parser/statement.ts
Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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();
Expand Down
6 changes: 3 additions & 3 deletions packages/babel-parser/src/plugins/flow/index.ts
Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand Down
97 changes: 58 additions & 39 deletions packages/babel-parser/src/plugins/typescript/index.ts
Expand Up @@ -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];
Expand Down Expand Up @@ -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" &&
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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
Expand All @@ -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);
}
Expand Down Expand Up @@ -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;`
Expand Down Expand Up @@ -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();
Expand Down
@@ -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}},
Expand Down
@@ -0,0 +1 @@
([]?, {}) => {}
@@ -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": []
}
}