From d762f55199ec652520b6a695f3957f9ed4da881b Mon Sep 17 00:00:00 2001 From: Nathan Shively-Sanders Date: Wed, 13 Sep 2017 09:23:57 -0700 Subject: [PATCH 1/3] Fix:Instantiate javascript constructor signatures getSignatureInstantation takes a parameter that tells whether the signature comes from Javascript and therefore is allowed to pass fewer than the required number of type arguments. (Defaults are chosen if this is the case.) Previously, getInstantiatedConstructorsForTypeArguments forgot to provide this argument, and constructors with insufficient type arguments would cause a crash because getSignatureInstantiation would not know to fill in the missing type arguments. --- src/compiler/checker.ts | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index c169dcb83ca..2375d52a6e9 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -4902,7 +4902,7 @@ namespace ts { function getInstantiatedConstructorsForTypeArguments(type: Type, typeArgumentNodes: ReadonlyArray, location: Node): Signature[] { const signatures = getConstructorsForTypeArguments(type, typeArgumentNodes, location); const typeArguments = map(typeArgumentNodes, getTypeFromTypeNode); - return sameMap(signatures, sig => some(sig.typeParameters) ? getSignatureInstantiation(sig, typeArguments) : sig); + return sameMap(signatures, sig => some(sig.typeParameters) ? getSignatureInstantiation(sig, typeArguments, isInJavaScriptFile(location)) : sig); } /** @@ -5498,7 +5498,7 @@ namespace ts { const minTypeArgumentCount = getMinTypeArgumentCount(baseSig.typeParameters); const typeParamCount = length(baseSig.typeParameters); if ((isJavaScript || typeArgCount >= minTypeArgumentCount) && typeArgCount <= typeParamCount) { - const sig = typeParamCount ? createSignatureInstantiation(baseSig, fillMissingTypeArguments(typeArguments, baseSig.typeParameters, minTypeArgumentCount, baseTypeNode)) : cloneSignature(baseSig); + const sig = typeParamCount ? createSignatureInstantiation(baseSig, fillMissingTypeArguments(typeArguments, baseSig.typeParameters, minTypeArgumentCount, isJavaScript)) : cloneSignature(baseSig); sig.typeParameters = classType.localTypeParameters; sig.resolvedReturnType = classType; result.push(sig); @@ -6361,11 +6361,10 @@ namespace ts { * @param typeParameters The requested type parameters. * @param minTypeArgumentCount The minimum number of required type arguments. */ - function fillMissingTypeArguments(typeArguments: Type[] | undefined, typeParameters: TypeParameter[] | undefined, minTypeArgumentCount: number, location?: Node) { + function fillMissingTypeArguments(typeArguments: Type[] | undefined, typeParameters: TypeParameter[] | undefined, minTypeArgumentCount: number, isJavaScript?: boolean) { const numTypeParameters = length(typeParameters); if (numTypeParameters) { const numTypeArguments = length(typeArguments); - const isJavaScript = isInJavaScriptFile(location); if ((isJavaScript || numTypeArguments >= minTypeArgumentCount) && numTypeArguments <= numTypeParameters) { if (!typeArguments) { typeArguments = []; @@ -6623,8 +6622,8 @@ namespace ts { return anyType; } - function getSignatureInstantiation(signature: Signature, typeArguments: Type[]): Signature { - typeArguments = fillMissingTypeArguments(typeArguments, signature.typeParameters, getMinTypeArgumentCount(signature.typeParameters)); + function getSignatureInstantiation(signature: Signature, typeArguments: Type[], isJavascript?: boolean): Signature { + typeArguments = fillMissingTypeArguments(typeArguments, signature.typeParameters, getMinTypeArgumentCount(signature.typeParameters), isJavascript); const instantiations = signature.instantiations || (signature.instantiations = createMap()); const id = getTypeListId(typeArguments); let instantiation = instantiations.get(id); @@ -6813,7 +6812,8 @@ namespace ts { if (typeParameters) { const numTypeArguments = length(node.typeArguments); const minTypeArgumentCount = getMinTypeArgumentCount(typeParameters); - if (!isInJavaScriptFile(node) && (numTypeArguments < minTypeArgumentCount || numTypeArguments > typeParameters.length)) { + const isJavascript = isInJavaScriptFile(node); + if (!isJavascript && (numTypeArguments < minTypeArgumentCount || numTypeArguments > typeParameters.length)) { error(node, minTypeArgumentCount === typeParameters.length ? Diagnostics.Generic_type_0_requires_1_type_argument_s @@ -6826,7 +6826,7 @@ namespace ts { // In a type reference, the outer type parameters of the referenced class or interface are automatically // supplied as type arguments and the type reference only specifies arguments for the local type parameters // of the class or interface. - const typeArguments = concatenate(type.outerTypeParameters, fillMissingTypeArguments(typeArgs, typeParameters, minTypeArgumentCount, node)); + const typeArguments = concatenate(type.outerTypeParameters, fillMissingTypeArguments(typeArgs, typeParameters, minTypeArgumentCount, isJavascript)); return createTypeReference(type, typeArguments); } if (node.typeArguments) { From 014f7ba8280d0cfa3936ebacb018c4b6d2407d91 Mon Sep 17 00:00:00 2001 From: Nathan Shively-Sanders Date: Wed, 13 Sep 2017 09:26:20 -0700 Subject: [PATCH 2/3] Test:javascript signature instantiation w/insufficient type args --- ...ssingTypeArgsOnJSConstructCalls.errors.txt | 48 +++++++++++++++++++ ...fillInMissingTypeArgsOnJSConstructCalls.ts | 27 +++++++++++ 2 files changed, 75 insertions(+) create mode 100644 tests/baselines/reference/fillInMissingTypeArgsOnJSConstructCalls.errors.txt create mode 100644 tests/cases/compiler/fillInMissingTypeArgsOnJSConstructCalls.ts diff --git a/tests/baselines/reference/fillInMissingTypeArgsOnJSConstructCalls.errors.txt b/tests/baselines/reference/fillInMissingTypeArgsOnJSConstructCalls.errors.txt new file mode 100644 index 00000000000..966e3e35613 --- /dev/null +++ b/tests/baselines/reference/fillInMissingTypeArgsOnJSConstructCalls.errors.txt @@ -0,0 +1,48 @@ +tests/cases/compiler/BaseB.js(2,24): error TS8004: 'type parameter declarations' can only be used in a .ts file. +tests/cases/compiler/BaseB.js(2,25): error TS1005: ',' expected. +tests/cases/compiler/BaseB.js(3,14): error TS2304: Cannot find name 'Class'. +tests/cases/compiler/BaseB.js(3,14): error TS8010: 'types' can only be used in a .ts file. +tests/cases/compiler/BaseB.js(4,25): error TS2304: Cannot find name 'Class'. +tests/cases/compiler/BaseB.js(4,25): error TS8010: 'types' can only be used in a .ts file. +tests/cases/compiler/SubB.js(3,41): error TS8011: 'type arguments' can only be used in a .ts file. + + +==== tests/cases/compiler/BaseA.js (0 errors) ==== + // regression test for #18254 + export default class BaseA { + } +==== tests/cases/compiler/SubA.js (0 errors) ==== + import BaseA from './BaseA'; + export default class SubA extends BaseA { + } +==== tests/cases/compiler/BaseB.js (6 errors) ==== + import BaseA from './BaseA'; + export default class B { + ~~~~~~~~ +!!! error TS8004: 'type parameter declarations' can only be used in a .ts file. + ~ +!!! error TS1005: ',' expected. + _AClass: Class; + ~~~~~ +!!! error TS2304: Cannot find name 'Class'. + ~~~~~~~~ +!!! error TS8010: 'types' can only be used in a .ts file. + constructor(AClass: Class) { + ~~~~~ +!!! error TS2304: Cannot find name 'Class'. + ~~~~~~~~ +!!! error TS8010: 'types' can only be used in a .ts file. + this._AClass = AClass; + } + } +==== tests/cases/compiler/SubB.js (1 errors) ==== + import SubA from './SubA'; + import BaseB from './BaseB'; + export default class SubB extends BaseB { + ~~~~ +!!! error TS8011: 'type arguments' can only be used in a .ts file. + constructor() { + super(SubA); + } + } + \ No newline at end of file diff --git a/tests/cases/compiler/fillInMissingTypeArgsOnJSConstructCalls.ts b/tests/cases/compiler/fillInMissingTypeArgsOnJSConstructCalls.ts new file mode 100644 index 00000000000..8ef0f903189 --- /dev/null +++ b/tests/cases/compiler/fillInMissingTypeArgsOnJSConstructCalls.ts @@ -0,0 +1,27 @@ +// @allowJs: true +// @checkJs: true +// @noEmit: true +// regression test for #18254 +// @Filename: BaseA.js +export default class BaseA { +} +// @Filename: SubA.js +import BaseA from './BaseA'; +export default class SubA extends BaseA { +} +// @Filename: BaseB.js +import BaseA from './BaseA'; +export default class B { + _AClass: Class; + constructor(AClass: Class) { + this._AClass = AClass; + } +} +// @Filename: SubB.js +import SubA from './SubA'; +import BaseB from './BaseB'; +export default class SubB extends BaseB { + constructor() { + super(SubA); + } +} From a1d1a2219b7d16a262af9eb52eaa242bc46ef657 Mon Sep 17 00:00:00 2001 From: Nathan Shively-Sanders Date: Wed, 13 Sep 2017 10:44:11 -0700 Subject: [PATCH 3/3] Make isJavascript parameters required This is a bit wordy, but will probably prevent bugs similar to #18254 in the future. --- src/compiler/checker.ts | 27 ++++++++++++++++----------- src/compiler/utilities.ts | 4 ++-- 2 files changed, 18 insertions(+), 13 deletions(-) diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index 2375d52a6e9..0ee3d549d48 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -6361,7 +6361,7 @@ namespace ts { * @param typeParameters The requested type parameters. * @param minTypeArgumentCount The minimum number of required type arguments. */ - function fillMissingTypeArguments(typeArguments: Type[] | undefined, typeParameters: TypeParameter[] | undefined, minTypeArgumentCount: number, isJavaScript?: boolean) { + function fillMissingTypeArguments(typeArguments: Type[] | undefined, typeParameters: TypeParameter[] | undefined, minTypeArgumentCount: number, isJavaScript: boolean) { const numTypeParameters = length(typeParameters); if (numTypeParameters) { const numTypeArguments = length(typeArguments); @@ -6622,7 +6622,7 @@ namespace ts { return anyType; } - function getSignatureInstantiation(signature: Signature, typeArguments: Type[], isJavascript?: boolean): Signature { + function getSignatureInstantiation(signature: Signature, typeArguments: Type[], isJavascript: boolean): Signature { typeArguments = fillMissingTypeArguments(typeArguments, signature.typeParameters, getMinTypeArgumentCount(signature.typeParameters), isJavascript); const instantiations = signature.instantiations || (signature.instantiations = createMap()); const id = getTypeListId(typeArguments); @@ -6661,7 +6661,10 @@ namespace ts { // where different generations of the same type parameter are in scope). This leads to a lot of new type // identities, and potentially a lot of work comparing those identities, so here we create an instantiation // that uses the original type identities for all unconstrained type parameters. - return getSignatureInstantiation(signature, map(signature.typeParameters, tp => tp.target && !getConstraintOfTypeParameter(tp.target) ? tp.target : tp)); + return getSignatureInstantiation( + signature, + map(signature.typeParameters, tp => tp.target && !getConstraintOfTypeParameter(tp.target) ? tp.target : tp), + isInJavaScriptFile(signature.declaration)); } function getOrCreateTypeFromSignature(signature: Signature): ObjectType { @@ -6843,7 +6846,7 @@ namespace ts { const id = getTypeListId(typeArguments); let instantiation = links.instantiations.get(id); if (!instantiation) { - links.instantiations.set(id, instantiation = instantiateType(type, createTypeMapper(typeParameters, fillMissingTypeArguments(typeArguments, typeParameters, getMinTypeArgumentCount(typeParameters))))); + links.instantiations.set(id, instantiation = instantiateType(type, createTypeMapper(typeParameters, fillMissingTypeArguments(typeArguments, typeParameters, getMinTypeArgumentCount(typeParameters), isInJavaScriptFile(symbol.valueDeclaration))))); } return instantiation; } @@ -13999,8 +14002,9 @@ namespace ts { const instantiatedSignatures = []; for (const signature of signatures) { if (signature.typeParameters) { - const typeArguments = fillMissingTypeArguments(/*typeArguments*/ undefined, signature.typeParameters, /*minTypeArgumentCount*/ 0); - instantiatedSignatures.push(getSignatureInstantiation(signature, typeArguments)); + const isJavascript = isInJavaScriptFile(node); + const typeArguments = fillMissingTypeArguments(/*typeArguments*/ undefined, signature.typeParameters, /*minTypeArgumentCount*/ 0, isJavascript); + instantiatedSignatures.push(getSignatureInstantiation(signature, typeArguments, isJavascript)); } else { instantiatedSignatures.push(signature); @@ -15257,7 +15261,7 @@ namespace ts { if (!contextualMapper) { inferTypes(context.inferences, getReturnTypeOfSignature(contextualSignature), getReturnTypeOfSignature(signature), InferencePriority.ReturnType); } - return getSignatureInstantiation(signature, getInferredTypes(context)); + return getSignatureInstantiation(signature, getInferredTypes(context), isInJavaScriptFile(contextualSignature.declaration)); } function inferTypeArguments(node: CallLikeExpression, signature: Signature, args: ReadonlyArray, excludeArgument: boolean[], context: InferenceContext): Type[] { @@ -15292,7 +15296,7 @@ namespace ts { // Above, the type of the 'value' parameter is inferred to be 'A'. const contextualSignature = getSingleCallSignature(instantiatedType); const inferenceSourceType = contextualSignature && contextualSignature.typeParameters ? - getOrCreateTypeFromSignature(getSignatureInstantiation(contextualSignature, contextualSignature.typeParameters)) : + getOrCreateTypeFromSignature(getSignatureInstantiation(contextualSignature, contextualSignature.typeParameters, isInJavaScriptFile(node))) : instantiatedType; const inferenceTargetType = getReturnTypeOfSignature(signature); // Inferences made from return types have lower priority than all other inferences. @@ -16008,8 +16012,9 @@ namespace ts { candidate = originalCandidate; if (candidate.typeParameters) { let typeArgumentTypes: Type[]; + const isJavascript = isInJavaScriptFile(candidate.declaration); if (typeArguments) { - typeArgumentTypes = fillMissingTypeArguments(map(typeArguments, getTypeFromTypeNode), candidate.typeParameters, getMinTypeArgumentCount(candidate.typeParameters)); + typeArgumentTypes = fillMissingTypeArguments(map(typeArguments, getTypeFromTypeNode), candidate.typeParameters, getMinTypeArgumentCount(candidate.typeParameters), isJavascript); if (!checkTypeArguments(candidate, typeArguments, typeArgumentTypes, /*reportErrors*/ false)) { candidateForTypeArgumentError = originalCandidate; break; @@ -16018,7 +16023,7 @@ namespace ts { else { typeArgumentTypes = inferTypeArguments(node, candidate, args, excludeArgument, inferenceContext); } - candidate = getSignatureInstantiation(candidate, typeArgumentTypes); + candidate = getSignatureInstantiation(candidate, typeArgumentTypes, isJavascript); } if (!checkApplicableSignature(node, args, candidate, relation, excludeArgument, /*reportErrors*/ false)) { candidateForArgumentError = candidate; @@ -18782,7 +18787,7 @@ namespace ts { const constraint = getConstraintOfTypeParameter(typeParameters[i]); if (constraint) { if (!typeArguments) { - typeArguments = fillMissingTypeArguments(map(typeArgumentNodes, getTypeFromTypeNode), typeParameters, minTypeArgumentCount); + typeArguments = fillMissingTypeArguments(map(typeArgumentNodes, getTypeFromTypeNode), typeParameters, minTypeArgumentCount, isInJavaScriptFile(typeArgumentNodes[i])); mapper = createTypeMapper(typeParameters, typeArguments); } const typeArgument = typeArguments[i]; diff --git a/src/compiler/utilities.ts b/src/compiler/utilities.ts index ee450344827..7eaf99c68e2 100644 --- a/src/compiler/utilities.ts +++ b/src/compiler/utilities.ts @@ -1325,11 +1325,11 @@ namespace ts { return isInJavaScriptFile(file); } - export function isInJavaScriptFile(node: Node): boolean { + export function isInJavaScriptFile(node: Node | undefined): boolean { return node && !!(node.flags & NodeFlags.JavaScriptFile); } - export function isInJSDoc(node: Node): boolean { + export function isInJSDoc(node: Node | undefined): boolean { return node && !!(node.flags & NodeFlags.JSDoc); }