Skip to content

Commit 0f55566

Browse files
sandersnsandersn
authoredJun 20, 2018
In JS, always check the extends tag of a class before its heritage clause (#25111)
* Check extends tag first in getClassExtendsHeritageClauseElement Previously getBaseTypeNodeOfClass checked, but this is only used in a few places. * Fix names and add test * Update API baseline * Move jsdocAugments tests to conformance/jsdoc * Better naming in checkClassLikeDeclaration
·
v5.8.2v3.0-rc
1 parent 3eeb36b commit 0f55566

18 files changed

+60
-25
lines changed
 

‎src/compiler/checker.ts

+12-17
Original file line numberDiff line numberDiff line change
@@ -5392,16 +5392,7 @@ namespace ts {
53925392
}
53935393

53945394
function getBaseTypeNodeOfClass(type: InterfaceType): ExpressionWithTypeArguments | undefined {
5395-
const decl = <ClassLikeDeclaration>type.symbol.valueDeclaration;
5396-
if (isInJavaScriptFile(decl)) {
5397-
// Prefer an @augments tag because it may have type parameters.
5398-
const tag = getJSDocAugmentsTag(decl);
5399-
if (tag) {
5400-
return tag.class;
5401-
}
5402-
}
5403-
5404-
return getClassExtendsHeritageClauseElement(decl);
5395+
return getEffectiveBaseTypeNode(type.symbol.valueDeclaration as ClassLikeDeclaration);
54055396
}
54065397

54075398
function getConstructorsForTypeArguments(type: Type, typeArgumentNodes: ReadonlyArray<TypeNode> | undefined, location: Node): Signature[] {
@@ -5428,7 +5419,7 @@ namespace ts {
54285419
function getBaseConstructorTypeOfClass(type: InterfaceType): Type {
54295420
if (!type.resolvedBaseConstructorType) {
54305421
const decl = <ClassLikeDeclaration>type.symbol.valueDeclaration;
5431-
const extended = getClassExtendsHeritageClauseElement(decl);
5422+
const extended = getEffectiveBaseTypeNode(decl);
54325423
const baseTypeNode = getBaseTypeNodeOfClass(type);
54335424
if (!baseTypeNode) {
54345425
return type.resolvedBaseConstructorType = undefinedType;
@@ -14764,7 +14755,7 @@ namespace ts {
1476414755

1476514756
function checkThisBeforeSuper(node: Node, container: Node, diagnosticMessage: DiagnosticMessage) {
1476614757
const containingClassDecl = <ClassDeclaration>container.parent;
14767-
const baseTypeNode = getClassExtendsHeritageClauseElement(containingClassDecl);
14758+
const baseTypeNode = getEffectiveBaseTypeNode(containingClassDecl);
1476814759

1476914760
// If a containing class does not have extends clause or the class extends null
1477014761
// skip checking whether super statement is called before "this" accessing.
@@ -15040,7 +15031,7 @@ namespace ts {
1504015031

1504115032
// at this point the only legal case for parent is ClassLikeDeclaration
1504215033
const classLikeDeclaration = <ClassLikeDeclaration>container.parent;
15043-
if (!getClassExtendsHeritageClauseElement(classLikeDeclaration)) {
15034+
if (!getEffectiveBaseTypeNode(classLikeDeclaration)) {
1504415035
error(node, Diagnostics.super_can_only_be_referenced_in_a_derived_class);
1504515036
return errorType;
1504615037
}
@@ -18678,7 +18669,7 @@ namespace ts {
1867818669
if (superType !== errorType) {
1867918670
// In super call, the candidate signatures are the matching arity signatures of the base constructor function instantiated
1868018671
// with the type arguments specified in the extends clause.
18681-
const baseTypeNode = getClassExtendsHeritageClauseElement(getContainingClass(node)!);
18672+
const baseTypeNode = getEffectiveBaseTypeNode(getContainingClass(node)!);
1868218673
if (baseTypeNode) {
1868318674
const baseConstructors = getInstantiatedConstructorsForTypeArguments(superType, baseTypeNode.typeArguments, baseTypeNode);
1868418675
return resolveCall(node, baseConstructors, candidatesOutArray);
@@ -21474,7 +21465,7 @@ namespace ts {
2147421465
// Constructors of classes with no extends clause may not contain super calls, whereas
2147521466
// constructors of derived classes must contain at least one super call somewhere in their function body.
2147621467
const containingClassDecl = <ClassDeclaration>node.parent;
21477-
if (getClassExtendsHeritageClauseElement(containingClassDecl)) {
21468+
if (getEffectiveBaseTypeNode(containingClassDecl)) {
2147821469
captureLexicalThis(node.parent, containingClassDecl);
2147921470
const classExtendsNull = classDeclarationExtendsNull(containingClassDecl);
2148021471
const superCall = getSuperCallInConstructor(node);
@@ -22651,7 +22642,7 @@ namespace ts {
2265122642
}
2265222643

2265322644
const name = getIdentifierFromEntityNameExpression(node.class.expression);
22654-
const extend = getClassExtendsHeritageClauseElement(classLike);
22645+
const extend = getClassExtendsHeritageElement(classLike);
2265522646
if (extend) {
2265622647
const className = getIdentifierFromEntityNameExpression(extend.expression);
2265722648
if (className && name.escapedText !== className.escapedText) {
@@ -24426,7 +24417,7 @@ namespace ts {
2442624417
checkClassForStaticPropertyNameConflicts(node);
2442724418
}
2442824419

24429-
const baseTypeNode = getClassExtendsHeritageClauseElement(node);
24420+
const baseTypeNode = getEffectiveBaseTypeNode(node);
2443024421
if (baseTypeNode) {
2443124422
if (languageVersion < ScriptTarget.ES2015) {
2443224423
checkExternalEmitHelpers(baseTypeNode.parent, ExternalEmitHelpers.Extends);
@@ -24439,6 +24430,10 @@ namespace ts {
2443924430
const staticBaseType = getApparentType(baseConstructorType);
2444024431
checkBaseTypeAccessibility(staticBaseType, baseTypeNode);
2444124432
checkSourceElement(baseTypeNode.expression);
24433+
const extendsNode = getClassExtendsHeritageElement(node);
24434+
if (extendsNode && extendsNode !== baseTypeNode) {
24435+
checkExpression(extendsNode.expression);
24436+
}
2444224437
if (some(baseTypeNode.typeArguments)) {
2444324438
forEach(baseTypeNode.typeArguments, checkSourceElement);
2444424439
for (const constructor of getConstructorsForTypeArguments(staticBaseType, baseTypeNode.typeArguments, baseTypeNode)) {

‎src/compiler/transformers/declarations.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -1022,7 +1022,7 @@ namespace ts {
10221022
}
10231023
const members = createNodeArray(concatenate(parameterProperties, visitNodes(input.members, visitDeclarationSubtree)));
10241024

1025-
const extendsClause = getClassExtendsHeritageClauseElement(input);
1025+
const extendsClause = getEffectiveBaseTypeNode(input);
10261026
if (extendsClause && !isEntityNameExpression(extendsClause.expression) && extendsClause.expression.kind !== SyntaxKind.NullKeyword) {
10271027
// We must add a temporary declaration for the extends clause expression
10281028

‎src/compiler/transformers/es2015.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -770,7 +770,7 @@ namespace ts {
770770
enableSubstitutionsForBlockScopedBindings();
771771
}
772772

773-
const extendsClauseElement = getClassExtendsHeritageClauseElement(node);
773+
const extendsClauseElement = getEffectiveBaseTypeNode(node);
774774
const classFunction = createFunctionExpression(
775775
/*modifiers*/ undefined,
776776
/*asteriskToken*/ undefined,

‎src/compiler/transformers/ts.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -586,7 +586,7 @@ namespace ts {
586586
function getClassFacts(node: ClassDeclaration, staticProperties: ReadonlyArray<PropertyDeclaration>) {
587587
let facts = ClassFacts.None;
588588
if (some(staticProperties)) facts |= ClassFacts.HasStaticInitializedProperties;
589-
const extendsClauseElement = getClassExtendsHeritageClauseElement(node);
589+
const extendsClauseElement = getEffectiveBaseTypeNode(node);
590590
if (extendsClauseElement && skipOuterExpressions(extendsClauseElement.expression).kind !== SyntaxKind.NullKeyword) facts |= ClassFacts.IsDerivedClass;
591591
if (shouldEmitDecorateCallForClass(node)) facts |= ClassFacts.HasConstructorDecorators;
592592
if (childIsDecorated(node)) facts |= ClassFacts.HasMemberDecorators;

‎src/compiler/utilities.ts

+13-2
Original file line numberDiff line numberDiff line change
@@ -2413,7 +2413,18 @@ namespace ts {
24132413
return isEntityNameExpression(e) || isClassExpression(e);
24142414
}
24152415

2416-
export function getClassExtendsHeritageClauseElement(node: ClassLikeDeclaration | InterfaceDeclaration) {
2416+
export function getEffectiveBaseTypeNode(node: ClassLikeDeclaration | InterfaceDeclaration) {
2417+
if (isInJavaScriptFile(node)) {
2418+
// Prefer an @augments tag because it may have type parameters.
2419+
const tag = getJSDocAugmentsTag(node);
2420+
if (tag) {
2421+
return tag.class;
2422+
}
2423+
}
2424+
return getClassExtendsHeritageElement(node);
2425+
}
2426+
2427+
export function getClassExtendsHeritageElement(node: ClassLikeDeclaration | InterfaceDeclaration) {
24172428
const heritageClause = getHeritageClause(node.heritageClauses, SyntaxKind.ExtendsKeyword);
24182429
return heritageClause && heritageClause.types.length > 0 ? heritageClause.types[0] : undefined;
24192430
}
@@ -2426,7 +2437,7 @@ namespace ts {
24262437
/** Returns the node in an `extends` or `implements` clause of a class or interface. */
24272438
export function getAllSuperTypeNodes(node: Node): ReadonlyArray<TypeNode> {
24282439
return isInterfaceDeclaration(node) ? getInterfaceBaseTypeNodes(node) || emptyArray
2429-
: isClassLike(node) ? concatenate(singleElementArray(getClassExtendsHeritageClauseElement(node)), getClassImplementsHeritageClauseElements(node)) || emptyArray
2440+
: isClassLike(node) ? concatenate(singleElementArray(getEffectiveBaseTypeNode(node)), getClassImplementsHeritageClauseElements(node)) || emptyArray
24302441
: emptyArray;
24312442
}
24322443

‎src/services/codefixes/fixClassDoesntImplementInheritedAbstractMember.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ namespace ts.codefix {
3333
}
3434

3535
function addMissingMembers(classDeclaration: ClassLikeDeclaration, sourceFile: SourceFile, checker: TypeChecker, changeTracker: textChanges.ChangeTracker, preferences: UserPreferences): void {
36-
const extendsNode = getClassExtendsHeritageClauseElement(classDeclaration)!;
36+
const extendsNode = getEffectiveBaseTypeNode(classDeclaration)!;
3737
const instantiatedExtendsType = checker.getTypeAtLocation(extendsNode)!;
3838

3939
// Note that this is ultimately derived from a map indexed by symbol names,

‎src/services/codefixes/fixClassIncorrectlyImplementsInterface.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,7 @@ namespace ts.codefix {
7171
}
7272

7373
function getHeritageClauseSymbolTable (classDeclaration: ClassLikeDeclaration, checker: TypeChecker): SymbolTable {
74-
const heritageClauseNode = getClassExtendsHeritageClauseElement(classDeclaration);
74+
const heritageClauseNode = getEffectiveBaseTypeNode(classDeclaration);
7575
if (!heritageClauseNode) return createSymbolTable();
7676
const heritageClauseType = checker.getTypeAtLocation(heritageClauseNode) as InterfaceType;
7777
const heritageClauseTypeSymbols = checker.getPropertiesOfType(heritageClauseType);

‎tests/baselines/reference/api/tsserverlibrary.d.ts

+2-1
Original file line numberDiff line numberDiff line change
@@ -6293,7 +6293,8 @@ declare namespace ts {
62936293
function isIdentifierName(node: Identifier): boolean;
62946294
function isAliasSymbolDeclaration(node: Node): boolean;
62956295
function exportAssignmentIsAlias(node: ExportAssignment | BinaryExpression): boolean;
6296-
function getClassExtendsHeritageClauseElement(node: ClassLikeDeclaration | InterfaceDeclaration): ExpressionWithTypeArguments | undefined;
6296+
function getEffectiveBaseTypeNode(node: ClassLikeDeclaration | InterfaceDeclaration): ExpressionWithTypeArguments | undefined;
6297+
function getClassExtendsHeritageElement(node: ClassLikeDeclaration | InterfaceDeclaration): ExpressionWithTypeArguments | undefined;
62976298
function getClassImplementsHeritageClauseElements(node: ClassLikeDeclaration): NodeArray<ExpressionWithTypeArguments> | undefined;
62986299
/** Returns the node in an `extends` or `implements` clause of a class or interface. */
62996300
function getAllSuperTypeNodes(node: Node): ReadonlyArray<TypeNode>;

‎tests/baselines/reference/extendsTag1.symbols

+9
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
=== tests/cases/conformance/jsdoc/bug25101.js ===
2+
/**
3+
* @template T
4+
* @extends {Set<T>} Should prefer this Set<T>, not the Set in the heritage clause
5+
*/
6+
class My extends Set {}
7+
>My : Symbol(My, Decl(bug25101.js, 0, 0))
8+
>Set : Symbol(Set, Decl(lib.es2015.collection.d.ts, --, --), Decl(lib.es2015.collection.d.ts, --, --), Decl(lib.es2015.iterable.d.ts, --, --), Decl(lib.es2015.symbol.wellknown.d.ts, --, --))
9+

‎tests/baselines/reference/extendsTag1.types

+9
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
=== tests/cases/conformance/jsdoc/bug25101.js ===
2+
/**
3+
* @template T
4+
* @extends {Set<T>} Should prefer this Set<T>, not the Set in the heritage clause
5+
*/
6+
class My extends Set {}
7+
>My : My<T>
8+
>Set : Set<T>
9+

‎tests/cases/conformance/jsdoc/extendsTag1.ts

+10
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
// @noEmit: true
2+
// @allowJs: true
3+
// @checkJs: true
4+
// @lib: esnext
5+
// @Filename: bug25101.js
6+
/**
7+
* @template T
8+
* @extends {Set<T>} Should prefer this Set<T>, not the Set in the heritage clause
9+
*/
10+
class My extends Set {}

0 commit comments

Comments
 (0)
Please sign in to comment.