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.

Sign up for GitHub

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

Jump to bottom

Raise recoverable error for type members with invalid modifiers #12785

Merged
merged 9 commits into from Feb 12, 2021
Merged

Raise recoverable error for type members with invalid modifiers #12785

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
e8a5fc1
tsCheckModifiers
sosukesuzuki Feb 9, 2021
9b1b5d8
Rasie error when tsParseModifiers
sosukesuzuki Feb 9, 2021
82a8e85
Fix tests
sosukesuzuki Feb 9, 2021
6739d43
Error for readonly method signature
sosukesuzuki Feb 9, 2021
ededec2
Use callback
sosukesuzuki Feb 9, 2021
c6c531a
Better interface
sosukesuzuki Feb 11, 2021
dc1cfe5
Add review
sosukesuzuki Feb 11, 2021
2a82123
Address review
sosukesuzuki Feb 11, 2021
b79fd6b
Use optional chaining
sosukesuzuki Feb 12, 2021
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
46 changes: 42 additions & 4 deletions packages/babel-parser/src/plugins/typescript/index.js
View file
Open in desktop
Expand Up @@ -82,6 +82,7 @@ const TSErrors = Object.freeze({
IndexSignatureHasStatic: "Index signatures cannot have the 'static' modifier",
IndexSignatureHasDeclare:
"Index signatures cannot have the 'declare' modifier",
InvalidModifierOnTypeMember: "'%0' modifier cannot appear on a type member.",
InvalidTupleMemberLabel:
"Tuple members must be labeled with a simple identifier.",
MixedLabeledAndUnlabeledElements:
Expand All @@ -98,6 +99,8 @@ const TSErrors = Object.freeze({
"Private elements cannot have the 'abstract' modifier.",
PrivateElementHasAccessibility:
"Private elements cannot have an accessibility modifier ('%0')",
ReadonlyForMethodSignature:
"'readonly' modifier can only appear on a property declaration or index signature.",
TypeAnnotationAfterAssign:
"Type annotations must come before default assignments, e.g. instead of `age = 25: number` use `age: number = 25`",
UnexpectedParameterModifier:
Expand Down Expand Up @@ -207,6 +210,7 @@ export default (superClass: Class<Parser>): Class<Parser> =>
accessibility?: N.Accessibility,
},
allowedModifiers: TsModifier[],
callback?: (startPos: number, modifer: TsModifier) => void,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alternative interface design

tsParseModifiers(
      modified: {
        [key: TsModifier]: ?true,
        accessibility?: N.Accessibility,
      },
      allowedModifiers: TsModifier[],
      disallowedModifiers: TsModifier[],
      errorTemplate: string
): void

So we can raise errors when a disallowed modifier is seen and it can be reused in other situations. Introducing callback to parsers can be very nasty.

): void {
for (;;) {
const startPos = this.state.start;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can pass down allowedModifiers.concat(disallowedModifier) to tsParserModifier so we don't have to duplicate disallowedModifier in the tsParseModifiers call.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: allowedModifiers.concat(disallowedModifier ?? [])

Expand All @@ -226,6 +230,10 @@ export default (superClass: Class<Parser>): Class<Parser> =>
}
modified[modifier] = true;
}

if (callback) {
callback(startPos, modifier);
}
}
}

Expand Down Expand Up @@ -537,7 +545,10 @@ export default (superClass: Class<Parser>): Class<Parser> =>
if (this.eat(tt.question)) node.optional = true;
const nodeAny: any = node;

if (!readonly && (this.match(tt.parenL) || this.isRelational("<"))) {
if (this.match(tt.parenL) || this.isRelational("<")) {
if (readonly) {
this.raise(node.start, TSErrors.ReadonlyForMethodSignature);
}
const method: N.TsMethodSignature = nodeAny;
this.tsFillSignature(tt.colon, method);
this.tsParseTypeMemberSemicolon();
Expand Down Expand Up @@ -573,16 +584,43 @@ export default (superClass: Class<Parser>): Class<Parser> =>
}
}

const readonly = !!this.tsParseModifier(["readonly"]);
const denyModifiers = [
"declare",
"abstract",
"private",
"protected",
"public",
"static",
];
this.tsParseModifiers(
node,
[
"readonly",
"declare",
"abstract",
"private",
"protected",
"public",
"static",
],
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
[
"readonly",
"declare",
"abstract",
"private",
"protected",
"public",
"static",
],
["readonly", ...denyModifiers],

so that the two lists don't get out of sync.

(startPos, modifier) => {
if (denyModifiers.includes(modifier)) {
this.raise(
startPos,
TSErrors.InvalidModifierOnTypeMember,
modifier,
);
}
},
);

const idx = this.tsTryParseIndexSignature(node);
if (idx) {
if (readonly) node.readonly = true;
return idx;
}

