From 91691f6079edaf77dce77df5e0c0d1176bc0a7b8 Mon Sep 17 00:00:00 2001 From: Anders Hejlsberg Date: Mon, 25 Sep 2017 16:59:18 -0700 Subject: [PATCH] Strict function type checking only for certain function types --- src/compiler/checker.ts | 120 +++++++++++++++++++++++++++++----------- src/compiler/types.ts | 2 + 2 files changed, 91 insertions(+), 31 deletions(-) diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index 25dc3030556..5d3957652d8 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -285,6 +285,7 @@ namespace ts { const markerSuperType = createType(TypeFlags.TypeParameter); const markerSubType = createType(TypeFlags.TypeParameter); markerSubType.constraint = markerSuperType; + const markerOtherType = createType(TypeFlags.TypeParameter); const anySignature = createSignature(undefined, undefined, undefined, emptyArray, anyType, /*typePredicate*/ undefined, 0, /*hasRestParameter*/ false, /*hasLiteralTypes*/ false); const unknownSignature = createSignature(undefined, undefined, undefined, emptyArray, unknownType, /*typePredicate*/ undefined, 0, /*hasRestParameter*/ false, /*hasLiteralTypes*/ false); @@ -3548,7 +3549,7 @@ namespace ts { return; } - if (resolved.callSignatures.length === 1 && !resolved.constructSignatures.length) { + if (resolved.callSignatures.length === 1 && !resolved.constructSignatures.length && isStrictSignature(resolved.callSignatures[0])) { const parenthesizeSignature = shouldAddParenthesisAroundFunctionType(resolved.callSignatures[0], flags); if (parenthesizeSignature) { writePunctuation(writer, SyntaxKind.OpenParenToken); @@ -3559,7 +3560,7 @@ namespace ts { } return; } - if (resolved.constructSignatures.length === 1 && !resolved.callSignatures.length) { + if (resolved.constructSignatures.length === 1 && !resolved.callSignatures.length && isStrictSignature(resolved.constructSignatures[0])) { if (flags & TypeFormatFlags.InElementType) { writePunctuation(writer, SyntaxKind.OpenParenToken); } @@ -8521,6 +8522,17 @@ namespace ts { /*errorReporter*/ undefined, compareTypesAssignable) !== Ternary.False; } + // A signature is considered strict if it is declared in a function type literal, a constructor type + // literal, a function expression, an arrow function, or a function declaration with no overloads. A + // strict signature is subject to strict checking in strictFunctionTypes mode. + function isStrictSignature(signature: Signature) { + const declaration = signature.declaration; + const kind = declaration ? declaration.kind : SyntaxKind.Unknown; + return kind === SyntaxKind.FunctionType || kind === SyntaxKind.ConstructorType || + kind === SyntaxKind.FunctionExpression || kind === SyntaxKind.ArrowFunction || + (kind === SyntaxKind.FunctionDeclaration && getSingleCallSignature(getTypeOfSymbol(getSymbolOfNode(declaration)))); + } + type ErrorReporter = (message: DiagnosticMessage, arg0?: string, arg1?: string) => void; /** @@ -8546,9 +8558,7 @@ namespace ts { source = instantiateSignatureInContextOf(source, target, /*contextualMapper*/ undefined, compareTypes); } - const targetKind = target.declaration ? target.declaration.kind : SyntaxKind.Unknown; - const strictVariance = strictFunctionTypes && targetKind !== SyntaxKind.MethodDeclaration && targetKind !== SyntaxKind.MethodSignature; - + const strictVariance = strictFunctionTypes && isStrictSignature(target); let result = Ternary.True; const sourceThisType = getThisTypeOfSignature(source); @@ -9215,15 +9225,41 @@ namespace ts { // in the process of computing variance information for recursive types and when // comparing 'this' type arguments. const variance = i < variances.length ? variances[i] : Variance.Covariant; - const s = sources[i]; - const t = targets[i]; - const related = variance === Variance.Covariant ? isRelatedTo(s, t, reportErrors) : - variance === Variance.Contravariant ? isRelatedTo(t, s, reportErrors) : - Ternary.False; - if (!related) { - return Ternary.False; + // We ignore arguments for independent type parameters (because they're never witnessed). + if (variance !== Variance.Independent) { + const s = sources[i]; + const t = targets[i]; + let related = Ternary.True; + if (variance === Variance.Covariant) { + related = isRelatedTo(s, t, reportErrors); + } + else if (variance === Variance.Contravariant) { + related = isRelatedTo(t, s, reportErrors); + } + else if (variance === Variance.Bivariant) { + // In the bivariant case we first compare contravariantly without reporting + // errors. Then, if that doesn't succeed, we compare covariantly with error + // reporting. Thus, error elaboration will be based on the the covariant check, + // which is generally easier to reason about. + related = isRelatedTo(t, s, /*reportErrors*/ false); + if (!related) { + related = isRelatedTo(s, t, reportErrors); + } + } + else { + // In the invariant case we first compare covariantly, and only when that + // succeeds do we proceed to compare contravariantly. Thus, error elaboration + // will typically be based on the covariant check. + related = isRelatedTo(s, t, reportErrors); + if (related) { + related &= isRelatedTo(t, s, reportErrors); + } + } + if (!related) { + return Ternary.False; + } + result &= related; } - result &= related; } return result; } @@ -9388,14 +9424,21 @@ namespace ts { !(source.flags & TypeFlags.MarkerType || target.flags & TypeFlags.MarkerType)) { // We have type references to the same generic type, and the type references are not marker // type references (which are intended by be compared structurally). Obtain the variance - // information for the type parameters and relate the type arguments accordingly. If we do - // not succeed, fall through and do a structural comparison instead (there are instances - // where the variance information isn't accurate, e.g. when type parameters are used only - // in bivariant positions or when a type argument is 'any' or 'void'.) + // information for the type parameters and relate the type arguments accordingly. const variances = getVariances((source).target); if (result = typeArgumentsRelatedTo(source, target, variances, reportErrors)) { return result; } + // The type arguments did not relate appropriately, but it may be because we have no variance + // information (in which case typeArgumentsRelatedTo defaulted to covariance for all type + // arguments). It might also be the case that the target type has a 'void' type argument for + // a covariant type parameter that is only used in return positions within the generic type + // (in which case any type argument is permitted on the source side). In those cases we proceed + // with a structural comparison. Otherwise, we know for certain the instantiations aren't + // related and we can return here. + if (variances !== emptyArray && !hasCovariantVoidArgument(target, variances)) { + return Ternary.False; + } } // Even if relationship doesn't hold for unions, intersections, or generic type references, // it may hold in a structural comparison. @@ -9814,14 +9857,12 @@ namespace ts { return result; } - // Return an array containing the variance of each type parameter. The variance information is - // computed by comparing instantiations of the generic type for type arguments with known relations. - // A type parameter is marked as covariant if a covariant comparison succeeds; otherwise, it is - // marked contravariant if a contravarint comparison succeeds; otherwise, it is marked invariant. - // One form of variance doesn't exclude another, so this information simply serves to indicate - // a "primary" relationship that can be checked as an optimization ahead of a full structural - // comparison. The function returns the emptyArray singleton if we're not in strictFunctionTypes - // mode or if the function has been invoked recursively for the given generic type. + // Return an array containing the variance of each type parameter. The variance is effectively + // a digest of the type comparisons that occur for each type argument when instantiations of the + // generic type are structurally compared. We infer the variance information by comparing + // instantiations of the generic type for type arguments with known relations. The function + // returns the emptyArray singleton if we're not in strictFunctionTypes mode or if the function + // has been invoked recursively for the given generic type. function getVariances(type: GenericType): Variance[] { if (!strictFunctionTypes) { return emptyArray; @@ -9838,14 +9879,20 @@ namespace ts { type.variances = emptyArray; variances = []; for (const tp of typeParameters) { - // We compare instantiations where the type parameter is replaced with marker types - // that have a known subtype relationship. From this we infer covariance, contravariance - // or invariance. + // We first compare instantiations where the type parameter is replaced with + // marker types that have a known subtype relationship. From this we can infer + // invariance, covariance, contravariance or bivariance. const typeWithSuper = getMarkerTypeReference(type, tp, markerSuperType); const typeWithSub = getMarkerTypeReference(type, tp, markerSubType); - const variance = isTypeAssignableTo(typeWithSub, typeWithSuper) ? Variance.Covariant : - isTypeAssignableTo(typeWithSuper, typeWithSub) ? Variance.Contravariant : - Variance.Invariant; + let variance = (isTypeAssignableTo(typeWithSub, typeWithSuper) ? Variance.Covariant : 0) | + (isTypeAssignableTo(typeWithSuper, typeWithSub) ? Variance.Contravariant : 0); + // If the instantiations appear to be related bivariantly it may be because the + // type parameter is independent (i.e. it isn't witnessed anywhere in the generic + // type). To determine this we compare instantiations where the type parameter is + // replaced with marker types that are known to be unrelated. + if (variance === Variance.Bivariant && isTypeAssignableTo(getMarkerTypeReference(type, tp, markerOtherType), typeWithSuper)) { + variance = Variance.Independent; + } variances.push(variance); } } @@ -9854,6 +9901,17 @@ namespace ts { return variances; } + // Return true if the given type reference has a 'void' type argument for a covariant type parameter. + // See comment at call in recursiveTypeRelatedTo for when this case matters. + function hasCovariantVoidArgument(type: TypeReference, variances: Variance[]): boolean { + for (let i = 0; i < variances.length; i++) { + if (variances[i] === Variance.Covariant && type.typeArguments[i].flags & TypeFlags.Void) { + return true; + } + } + return false; + } + function isUnconstrainedTypeParameter(type: Type) { return type.flags & TypeFlags.TypeParameter && !getConstraintFromTypeParameter(type); } diff --git a/src/compiler/types.ts b/src/compiler/types.ts index 8ab999032bc..a867d9ebe2a 100644 --- a/src/compiler/types.ts +++ b/src/compiler/types.ts @@ -3348,6 +3348,8 @@ namespace ts { Invariant = 0, // Neither covariant nor contravariant Covariant = 1, // Covariant Contravariant = 2, // Contravariant + Bivariant = 3, // Both covariant and contravariant + Independent = 4, // Unwitnessed type parameter } // Generic class and interface types