From a14b22718ab1bc7d395c981b0a8f95f4d6e361b6 Mon Sep 17 00:00:00 2001 From: Andrew Branch Date: Wed, 28 Apr 2021 16:41:28 -0700 Subject: [PATCH] Enforce keyword order of `abstract` and `override` (#43829) * Enforce keyword order of abstract and override * Update baselines * Update existing test --- src/compiler/checker.ts | 3 +++ tests/baselines/reference/override10.js | 4 +--- tests/baselines/reference/override10.symbols | 7 +++--- tests/baselines/reference/override10.types | 3 +-- .../baselines/reference/override5.errors.txt | 8 ++++++- .../baselines/reference/override7.errors.txt | 8 ++++++- .../reference/overrideKeywordOrder.errors.txt | 24 ++++++++++++++----- .../cases/conformance/override/override10.ts | 3 +-- .../override/overrideKeywordOrder.ts | 15 +++++++++--- 9 files changed, 53 insertions(+), 22 deletions(-) diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index 95ccd4da3242e..bbd7def0d784f 100644 --- 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 index 61de880a2ffad..51f22a4faccc1 100644 --- 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 index 99382520fd3bf..50c8690d5be2f 100644 --- 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 index ce42a87972701..7781735340640 100644 --- 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 index 03711150037b1..a345a752f03fb 100644 --- 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 index 89240a4782ffa..da7370028beb5 100644 --- 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 index b4842347dbda4..487baa22c009f 100644 --- 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; } \ No newline at end of file diff --git a/tests/cases/conformance/override/override10.ts b/tests/cases/conformance/override/override10.ts index 98de0c7f1c6eb..3a8eca1632434 100644 --- 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() { } } \ No newline at end of file diff --git a/tests/cases/conformance/override/overrideKeywordOrder.ts b/tests/cases/conformance/override/overrideKeywordOrder.ts index c024ef6ec04aa..51bd95a9e7f2b 100644 --- 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; }