this.parsePropertyName(node, /* isPrivateNameAllowed */ false);
return this.tsParsePropertyOrMethodSignature(node, readonly);
return this.tsParsePropertyOrMethodSignature(node, !!node.readonly);
}

tsParseTypeLiteral(): N.TsTypeLiteral {
Expand Down
9 changes: 9 additions & 0 deletions packages/babel-parser/test/fixtures/typescript/interface/invalid-modifiers-method/input.ts
View file
Open in desktop
@@ -0,0 +1,9 @@
interface Foo {
private a();
public b();
protected c();
static d();
declare e();
abstract f();
readonly g();
}
121 changes: 121 additions & 0 deletions ...ages/babel-parser/test/fixtures/typescript/interface/invalid-modifiers-method/output.json
View file
Open in desktop
@@ -0,0 +1,121 @@
{
"type": "File",
"start":0,"end":124,"loc":{"start":{"line":1,"column":0},"end":{"line":9,"column":1}},
"errors": [
"SyntaxError: 'private' modifier cannot appear on a type member. (2:2)",
"SyntaxError: 'public' modifier cannot appear on a type member. (3:2)",
"SyntaxError: 'protected' modifier cannot appear on a type member. (4:2)",
"SyntaxError: 'static' modifier cannot appear on a type member. (5:2)",
"SyntaxError: 'declare' modifier cannot appear on a type member. (6:2)",
"SyntaxError: 'abstract' modifier cannot appear on a type member. (7:2)",
"SyntaxError: 'readonly' modifier can only appear on a property declaration or index signature. (8:2)"
],
"program": {
"type": "Program",
"start":0,"end":124,"loc":{"start":{"line":1,"column":0},"end":{"line":9,"column":1}},
"sourceType": "module",
"interpreter": null,
"body": [
{
"type": "TSInterfaceDeclaration",
"start":0,"end":124,"loc":{"start":{"line":1,"column":0},"end":{"line":9,"column":1}},
"id": {
"type": "Identifier",
"start":10,"end":13,"loc":{"start":{"line":1,"column":10},"end":{"line":1,"column":13},"identifierName":"Foo"},
"name": "Foo"
},
"body": {
"type": "TSInterfaceBody",
"start":14,"end":124,"loc":{"start":{"line":1,"column":14},"end":{"line":9,"column":1}},
"body": [
{
"type": "TSMethodSignature",
"start":18,"end":30,"loc":{"start":{"line":2,"column":2},"end":{"line":2,"column":14}},
"accessibility": "private",
"key": {
"type": "Identifier",
"start":26,"end":27,"loc":{"start":{"line":2,"column":10},"end":{"line":2,"column":11},"identifierName":"a"},
"name": "a"
},
"computed": false,
"parameters": []
},
{
"type": "TSMethodSignature",
"start":33,"end":44,"loc":{"start":{"line":3,"column":2},"end":{"line":3,"column":13}},
"accessibility": "public",
"key": {
"type": "Identifier",
"start":40,"end":41,"loc":{"start":{"line":3,"column":9},"end":{"line":3,"column":10},"identifierName":"b"},
"name": "b"
},
"computed": false,
"parameters": []
},
{
"type": "TSMethodSignature",
"start":47,"end":61,"loc":{"start":{"line":4,"column":2},"end":{"line":4,"column":16}},
"accessibility": "protected",
"key": {
"type": "Identifier",
"start":57,"end":58,"loc":{"start":{"line":4,"column":12},"end":{"line":4,"column":13},"identifierName":"c"},
"name": "c"
},
"computed": false,
"parameters": []
},
{
"type": "TSMethodSignature",
"start":64,"end":75,"loc":{"start":{"line":5,"column":2},"end":{"line":5,"column":13}},
"static": true,
"key": {
"type": "Identifier",
"start":71,"end":72,"loc":{"start":{"line":5,"column":9},"end":{"line":5,"column":10},"identifierName":"d"},
"name": "d"
},
"computed": false,
"parameters": []
},
{
"type": "TSMethodSignature",
"start":78,"end":90,"loc":{"start":{"line":6,"column":2},"end":{"line":6,"column":14}},
"declare": true,
"key": {
"type": "Identifier",
"start":86,"end":87,"loc":{"start":{"line":6,"column":10},"end":{"line":6,"column":11},"identifierName":"e"},
"name": "e"
},
"computed": false,
"parameters": []
},
{
"type": "TSMethodSignature",
"start":93,"end":106,"loc":{"start":{"line":7,"column":2},"end":{"line":7,"column":15}},
"abstract": true,
"key": {
"type": "Identifier",
"start":102,"end":103,"loc":{"start":{"line":7,"column":11},"end":{"line":7,"column":12},"identifierName":"f"},
"name": "f"
},
"computed": false,
"parameters": []
},
{
"type": "TSMethodSignature",
"start":109,"end":122,"loc":{"start":{"line":8,"column":2},"end":{"line":8,"column":15}},
"readonly": true,
"key": {
"type": "Identifier",
"start":118,"end":119,"loc":{"start":{"line":8,"column":11},"end":{"line":8,"column":12},"identifierName":"g"},
"name": "g"
},
"computed": false,
"parameters": []
}
]
}
}
],
"directives": []
}
}
8 changes: 8 additions & 0 deletions packages/babel-parser/test/fixtures/typescript/interface/invalid-modifiers-property/input.ts
View file
Open in desktop
@@ -0,0 +1,8 @@
interface Foo {
private a;
public b;
protected c;
static d;
declare e;
abstract f;
}
102 changes: 102 additions & 0 deletions ...es/babel-parser/test/fixtures/typescript/interface/invalid-modifiers-property/output.json
View file
Open in desktop
@@ -0,0 +1,102 @@
{
"type": "File",
"start":0,"end":96,"loc":{"start":{"line":1,"column":0},"end":{"line":8,"column":1}},
"errors": [
"SyntaxError: 'private' modifier cannot appear on a type member. (2:2)",
"SyntaxError: 'public' modifier cannot appear on a type member. (3:2)",
"SyntaxError: 'protected' modifier cannot appear on a type member. (4:2)",
"SyntaxError: 'static' modifier cannot appear on a type member. (5:2)",
"SyntaxError: 'declare' modifier cannot appear on a type member. (6:2)",
"SyntaxError: 'abstract' modifier cannot appear on a type member. (7:2)"
],
"program": {
"type": "Program",
"start":0,"end":96,"loc":{"start":{"line":1,"column":0},"end":{"line":8,"column":1}},
"sourceType": "module",
"interpreter": null,
"body": [
{
"type": "TSInterfaceDeclaration",
"start":0,"end":96,"loc":{"start":{"line":1,"column":0},"end":{"line":8,"column":1}},
"id": {
"type": "Identifier",
"start":10,"end":13,"loc":{"start":{"line":1,"column":10},"end":{"line":1,"column":13},"identifierName":"Foo"},
"name": "Foo"
},
"body": {
"type": "TSInterfaceBody",
"start":14,"end":96,"loc":{"start":{"line":1,"column":14},"end":{"line":8,"column":1}},
"body": [
{
"type": "TSPropertySignature",
"start":18,"end":28,"loc":{"start":{"line":2,"column":2},"end":{"line":2,"column":12}},
"accessibility": "private",
"key": {
"type": "Identifier",
"start":26,"end":27,"loc":{"start":{"line":2,"column":10},"end":{"line":2,"column":11},"identifierName":"a"},
"name": "a"
},
"computed": false
},
{
"type": "TSPropertySignature",
"start":31,"end":40,"loc":{"start":{"line":3,"column":2},"end":{"line":3,"column":11}},
"accessibility": "public",
"key": {
"type": "Identifier",
"start":38,"end":39,"loc":{"start":{"line":3,"column":9},"end":{"line":3,"column":10},"identifierName":"b"},
"name": "b"
},
"computed": false
},
{
"type": "TSPropertySignature",
"start":43,"end":55,"loc":{"start":{"line":4,"column":2},"end":{"line":4,"column":14}},
"accessibility": "protected",
"key": {
"type": "Identifier",
"start":53,"end":54,"loc":{"start":{"line":4,"column":12},"end":{"line":4,"column":13},"identifierName":"c"},
"name": "c"
},
"computed": false
},
{
"type": "TSPropertySignature",
"start":58,"end":67,"loc":{"start":{"line":5,"column":2},"end":{"line":5,"column":11}},
"static": true,
"key": {
"type": "Identifier",
"start":65,"end":66,"loc":{"start":{"line":5,"column":9},"end":{"line":5,"column":10},"identifierName":"d"},
"name": "d"
},
"computed": false
},
{
"type": "TSPropertySignature",
"start":70,"end":80,"loc":{"start":{"line":6,"column":2},"end":{"line":6,"column":12}},
"declare": true,
"key": {
"type": "Identifier",
"start":78,"end":79,"loc":{"start":{"line":6,"column":10},"end":{"line":6,"column":11},"identifierName":"e"},
"name": "e"
},
"computed": false
},
{
"type": "TSPropertySignature",
"start":83,"end":94,"loc":{"start":{"line":7,"column":2},"end":{"line":7,"column":13}},
"abstract": true,
"key": {
"type": "Identifier",
"start":92,"end":93,"loc":{"start":{"line":7,"column":11},"end":{"line":7,"column":12},"identifierName":"f"},
"name": "f"
},
"computed": false
}
]
}
}
],
"directives": []
}
}