Enforce keyword order of abstract and override (#43829)

andrewbranch · web-flow · commit a14b22718ab1 · 2021-04-28T16:41:28.000-07:00
* Enforce keyword order of abstract and override

* Update baselines

* Update existing test
diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts
@@ -40837,6 +40837,9 @@ namespace ts {
                             if (flags & ModifierFlags.Async && lastAsync) {
                                 return grammarErrorOnNode(lastAsync, Diagnostics._0_modifier_cannot_be_used_with_1_modifier, "async", "abstract");
                             }
+                            if (flags & ModifierFlags.Override) {
+                                return grammarErrorOnNode(modifier, Diagnostics._0_modifier_must_precede_1_modifier, "abstract", "override");
+                            }
                         }
                         if (isNamedDeclaration(node) && node.name.kind === SyntaxKind.PrivateIdentifier) {
                             return grammarErrorOnNode(modifier, Diagnostics._0_modifier_cannot_be_used_with_a_private_identifier, "abstract");
diff --git a/tests/baselines/reference/override10.js b/tests/baselines/reference/override10.js
@@ -4,9 +4,8 @@ abstract class Base {
     abstract bar(): void;
 }
 
-// No errors:
 abstract class Sub extends Base {
-    override abstract foo(): number;
+    abstract override foo(): number;
     bar() { }
 }
 
@@ -31,7 +30,6 @@ var Base = /** @class */ (function () {
     }
     return Base;
 }());
-// No errors:
 var Sub = /** @class */ (function (_super) {
     __extends(Sub, _super);
     function Sub() {
diff --git a/tests/baselines/reference/override10.symbols b/tests/baselines/reference/override10.symbols
@@ -9,14 +9,13 @@ abstract class Base {
 >bar : Symbol(Base.bar, Decl(override10.ts, 1, 28))
 }
 
-// No errors:
 abstract class Sub extends Base {
 >Sub : Symbol(Sub, Decl(override10.ts, 3, 1))
 >Base : Symbol(Base, Decl(override10.ts, 0, 0))
 
-    override abstract foo(): number;
->foo : Symbol(Sub.foo, Decl(override10.ts, 6, 33))
+    abstract override foo(): number;
+>foo : Symbol(Sub.foo, Decl(override10.ts, 5, 33))
 
     bar() { }
->bar : Symbol(Sub.bar, Decl(override10.ts, 7, 36))
+>bar : Symbol(Sub.bar, Decl(override10.ts, 6, 36))
 }
diff --git a/tests/baselines/reference/override10.types b/tests/baselines/reference/override10.types
@@ -9,12 +9,11 @@ abstract class Base {
 >bar : () => void
 }
 
-// No errors:
 abstract class Sub extends Base {
 >Sub : Sub
 >Base : Base
 
-    override abstract foo(): number;
+    abstract override foo(): number;
 >foo : () => number
 
     bar() { }
diff --git a/tests/baselines/reference/override5.errors.txt b/tests/baselines/reference/override5.errors.txt
@@ -4,11 +4,13 @@ tests/cases/conformance/override/override5.ts(20,21): error TS4113: This member
 tests/cases/conformance/override/override5.ts(22,14): error TS1030: 'override' modifier already seen.
 tests/cases/conformance/override/override5.ts(25,14): error TS1029: 'public' modifier must precede 'override' modifier.
 tests/cases/conformance/override/override5.ts(27,5): error TS1089: 'override' modifier cannot appear on a constructor declaration.
+tests/cases/conformance/override/override5.ts(39,14): error TS1029: 'abstract' modifier must precede 'override' modifier.
+tests/cases/conformance/override/override5.ts(44,14): error TS1029: 'abstract' modifier must precede 'override' modifier.
 tests/cases/conformance/override/override5.ts(44,23): error TS4112: This member cannot have an 'override' modifier because its containing class 'AND' does not extend another class.
 tests/cases/conformance/override/override5.ts(45,23): error TS4112: This member cannot have an 'override' modifier because its containing class 'AND' does not extend another class.
 
 
-==== tests/cases/conformance/override/override5.ts (8 errors) ====
+==== tests/cases/conformance/override/override5.ts (10 errors) ====
     class B {
         p1: number = 1;
         p2: number = 2;
@@ -60,11 +62,15 @@ tests/cases/conformance/override/override5.ts(45,23): error TS4112: This member
     
     abstract class AD extends AB {
         override abstract f(): void;
+                 ~~~~~~~~
+!!! error TS1029: 'abstract' modifier must precede 'override' modifier.
         abstract override b(): void;
     }
     
     abstract class AND {
         override abstract f(): void;
+                 ~~~~~~~~
+!!! error TS1029: 'abstract' modifier must precede 'override' modifier.
                           ~
 !!! error TS4112: This member cannot have an 'override' modifier because its containing class 'AND' does not extend another class.
         abstract override b(): void;
diff --git a/tests/baselines/reference/override7.errors.txt b/tests/baselines/reference/override7.errors.txt
@@ -7,11 +7,13 @@ tests/cases/conformance/override/override7.ts(21,21): error TS4113: This member
 tests/cases/conformance/override/override7.ts(22,14): error TS1029: 'public' modifier must precede 'override' modifier.
 tests/cases/conformance/override/override7.ts(22,21): error TS4113: This member cannot have an 'override' modifier because it is not declared in the base class 'B'.
 tests/cases/conformance/override/override7.ts(24,5): error TS1089: 'override' modifier cannot appear on a constructor declaration.
+tests/cases/conformance/override/override7.ts(36,14): error TS1029: 'abstract' modifier must precede 'override' modifier.
+tests/cases/conformance/override/override7.ts(41,14): error TS1029: 'abstract' modifier must precede 'override' modifier.
 tests/cases/conformance/override/override7.ts(41,23): error TS4112: This member cannot have an 'override' modifier because its containing class 'AND' does not extend another class.
 tests/cases/conformance/override/override7.ts(42,23): error TS4112: This member cannot have an 'override' modifier because its containing class 'AND' does not extend another class.
 
 
-==== tests/cases/conformance/override/override7.ts (11 errors) ====
+==== tests/cases/conformance/override/override7.ts (13 errors) ====
     class B {
         p1: number = 1;
         p2: number = 2;
@@ -66,11 +68,15 @@ tests/cases/conformance/override/override7.ts(42,23): error TS4112: This member
     
     abstract class AD extends AB {
         override abstract f(): void;
+                 ~~~~~~~~
+!!! error TS1029: 'abstract' modifier must precede 'override' modifier.
         abstract override b(): void;
     }
     
     abstract class AND {
         override abstract f(): void;
+                 ~~~~~~~~
+!!! error TS1029: 'abstract' modifier must precede 'override' modifier.
                           ~
 !!! error TS4112: This member cannot have an 'override' modifier because its containing class 'AND' does not extend another class.
         abstract override b(): void;
diff --git a/tests/baselines/reference/overrideKeywordOrder.errors.txt b/tests/baselines/reference/overrideKeywordOrder.errors.txt
@@ -1,15 +1,16 @@
 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(21,12): error TS1029: 'public' modifier must precede 'override' modifier.
+tests/cases/conformance/override/overrideKeywordOrder.ts(26,12): error TS1029: 'override' modifier must precede 'readonly' modifier.
+tests/cases/conformance/override/overrideKeywordOrder.ts(32,12): error TS1029: 'abstract' modifier must precede 'override' modifier.
 
 
-==== tests/cases/conformance/override/overrideKeywordOrder.ts (4 errors) ====
-    class Base {
+==== tests/cases/conformance/override/overrideKeywordOrder.ts (5 errors) ====
+    abstract class Base {
       static s1() {}
       static s2() {}
-      m1() {}
-      m2() {}
+      abstract m1(): void;
+      abstract m2(): void;
       p1: any;
       p2: any;
     }
@@ -25,6 +26,8 @@ tests/cases/conformance/override/overrideKeywordOrder.ts(24,12): error TS1029: '
                ~~~~~~
 !!! error TS1029: 'static' modifier must precede 'override' modifier.
       static override s2() {}
+      m1() {}
+      m2() {}
     }
     class Test3 extends Base {
       override public m1() {} // error
@@ -37,5 +40,14 @@ tests/cases/conformance/override/overrideKeywordOrder.ts(24,12): error TS1029: '
       readonly override p2: any; // error
                ~~~~~~~~
 !!! error TS1029: 'override' modifier must precede 'readonly' modifier.
+      m1() {}
+      m2() {}
+    }
+    
+    abstract class Test5 extends Base {
+      override abstract m1(): void; // error
+               ~~~~~~~~
+!!! error TS1029: 'abstract' modifier must precede 'override' modifier.
+      abstract override m2(): void;
     }
     
diff --git a/tests/cases/conformance/override/override10.ts b/tests/cases/conformance/override/override10.ts
@@ -6,8 +6,7 @@ abstract class Base {
     abstract bar(): void;
 }
 
-// No errors:
 abstract class Sub extends Base {
-    override abstract foo(): number;
+    abstract override foo(): number;
     bar() { }
 }
diff --git a/tests/cases/conformance/override/overrideKeywordOrder.ts b/tests/cases/conformance/override/overrideKeywordOrder.ts
@@ -1,11 +1,11 @@
 // @noTypesAndSymbols: true
 // @noEmit: true
 
-class Base {
+abstract class Base {
   static s1() {}
   static s2() {}
-  m1() {}
-  m2() {}
+  abstract m1(): void;
+  abstract m2(): void;
   p1: any;
   p2: any;
 }
@@ -17,6 +17,8 @@ class Test1 extends Base {
 class Test2 extends Base {
   override static s1() {} // error
   static override s2() {}
+  m1() {}
+  m2() {}
 }
 class Test3 extends Base {
   override public m1() {} // error
@@ -25,4 +27,11 @@ class Test3 extends Base {
 class Test4 extends Base {
   override readonly p1: any;
   readonly override p2: any; // error
+  m1() {}
+  m2() {}
+}
+
+abstract class Test5 extends Base {
+  override abstract m1(): void; // error
+  abstract override m2(): void;
 }

Original file line number	Diff line number	Diff line change
`@@ -40837,6 +40837,9 @@ namespace ts {`
`40837`	`40837`	`if (flags & ModifierFlags.Async && lastAsync) {`
`40838`	`40838`	`return grammarErrorOnNode(lastAsync, Diagnostics._0_modifier_cannot_be_used_with_1_modifier, "async", "abstract");`
`40839`	`40839`	`}`
	`40840`	`+ if (flags & ModifierFlags.Override) {`
	`40841`	`+ return grammarErrorOnNode(modifier, Diagnostics._0_modifier_must_precede_1_modifier, "abstract", "override");`
	`40842`	`+ }`
`40840`	`40843`	`}`
`40841`	`40844`	`if (isNamedDeclaration(node) && node.name.kind === SyntaxKind.PrivateIdentifier) {`
`40842`	`40845`	`return grammarErrorOnNode(modifier, Diagnostics._0_modifier_cannot_be_used_with_a_private_identifier, "abstract");`
Original file line number	Diff line number	Diff line change
`@@ -4,9 +4,8 @@ abstract class Base {`
`4`	`4`	`abstract bar(): void;`
`5`	`5`	`}`
`6`	`6`
`7`		`-// No errors:`
`8`	`7`	`abstract class Sub extends Base {`
`9`		`- override abstract foo(): number;`
	`8`	`+ abstract override foo(): number;`
`10`	`9`	`bar() { }`
`11`	`10`	`}`
`12`	`11`
`@@ -31,7 +30,6 @@ var Base = /** @class */ (function () {`
`31`	`30`	`}`
`32`	`31`	`return Base;`
`33`	`32`	`}());`
`34`		`-// No errors:`
`35`	`33`	`var Sub = /** @class */ (function (_super) {`
`36`	`34`	`__extends(Sub, _super);`
`37`	`35`	`function Sub() {`
Original file line number	Diff line number	Diff line change
`@@ -6,8 +6,7 @@ abstract class Base {`
`6`	`6`	`abstract bar(): void;`
`7`	`7`	`}`
`8`	`8`
`9`		`-// No errors:`
`10`	`9`	`abstract class Sub extends Base {`
`11`		`- override abstract foo(): number;`
	`10`	`+ abstract override foo(): number;`
`12`	`11`	`bar() { }`
`13`	`12`	`}`