From d52812fadd73e7f537ee998638dd31d7402e9911 Mon Sep 17 00:00:00 2001 From: Andy Hanson Date: Thu, 5 Oct 2017 15:38:25 -0700 Subject: [PATCH] Fix bugs for unions of type predicates --- src/compiler/checker.ts | 121 +++++++++++++----- .../reference/typePredicatesInUnion.js | 23 ++++ .../reference/typePredicatesInUnion.symbols | 40 ++++++ .../reference/typePredicatesInUnion.types | 41 ++++++ .../typePredicatesInUnion_noMatch.js | 25 ++++ .../typePredicatesInUnion_noMatch.symbols | 47 +++++++ .../typePredicatesInUnion_noMatch.types | 48 +++++++ tests/cases/compiler/typePredicatesInUnion.ts | 14 ++ .../compiler/typePredicatesInUnion_noMatch.ts | 15 +++ 9 files changed, 345 insertions(+), 29 deletions(-) create mode 100644 tests/baselines/reference/typePredicatesInUnion.js create mode 100644 tests/baselines/reference/typePredicatesInUnion.symbols create mode 100644 tests/baselines/reference/typePredicatesInUnion.types create mode 100644 tests/baselines/reference/typePredicatesInUnion_noMatch.js create mode 100644 tests/baselines/reference/typePredicatesInUnion_noMatch.symbols create mode 100644 tests/baselines/reference/typePredicatesInUnion_noMatch.types create mode 100644 tests/cases/compiler/typePredicatesInUnion.ts create mode 100644 tests/cases/compiler/typePredicatesInUnion_noMatch.ts diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index 131c6224b1495..dfe741e31daa4 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -298,12 +298,12 @@ namespace ts { markerSubType.constraint = markerSuperType; const markerOtherType = createType(TypeFlags.TypeParameter); - const noTypePredicate: IdentifierTypePredicate = { kind: TypePredicateKind.Identifier, parameterName: "<>", parameterIndex: 0, type: anyType }; + const noTypePredicate = createIdentifierTypePredicate("<>", 0, anyType); - 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); - const resolvingSignature = createSignature(undefined, undefined, undefined, emptyArray, anyType, /*typePredicate*/ undefined, 0, /*hasRestParameter*/ false, /*hasLiteralTypes*/ false); - const silentNeverSignature = createSignature(undefined, undefined, undefined, emptyArray, silentNeverType, /*typePredicate*/ undefined, 0, /*hasRestParameter*/ false, /*hasLiteralTypes*/ false); + const anySignature = createSignature(undefined, undefined, undefined, emptyArray, anyType, noTypePredicate, 0, /*hasRestParameter*/ false, /*hasLiteralTypes*/ false); + const unknownSignature = createSignature(undefined, undefined, undefined, emptyArray, unknownType, noTypePredicate, 0, /*hasRestParameter*/ false, /*hasLiteralTypes*/ false); + const resolvingSignature = createSignature(undefined, undefined, undefined, emptyArray, anyType, noTypePredicate, 0, /*hasRestParameter*/ false, /*hasLiteralTypes*/ false); + const silentNeverSignature = createSignature(undefined, undefined, undefined, emptyArray, silentNeverType, noTypePredicate, 0, /*hasRestParameter*/ false, /*hasLiteralTypes*/ false); const enumNumberIndexInfo = createIndexInfo(stringType, /*isReadonly*/ true); const jsObjectLiteralIndexInfo = createIndexInfo(anyType, /*isReadonly*/ false); @@ -5490,8 +5490,17 @@ namespace ts { resolveObjectTypeMembers(type, source, typeParameters, typeArguments); } - function createSignature(declaration: SignatureDeclaration, typeParameters: TypeParameter[], thisParameter: Symbol | undefined, parameters: Symbol[], - resolvedReturnType: Type | undefined, resolvedTypePredicate: TypePredicate | undefined, minArgumentCount: number, hasRestParameter: boolean, hasLiteralTypes: boolean): Signature { + function createSignature( + declaration: SignatureDeclaration, + typeParameters: TypeParameter[], + thisParameter: Symbol | undefined, + parameters: Symbol[], + resolvedReturnType: Type | undefined, + resolvedTypePredicate: TypePredicate | undefined, + minArgumentCount: number, + hasRestParameter: boolean, + hasLiteralTypes: boolean, + ): Signature { const sig = new Signature(checker); sig.declaration = declaration; sig.typeParameters = typeParameters; @@ -5506,8 +5515,8 @@ namespace ts { } function cloneSignature(sig: Signature): Signature { - return createSignature(sig.declaration, sig.typeParameters, sig.thisParameter, sig.parameters, sig.resolvedReturnType, - sig.resolvedTypePredicate, sig.minArgumentCount, sig.hasRestParameter, sig.hasLiteralTypes); + return createSignature(sig.declaration, sig.typeParameters, sig.thisParameter, sig.parameters, /*resolvedReturnType*/ undefined, + /*resolvedTypePredicate*/ undefined, sig.minArgumentCount, sig.hasRestParameter, sig.hasLiteralTypes); } function getDefaultConstructSignatures(classType: InterfaceType): Signature[] { @@ -5525,9 +5534,12 @@ 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, isJavaScript)) : cloneSignature(baseSig); + const sig = typeParamCount + ? createSignatureInstantiation(baseSig, fillMissingTypeArguments(typeArguments, baseSig.typeParameters, minTypeArgumentCount, isJavaScript)) + : cloneSignature(baseSig); sig.typeParameters = classType.localTypeParameters; sig.resolvedReturnType = classType; + sig.resolvedTypePredicate = noTypePredicate; result.push(sig); } } @@ -5589,8 +5601,6 @@ namespace ts { const thisType = getUnionType(map(unionSignatures, sig => getTypeOfSymbol(sig.thisParameter) || anyType), /*subtypeReduction*/ true); s.thisParameter = createSymbolWithType(signature.thisParameter, thisType); } - // Clear resolved return type we possibly got from cloneSignature - s.resolvedReturnType = undefined; s.unionSignatures = unionSignatures; } (result || (result = [])).push(s); @@ -5674,6 +5684,7 @@ namespace ts { signatures = map(signatures, s => { const clone = cloneSignature(s); clone.resolvedReturnType = includeMixinType(getReturnTypeOfSignature(s), types, i); + clone.resolvedTypePredicate = noTypePredicate; // TODO: GH#17757 return clone; }); } @@ -6103,14 +6114,13 @@ namespace ts { function createUnionOrIntersectionProperty(containingType: UnionOrIntersectionType, name: __String): Symbol { let props: Symbol[]; - const types = containingType.types; const isUnion = containingType.flags & TypeFlags.Union; const excludeModifiers = isUnion ? ModifierFlags.NonPublicAccessibilityModifier : 0; // Flags we want to propagate to the result if they exist in all source symbols let commonFlags = isUnion ? SymbolFlags.None : SymbolFlags.Optional; let syntheticFlag = CheckFlags.SyntheticMethod; let checkFlags = 0; - for (const current of types) { + for (const current of containingType.types) { const type = getApparentType(current); if (type !== unknownType) { const prop = getPropertyOfType(type, name); @@ -6344,22 +6354,26 @@ namespace ts { function createTypePredicateFromTypePredicateNode(node: TypePredicateNode): IdentifierTypePredicate | ThisTypePredicate { const { parameterName } = node; + const type = getTypeFromTypeNode(node.type); if (parameterName.kind === SyntaxKind.Identifier) { - return { - kind: TypePredicateKind.Identifier, - parameterName: parameterName ? parameterName.escapedText : undefined, - parameterIndex: parameterName ? getTypePredicateParameterIndex((node.parent as SignatureDeclaration).parameters, parameterName) : undefined, - type: getTypeFromTypeNode(node.type) - } as IdentifierTypePredicate; + return createIdentifierTypePredicate( + parameterName && parameterName.escapedText as string, // TODO: GH#18217 + parameterName && getTypePredicateParameterIndex((node.parent as SignatureDeclaration).parameters, parameterName), + type); } else { - return { - kind: TypePredicateKind.This, - type: getTypeFromTypeNode(node.type) - }; + return createThisTypePredicate(type); } } + function createIdentifierTypePredicate(parameterName: string | undefined, parameterIndex: number | undefined, type: Type): IdentifierTypePredicate { + return { kind: TypePredicateKind.Identifier, parameterName, parameterIndex, type }; + } + + function createThisTypePredicate(type: Type): ThisTypePredicate { + return { kind: TypePredicateKind.This, type }; + } + /** * Gets the minimum number of type arguments needed to satisfy all non-optional type * parameters. @@ -6604,8 +6618,14 @@ namespace ts { function getTypePredicateOfSignature(signature: Signature): TypePredicate | undefined { if (!signature.resolvedTypePredicate) { - const targetTypePredicate = getTypePredicateOfSignature(signature.target); - signature.resolvedTypePredicate = targetTypePredicate ? instantiateTypePredicate(targetTypePredicate, signature.mapper) : noTypePredicate; + if (signature.target) { + const targetTypePredicate = getTypePredicateOfSignature(signature.target); + signature.resolvedTypePredicate = targetTypePredicate ? instantiateTypePredicate(targetTypePredicate, signature.mapper) : noTypePredicate; + } + else { + Debug.assert(!!signature.unionSignatures); + signature.resolvedTypePredicate = getUnionTypePredicate(signature.unionSignatures); + } } return signature.resolvedTypePredicate === noTypePredicate ? undefined : signature.resolvedTypePredicate; } @@ -7480,6 +7500,42 @@ namespace ts { return getUnionTypeFromSortedList(typeSet, aliasSymbol, aliasTypeArguments); } + function getUnionTypePredicate(signatures: ReadonlyArray): TypePredicate | undefined { + let first: TypePredicate | undefined; + const types: Type[] = []; + for (let i = 0; i < signatures.length; i++) { + const pred = getTypePredicateOfSignature(signatures[i]); + if (!pred) { + continue; + } + + if (first) { + if (!typePredicateKindsMatch(first, pred)) { + // No common type predicate. + return undefined; + } + } + else { + first = pred; + } + types.push(pred.type); + } + if (!first) { + // No union signatures had a type predicate. + return undefined; + } + const unionType = getUnionType(types); + return isIdentifierTypePredicate(first) + ? createIdentifierTypePredicate(first.parameterName, first.parameterIndex, unionType) + : createThisTypePredicate(unionType); + } + + function typePredicateKindsMatch(a: TypePredicate, b: TypePredicate): boolean { + return isIdentifierTypePredicate(a) + ? isIdentifierTypePredicate(b) && a.parameterIndex === b.parameterIndex + : !isIdentifierTypePredicate(b); + } + // This function assumes the constituent type list is sorted and deduplicated. function getUnionTypeFromSortedList(types: Type[], aliasSymbol?: Symbol, aliasTypeArguments?: Type[]): Type { if (types.length === 0) { @@ -10167,11 +10223,20 @@ namespace ts { result &= related; } if (!ignoreReturnTypes) { - result &= compareTypes(getReturnTypeOfSignature(source), getReturnTypeOfSignature(target)); + const sourceTypePredicate = getTypePredicateOfSignature(source); + const targetTypePredicate = getTypePredicateOfSignature(target); + result &= sourceTypePredicate !== undefined || targetTypePredicate !== undefined + ? compareTypePredicatesIdentical(sourceTypePredicate, targetTypePredicate, compareTypes) + // If they're both type predicates their return types will both be `boolean`, so no need to compare those. + : compareTypes(getReturnTypeOfSignature(source), getReturnTypeOfSignature(target)); } return result; } + function compareTypePredicatesIdentical(source: TypePredicate | undefined, target: TypePredicate | undefined, compareTypes: (s: Type, t: Type) => Ternary): Ternary { + return source === undefined || target === undefined || !typePredicateKindsMatch(source, target) ? Ternary.False : compareTypes(source.type, target.type); + } + function isRestParameterIndex(signature: Signature, parameterIndex: number) { return signature.hasRestParameter && parameterIndex >= signature.parameters.length - 1; } @@ -13612,8 +13677,6 @@ namespace ts { let result: Signature; if (signatureList) { result = cloneSignature(signatureList[0]); - // Clear resolved return type we possibly got from cloneSignature - result.resolvedReturnType = undefined; result.unionSignatures = signatureList; } return result; diff --git a/tests/baselines/reference/typePredicatesInUnion.js b/tests/baselines/reference/typePredicatesInUnion.js new file mode 100644 index 0000000000000..4527455af60cf --- /dev/null +++ b/tests/baselines/reference/typePredicatesInUnion.js @@ -0,0 +1,23 @@ +//// [typePredicatesInUnion.ts] +interface A { + pred(x: {}): x is boolean; +} +interface B { + pred(x: {}): x is string; +} + +type Or = A | B; + +function f(o: Or, x: {}) { + if (o.pred(x)) { + x; + } +} + + +//// [typePredicatesInUnion.js] +function f(o, x) { + if (o.pred(x)) { + x; + } +} diff --git a/tests/baselines/reference/typePredicatesInUnion.symbols b/tests/baselines/reference/typePredicatesInUnion.symbols new file mode 100644 index 0000000000000..eb149af9e0c39 --- /dev/null +++ b/tests/baselines/reference/typePredicatesInUnion.symbols @@ -0,0 +1,40 @@ +=== tests/cases/compiler/typePredicatesInUnion.ts === +interface A { +>A : Symbol(A, Decl(typePredicatesInUnion.ts, 0, 0)) + + pred(x: {}): x is boolean; +>pred : Symbol(A.pred, Decl(typePredicatesInUnion.ts, 0, 13)) +>x : Symbol(x, Decl(typePredicatesInUnion.ts, 1, 9)) +>x : Symbol(x, Decl(typePredicatesInUnion.ts, 1, 9)) +} +interface B { +>B : Symbol(B, Decl(typePredicatesInUnion.ts, 2, 1)) + + pred(x: {}): x is string; +>pred : Symbol(B.pred, Decl(typePredicatesInUnion.ts, 3, 13)) +>x : Symbol(x, Decl(typePredicatesInUnion.ts, 4, 9)) +>x : Symbol(x, Decl(typePredicatesInUnion.ts, 4, 9)) +} + +type Or = A | B; +>Or : Symbol(Or, Decl(typePredicatesInUnion.ts, 5, 1)) +>A : Symbol(A, Decl(typePredicatesInUnion.ts, 0, 0)) +>B : Symbol(B, Decl(typePredicatesInUnion.ts, 2, 1)) + +function f(o: Or, x: {}) { +>f : Symbol(f, Decl(typePredicatesInUnion.ts, 7, 16)) +>o : Symbol(o, Decl(typePredicatesInUnion.ts, 9, 11)) +>Or : Symbol(Or, Decl(typePredicatesInUnion.ts, 5, 1)) +>x : Symbol(x, Decl(typePredicatesInUnion.ts, 9, 17)) + + if (o.pred(x)) { +>o.pred : Symbol(pred, Decl(typePredicatesInUnion.ts, 0, 13), Decl(typePredicatesInUnion.ts, 3, 13)) +>o : Symbol(o, Decl(typePredicatesInUnion.ts, 9, 11)) +>pred : Symbol(pred, Decl(typePredicatesInUnion.ts, 0, 13), Decl(typePredicatesInUnion.ts, 3, 13)) +>x : Symbol(x, Decl(typePredicatesInUnion.ts, 9, 17)) + + x; +>x : Symbol(x, Decl(typePredicatesInUnion.ts, 9, 17)) + } +} + diff --git a/tests/baselines/reference/typePredicatesInUnion.types b/tests/baselines/reference/typePredicatesInUnion.types new file mode 100644 index 0000000000000..fee8fea68d0ce --- /dev/null +++ b/tests/baselines/reference/typePredicatesInUnion.types @@ -0,0 +1,41 @@ +=== tests/cases/compiler/typePredicatesInUnion.ts === +interface A { +>A : A + + pred(x: {}): x is boolean; +>pred : (x: {}) => x is boolean +>x : {} +>x : any +} +interface B { +>B : B + + pred(x: {}): x is string; +>pred : (x: {}) => x is string +>x : {} +>x : any +} + +type Or = A | B; +>Or : Or +>A : A +>B : B + +function f(o: Or, x: {}) { +>f : (o: Or, x: {}) => void +>o : Or +>Or : Or +>x : {} + + if (o.pred(x)) { +>o.pred(x) : boolean +>o.pred : ((x: {}) => x is boolean) | ((x: {}) => x is string) +>o : Or +>pred : ((x: {}) => x is boolean) | ((x: {}) => x is string) +>x : {} + + x; +>x : string | boolean + } +} + diff --git a/tests/baselines/reference/typePredicatesInUnion_noMatch.js b/tests/baselines/reference/typePredicatesInUnion_noMatch.js new file mode 100644 index 0000000000000..c0e3ed68fadaf --- /dev/null +++ b/tests/baselines/reference/typePredicatesInUnion_noMatch.js @@ -0,0 +1,25 @@ +//// [typePredicatesInUnion_noMatch.ts] +interface A { + pred(x: {}, y: {}): x is boolean; +} +interface B { + pred(x: {}, y: {}): y is string; +} + +type Or = A | B; + +function f(o: Or, x: {}, y: {}) { + if (o.pred(x, y)) { + x; + y; + } +} + + +//// [typePredicatesInUnion_noMatch.js] +function f(o, x, y) { + if (o.pred(x, y)) { + x; + y; + } +} diff --git a/tests/baselines/reference/typePredicatesInUnion_noMatch.symbols b/tests/baselines/reference/typePredicatesInUnion_noMatch.symbols new file mode 100644 index 0000000000000..3fe467a9cec1e --- /dev/null +++ b/tests/baselines/reference/typePredicatesInUnion_noMatch.symbols @@ -0,0 +1,47 @@ +=== tests/cases/compiler/typePredicatesInUnion_noMatch.ts === +interface A { +>A : Symbol(A, Decl(typePredicatesInUnion_noMatch.ts, 0, 0)) + + pred(x: {}, y: {}): x is boolean; +>pred : Symbol(A.pred, Decl(typePredicatesInUnion_noMatch.ts, 0, 13)) +>x : Symbol(x, Decl(typePredicatesInUnion_noMatch.ts, 1, 9)) +>y : Symbol(y, Decl(typePredicatesInUnion_noMatch.ts, 1, 15)) +>x : Symbol(x, Decl(typePredicatesInUnion_noMatch.ts, 1, 9)) +} +interface B { +>B : Symbol(B, Decl(typePredicatesInUnion_noMatch.ts, 2, 1)) + + pred(x: {}, y: {}): y is string; +>pred : Symbol(B.pred, Decl(typePredicatesInUnion_noMatch.ts, 3, 13)) +>x : Symbol(x, Decl(typePredicatesInUnion_noMatch.ts, 4, 9)) +>y : Symbol(y, Decl(typePredicatesInUnion_noMatch.ts, 4, 15)) +>y : Symbol(y, Decl(typePredicatesInUnion_noMatch.ts, 4, 15)) +} + +type Or = A | B; +>Or : Symbol(Or, Decl(typePredicatesInUnion_noMatch.ts, 5, 1)) +>A : Symbol(A, Decl(typePredicatesInUnion_noMatch.ts, 0, 0)) +>B : Symbol(B, Decl(typePredicatesInUnion_noMatch.ts, 2, 1)) + +function f(o: Or, x: {}, y: {}) { +>f : Symbol(f, Decl(typePredicatesInUnion_noMatch.ts, 7, 16)) +>o : Symbol(o, Decl(typePredicatesInUnion_noMatch.ts, 9, 11)) +>Or : Symbol(Or, Decl(typePredicatesInUnion_noMatch.ts, 5, 1)) +>x : Symbol(x, Decl(typePredicatesInUnion_noMatch.ts, 9, 17)) +>y : Symbol(y, Decl(typePredicatesInUnion_noMatch.ts, 9, 24)) + + if (o.pred(x, y)) { +>o.pred : Symbol(pred, Decl(typePredicatesInUnion_noMatch.ts, 0, 13), Decl(typePredicatesInUnion_noMatch.ts, 3, 13)) +>o : Symbol(o, Decl(typePredicatesInUnion_noMatch.ts, 9, 11)) +>pred : Symbol(pred, Decl(typePredicatesInUnion_noMatch.ts, 0, 13), Decl(typePredicatesInUnion_noMatch.ts, 3, 13)) +>x : Symbol(x, Decl(typePredicatesInUnion_noMatch.ts, 9, 17)) +>y : Symbol(y, Decl(typePredicatesInUnion_noMatch.ts, 9, 24)) + + x; +>x : Symbol(x, Decl(typePredicatesInUnion_noMatch.ts, 9, 17)) + + y; +>y : Symbol(y, Decl(typePredicatesInUnion_noMatch.ts, 9, 24)) + } +} + diff --git a/tests/baselines/reference/typePredicatesInUnion_noMatch.types b/tests/baselines/reference/typePredicatesInUnion_noMatch.types new file mode 100644 index 0000000000000..2dbae00fccd7d --- /dev/null +++ b/tests/baselines/reference/typePredicatesInUnion_noMatch.types @@ -0,0 +1,48 @@ +=== tests/cases/compiler/typePredicatesInUnion_noMatch.ts === +interface A { +>A : A + + pred(x: {}, y: {}): x is boolean; +>pred : (x: {}, y: {}) => x is boolean +>x : {} +>y : {} +>x : any +} +interface B { +>B : B + + pred(x: {}, y: {}): y is string; +>pred : (x: {}, y: {}) => y is string +>x : {} +>y : {} +>y : any +} + +type Or = A | B; +>Or : Or +>A : A +>B : B + +function f(o: Or, x: {}, y: {}) { +>f : (o: Or, x: {}, y: {}) => void +>o : Or +>Or : Or +>x : {} +>y : {} + + if (o.pred(x, y)) { +>o.pred(x, y) : boolean +>o.pred : ((x: {}, y: {}) => x is boolean) | ((x: {}, y: {}) => y is string) +>o : Or +>pred : ((x: {}, y: {}) => x is boolean) | ((x: {}, y: {}) => y is string) +>x : {} +>y : {} + + x; +>x : {} + + y; +>y : {} + } +} + diff --git a/tests/cases/compiler/typePredicatesInUnion.ts b/tests/cases/compiler/typePredicatesInUnion.ts new file mode 100644 index 0000000000000..7f2fe9ddbb246 --- /dev/null +++ b/tests/cases/compiler/typePredicatesInUnion.ts @@ -0,0 +1,14 @@ +interface A { + pred(x: {}): x is boolean; +} +interface B { + pred(x: {}): x is string; +} + +type Or = A | B; + +function f(o: Or, x: {}) { + if (o.pred(x)) { + x; + } +} diff --git a/tests/cases/compiler/typePredicatesInUnion_noMatch.ts b/tests/cases/compiler/typePredicatesInUnion_noMatch.ts new file mode 100644 index 0000000000000..84df39cc86f20 --- /dev/null +++ b/tests/cases/compiler/typePredicatesInUnion_noMatch.ts @@ -0,0 +1,15 @@ +interface A { + pred(x: {}, y: {}): x is boolean; +} +interface B { + pred(x: {}, y: {}): y is string; +} + +type Or = A | B; + +function f(o: Or, x: {}, y: {}) { + if (o.pred(x, y)) { + x; + y; + } +}