Skip to content

Commit 5b1e873

Browse files
authored
Update diagnostic message and quickfix for parameter property (#44010)
1 parent 3125398 commit 5b1e873

13 files changed

+110
-27
lines changed

src/compiler/checker.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -37214,7 +37214,7 @@ namespace ts {
3721437214

3721537215
if (!baseHasAbstract) {
3721637216
const diag = memberIsParameterProperty ?
37217-
Diagnostics.This_parameter_property_must_be_rewritten_as_a_property_declaration_with_an_override_modifier_because_it_overrides_a_member_in_base_class_0 :
37217+
Diagnostics.This_parameter_property_must_have_an_override_modifier_because_it_overrides_a_member_in_base_class_0 :
3721837218
Diagnostics.This_member_must_have_an_override_modifier_because_it_overrides_a_member_in_the_base_class_0;
3721937219
error(member, diag, baseClassName);
3722037220
}

src/compiler/diagnosticMessages.json

+1-1
Original file line numberDiff line numberDiff line change
@@ -3717,7 +3717,7 @@
37173717
"category": "Error",
37183718
"code": 4114
37193719
},
3720-
"This parameter property must be rewritten as a property declaration with an 'override' modifier because it overrides a member in base class '{0}'.": {
3720+
"This parameter property must have an 'override' modifier because it overrides a member in base class '{0}'.": {
37213721
"category": "Error",
37223722
"code": 4115
37233723
},

src/services/codefixes/fixOverrideModifier.ts

+17-9
Original file line numberDiff line numberDiff line change
@@ -4,18 +4,20 @@ namespace ts.codefix {
44
const fixAddOverrideId = "fixAddOverrideModifier";
55
const fixRemoveOverrideId = "fixRemoveOverrideModifier";
66

7-
type ClassElementHasJSDoc =
7+
type ClassElementLikeHasJSDoc =
88
| ConstructorDeclaration
99
| PropertyDeclaration
1010
| MethodDeclaration
1111
| GetAccessorDeclaration
12-
| SetAccessorDeclaration;
12+
| SetAccessorDeclaration
13+
| ParameterPropertyDeclaration;
1314

1415
const errorCodes = [
1516
Diagnostics.This_member_cannot_have_an_override_modifier_because_it_is_not_declared_in_the_base_class_0.code,
1617
Diagnostics.This_member_cannot_have_an_override_modifier_because_its_containing_class_0_does_not_extend_another_class.code,
1718
Diagnostics.This_member_must_have_an_override_modifier_because_it_overrides_an_abstract_method_that_is_declared_in_the_base_class_0.code,
18-
Diagnostics.This_member_must_have_an_override_modifier_because_it_overrides_a_member_in_the_base_class_0.code
19+
Diagnostics.This_member_must_have_an_override_modifier_because_it_overrides_a_member_in_the_base_class_0.code,
20+
Diagnostics.This_parameter_property_must_have_an_override_modifier_because_it_overrides_a_member_in_base_class_0.code
1921
];
2022

2123
const errorCodeFixIdMap: Record<number, [DiagnosticMessage, string | undefined, DiagnosticMessage | undefined]> = {
@@ -25,6 +27,9 @@ namespace ts.codefix {
2527
[Diagnostics.This_member_cannot_have_an_override_modifier_because_its_containing_class_0_does_not_extend_another_class.code]: [
2628
Diagnostics.Remove_override_modifier, fixRemoveOverrideId, Diagnostics.Remove_all_unnecessary_override_modifiers
2729
],
30+
[Diagnostics.This_parameter_property_must_have_an_override_modifier_because_it_overrides_a_member_in_base_class_0.code]: [
31+
Diagnostics.Add_override_modifier, fixAddOverrideId, Diagnostics.Add_all_missing_override_modifiers,
32+
],
2833
[Diagnostics.This_member_must_have_an_override_modifier_because_it_overrides_an_abstract_method_that_is_declared_in_the_base_class_0.code]: [
2934
Diagnostics.Add_override_modifier, fixAddOverrideId, Diagnostics.Remove_all_unnecessary_override_modifiers
3035
],
@@ -70,6 +75,7 @@ namespace ts.codefix {
7075
switch (errorCode) {
7176
case Diagnostics.This_member_must_have_an_override_modifier_because_it_overrides_a_member_in_the_base_class_0.code:
7277
case Diagnostics.This_member_must_have_an_override_modifier_because_it_overrides_an_abstract_method_that_is_declared_in_the_base_class_0.code:
78+
case Diagnostics.This_parameter_property_must_have_an_override_modifier_because_it_overrides_a_member_in_base_class_0.code:
7379
return doAddOverrideModifierChange(changeTracker, context.sourceFile, pos);
7480
case Diagnostics.This_member_cannot_have_an_override_modifier_because_it_is_not_declared_in_the_base_class_0.code:
7581
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 {
8086
}
8187

8288
function doAddOverrideModifierChange(changeTracker: textChanges.ChangeTracker, sourceFile: SourceFile, pos: number) {
83-
const classElement = findContainerClassElement(sourceFile, pos);
89+
const classElement = findContainerClassElementLike(sourceFile, pos);
8490
const modifiers = classElement.modifiers || emptyArray;
8591
const staticModifier = find(modifiers, isStaticModifier);
8692
const accessibilityModifier = find(modifiers, m => isAccessibilityModifier(m.kind));
@@ -92,34 +98,36 @@ namespace ts.codefix {
9298
}
9399

94100
function doRemoveOverrideModifierChange(changeTracker: textChanges.ChangeTracker, sourceFile: SourceFile, pos: number) {
95-
const classElement = findContainerClassElement(sourceFile, pos);
101+
const classElement = findContainerClassElementLike(sourceFile, pos);
96102
const overrideModifier = classElement.modifiers && find(classElement.modifiers, modifier => modifier.kind === SyntaxKind.OverrideKeyword);
97103
Debug.assertIsDefined(overrideModifier);
98104

99105
changeTracker.deleteModifier(sourceFile, overrideModifier);
100106
}
101107

102-
function isClassElementHasJSDoc(node: Node): node is ClassElementHasJSDoc {
108+
function isClassElementLikeHasJSDoc(node: Node): node is ClassElementLikeHasJSDoc {
103109
switch (node.kind) {
104110
case SyntaxKind.Constructor:
105111
case SyntaxKind.PropertyDeclaration:
106112
case SyntaxKind.MethodDeclaration:
107113
case SyntaxKind.GetAccessor:
108114
case SyntaxKind.SetAccessor:
109115
return true;
116+
case SyntaxKind.Parameter:
117+
return isParameterPropertyDeclaration(node, node.parent);
110118
default:
111119
return false;
112120
}
113121
}
114122

115-
function findContainerClassElement(sourceFile: SourceFile, pos: number) {
123+
function findContainerClassElementLike(sourceFile: SourceFile, pos: number) {
116124
const token = getTokenAtPosition(sourceFile, pos);
117125
const classElement = findAncestor(token, node => {
118126
if (isClassLike(node)) return "quit";
119-
return isClassElementHasJSDoc(node);
127+
return isClassElementLikeHasJSDoc(node);
120128
});
121129

122-
Debug.assert(classElement && isClassElementHasJSDoc(classElement));
130+
Debug.assert(classElement && isClassElementLikeHasJSDoc(classElement));
123131
return classElement;
124132
}
125133
}

tests/baselines/reference/override6.errors.txt

+4-4
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
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'.
2-
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'.
3-
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'.
2+
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'.
3+
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'.
44

55

66
==== tests/cases/conformance/override/override6.ts (3 errors) ====
@@ -17,9 +17,9 @@ tests/cases/conformance/override/override6.ts(10,37): error TS4115: This paramet
1717
!!! error TS4114: This member must have an 'override' modifier because it overrides a member in the base class 'B'.
1818
constructor(public foo: string, public baz: number) {
1919
~~~~~~~~~~~~~~~~~~
20-
!!! 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'.
20+
!!! error TS4115: This parameter property must have an 'override' modifier because it overrides a member in base class 'B'.
2121
~~~~~~~~~~~~~~~~~~
22-
!!! 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'.
22+
!!! error TS4115: This parameter property must have an 'override' modifier because it overrides a member in base class 'B'.
2323
super(foo, 42)
2424
}
2525
}

tests/baselines/reference/override8.errors.txt

+4-4
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
1-
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'.
2-
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'.
1+
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'.
2+
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'.
33
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'.
44

55

@@ -11,7 +11,7 @@ tests/cases/conformance/override/override8.ts(24,12): error TS4114: This member
1111
class D extends B {
1212
constructor(public a: string, public b: string) {
1313
~~~~~~~~~~~~~~~~
14-
!!! 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'.
14+
!!! error TS4115: This parameter property must have an 'override' modifier because it overrides a member in base class 'B'.
1515
super();
1616
}
1717
}
@@ -25,7 +25,7 @@ tests/cases/conformance/override/override8.ts(24,12): error TS4114: This member
2525
class DD extends BB {
2626
constructor(public a: string) {
2727
~~~~~~~~~~~~~~~~
28-
!!! 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'.
28+
!!! error TS4115: This parameter property must have an 'override' modifier because it overrides a member in base class 'BB'.
2929
super(a)
3030
}
3131
}

tests/baselines/reference/overrideParameterProperty.errors.txt

+10-2
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,9 @@
11
tests/cases/conformance/override/overrideParameterProperty.ts(20,24): error TS1029: 'public' modifier must precede 'override' modifier.
22
tests/cases/conformance/override/overrideParameterProperty.ts(25,5): error TS2369: A parameter property is only allowed in a constructor implementation.
3+
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'.
34

45

5-
==== tests/cases/conformance/override/overrideParameterProperty.ts (2 errors) ====
6+
==== tests/cases/conformance/override/overrideParameterProperty.ts (3 errors) ====
67
class Base {
78
p1!: string;
89
}
@@ -33,4 +34,11 @@ tests/cases/conformance/override/overrideParameterProperty.ts(25,5): error TS236
3334
~~~~~~~~~~~~~~~~~~~~
3435
!!! error TS2369: A parameter property is only allowed in a constructor implementation.
3536
}
36-
37+
38+
class C4 extends Base {
39+
constructor(public override p2: string) {
40+
~~~~~~~~~~~~~~~~~~~~~~~~~~
41+
!!! error TS4113: This member cannot have an 'override' modifier because it is not declared in the base class 'Base'.
42+
super();
43+
}
44+
}

tests/baselines/reference/overrideParameterProperty.js

+15-1
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,12 @@ class C3 extends Base {
2525

2626
m(override p1: "hello") {}
2727
}
28-
28+
29+
class C4 extends Base {
30+
constructor(public override p2: string) {
31+
super();
32+
}
33+
}
2934

3035
//// [overrideParameterProperty.js]
3136
var __extends = (this && this.__extends) || (function () {
@@ -79,3 +84,12 @@ var C3 = /** @class */ (function (_super) {
7984
C3.prototype.m = function (p1) { };
8085
return C3;
8186
}(Base));
87+
var C4 = /** @class */ (function (_super) {
88+
__extends(C4, _super);
89+
function C4(p2) {
90+
var _this = _super.call(this) || this;
91+
_this.p2 = p2;
92+
return _this;
93+
}
94+
return C4;
95+
}(Base));

tests/baselines/reference/overrideParameterProperty.symbols

+11
Original file line numberDiff line numberDiff line change
@@ -61,3 +61,14 @@ class C3 extends Base {
6161
>p1 : Symbol(p1, Decl(overrideParameterProperty.ts, 24, 4))
6262
}
6363

64+
class C4 extends Base {
65+
>C4 : Symbol(C4, Decl(overrideParameterProperty.ts, 25, 1))
66+
>Base : Symbol(Base, Decl(overrideParameterProperty.ts, 0, 0))
67+
68+
constructor(public override p2: string) {
69+
>p2 : Symbol(C4.p2, Decl(overrideParameterProperty.ts, 28, 14))
70+
71+
super();
72+
>super : Symbol(Base, Decl(overrideParameterProperty.ts, 0, 0))
73+
}
74+
}

tests/baselines/reference/overrideParameterProperty.types

+12
Original file line numberDiff line numberDiff line change
@@ -64,3 +64,15 @@ class C3 extends Base {
6464
>p1 : "hello"
6565
}
6666

67+
class C4 extends Base {
68+
>C4 : C4
69+
>Base : Base
70+
71+
constructor(public override p2: string) {
72+
>p2 : string
73+
74+
super();
75+
>super() : void
76+
>super : typeof Base
77+
}
78+
}

tests/cases/conformance/override/overrideParameterProperty.ts

+6
Original file line numberDiff line numberDiff line change
@@ -26,3 +26,9 @@ class C3 extends Base {
2626

2727
m(override p1: "hello") {}
2828
}
29+
30+
class C4 extends Base {
31+
constructor(public override p2: string) {
32+
super();
33+
}
34+
}

tests/cases/fourslash/codeFixOverrideModifier10.ts

+6-2
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,13 @@
77
//// }
88
//// class D extends B {
99
//// c = 10;
10-
//// constructor(public a: string, public readonly b: string) {
10+
//// constructor(public a: string, [|public readonly b: string|]) {
1111
//// super();
1212
//// }
1313
//// }
1414

15-
verify.not.codeFixAvailable();
15+
verify.codeFix({
16+
description: "Add 'override' modifier",
17+
newRangeContent: "public override readonly b: string",
18+
index: 0
19+
})
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
/// <reference path='fourslash.ts' />
2+
3+
// @noImplicitOverride: true
4+
5+
//// class B { }
6+
//// class D extends B {
7+
//// c = 10;
8+
//// constructor([|public override a: string|]) {
9+
//// super();
10+
//// }
11+
//// }
12+
13+
verify.codeFix({
14+
description: "Remove 'override' modifier",
15+
newRangeContent: "public a: string",
16+
index: 0
17+
})

tests/cases/fourslash/codeFixOverrideModifier9.ts

+6-3
Original file line numberDiff line numberDiff line change
@@ -6,10 +6,13 @@
66
//// a: string
77
//// }
88
//// class D extends B {
9-
//// constructor(public a: string, public b: string) {
9+
//// constructor([|public a: string|], public b: string) {
1010
//// super();
1111
//// }
1212
//// }
1313

14-
verify.not.codeFixAvailable();
15-
14+
verify.codeFix({
15+
description: "Add 'override' modifier",
16+
newRangeContent: "public override a: string",
17+
index: 0
18+
})

0 commit comments

Comments
 (0)