From 15c7df92644cf2ad9b9664ba7e40424851048335 Mon Sep 17 00:00:00 2001 From: kingwl Date: Sat, 8 May 2021 13:53:53 +0800 Subject: [PATCH] Update diagnostic message and quickfix for parameter property --- src/compiler/checker.ts | 2 +- src/compiler/diagnosticMessages.json | 2 +- src/services/codefixes/fixOverrideModifier.ts | 26 ++++++++++++------- .../baselines/reference/override6.errors.txt | 8 +++--- .../baselines/reference/override8.errors.txt | 8 +++--- .../overrideParameterProperty.errors.txt | 12 +++++++-- .../reference/overrideParameterProperty.js | 16 +++++++++++- .../overrideParameterProperty.symbols | 11 ++++++++ .../reference/overrideParameterProperty.types | 12 +++++++++ .../override/overrideParameterProperty.ts | 6 +++++ .../fourslash/codeFixOverrideModifier10.ts | 8 ++++-- .../fourslash/codeFixOverrideModifier21.ts | 17 ++++++++++++ .../fourslash/codeFixOverrideModifier9.ts | 9 ++++--- 13 files changed, 110 insertions(+), 27 deletions(-) create mode 100644 tests/cases/fourslash/codeFixOverrideModifier21.ts diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index 62f0edcf119ce..b58012e3188bf 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -37214,7 +37214,7 @@ namespace ts { if (!baseHasAbstract) { const diag = memberIsParameterProperty ? - Diagnostics.This_parameter_property_must_be_rewritten_as_a_property_declaration_with_an_override_modifier_because_it_overrides_a_member_in_base_class_0 : + Diagnostics.This_parameter_property_must_have_an_override_modifier_because_it_overrides_a_member_in_base_class_0 : Diagnostics.This_member_must_have_an_override_modifier_because_it_overrides_a_member_in_the_base_class_0; error(member, diag, baseClassName); } diff --git a/src/compiler/diagnosticMessages.json b/src/compiler/diagnosticMessages.json index 155e176eaa73e..8a48d220fb938 100644 --- a/src/compiler/diagnosticMessages.json +++ b/src/compiler/diagnosticMessages.json @@ -3717,7 +3717,7 @@ "category": "Error", "code": 4114 }, - "This parameter property must be rewritten as a property declaration with an 'override' modifier because it overrides a member in base class '{0}'.": { + "This parameter property must have an 'override' modifier because it overrides a member in base class '{0}'.": { "category": "Error", "code": 4115 }, diff --git a/src/services/codefixes/fixOverrideModifier.ts b/src/services/codefixes/fixOverrideModifier.ts index 046a1c8c19479..a804a5f97dbb9 100644 --- a/src/services/codefixes/fixOverrideModifier.ts +++ b/src/services/codefixes/fixOverrideModifier.ts @@ -4,18 +4,20 @@ namespace ts.codefix { const fixAddOverrideId = "fixAddOverrideModifier"; const fixRemoveOverrideId = "fixRemoveOverrideModifier"; - type ClassElementHasJSDoc = + type ClassElementLikeHasJSDoc = | ConstructorDeclaration | PropertyDeclaration | MethodDeclaration | GetAccessorDeclaration - | SetAccessorDeclaration; + | SetAccessorDeclaration + | ParameterPropertyDeclaration; const errorCodes = [ Diagnostics.This_member_cannot_have_an_override_modifier_because_it_is_not_declared_in_the_base_class_0.code, Diagnostics.This_member_cannot_have_an_override_modifier_because_its_containing_class_0_does_not_extend_another_class.code, Diagnostics.This_member_must_have_an_override_modifier_because_it_overrides_an_abstract_method_that_is_declared_in_the_base_class_0.code, - Diagnostics.This_member_must_have_an_override_modifier_because_it_overrides_a_member_in_the_base_class_0.code + Diagnostics.This_member_must_have_an_override_modifier_because_it_overrides_a_member_in_the_base_class_0.code, + Diagnostics.This_parameter_property_must_have_an_override_modifier_because_it_overrides_a_member_in_base_class_0.code ]; const errorCodeFixIdMap: Record = { @@ -25,6 +27,9 @@ namespace ts.codefix { [Diagnostics.This_member_cannot_have_an_override_modifier_because_its_containing_class_0_does_not_extend_another_class.code]: [ Diagnostics.Remove_override_modifier, fixRemoveOverrideId, Diagnostics.Remove_all_unnecessary_override_modifiers ], + [Diagnostics.This_parameter_property_must_have_an_override_modifier_because_it_overrides_a_member_in_base_class_0.code]: [ + Diagnostics.Add_override_modifier, fixAddOverrideId, Diagnostics.Add_all_missing_override_modifiers, + ], [Diagnostics.This_member_must_have_an_override_modifier_because_it_overrides_an_abstract_method_that_is_declared_in_the_base_class_0.code]: [ Diagnostics.Add_override_modifier, fixAddOverrideId, Diagnostics.Remove_all_unnecessary_override_modifiers ], @@ -70,6 +75,7 @@ namespace ts.codefix { switch (errorCode) { case Diagnostics.This_member_must_have_an_override_modifier_because_it_overrides_a_member_in_the_base_class_0.code: case Diagnostics.This_member_must_have_an_override_modifier_because_it_overrides_an_abstract_method_that_is_declared_in_the_base_class_0.code: + case Diagnostics.This_parameter_property_must_have_an_override_modifier_because_it_overrides_a_member_in_base_class_0.code: return doAddOverrideModifierChange(changeTracker, context.sourceFile, pos); case Diagnostics.This_member_cannot_have_an_override_modifier_because_it_is_not_declared_in_the_base_class_0.code: case Diagnostics.This_member_cannot_have_an_override_modifier_because_its_containing_class_0_does_not_extend_another_class.code: @@ -80,7 +86,7 @@ namespace ts.codefix { } function doAddOverrideModifierChange(changeTracker: textChanges.ChangeTracker, sourceFile: SourceFile, pos: number) { - const classElement = findContainerClassElement(sourceFile, pos); + const classElement = findContainerClassElementLike(sourceFile, pos); const modifiers = classElement.modifiers || emptyArray; const staticModifier = find(modifiers, isStaticModifier); const accessibilityModifier = find(modifiers, m => isAccessibilityModifier(m.kind)); @@ -92,14 +98,14 @@ namespace ts.codefix { } function doRemoveOverrideModifierChange(changeTracker: textChanges.ChangeTracker, sourceFile: SourceFile, pos: number) { - const classElement = findContainerClassElement(sourceFile, pos); + const classElement = findContainerClassElementLike(sourceFile, pos); const overrideModifier = classElement.modifiers && find(classElement.modifiers, modifier => modifier.kind === SyntaxKind.OverrideKeyword); Debug.assertIsDefined(overrideModifier); changeTracker.deleteModifier(sourceFile, overrideModifier); } - function isClassElementHasJSDoc(node: Node): node is ClassElementHasJSDoc { + function isClassElementLikeHasJSDoc(node: Node): node is ClassElementLikeHasJSDoc { switch (node.kind) { case SyntaxKind.Constructor: case SyntaxKind.PropertyDeclaration: @@ -107,19 +113,21 @@ namespace ts.codefix { case SyntaxKind.GetAccessor: case SyntaxKind.SetAccessor: return true; + case SyntaxKind.Parameter: + return isParameterPropertyDeclaration(node, node.parent); default: return false; } } - function findContainerClassElement(sourceFile: SourceFile, pos: number) { + function findContainerClassElementLike(sourceFile: SourceFile, pos: number) { const token = getTokenAtPosition(sourceFile, pos); const classElement = findAncestor(token, node => { if (isClassLike(node)) return "quit"; - return isClassElementHasJSDoc(node); + return isClassElementLikeHasJSDoc(node); }); - Debug.assert(classElement && isClassElementHasJSDoc(classElement)); + Debug.assert(classElement && isClassElementLikeHasJSDoc(classElement)); return classElement; } } diff --git a/tests/baselines/reference/override6.errors.txt b/tests/baselines/reference/override6.errors.txt index cd05376f2a63f..4a1fb1a0db185 100644 --- a/tests/baselines/reference/override6.errors.txt +++ b/tests/baselines/reference/override6.errors.txt @@ -1,6 +1,6 @@ tests/cases/conformance/override/override6.ts(9,12): error TS4114: This member must have an 'override' modifier because it overrides a member in the base class 'B'. -tests/cases/conformance/override/override6.ts(10,17): error TS4115: This parameter property must be rewritten as a property declaration with an 'override' modifier because it overrides a member in base class 'B'. -tests/cases/conformance/override/override6.ts(10,37): error TS4115: This parameter property must be rewritten as a property declaration with an 'override' modifier because it overrides a member in base class 'B'. +tests/cases/conformance/override/override6.ts(10,17): error TS4115: This parameter property must have an 'override' modifier because it overrides a member in base class 'B'. +tests/cases/conformance/override/override6.ts(10,37): error TS4115: This parameter property must have an 'override' modifier because it overrides a member in base class 'B'. ==== tests/cases/conformance/override/override6.ts (3 errors) ==== @@ -17,9 +17,9 @@ tests/cases/conformance/override/override6.ts(10,37): error TS4115: This paramet !!! error TS4114: This member must have an 'override' modifier because it overrides a member in the base class 'B'. constructor(public foo: string, public baz: number) { ~~~~~~~~~~~~~~~~~~ -!!! error TS4115: This parameter property must be rewritten as a property declaration with an 'override' modifier because it overrides a member in base class 'B'. +!!! error TS4115: This parameter property must have an 'override' modifier because it overrides a member in base class 'B'. ~~~~~~~~~~~~~~~~~~ -!!! error TS4115: This parameter property must be rewritten as a property declaration with an 'override' modifier because it overrides a member in base class 'B'. +!!! error TS4115: This parameter property must have an 'override' modifier because it overrides a member in base class 'B'. super(foo, 42) } } diff --git a/tests/baselines/reference/override8.errors.txt b/tests/baselines/reference/override8.errors.txt index f1b2fa8654b5f..629cad4be6ab8 100644 --- a/tests/baselines/reference/override8.errors.txt +++ b/tests/baselines/reference/override8.errors.txt @@ -1,5 +1,5 @@ -tests/cases/conformance/override/override8.ts(6,17): error TS4115: This parameter property must be rewritten as a property declaration with an 'override' modifier because it overrides a member in base class 'B'. -tests/cases/conformance/override/override8.ts(18,17): error TS4115: This parameter property must be rewritten as a property declaration with an 'override' modifier because it overrides a member in base class 'BB'. +tests/cases/conformance/override/override8.ts(6,17): error TS4115: This parameter property must have an 'override' modifier because it overrides a member in base class 'B'. +tests/cases/conformance/override/override8.ts(18,17): error TS4115: This parameter property must have an 'override' modifier because it overrides a member in base class 'BB'. tests/cases/conformance/override/override8.ts(24,12): error TS4114: This member must have an 'override' modifier because it overrides a member in the base class 'BB'. @@ -11,7 +11,7 @@ tests/cases/conformance/override/override8.ts(24,12): error TS4114: This member class D extends B { constructor(public a: string, public b: string) { ~~~~~~~~~~~~~~~~ -!!! error TS4115: This parameter property must be rewritten as a property declaration with an 'override' modifier because it overrides a member in base class 'B'. +!!! error TS4115: This parameter property must have an 'override' modifier because it overrides a member in base class 'B'. super(); } } @@ -25,7 +25,7 @@ tests/cases/conformance/override/override8.ts(24,12): error TS4114: This member class DD extends BB { constructor(public a: string) { ~~~~~~~~~~~~~~~~ -!!! error TS4115: This parameter property must be rewritten as a property declaration with an 'override' modifier because it overrides a member in base class 'BB'. +!!! error TS4115: This parameter property must have an 'override' modifier because it overrides a member in base class 'BB'. super(a) } } diff --git a/tests/baselines/reference/overrideParameterProperty.errors.txt b/tests/baselines/reference/overrideParameterProperty.errors.txt index 10929ce6e3cae..12a9e8e8bab41 100644 --- a/tests/baselines/reference/overrideParameterProperty.errors.txt +++ b/tests/baselines/reference/overrideParameterProperty.errors.txt @@ -1,8 +1,9 @@ tests/cases/conformance/override/overrideParameterProperty.ts(20,24): error TS1029: 'public' modifier must precede 'override' modifier. tests/cases/conformance/override/overrideParameterProperty.ts(25,5): error TS2369: A parameter property is only allowed in a constructor implementation. +tests/cases/conformance/override/overrideParameterProperty.ts(29,15): error TS4113: This member cannot have an 'override' modifier because it is not declared in the base class 'Base'. -==== tests/cases/conformance/override/overrideParameterProperty.ts (2 errors) ==== +==== tests/cases/conformance/override/overrideParameterProperty.ts (3 errors) ==== class Base { p1!: string; } @@ -33,4 +34,11 @@ tests/cases/conformance/override/overrideParameterProperty.ts(25,5): error TS236 ~~~~~~~~~~~~~~~~~~~~ !!! error TS2369: A parameter property is only allowed in a constructor implementation. } - \ No newline at end of file + + class C4 extends Base { + constructor(public override p2: string) { + ~~~~~~~~~~~~~~~~~~~~~~~~~~ +!!! error TS4113: This member cannot have an 'override' modifier because it is not declared in the base class 'Base'. + super(); + } + } \ No newline at end of file diff --git a/tests/baselines/reference/overrideParameterProperty.js b/tests/baselines/reference/overrideParameterProperty.js index ad063d8e5ef8f..811cda11c7d63 100644 --- a/tests/baselines/reference/overrideParameterProperty.js +++ b/tests/baselines/reference/overrideParameterProperty.js @@ -25,7 +25,12 @@ class C3 extends Base { m(override p1: "hello") {} } - + +class C4 extends Base { + constructor(public override p2: string) { + super(); + } +} //// [overrideParameterProperty.js] var __extends = (this && this.__extends) || (function () { @@ -79,3 +84,12 @@ var C3 = /** @class */ (function (_super) { C3.prototype.m = function (p1) { }; return C3; }(Base)); +var C4 = /** @class */ (function (_super) { + __extends(C4, _super); + function C4(p2) { + var _this = _super.call(this) || this; + _this.p2 = p2; + return _this; + } + return C4; +}(Base)); diff --git a/tests/baselines/reference/overrideParameterProperty.symbols b/tests/baselines/reference/overrideParameterProperty.symbols index 88eefef58290e..c6d55d95b1bbb 100644 --- a/tests/baselines/reference/overrideParameterProperty.symbols +++ b/tests/baselines/reference/overrideParameterProperty.symbols @@ -61,3 +61,14 @@ class C3 extends Base { >p1 : Symbol(p1, Decl(overrideParameterProperty.ts, 24, 4)) } +class C4 extends Base { +>C4 : Symbol(C4, Decl(overrideParameterProperty.ts, 25, 1)) +>Base : Symbol(Base, Decl(overrideParameterProperty.ts, 0, 0)) + + constructor(public override p2: string) { +>p2 : Symbol(C4.p2, Decl(overrideParameterProperty.ts, 28, 14)) + + super(); +>super : Symbol(Base, Decl(overrideParameterProperty.ts, 0, 0)) + } +} diff --git a/tests/baselines/reference/overrideParameterProperty.types b/tests/baselines/reference/overrideParameterProperty.types index 33aca1cfbca47..8ff0f11cc8096 100644 --- a/tests/baselines/reference/overrideParameterProperty.types +++ b/tests/baselines/reference/overrideParameterProperty.types @@ -64,3 +64,15 @@ class C3 extends Base { >p1 : "hello" } +class C4 extends Base { +>C4 : C4 +>Base : Base + + constructor(public override p2: string) { +>p2 : string + + super(); +>super() : void +>super : typeof Base + } +} diff --git a/tests/cases/conformance/override/overrideParameterProperty.ts b/tests/cases/conformance/override/overrideParameterProperty.ts index 67e25674010a2..3f11baf8b8a20 100644 --- a/tests/cases/conformance/override/overrideParameterProperty.ts +++ b/tests/cases/conformance/override/overrideParameterProperty.ts @@ -26,3 +26,9 @@ class C3 extends Base { m(override p1: "hello") {} } + +class C4 extends Base { + constructor(public override p2: string) { + super(); + } +} \ No newline at end of file diff --git a/tests/cases/fourslash/codeFixOverrideModifier10.ts b/tests/cases/fourslash/codeFixOverrideModifier10.ts index fd70f63f08629..362fd859608bc 100644 --- a/tests/cases/fourslash/codeFixOverrideModifier10.ts +++ b/tests/cases/fourslash/codeFixOverrideModifier10.ts @@ -7,9 +7,13 @@ //// } //// class D extends B { //// c = 10; -//// constructor(public a: string, public readonly b: string) { +//// constructor(public a: string, [|public readonly b: string|]) { //// super(); //// } //// } -verify.not.codeFixAvailable(); +verify.codeFix({ + description: "Add 'override' modifier", + newRangeContent: "public override readonly b: string", + index: 0 +}) diff --git a/tests/cases/fourslash/codeFixOverrideModifier21.ts b/tests/cases/fourslash/codeFixOverrideModifier21.ts new file mode 100644 index 0000000000000..3cc0eef124e07 --- /dev/null +++ b/tests/cases/fourslash/codeFixOverrideModifier21.ts @@ -0,0 +1,17 @@ +/// + +// @noImplicitOverride: true + +//// class B { } +//// class D extends B { +//// c = 10; +//// constructor([|public override a: string|]) { +//// super(); +//// } +//// } + +verify.codeFix({ + description: "Remove 'override' modifier", + newRangeContent: "public a: string", + index: 0 +}) diff --git a/tests/cases/fourslash/codeFixOverrideModifier9.ts b/tests/cases/fourslash/codeFixOverrideModifier9.ts index 77e24bf09ca3c..0361a75e52fe2 100644 --- a/tests/cases/fourslash/codeFixOverrideModifier9.ts +++ b/tests/cases/fourslash/codeFixOverrideModifier9.ts @@ -6,10 +6,13 @@ //// a: string //// } //// class D extends B { -//// constructor(public a: string, public b: string) { +//// constructor([|public a: string|], public b: string) { //// super(); //// } //// } -verify.not.codeFixAvailable(); - +verify.codeFix({ + description: "Add 'override' modifier", + newRangeContent: "public override a: string", + index: 0 +}) \ No newline at end of file