Skip to content

Commit d71af8a

Browse files
committed
Addressing CR feedback
1 parent f6bcf70 commit d71af8a

File tree

6 files changed

+35
-26
lines changed

6 files changed

+35
-26
lines changed

Diff for: src/compiler/checker.ts

+25-16
Original file line numberDiff line numberDiff line change
@@ -2639,15 +2639,16 @@ namespace ts {
26392639
}
26402640
let baseConstructorType = checkExpression(baseTypeNode.expression);
26412641
if (baseConstructorType.flags & TypeFlags.ObjectType) {
2642-
// Force resolution of members such that we catch circularities
2642+
// Resolving the members of a class requires us to resolve the base class of that class.
2643+
// We force resolution here such that we catch circularities now.
26432644
resolveObjectOrUnionTypeMembers(baseConstructorType);
26442645
}
26452646
if (!popTypeResolution()) {
26462647
error(type.symbol.valueDeclaration, Diagnostics._0_is_referenced_directly_or_indirectly_in_its_own_base_expression, symbolToString(type.symbol));
26472648
return type.resolvedBaseConstructorType = unknownType;
26482649
}
26492650
if (baseConstructorType !== unknownType && baseConstructorType !== nullType && !isConstructorType(baseConstructorType)) {
2650-
error(baseTypeNode.expression, Diagnostics.Base_expression_is_not_of_a_constructor_function_type);
2651+
error(baseTypeNode.expression, Diagnostics.Type_0_is_not_a_constructor_function_type, typeToString(baseConstructorType));
26512652
return type.resolvedBaseConstructorType = unknownType;
26522653
}
26532654
type.resolvedBaseConstructorType = baseConstructorType;
@@ -2699,7 +2700,7 @@ namespace ts {
26992700
return;
27002701
}
27012702
if (!(getTargetType(baseType).flags & (TypeFlags.Class | TypeFlags.Interface))) {
2702-
error(baseTypeNode.expression, Diagnostics.Base_constructor_does_not_return_a_class_or_interface_type);
2703+
error(baseTypeNode.expression, Diagnostics.Base_constructor_return_type_0_is_not_a_class_or_interface_type, typeToString(baseType));
27032704
return;
27042705
}
27052706
if (type === baseType || hasBaseType(<InterfaceType>baseType, type)) {
@@ -5962,7 +5963,9 @@ namespace ts {
59625963
let baseClassType = classType && getBaseTypes(classType)[0];
59635964

59645965
if (!baseClassType) {
5965-
error(node, Diagnostics.super_can_only_be_referenced_in_a_derived_class);
5966+
if (!classDeclaration || !getClassExtendsHeritageClauseElement(classDeclaration)) {
5967+
error(node, Diagnostics.super_can_only_be_referenced_in_a_derived_class);
5968+
}
59665969
return unknownType;
59675970
}
59685971

@@ -8970,13 +8973,14 @@ namespace ts {
89708973
checkDecorators(node);
89718974
}
89728975

8973-
function checkTypeArgumentsAndConstraints(typeParameters: TypeParameter[], typeArguments: TypeNode[]) {
8974-
for (let i = 0; i < typeArguments.length; i++) {
8975-
let typeArgument = typeArguments[i];
8976-
checkSourceElement(typeArgument);
8977-
let constraint = getConstraintOfTypeParameter(typeParameters[i]);
8978-
if (produceDiagnostics && constraint) {
8979-
checkTypeAssignableTo(getTypeFromTypeNode(typeArgument), constraint, typeArgument, Diagnostics.Type_0_does_not_satisfy_the_constraint_1);
8976+
function checkTypeArgumentConstraints(typeParameters: TypeParameter[], typeArguments: TypeNode[]) {
8977+
if (produceDiagnostics) {
8978+
for (let i = 0; i < typeParameters.length; i++) {
8979+
let constraint = getConstraintOfTypeParameter(typeParameters[i]);
8980+
if (constraint) {
8981+
let typeArgument = typeArguments[i];
8982+
checkTypeAssignableTo(getTypeFromTypeNode(typeArgument), constraint, typeArgument, Diagnostics.Type_0_does_not_satisfy_the_constraint_1);
8983+
}
89808984
}
89818985
}
89828986
}
@@ -8986,9 +8990,10 @@ namespace ts {
89868990
let type = getTypeFromTypeReference(node);
89878991
if (type !== unknownType && node.typeArguments) {
89888992
// Do type argument local checks only if referenced type is successfully resolved
8993+
forEach(node.typeArguments, checkSourceElement);
89898994
let symbol = getNodeLinks(node).resolvedSymbol;
89908995
let typeParameters = symbol.flags & SymbolFlags.TypeAlias ? getSymbolLinks(symbol).typeParameters : (<TypeReference>type).target.localTypeParameters;
8991-
checkTypeArgumentsAndConstraints(typeParameters, node.typeArguments);
8996+
checkTypeArgumentConstraints(typeParameters, node.typeArguments);
89928997
}
89938998
}
89948999

@@ -10588,15 +10593,19 @@ namespace ts {
1058810593
let baseType = baseTypes[0];
1058910594
let staticBaseType = getBaseConstructorTypeOfClass(type);
1059010595
if (baseTypeNode.typeArguments) {
10591-
let constructors = getConstructorsForTypeArguments(staticBaseType, baseTypeNode.typeArguments);
10592-
checkTypeArgumentsAndConstraints(constructors[0].typeParameters, baseTypeNode.typeArguments);
10596+
forEach(baseTypeNode.typeArguments, checkSourceElement);
10597+
for (let constructor of getConstructorsForTypeArguments(staticBaseType, baseTypeNode.typeArguments)) {
10598+
checkTypeArgumentConstraints(constructor.typeParameters, baseTypeNode.typeArguments);
10599+
}
1059310600
}
1059410601
checkTypeAssignableTo(type, baseType, node.name || node, Diagnostics.Class_0_incorrectly_extends_base_class_1);
1059510602
checkTypeAssignableTo(staticType, getTypeWithoutSignatures(staticBaseType), node.name || node,
1059610603
Diagnostics.Class_static_side_0_incorrectly_extends_base_class_static_side_1);
1059710604
if (!(staticBaseType.symbol && staticBaseType.symbol.flags & SymbolFlags.Class)) {
1059810605
// When the static base type is a "class-like" constructor function (but not actually a class), we verify
10599-
// that all instantiated base constructor signatures return the same type.
10606+
// that all instantiated base constructor signatures return the same type. We can simply compare the type
10607+
// references (as opposed to checking the structure of the types) because elsewhere we have already checked
10608+
// that the base type is a class or interface type (and not, for example, an anonymous object type).
1060010609
let constructors = getInstantiatedConstructorsForTypeArguments(staticBaseType, baseTypeNode.typeArguments);
1060110610
if (forEach(constructors, sig => getReturnTypeOfSignature(sig) !== baseType)) {
1060210611
error(baseTypeNode.expression, Diagnostics.Base_constructors_must_all_have_the_same_return_type);
@@ -11997,7 +12006,7 @@ namespace ts {
1199712006
return getTypeOfExpression(<Expression>node);
1199812007
}
1199912008

12000-
if (isClassExtendsExpressionWithTypeArguments(node)) {
12009+
if (isExpressionWithTypeArgumentsInClassExtendsClause(node)) {
1200112010
// A SyntaxKind.ExpressionWithTypeArguments is considered a type node, except when it occurs in the
1200212011
// extends clause of a class. We handle that case here.
1200312012
return getBaseTypes(<InterfaceType>getDeclaredTypeOfSymbol(getSymbolOfNode(node.parent.parent)))[0];

Diff for: src/compiler/diagnosticInformationMap.generated.ts

+2-2
Original file line numberDiff line numberDiff line change
@@ -385,9 +385,9 @@ namespace ts {
385385
No_best_common_type_exists_among_yield_expressions: { code: 2504, category: DiagnosticCategory.Error, key: "No best common type exists among yield expressions." },
386386
A_generator_cannot_have_a_void_type_annotation: { code: 2505, category: DiagnosticCategory.Error, key: "A generator cannot have a 'void' type annotation." },
387387
_0_is_referenced_directly_or_indirectly_in_its_own_base_expression: { code: 2506, category: DiagnosticCategory.Error, key: "'{0}' is referenced directly or indirectly in its own base expression." },
388-
Base_expression_is_not_of_a_constructor_function_type: { code: 2507, category: DiagnosticCategory.Error, key: "Base expression is not of a constructor function type." },
388+
Type_0_is_not_a_constructor_function_type: { code: 2507, category: DiagnosticCategory.Error, key: "Type '{0}' is not a constructor function type." },
389389
No_base_constructor_has_the_specified_number_of_type_arguments: { code: 2508, category: DiagnosticCategory.Error, key: "No base constructor has the specified number of type arguments." },
390-
Base_constructor_does_not_return_a_class_or_interface_type: { code: 2509, category: DiagnosticCategory.Error, key: "Base constructor does not return a class or interface type." },
390+
Base_constructor_return_type_0_is_not_a_class_or_interface_type: { code: 2509, category: DiagnosticCategory.Error, key: "Base constructor return type '{0}' is not a class or interface type." },
391391
Base_constructors_must_all_have_the_same_return_type: { code: 2510, category: DiagnosticCategory.Error, key: "Base constructors must all have the same return type." },
392392
Import_declaration_0_is_using_private_name_1: { code: 4000, category: DiagnosticCategory.Error, key: "Import declaration '{0}' is using private name '{1}'." },
393393
Type_parameter_0_of_exported_class_has_or_is_using_private_name_1: { code: 4002, category: DiagnosticCategory.Error, key: "Type parameter '{0}' of exported class has or is using private name '{1}'." },

Diff for: src/compiler/diagnosticMessages.json

+2-2
Original file line numberDiff line numberDiff line change
@@ -1529,15 +1529,15 @@
15291529
"category": "Error",
15301530
"code": 2506
15311531
},
1532-
"Base expression is not of a constructor function type.": {
1532+
"Type '{0}' is not a constructor function type.": {
15331533
"category": "Error",
15341534
"code": 2507
15351535
},
15361536
"No base constructor has the specified number of type arguments.": {
15371537
"category": "Error",
15381538
"code": 2508
15391539
},
1540-
"Base constructor does not return a class or interface type.": {
1540+
"Base constructor return type '{0}' is not a class or interface type.": {
15411541
"category": "Error",
15421542
"code": 2509
15431543
},

Diff for: src/compiler/utilities.ts

+4-4
Original file line numberDiff line numberDiff line change
@@ -426,7 +426,7 @@ namespace ts {
426426
// Specialized signatures can have string literals as their parameters' type names
427427
return node.parent.kind === SyntaxKind.Parameter;
428428
case SyntaxKind.ExpressionWithTypeArguments:
429-
return !isClassExtendsExpressionWithTypeArguments(node);
429+
return !isExpressionWithTypeArgumentsInClassExtendsClause(node);
430430

431431
// Identifiers and qualified names may be type nodes, depending on their context. Climb
432432
// above them to find the lowest container
@@ -460,7 +460,7 @@ namespace ts {
460460
}
461461
switch (parent.kind) {
462462
case SyntaxKind.ExpressionWithTypeArguments:
463-
return !isClassExtendsExpressionWithTypeArguments(parent);
463+
return !isExpressionWithTypeArgumentsInClassExtendsClause(parent);
464464
case SyntaxKind.TypeParameter:
465465
return node === (<TypeParameterDeclaration>parent).constraint;
466466
case SyntaxKind.PropertyDeclaration:
@@ -920,7 +920,7 @@ namespace ts {
920920
case SyntaxKind.Decorator:
921921
return true;
922922
case SyntaxKind.ExpressionWithTypeArguments:
923-
return isClassExtendsExpressionWithTypeArguments(parent);
923+
return (<ExpressionWithTypeArguments>parent).expression === node && isExpressionWithTypeArgumentsInClassExtendsClause(parent);
924924
default:
925925
if (isExpression(parent)) {
926926
return true;
@@ -1914,7 +1914,7 @@ namespace ts {
19141914
return token >= SyntaxKind.FirstAssignment && token <= SyntaxKind.LastAssignment;
19151915
}
19161916

1917-
export function isClassExtendsExpressionWithTypeArguments(node: Node): boolean {
1917+
export function isExpressionWithTypeArgumentsInClassExtendsClause(node: Node): boolean {
19181918
return node.kind === SyntaxKind.ExpressionWithTypeArguments &&
19191919
(<HeritageClause>node.parent).token === SyntaxKind.ExtendsKeyword &&
19201920
node.parent.parent.kind === SyntaxKind.ClassDeclaration;

Diff for: src/harness/typeWriter.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ class TypeWriterWalker {
4343

4444
// Workaround to ensure we output 'C' instead of 'typeof C' for base class expressions
4545
// var type = this.checker.getTypeAtLocation(node);
46-
var type = node.parent && ts.isClassExtendsExpressionWithTypeArguments(node.parent) && this.checker.getTypeAtLocation(node.parent) || this.checker.getTypeAtLocation(node);
46+
var type = node.parent && ts.isExpressionWithTypeArgumentsInClassExtendsClause(node.parent) && this.checker.getTypeAtLocation(node.parent) || this.checker.getTypeAtLocation(node);
4747

4848
ts.Debug.assert(type !== undefined, "type doesn't exist");
4949
var symbol = this.checker.getSymbolAtLocation(node);

Diff for: src/services/services.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -5812,7 +5812,7 @@ namespace ts {
58125812
}
58135813

58145814
return node.parent.kind === SyntaxKind.TypeReference ||
5815-
(node.parent.kind === SyntaxKind.ExpressionWithTypeArguments && !isClassExtendsExpressionWithTypeArguments(<ExpressionWithTypeArguments>node.parent));
5815+
(node.parent.kind === SyntaxKind.ExpressionWithTypeArguments && !isExpressionWithTypeArgumentsInClassExtendsClause(<ExpressionWithTypeArguments>node.parent));
58165816
}
58175817

58185818
function isNamespaceReference(node: Node): boolean {

0 commit comments

Comments
 (0)