Skip to content

Commit

Permalink
[ts] Enforce order for the override modifier (#13209)
Browse files Browse the repository at this point in the history
* [ts] Enforce order for the `override` modifier

* generator

* Add more checks

* Update TS tests
  • Loading branch information
nicolo-ribaudo committed Apr 27, 2021
1 parent d6e7535 commit 52a59b1
Show file tree
Hide file tree
Showing 14 changed files with 682 additions and 266 deletions.
2 changes: 1 addition & 1 deletion Makefile
@@ -1,6 +1,6 @@
FLOW_COMMIT = a1f9a4c709dcebb27a5084acf47755fbae699c25
TEST262_COMMIT = eca69e2c95972a4c5780ef58fe1f1e53e871b9b1
TYPESCRIPT_COMMIT = dd1ef88d016dc40a145eafc0a1169e7f0a4a9ebe
TYPESCRIPT_COMMIT = 3de706a8525c2ded782fc032fa4afe2e485100d3

# Fix color output until TravisCI fixes https://github.com/travis-ci/travis-ci/issues/7967
export FORCE_COLOR = true
Expand Down
8 changes: 4 additions & 4 deletions packages/babel-generator/src/generators/typescript.ts
Expand Up @@ -649,10 +649,6 @@ export function tsPrintClassMemberModifiers(this: Printer, node: any, isField) {
this.word("declare");
this.space();
}
if (node.override) {
this.word("override");
this.space();
}
if (node.accessibility) {
this.word(node.accessibility);
this.space();
Expand All @@ -661,6 +657,10 @@ export function tsPrintClassMemberModifiers(this: Printer, node: any, isField) {
this.word("static");
this.space();
}
if (node.override) {
this.word("override");
this.space();
}
if (node.abstract) {
this.word("abstract");
this.space();
Expand Down
@@ -1,8 +1,6 @@
class MyClass extends BaseClass {
override show() {}
override public show() {}
public override show() {}
override size = 5;
override readonly size = 5;
readonly override size = 5;
public static override readonly size = 5;
}
@@ -1,11 +1,8 @@
class MyClass extends BaseClass {
override show() {}

override public show() {}

override public show() {}
public override show() {}

override size = 5;
override readonly size = 5;
override readonly size = 5;
}
public static override readonly size = 5;
}
73 changes: 35 additions & 38 deletions packages/babel-parser/src/plugins/typescript/index.js
Expand Up @@ -253,6 +253,20 @@ export default (superClass: Class<Parser>): Class<Parser> =>
disallowedModifiers?: TsModifier[],
errorTemplate?: ErrorTemplate,
): void {
const enforceOrder = (pos, modifier, before, after) => {
if (modifier === before && modified[after]) {
this.raise(pos, TSErrors.InvalidModifiersOrder, before, after);
}
};
const incompatible = (pos, modifier, mod1, mod2) => {
if (
(modified[mod1] && modifier === mod2) ||
(modified[mod2] && modifier === mod1)
) {
this.raise(pos, TSErrors.IncompatibleModifiers, mod1, mod2);
}
};

for (;;) {
const startPos = this.state.start;
const modifier: ?TsModifier = this.tsParseModifier(
Expand All @@ -265,28 +279,22 @@ export default (superClass: Class<Parser>): Class<Parser> =>
if (modified.accessibility) {
this.raise(startPos, TSErrors.DuplicateAccessibilityModifier);
} else {
enforceOrder(startPos, modifier, modifier, "override");
enforceOrder(startPos, modifier, modifier, "static");

modified.accessibility = modifier;
}
} else {
if (Object.hasOwnProperty.call(modified, modifier)) {
this.raise(startPos, TSErrors.DuplicateModifier, modifier);
} else if (modified.readonly && modifier === "static") {
this.raise(
startPos,
TSErrors.InvalidModifiersOrder,
"static",
"readonly",
);
} else if (
(modified.declare && modifier === "override") ||
(modified.override && modifier === "declare")
) {
this.raise(
startPos,
TSErrors.IncompatibleModifiers,
"declare",
"override",
);
} else {
enforceOrder(startPos, modifier, "static", "readonly");
enforceOrder(startPos, modifier, "static", "override");
enforceOrder(startPos, modifier, "override", "readonly");
enforceOrder(startPos, modifier, "abstract", "override");

incompatible(startPos, modifier, "declare", "override");
incompatible(startPos, modifier, "static", "abstract");
}
modified[modifier] = true;
}
Expand Down Expand Up @@ -2320,10 +2328,18 @@ export default (superClass: Class<Parser>): Class<Parser> =>
"public",
"protected",
"override",
"static",
"abstract",
"readonly",
]);

const callParseClassMember = () => {
super.parseClassMember(classBody, member, state);
this.parseClassMemberWithIsStatic(
classBody,
member,
state,
!!member.static,
);
};
if (member.declare) {
this.tsInDeclareContext(callParseClassMember);
Expand All @@ -2338,18 +2354,6 @@ export default (superClass: Class<Parser>): Class<Parser> =>
state: N.ParseClassMemberState,
isStatic: boolean,
): void {
this.tsParseModifiers(member, [
"abstract",
"readonly",
"declare",
"static",
"override",
]);

if (isStatic) {
member.static = true;
}

const idx = this.tsTryParseIndexSignature(member);
if (idx) {
classBody.body.push(idx);
Expand Down Expand Up @@ -2379,14 +2383,7 @@ export default (superClass: Class<Parser>): Class<Parser> =>
}

if ((member: any).override) {
if (isStatic) {
this.raise(
member.start,
TSErrors.IncompatibleModifiers,
"static",
"override",
);
} else if (!state.hadSuperClass) {
if (!state.hadSuperClass) {
this.raise(member.start, TSErrors.OverrideNotInSubClass);
}
}
Expand Down
@@ -0,0 +1,7 @@
abstract class A extends Base {
static abstract m1();
abstract static m1();

declare override prop1: any;
override declare prop2: any;
}
@@ -0,0 +1,118 @@
{
"type": "File",
"start":0,"end":144,"loc":{"start":{"line":1,"column":0},"end":{"line":7,"column":1}},
"errors": [
"SyntaxError: 'static' modifier cannot be used with 'abstract' modifier. (2:9)",
"SyntaxError: 'static' modifier cannot be used with 'abstract' modifier. (3:11)",
"SyntaxError: 'declare' modifier cannot be used with 'override' modifier. (5:10)",
"SyntaxError: 'declare' modifier cannot be used with 'override' modifier. (6:11)"
],
"program": {
"type": "Program",
"start":0,"end":144,"loc":{"start":{"line":1,"column":0},"end":{"line":7,"column":1}},
"sourceType": "module",
"interpreter": null,
"body": [
{
"type": "ClassDeclaration",
"start":0,"end":144,"loc":{"start":{"line":1,"column":0},"end":{"line":7,"column":1}},
"abstract": true,
"id": {
"type": "Identifier",
"start":15,"end":16,"loc":{"start":{"line":1,"column":15},"end":{"line":1,"column":16},"identifierName":"A"},
"name": "A"
},
"superClass": {
"type": "Identifier",
"start":25,"end":29,"loc":{"start":{"line":1,"column":25},"end":{"line":1,"column":29},"identifierName":"Base"},
"name": "Base"
},
"body": {
"type": "ClassBody",
"start":30,"end":144,"loc":{"start":{"line":1,"column":30},"end":{"line":7,"column":1}},
"body": [
{
"type": "TSDeclareMethod",
"start":34,"end":55,"loc":{"start":{"line":2,"column":2},"end":{"line":2,"column":23}},
"static": true,
"abstract": true,
"key": {
"type": "Identifier",
"start":50,"end":52,"loc":{"start":{"line":2,"column":18},"end":{"line":2,"column":20},"identifierName":"m1"},
"name": "m1"
},
"computed": false,
"kind": "method",
"id": null,
"generator": false,
"async": false,
"params": []
},
{
"type": "TSDeclareMethod",
"start":58,"end":79,"loc":{"start":{"line":3,"column":2},"end":{"line":3,"column":23}},
"abstract": true,
"static": true,
"key": {
"type": "Identifier",
"start":74,"end":76,"loc":{"start":{"line":3,"column":18},"end":{"line":3,"column":20},"identifierName":"m1"},
"name": "m1"
},
"computed": false,
"kind": "method",
"id": null,
"generator": false,
"async": false,
"params": []
},
{
"type": "ClassProperty",
"start":83,"end":111,"loc":{"start":{"line":5,"column":2},"end":{"line":5,"column":30}},
"declare": true,
"override": true,
"static": false,
"key": {
"type": "Identifier",
"start":100,"end":105,"loc":{"start":{"line":5,"column":19},"end":{"line":5,"column":24},"identifierName":"prop1"},
"name": "prop1"
},
"computed": false,
"typeAnnotation": {
"type": "TSTypeAnnotation",
"start":105,"end":110,"loc":{"start":{"line":5,"column":24},"end":{"line":5,"column":29}},
"typeAnnotation": {
"type": "TSAnyKeyword",
"start":107,"end":110,"loc":{"start":{"line":5,"column":26},"end":{"line":5,"column":29}}
}
},
"value": null
},
{
"type": "ClassProperty",
"start":114,"end":142,"loc":{"start":{"line":6,"column":2},"end":{"line":6,"column":30}},
"override": true,
"declare": true,
"static": false,
"key": {
"type": "Identifier",
"start":131,"end":136,"loc":{"start":{"line":6,"column":19},"end":{"line":6,"column":24},"identifierName":"prop2"},
"name": "prop2"
},
"computed": false,
"typeAnnotation": {
"type": "TSTypeAnnotation",
"start":136,"end":141,"loc":{"start":{"line":6,"column":24},"end":{"line":6,"column":29}},
"typeAnnotation": {
"type": "TSAnyKeyword",
"start":138,"end":141,"loc":{"start":{"line":6,"column":26},"end":{"line":6,"column":29}}
}
},
"value": null
}
]
}
}
],
"directives": []
}
}
@@ -0,0 +1,28 @@
abstract class A extends B {
static override m1() {}
override static m2() {}

override readonly p4;
readonly override p3;

public override m5() {}
override public m6() {}

protected override m7() {}
override protected m8() {}

private override m9() {}
override private m10() {}

abstract override m12();
override abstract m11();

public static m14() {}
static public m13() {}

protected static m16() {}
static protected m15() {}

private static m18() {}
static private m17() {}
}

0 comments on commit 52a59b1

Please sign in to comment.