Skip to content

Commit

Permalink
Enforce keyword order between override and static/async (#43660)
Browse files Browse the repository at this point in the history
* Enforce keyword order between override and static/async

* Update old tests for new keyword order
  • Loading branch information
andrewbranch authored Apr 15, 2021
1 parent 0987ee9 commit 06f25c0
Show file tree
Hide file tree
Showing 13 changed files with 85 additions and 10 deletions.
6 changes: 6 additions & 0 deletions src/compiler/checker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40529,6 +40529,9 @@ namespace ts {
else if (flags & ModifierFlags.Readonly) {
return grammarErrorOnNode(modifier, Diagnostics._0_modifier_must_precede_1_modifier, "override", "readonly");
}
else if (flags & ModifierFlags.Async) {
return grammarErrorOnNode(modifier, Diagnostics._0_modifier_must_precede_1_modifier, "override", "async");
}
if (node.kind === SyntaxKind.Parameter) {
return grammarErrorOnNode(modifier, Diagnostics._0_modifier_cannot_appear_on_a_parameter, "override");
}
Expand Down Expand Up @@ -40592,6 +40595,9 @@ namespace ts {
else if (flags & ModifierFlags.Abstract) {
return grammarErrorOnNode(modifier, Diagnostics._0_modifier_cannot_be_used_with_1_modifier, "static", "abstract");
}
else if (flags & ModifierFlags.Override) {
return grammarErrorOnNode(modifier, Diagnostics._0_modifier_must_precede_1_modifier, "static", "override");
}
flags |= ModifierFlags.Static;
lastStatic = modifier;
break;
Expand Down
2 changes: 1 addition & 1 deletion tests/baselines/reference/override5.errors.txt
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ tests/cases/conformance/override/override5.ts(45,23): error TS4112: This member

override readonly p4: number;

override static sp: number;
static override sp: number;
~~
!!! error TS4113: This member cannot have an 'override' modifier because it is not declared in the base class 'B'.

Expand Down
2 changes: 1 addition & 1 deletion tests/baselines/reference/override5.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ class D extends B{

override readonly p4: number;

override static sp: number;
static override sp: number;

override override oop: number;

Expand Down
2 changes: 1 addition & 1 deletion tests/baselines/reference/override5.symbols
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ class D extends B{
override readonly p4: number;
>p4 : Symbol(D.p4, Decl(override5.ts, 15, 33))

override static sp: number;
static override sp: number;
>sp : Symbol(D.sp, Decl(override5.ts, 17, 33))

override override oop: number;
Expand Down
2 changes: 1 addition & 1 deletion tests/baselines/reference/override5.types
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ class D extends B{
override readonly p4: number;
>p4 : number

override static sp: number;
static override sp: number;
>sp : number

override override oop: number;
Expand Down
2 changes: 1 addition & 1 deletion tests/baselines/reference/override7.errors.txt
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ tests/cases/conformance/override/override7.ts(42,23): error TS4112: This member

override readonly p4: number;

override static sp: number;
static override sp: number;
~~
!!! error TS4113: This member cannot have an 'override' modifier because it is not declared in the base class 'B'.

Expand Down
2 changes: 1 addition & 1 deletion tests/baselines/reference/override7.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ class D extends B{

override readonly p4: number;

override static sp: number;
static override sp: number;

override override oop: number;

Expand Down
2 changes: 1 addition & 1 deletion tests/baselines/reference/override7.symbols
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ class D extends B{
override readonly p4: number;
>p4 : Symbol(D.p4, Decl(override7.ts, 12, 33))

override static sp: number;
static override sp: number;
>sp : Symbol(D.sp, Decl(override7.ts, 14, 33))

override override oop: number;
Expand Down
2 changes: 1 addition & 1 deletion tests/baselines/reference/override7.types
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ class D extends B{
override readonly p4: number;
>p4 : number

override static sp: number;
static override sp: number;
>sp : number

override override oop: number;
Expand Down
41 changes: 41 additions & 0 deletions tests/baselines/reference/overrideKeywordOrder.errors.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
tests/cases/conformance/override/overrideKeywordOrder.ts(12,9): error TS1029: 'override' modifier must precede 'async' modifier.
tests/cases/conformance/override/overrideKeywordOrder.ts(15,12): error TS1029: 'static' modifier must precede 'override' modifier.
tests/cases/conformance/override/overrideKeywordOrder.ts(19,12): error TS1029: 'public' modifier must precede 'override' modifier.
tests/cases/conformance/override/overrideKeywordOrder.ts(24,12): error TS1029: 'override' modifier must precede 'readonly' modifier.


==== tests/cases/conformance/override/overrideKeywordOrder.ts (4 errors) ====
class Base {
static s1() {}
static s2() {}
m1() {}
m2() {}
p1: any;
p2: any;
}

class Test1 extends Base {
override async m1() {}
async override m2() {} // error
~~~~~~~~
!!! error TS1029: 'override' modifier must precede 'async' modifier.
}
class Test2 extends Base {
override static s1() {} // error
~~~~~~
!!! error TS1029: 'static' modifier must precede 'override' modifier.
static override s2() {}
}
class Test3 extends Base {
override public m1() {} // error
~~~~~~
!!! error TS1029: 'public' modifier must precede 'override' modifier.
public override m2() {}
}
class Test4 extends Base {
override readonly p1: any;
readonly override p2: any; // error
~~~~~~~~
!!! error TS1029: 'override' modifier must precede 'readonly' modifier.
}

2 changes: 1 addition & 1 deletion tests/cases/conformance/override/override5.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ class D extends B{

override readonly p4: number;

override static sp: number;
static override sp: number;

override override oop: number;

Expand Down
2 changes: 1 addition & 1 deletion tests/cases/conformance/override/override7.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ class D extends B{

override readonly p4: number;

override static sp: number;
static override sp: number;

override override oop: number;

Expand Down
28 changes: 28 additions & 0 deletions tests/cases/conformance/override/overrideKeywordOrder.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
// @noTypesAndSymbols: true
// @noEmit: true

class Base {
static s1() {}
static s2() {}
m1() {}
m2() {}
p1: any;
p2: any;
}

class Test1 extends Base {
override async m1() {}
async override m2() {} // error
}
class Test2 extends Base {
override static s1() {} // error
static override s2() {}
}
class Test3 extends Base {
override public m1() {} // error
public override m2() {}
}
class Test4 extends Base {
override readonly p1: any;
readonly override p2: any; // error
}

0 comments on commit 06f25c0

Please sign in to comment.