Skip to content

Commit fdd8a52

Browse files
authored
Offer per-member diagnostics for incorrectly implemented inherited members (#21036)
* Offer per-member diagnostics for incorrectly implemented inherited members on classes * Revise error message, make containingChain a thunk * Fix typo in comment
1 parent ba979fa commit fdd8a52

File tree

40 files changed

+798
-669
lines changed

40 files changed

+798
-669
lines changed

src/compiler/checker.ts

Lines changed: 52 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -9169,15 +9169,15 @@ namespace ts {
91699169
return isTypeComparableTo(type1, type2) || isTypeComparableTo(type2, type1);
91709170
}
91719171

9172-
function checkTypeAssignableTo(source: Type, target: Type, errorNode: Node, headMessage?: DiagnosticMessage, containingMessageChain?: DiagnosticMessageChain): boolean {
9172+
function checkTypeAssignableTo(source: Type, target: Type, errorNode: Node, headMessage?: DiagnosticMessage, containingMessageChain?: () => DiagnosticMessageChain | undefined): boolean {
91739173
return checkTypeRelatedTo(source, target, assignableRelation, errorNode, headMessage, containingMessageChain);
91749174
}
91759175

91769176
/**
91779177
* This is *not* a bi-directional relationship.
91789178
* If one needs to check both directions for comparability, use a second call to this function or 'isTypeComparableTo'.
91799179
*/
9180-
function checkTypeComparableTo(source: Type, target: Type, errorNode: Node, headMessage?: DiagnosticMessage, containingMessageChain?: DiagnosticMessageChain): boolean {
9180+
function checkTypeComparableTo(source: Type, target: Type, errorNode: Node, headMessage?: DiagnosticMessage, containingMessageChain?: () => DiagnosticMessageChain | undefined): boolean {
91819181
return checkTypeRelatedTo(source, target, comparableRelation, errorNode, headMessage, containingMessageChain);
91829182
}
91839183

@@ -9511,7 +9511,7 @@ namespace ts {
95119511
relation: Map<RelationComparisonResult>,
95129512
errorNode: Node,
95139513
headMessage?: DiagnosticMessage,
9514-
containingMessageChain?: DiagnosticMessageChain): boolean {
9514+
containingMessageChain?: () => DiagnosticMessageChain | undefined): boolean {
95159515

95169516
let errorInfo: DiagnosticMessageChain;
95179517
let maybeKeys: string[];
@@ -9531,7 +9531,10 @@ namespace ts {
95319531
}
95329532
else if (errorInfo) {
95339533
if (containingMessageChain) {
9534-
errorInfo = concatenateDiagnosticMessageChains(containingMessageChain, errorInfo);
9534+
const chain = containingMessageChain();
9535+
if (chain) {
9536+
errorInfo = concatenateDiagnosticMessageChains(chain, errorInfo);
9537+
}
95359538
}
95369539

95379540
diagnostics.add(createDiagnosticForNodeFromMessageChain(errorNode, errorInfo));
@@ -16675,7 +16678,7 @@ namespace ts {
1667516678
const constraint = getConstraintOfTypeParameter(typeParameters[i]);
1667616679
if (!constraint) continue;
1667716680

16678-
const errorInfo = reportErrors && headMessage && chainDiagnosticMessages(/*details*/ undefined, Diagnostics.Type_0_does_not_satisfy_the_constraint_1);
16681+
const errorInfo = reportErrors && headMessage && (() => chainDiagnosticMessages(/*details*/ undefined, Diagnostics.Type_0_does_not_satisfy_the_constraint_1));
1667916682
const typeArgumentHeadMessage = headMessage || Diagnostics.Type_0_does_not_satisfy_the_constraint_1;
1668016683
if (!mapper) {
1668116684
mapper = createTypeMapper(typeParameters, typeArgumentTypes);
@@ -19697,7 +19700,7 @@ namespace ts {
1969719700
Diagnostics.A_type_predicate_cannot_reference_a_rest_parameter);
1969819701
}
1969919702
else {
19700-
const leadingError = chainDiagnosticMessages(/*details*/ undefined, Diagnostics.A_type_predicate_s_type_must_be_assignable_to_its_parameter_s_type);
19703+
const leadingError = () => chainDiagnosticMessages(/*details*/ undefined, Diagnostics.A_type_predicate_s_type_must_be_assignable_to_its_parameter_s_type);
1970119704
checkTypeAssignableTo(typePredicate.type,
1970219705
getTypeOfNode(parent.parameters[typePredicate.parameterIndex]),
1970319706
node.type,
@@ -20984,7 +20987,7 @@ namespace ts {
2098420987
expectedReturnType,
2098520988
node,
2098620989
headMessage,
20987-
errorInfo);
20990+
() => errorInfo);
2098820991
}
2098920992

2099020993
/**
@@ -22907,7 +22910,10 @@ namespace ts {
2290722910
}
2290822911
}
2290922912
}
22910-
checkTypeAssignableTo(typeWithThis, getTypeWithThisArgument(baseType, type.thisType), node.name || node, Diagnostics.Class_0_incorrectly_extends_base_class_1);
22913+
const baseWithThis = getTypeWithThisArgument(baseType, type.thisType);
22914+
if (!checkTypeAssignableTo(typeWithThis, baseWithThis, /*errorNode*/ undefined)) {
22915+
issueMemberSpecificError(node, typeWithThis, baseWithThis, Diagnostics.Class_0_incorrectly_extends_base_class_1);
22916+
}
2291122917
checkTypeAssignableTo(staticType, getTypeWithoutSignatures(staticBaseType), node.name || node,
2291222918
Diagnostics.Class_static_side_0_incorrectly_extends_base_class_static_side_1);
2291322919
if (baseConstructorType.flags & TypeFlags.TypeVariable && !isMixinConstructorType(staticType)) {
@@ -22939,12 +22945,13 @@ namespace ts {
2293922945
const t = getTypeFromTypeNode(typeRefNode);
2294022946
if (t !== unknownType) {
2294122947
if (isValidBaseType(t)) {
22942-
checkTypeAssignableTo(typeWithThis,
22943-
getTypeWithThisArgument(t, type.thisType),
22944-
node.name || node,
22945-
t.symbol && t.symbol.flags & SymbolFlags.Class ?
22946-
Diagnostics.Class_0_incorrectly_implements_class_1_Did_you_mean_to_extend_1_and_inherit_its_members_as_a_subclass :
22947-
Diagnostics.Class_0_incorrectly_implements_interface_1);
22948+
const genericDiag = t.symbol && t.symbol.flags & SymbolFlags.Class ?
22949+
Diagnostics.Class_0_incorrectly_implements_class_1_Did_you_mean_to_extend_1_and_inherit_its_members_as_a_subclass :
22950+
Diagnostics.Class_0_incorrectly_implements_interface_1;
22951+
const baseWithThis = getTypeWithThisArgument(t, type.thisType);
22952+
if (!checkTypeAssignableTo(typeWithThis, baseWithThis, /*errorNode*/ undefined)) {
22953+
issueMemberSpecificError(node, typeWithThis, baseWithThis, genericDiag);
22954+
}
2294822955
}
2294922956
else {
2295022957
error(typeRefNode, Diagnostics.A_class_may_only_implement_another_class_or_interface);
@@ -22961,6 +22968,37 @@ namespace ts {
2296122968
}
2296222969
}
2296322970

22971+
function issueMemberSpecificError(node: ClassLikeDeclaration, typeWithThis: Type, baseWithThis: Type, broadDiag: DiagnosticMessage) {
22972+
// iterate over all implemented properties and issue errors on each one which isn't compatible, rather than the class as a whole, if possible
22973+
let issuedMemberError = false;
22974+
for (const member of node.members) {
22975+
if (hasStaticModifier(member)) {
22976+
continue;
22977+
}
22978+
const declaredProp = member.name && getSymbolAtLocation(member.name) || getSymbolAtLocation(member);
22979+
if (declaredProp) {
22980+
const prop = getPropertyOfType(typeWithThis, declaredProp.escapedName);
22981+
const baseProp = getPropertyOfType(baseWithThis, declaredProp.escapedName);
22982+
if (prop && baseProp) {
22983+
const rootChain = () => chainDiagnosticMessages(
22984+
/*details*/ undefined,
22985+
Diagnostics.Property_0_in_type_1_is_not_assignable_to_the_same_property_in_base_type_2,
22986+
unescapeLeadingUnderscores(declaredProp.escapedName),
22987+
typeToString(typeWithThis),
22988+
typeToString(getTypeOfSymbol(baseProp))
22989+
);
22990+
if (!checkTypeAssignableTo(getTypeOfSymbol(prop), getTypeOfSymbol(baseProp), member.name || member, /*message*/ undefined, rootChain)) {
22991+
issuedMemberError = true;
22992+
}
22993+
}
22994+
}
22995+
}
22996+
if (!issuedMemberError) {
22997+
// check again with diagnostics to generate a less-specific error
22998+
checkTypeAssignableTo(typeWithThis, baseWithThis, node.name || node, broadDiag);
22999+
}
23000+
}
23001+
2296423002
function checkBaseTypeAccessibility(type: Type, node: ExpressionWithTypeArguments) {
2296523003
const signatures = getSignaturesOfType(type, SignatureKind.Construct);
2296623004
if (signatures.length) {

src/compiler/diagnosticMessages.json

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1404,6 +1404,10 @@
14041404
"category": "Error",
14051405
"code": 2415
14061406
},
1407+
"Property '{0}' in type '{1}' is not assignable to the same property in base type '{2}'.": {
1408+
"category": "Error",
1409+
"code": 2416
1410+
},
14071411
"Class static side '{0}' incorrectly extends base class static side '{1}'.": {
14081412
"category": "Error",
14091413
"code": 2417

tests/baselines/reference/abstractPropertyNegative.errors.txt

Lines changed: 15 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -7,15 +7,12 @@ tests/cases/compiler/abstractPropertyNegative.ts(13,7): error TS2515: Non-abstra
77
tests/cases/compiler/abstractPropertyNegative.ts(15,5): error TS1244: Abstract methods can only appear within an abstract class.
88
tests/cases/compiler/abstractPropertyNegative.ts(16,37): error TS1005: '{' expected.
99
tests/cases/compiler/abstractPropertyNegative.ts(19,3): error TS2540: Cannot assign to 'ro' because it is a constant or a read-only property.
10-
tests/cases/compiler/abstractPropertyNegative.ts(24,7): error TS2415: Class 'WrongTypePropertyImpl' incorrectly extends base class 'WrongTypeProperty'.
11-
Types of property 'num' are incompatible.
12-
Type 'string' is not assignable to type 'number'.
13-
tests/cases/compiler/abstractPropertyNegative.ts(30,7): error TS2415: Class 'WrongTypeAccessorImpl' incorrectly extends base class 'WrongTypeAccessor'.
14-
Types of property 'num' are incompatible.
15-
Type 'string' is not assignable to type 'number'.
16-
tests/cases/compiler/abstractPropertyNegative.ts(33,7): error TS2415: Class 'WrongTypeAccessorImpl2' incorrectly extends base class 'WrongTypeAccessor'.
17-
Types of property 'num' are incompatible.
18-
Type 'string' is not assignable to type 'number'.
10+
tests/cases/compiler/abstractPropertyNegative.ts(25,5): error TS2416: Property 'num' in type 'WrongTypePropertyImpl' is not assignable to the same property in base type 'number'.
11+
Type 'string' is not assignable to type 'number'.
12+
tests/cases/compiler/abstractPropertyNegative.ts(31,9): error TS2416: Property 'num' in type 'WrongTypeAccessorImpl' is not assignable to the same property in base type 'number'.
13+
Type 'string' is not assignable to type 'number'.
14+
tests/cases/compiler/abstractPropertyNegative.ts(34,5): error TS2416: Property 'num' in type 'WrongTypeAccessorImpl2' is not assignable to the same property in base type 'number'.
15+
Type 'string' is not assignable to type 'number'.
1916
tests/cases/compiler/abstractPropertyNegative.ts(38,18): error TS2676: Accessors must both be abstract or non-abstract.
2017
tests/cases/compiler/abstractPropertyNegative.ts(39,9): error TS2676: Accessors must both be abstract or non-abstract.
2118
tests/cases/compiler/abstractPropertyNegative.ts(40,9): error TS2676: Accessors must both be abstract or non-abstract.
@@ -65,28 +62,25 @@ tests/cases/compiler/abstractPropertyNegative.ts(41,18): error TS2676: Accessors
6562
abstract num: number;
6663
}
6764
class WrongTypePropertyImpl extends WrongTypeProperty {
68-
~~~~~~~~~~~~~~~~~~~~~
69-
!!! error TS2415: Class 'WrongTypePropertyImpl' incorrectly extends base class 'WrongTypeProperty'.
70-
!!! error TS2415: Types of property 'num' are incompatible.
71-
!!! error TS2415: Type 'string' is not assignable to type 'number'.
7265
num = "nope, wrong";
66+
~~~
67+
!!! error TS2416: Property 'num' in type 'WrongTypePropertyImpl' is not assignable to the same property in base type 'number'.
68+
!!! error TS2416: Type 'string' is not assignable to type 'number'.
7369
}
7470
abstract class WrongTypeAccessor {
7571
abstract get num(): number;
7672
}
7773
class WrongTypeAccessorImpl extends WrongTypeAccessor {
78-
~~~~~~~~~~~~~~~~~~~~~
79-
!!! error TS2415: Class 'WrongTypeAccessorImpl' incorrectly extends base class 'WrongTypeAccessor'.
80-
!!! error TS2415: Types of property 'num' are incompatible.
81-
!!! error TS2415: Type 'string' is not assignable to type 'number'.
8274
get num() { return "nope, wrong"; }
75+
~~~
76+
!!! error TS2416: Property 'num' in type 'WrongTypeAccessorImpl' is not assignable to the same property in base type 'number'.
77+
!!! error TS2416: Type 'string' is not assignable to type 'number'.
8378
}
8479
class WrongTypeAccessorImpl2 extends WrongTypeAccessor {
85-
~~~~~~~~~~~~~~~~~~~~~~
86-
!!! error TS2415: Class 'WrongTypeAccessorImpl2' incorrectly extends base class 'WrongTypeAccessor'.
87-
!!! error TS2415: Types of property 'num' are incompatible.
88-
!!! error TS2415: Type 'string' is not assignable to type 'number'.
8980
num = "nope, wrong";
81+
~~~
82+
!!! error TS2416: Property 'num' in type 'WrongTypeAccessorImpl2' is not assignable to the same property in base type 'number'.
83+
!!! error TS2416: Type 'string' is not assignable to type 'number'.
9084
}
9185

9286
abstract class AbstractAccessorMismatch {

tests/baselines/reference/apparentTypeSubtyping.errors.txt

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
1-
tests/cases/conformance/types/typeRelationships/apparentType/apparentTypeSubtyping.ts(9,7): error TS2415: Class 'Derived<U>' incorrectly extends base class 'Base<string>'.
2-
Types of property 'x' are incompatible.
3-
Type 'String' is not assignable to type 'string'.
4-
'string' is a primitive, but 'String' is a wrapper object. Prefer using 'string' when possible.
1+
tests/cases/conformance/types/typeRelationships/apparentType/apparentTypeSubtyping.ts(10,5): error TS2416: Property 'x' in type 'Derived<U>' is not assignable to the same property in base type 'string'.
2+
Type 'String' is not assignable to type 'string'.
3+
'string' is a primitive, but 'String' is a wrapper object. Prefer using 'string' when possible.
54

65

76
==== tests/cases/conformance/types/typeRelationships/apparentType/apparentTypeSubtyping.ts (1 errors) ====
@@ -14,12 +13,11 @@ tests/cases/conformance/types/typeRelationships/apparentType/apparentTypeSubtypi
1413

1514
// is String (S) a subtype of U extends String (T)? Would only be true if we used the apparent type of U (T)
1615
class Derived<U> extends Base<string> { // error
17-
~~~~~~~
18-
!!! error TS2415: Class 'Derived<U>' incorrectly extends base class 'Base<string>'.
19-
!!! error TS2415: Types of property 'x' are incompatible.
20-
!!! error TS2415: Type 'String' is not assignable to type 'string'.
21-
!!! error TS2415: 'string' is a primitive, but 'String' is a wrapper object. Prefer using 'string' when possible.
2216
x: String;
17+
~
18+
!!! error TS2416: Property 'x' in type 'Derived<U>' is not assignable to the same property in base type 'string'.
19+
!!! error TS2416: Type 'String' is not assignable to type 'string'.
20+
!!! error TS2416: 'string' is a primitive, but 'String' is a wrapper object. Prefer using 'string' when possible.
2321
}
2422

2523
class Base2 {
Lines changed: 9 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,7 @@
1-
tests/cases/conformance/types/typeRelationships/apparentType/apparentTypeSupertype.ts(9,7): error TS2415: Class 'Derived<U>' incorrectly extends base class 'Base'.
2-
Types of property 'x' are incompatible.
3-
Type 'U' is not assignable to type 'string'.
4-
Type 'String' is not assignable to type 'string'.
5-
'string' is a primitive, but 'String' is a wrapper object. Prefer using 'string' when possible.
1+
tests/cases/conformance/types/typeRelationships/apparentType/apparentTypeSupertype.ts(10,5): error TS2416: Property 'x' in type 'Derived<U>' is not assignable to the same property in base type 'string'.
2+
Type 'U' is not assignable to type 'string'.
3+
Type 'String' is not assignable to type 'string'.
4+
'string' is a primitive, but 'String' is a wrapper object. Prefer using 'string' when possible.
65

76

87
==== tests/cases/conformance/types/typeRelationships/apparentType/apparentTypeSupertype.ts (1 errors) ====
@@ -15,11 +14,10 @@ tests/cases/conformance/types/typeRelationships/apparentType/apparentTypeSuperty
1514

1615
// is String (S) a subtype of U extends String (T)? Would only be true if we used the apparent type of U (T)
1716
class Derived<U extends String> extends Base { // error
18-
~~~~~~~
19-
!!! error TS2415: Class 'Derived<U>' incorrectly extends base class 'Base'.
20-
!!! error TS2415: Types of property 'x' are incompatible.
21-
!!! error TS2415: Type 'U' is not assignable to type 'string'.
22-
!!! error TS2415: Type 'String' is not assignable to type 'string'.
23-
!!! error TS2415: 'string' is a primitive, but 'String' is a wrapper object. Prefer using 'string' when possible.
2417
x: U;
18+
~
19+
!!! error TS2416: Property 'x' in type 'Derived<U>' is not assignable to the same property in base type 'string'.
20+
!!! error TS2416: Type 'U' is not assignable to type 'string'.
21+
!!! error TS2416: Type 'String' is not assignable to type 'string'.
22+
!!! error TS2416: 'string' is a primitive, but 'String' is a wrapper object. Prefer using 'string' when possible.
2523
}

0 commit comments

Comments
 (0)