From 70148a4b5514175f7bd3ee51cfa208e6d2c7a95d Mon Sep 17 00:00:00 2001 From: Anders Hejlsberg Date: Wed, 9 Jan 2019 16:10:28 -0800 Subject: [PATCH 1/3] Improve logic that determines when to resolve conditional types --- src/compiler/checker.ts | 134 ++++++++++++++++++++-------------------- src/compiler/types.ts | 4 +- 2 files changed, 71 insertions(+), 67 deletions(-) diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index de44e8bb37ca7..44cc7478e1382 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -444,10 +444,10 @@ namespace ts { const circularConstraintType = createAnonymousType(undefined, emptySymbols, emptyArray, emptyArray, undefined, undefined); const resolvingDefaultType = createAnonymousType(undefined, emptySymbols, emptyArray, emptyArray, undefined, undefined); - const markerSuperType = createType(TypeFlags.TypeParameter); - const markerSubType = createType(TypeFlags.TypeParameter); + const markerSuperType = createTypeParameter(); + const markerSubType = createTypeParameter(); markerSubType.constraint = markerSuperType; - const markerOtherType = createType(TypeFlags.TypeParameter); + const markerOtherType = createTypeParameter(); const noTypePredicate = createIdentifierTypePredicate("<>", 0, anyType); @@ -663,7 +663,6 @@ namespace ts { const subtypeRelation = createMap(); const assignableRelation = createMap(); - const definitelyAssignableRelation = createMap(); const comparableRelation = createMap(); const identityRelation = createMap(); const enumRelation = createMap(); @@ -2745,6 +2744,12 @@ namespace ts { return getUnionType(arrayFrom(typeofEQFacts.keys(), getLiteralType)); } + function createTypeParameter(symbol?: Symbol) { + const type = createType(TypeFlags.TypeParameter); + if (symbol) type.symbol = symbol; + return type; + } + // A reserved member name starts with two underscores, but the third character cannot be an underscore // or the @ symbol. A third underscore indicates an escaped form of an identifer that started // with at least two underscores. The @ character indicates that the name is denoted by a well known ES @@ -6085,9 +6090,8 @@ namespace ts { (type).instantiations.set(getTypeListId(type.typeParameters), type); (type).target = type; (type).typeArguments = type.typeParameters; - type.thisType = createType(TypeFlags.TypeParameter); + type.thisType = createTypeParameter(symbol); type.thisType.isThisType = true; - type.thisType.symbol = symbol; type.thisType.constraint = type; } } @@ -6228,20 +6232,12 @@ namespace ts { function getDeclaredTypeOfTypeParameter(symbol: Symbol): TypeParameter { const links = getSymbolLinks(symbol); - if (!links.declaredType) { - const type = createType(TypeFlags.TypeParameter); - type.symbol = symbol; - links.declaredType = type; - } - return links.declaredType; + return links.declaredType || (links.declaredType = createTypeParameter(symbol)); } function getDeclaredTypeOfAlias(symbol: Symbol): Type { const links = getSymbolLinks(symbol); - if (!links.declaredType) { - links.declaredType = getDeclaredTypeOfSymbol(resolveAlias(symbol)); - } - return links.declaredType; + return links.declaredType || (links.declaredType = getDeclaredTypeOfSymbol(resolveAlias(symbol))); } function getDeclaredTypeOfSymbol(symbol: Symbol): Type { @@ -7413,7 +7409,7 @@ namespace ts { if (type.root.isDistributive) { const simplified = getSimplifiedType(type.checkType); const constraint = simplified === type.checkType ? getConstraintOfType(simplified) : simplified; - if (constraint) { + if (constraint && constraint !== type.checkType) { const mapper = makeUnaryTypeMapper(type.root.checkType, constraint); const instantiated = getConditionalTypeInstantiation(type, combineTypeMappers(mapper, type.mapper)); if (!(instantiated.flags & TypeFlags.Never)) { @@ -9049,7 +9045,7 @@ namespace ts { if (arity) { typeParameters = new Array(arity); for (let i = 0; i < arity; i++) { - const typeParameter = typeParameters[i] = createType(TypeFlags.TypeParameter); + const typeParameter = typeParameters[i] = createTypeParameter(); if (i < maxLength) { const property = createSymbol(SymbolFlags.Property | (i >= minLength ? SymbolFlags.Optional : 0), "" + i as __String); property.type = typeParameter; @@ -9070,7 +9066,7 @@ namespace ts { type.instantiations.set(getTypeListId(type.typeParameters), type); type.target = type; type.typeArguments = type.typeParameters; - type.thisType = createType(TypeFlags.TypeParameter); + type.thisType = createTypeParameter(); type.thisType.isThisType = true; type.thisType.constraint = type; type.declaredProperties = properties; @@ -9971,7 +9967,7 @@ namespace ts { if (checkType === wildcardType || extendsType === wildcardType) { return wildcardType; } - const checkTypeInstantiable = maybeTypeOfKind(checkType, TypeFlags.Instantiable); + const checkTypeInstantiable = maybeTypeOfKind(checkType, TypeFlags.Instantiable | TypeFlags.GenericMappedType); let combinedMapper: TypeMapper | undefined; if (root.inferTypeParameters) { const context = createInferenceContext(root.inferTypeParameters, /*signature*/ undefined, InferenceFlags.None); @@ -9986,7 +9982,7 @@ namespace ts { // Instantiate the extends type including inferences for 'infer T' type parameters const inferredExtendsType = combinedMapper ? instantiateType(root.extendsType, combinedMapper) : extendsType; // We attempt to resolve the conditional type only when the check and extends types are non-generic - if (!checkTypeInstantiable && !maybeTypeOfKind(inferredExtendsType, TypeFlags.Instantiable)) { + if (!checkTypeInstantiable && !maybeTypeOfKind(inferredExtendsType, TypeFlags.Instantiable | TypeFlags.GenericMappedType)) { if (inferredExtendsType.flags & TypeFlags.AnyOrUnknown) { return instantiateType(root.trueType, mapper); } @@ -9998,14 +9994,15 @@ namespace ts { // types with type parameters mapped to the wildcard type, the most permissive instantiations // possible (the wildcard type is assignable to and from all types). If those are not related, // then no instatiations will be and we can just return the false branch type. - if (!isTypeAssignableTo(getWildcardInstantiation(checkType), getWildcardInstantiation(inferredExtendsType))) { + if (!isTypeAssignableTo(getPermissiveInstantiation(checkType), getPermissiveInstantiation(inferredExtendsType))) { return instantiateType(root.falseType, mapper); } - // Return trueType for a definitely true extends check. The definitely assignable relation excludes - // type variable constraints from consideration. Without the definitely assignable relation, the type + // Return trueType for a definitely true extends check. We check instantiations of the two + // types with type parameters mapped to their restrictive form, i.e. a form of the type parameter + // that has no constraint. This ensures that, for example, the type // type Foo = T extends { x: string } ? string : number - // would immediately resolve to 'string' instead of being deferred. - if (checkTypeRelatedTo(checkType, inferredExtendsType, definitelyAssignableRelation, /*errorNode*/ undefined)) { + // doesn't immediately resolve to 'string' instead of being deferred. + if (isTypeAssignableTo(getRestrictiveInstantiation(checkType), getRestrictiveInstantiation(inferredExtendsType))) { return instantiateType(root.trueType, combinedMapper || mapper); } } @@ -10611,13 +10608,20 @@ namespace ts { return t => t === source ? target : baseMapper(t); } - function wildcardMapper(type: Type) { + function permissiveMapper(type: Type) { return type.flags & TypeFlags.TypeParameter ? wildcardType : type; } + function getRestrictiveTypeParameter(tp: TypeParameter) { + return !tp.constraint ? tp : tp.restrictiveInstantiation || (tp.restrictiveInstantiation = createTypeParameter(tp.symbol)); + } + + function restrictiveMapper(type: Type) { + return type.flags & TypeFlags.TypeParameter ? getRestrictiveTypeParameter(type) : type; + } + function cloneTypeParameter(typeParameter: TypeParameter): TypeParameter { - const result = createType(TypeFlags.TypeParameter); - result.symbol = typeParameter.symbol; + const result = createTypeParameter(typeParameter.symbol); result.target = typeParameter; return result; } @@ -10969,9 +10973,14 @@ namespace ts { return type; } - function getWildcardInstantiation(type: Type) { + function getPermissiveInstantiation(type: Type) { return type.flags & (TypeFlags.Primitive | TypeFlags.AnyOrUnknown | TypeFlags.Never) ? type : - type.wildcardInstantiation || (type.wildcardInstantiation = instantiateType(type, wildcardMapper)); + type.permissiveInstantiation || (type.permissiveInstantiation = instantiateType(type, permissiveMapper)); + } + + function getRestrictiveInstantiation(type: Type) { + return type.flags & (TypeFlags.Primitive | TypeFlags.AnyOrUnknown | TypeFlags.Never) ? type : + type.restrictiveInstantiation || (type.restrictiveInstantiation = instantiateType(type, restrictiveMapper)); } function instantiateIndexInfo(info: IndexInfo | undefined, mapper: TypeMapper): IndexInfo | undefined { @@ -11644,7 +11653,7 @@ namespace ts { if (s & TypeFlags.Null && (!strictNullChecks || t & TypeFlags.Null)) return true; if (s & TypeFlags.Object && t & TypeFlags.NonPrimitive) return true; if (s & TypeFlags.UniqueESSymbol || t & TypeFlags.UniqueESSymbol) return false; - if (relation === assignableRelation || relation === definitelyAssignableRelation || relation === comparableRelation) { + if (relation === assignableRelation || relation === comparableRelation) { if (s & TypeFlags.Any) return true; // Type number or any numeric literal type is assignable to any numeric enum type or any // numeric enum literal type. This rule exists for backwards compatibility reasons because @@ -11836,7 +11845,7 @@ namespace ts { target = (target).regularType; } if (source.flags & TypeFlags.Substitution) { - source = relation === definitelyAssignableRelation ? (source).typeVariable : (source).substitute; + source = (source).substitute; } if (target.flags & TypeFlags.Substitution) { target = (target).typeVariable; @@ -12022,7 +12031,7 @@ namespace ts { } if (isExcessPropertyCheckTarget(target)) { const isComparingJsxAttributes = !!(getObjectFlags(source) & ObjectFlags.JsxAttributes); - if ((relation === assignableRelation || relation === definitelyAssignableRelation || relation === comparableRelation) && + if ((relation === assignableRelation || relation === comparableRelation) && (isTypeSubsetOf(globalObjectType, target) || (!isComparingJsxAttributes && isEmptyObjectType(target)))) { return false; } @@ -12425,24 +12434,22 @@ namespace ts { } // A type S is assignable to keyof T if S is assignable to keyof C, where C is the // simplified form of T or, if T doesn't simplify, the constraint of T. - if (relation !== definitelyAssignableRelation) { - const simplified = getSimplifiedType((target).type); - const constraint = simplified !== (target).type ? simplified : getConstraintOfType((target).type); - if (constraint) { - // We require Ternary.True here such that circular constraints don't cause - // false positives. For example, given 'T extends { [K in keyof T]: string }', - // 'keyof T' has itself as its constraint and produces a Ternary.Maybe when - // related to other types. - if (isRelatedTo(source, getIndexType(constraint, (target as IndexType).stringsOnly), reportErrors) === Ternary.True) { - return Ternary.True; - } + const simplified = getSimplifiedType((target).type); + const constraint = simplified !== (target).type ? simplified : getConstraintOfType((target).type); + if (constraint) { + // We require Ternary.True here such that circular constraints don't cause + // false positives. For example, given 'T extends { [K in keyof T]: string }', + // 'keyof T' has itself as its constraint and produces a Ternary.Maybe when + // related to other types. + if (isRelatedTo(source, getIndexType(constraint, (target as IndexType).stringsOnly), reportErrors) === Ternary.True) { + return Ternary.True; } } } else if (target.flags & TypeFlags.IndexedAccess) { // A type S is related to a type T[K], where T and K aren't both type variables, if S is related to C, // where C is the base constraint of T[K] - if (relation !== identityRelation && relation !== definitelyAssignableRelation && + if (relation !== identityRelation && !(isGenericObjectType((target).objectType) && isGenericIndexType((target).indexType))) { const constraint = getBaseConstraintOfType(target); if (constraint && constraint !== target) { @@ -12485,26 +12492,24 @@ namespace ts { return result; } } - if (relation !== definitelyAssignableRelation) { - const constraint = getConstraintOfType(source); - if (!constraint || (source.flags & TypeFlags.TypeParameter && constraint.flags & TypeFlags.Any)) { - // A type variable with no constraint is not related to the non-primitive object type. - if (result = isRelatedTo(emptyObjectType, extractTypesOfKind(target, ~TypeFlags.NonPrimitive))) { - errorInfo = saveErrorInfo; - return result; - } - } - // hi-speed no-this-instantiation check (less accurate, but avoids costly `this`-instantiation when the constraint will suffice), see #28231 for report on why this is needed - else if (result = isRelatedTo(constraint, target, /*reportErrors*/ false, /*headMessage*/ undefined, isIntersectionConstituent)) { - errorInfo = saveErrorInfo; - return result; - } - // slower, fuller, this-instantiated check (necessary when comparing raw `this` types from base classes), see `subclassWithPolymorphicThisIsAssignable.ts` test for example - else if (result = isRelatedTo(getTypeWithThisArgument(constraint, source), target, reportErrors, /*headMessage*/ undefined, isIntersectionConstituent)) { + const constraint = getConstraintOfType(source); + if (!constraint || (source.flags & TypeFlags.TypeParameter && constraint.flags & TypeFlags.Any)) { + // A type variable with no constraint is not related to the non-primitive object type. + if (result = isRelatedTo(emptyObjectType, extractTypesOfKind(target, ~TypeFlags.NonPrimitive))) { errorInfo = saveErrorInfo; return result; } } + // hi-speed no-this-instantiation check (less accurate, but avoids costly `this`-instantiation when the constraint will suffice), see #28231 for report on why this is needed + else if (result = isRelatedTo(constraint, target, /*reportErrors*/ false, /*headMessage*/ undefined, isIntersectionConstituent)) { + errorInfo = saveErrorInfo; + return result; + } + // slower, fuller, this-instantiated check (necessary when comparing raw `this` types from base classes), see `subclassWithPolymorphicThisIsAssignable.ts` test for example + else if (result = isRelatedTo(getTypeWithThisArgument(constraint, source), target, reportErrors, /*headMessage*/ undefined, isIntersectionConstituent)) { + errorInfo = saveErrorInfo; + return result; + } } else if (source.flags & TypeFlags.Index) { if (result = isRelatedTo(keyofConstraintType, target, reportErrors)) { @@ -12528,7 +12533,7 @@ namespace ts { } } } - else if (relation !== definitelyAssignableRelation) { + else { const distributiveConstraint = getConstraintOfDistributiveConditionalType(source); if (distributiveConstraint) { if (result = isRelatedTo(distributiveConstraint, target, reportErrors)) { @@ -12559,9 +12564,6 @@ namespace ts { } return Ternary.False; } - if (relation === definitelyAssignableRelation && isGenericMappedType(source)) { - return Ternary.False; - } const sourceIsPrimitive = !!(source.flags & TypeFlags.Primitive); if (relation !== identityRelation) { source = getApparentType(source); diff --git a/src/compiler/types.ts b/src/compiler/types.ts index 5c42d3eb5104c..e46a3d7040318 100644 --- a/src/compiler/types.ts +++ b/src/compiler/types.ts @@ -3916,7 +3916,9 @@ namespace ts { aliasTypeArguments?: ReadonlyArray; // Alias type arguments (if any) /* @internal */ aliasTypeArgumentsContainsMarker?: boolean; // Alias type arguments (if any) /* @internal */ - wildcardInstantiation?: Type; // Instantiation with type parameters mapped to wildcard type + permissiveInstantiation?: Type; // Instantiation with type parameters mapped to wildcard type + /* @internal */ + restrictiveInstantiation?: Type; // Instantiation with type parameters mapped to unconstrained form /* @internal */ immediateBaseConstraint?: Type; // Immediate base constraint cache } From 9fda7014cac88cd0885af42fccfb113ef289aadc Mon Sep 17 00:00:00 2001 From: Anders Hejlsberg Date: Wed, 9 Jan 2019 16:16:24 -0800 Subject: [PATCH 2/3] Add regression tests --- .../conformance/types/conditional/conditionalTypes1.ts | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/tests/cases/conformance/types/conditional/conditionalTypes1.ts b/tests/cases/conformance/types/conditional/conditionalTypes1.ts index f5c67f678eb3f..2e8690a228b45 100644 --- a/tests/cases/conformance/types/conditional/conditionalTypes1.ts +++ b/tests/cases/conformance/types/conditional/conditionalTypes1.ts @@ -353,3 +353,11 @@ declare function assign(o: T, a: RecursivePartial): void; var a = {o: 1, b: 2, c: [{a: 1, c: '213'}]} assign(a, {o: 2, c: {0: {a: 2, c: '213123'}}}) + +// Repros from #23843 + +type Weird1 = ((a: U) => never) extends + ((a: U) => never) ? never : never; + +type Weird2 = ((a: U) => U) extends + ((a: U) => infer T) ? T : never; From 0c1c97e5013e957da26e15390e5268cb63c1884e Mon Sep 17 00:00:00 2001 From: Anders Hejlsberg Date: Wed, 9 Jan 2019 16:16:31 -0800 Subject: [PATCH 3/3] Accept new baselines --- .../reference/conditionalTypes1.errors.txt | 8 ++++++ .../baselines/reference/conditionalTypes1.js | 10 +++++++ .../reference/conditionalTypes1.symbols | 27 +++++++++++++++++++ .../reference/conditionalTypes1.types | 18 +++++++++++++ 4 files changed, 63 insertions(+) diff --git a/tests/baselines/reference/conditionalTypes1.errors.txt b/tests/baselines/reference/conditionalTypes1.errors.txt index 8bf05c043ce53..62239eca7b821 100644 --- a/tests/baselines/reference/conditionalTypes1.errors.txt +++ b/tests/baselines/reference/conditionalTypes1.errors.txt @@ -471,4 +471,12 @@ tests/cases/conformance/types/conditional/conditionalTypes1.ts(288,43): error TS var a = {o: 1, b: 2, c: [{a: 1, c: '213'}]} assign(a, {o: 2, c: {0: {a: 2, c: '213123'}}}) + + // Repros from #23843 + + type Weird1 = ((a: U) => never) extends + ((a: U) => never) ? never : never; + + type Weird2 = ((a: U) => U) extends + ((a: U) => infer T) ? T : never; \ No newline at end of file diff --git a/tests/baselines/reference/conditionalTypes1.js b/tests/baselines/reference/conditionalTypes1.js index 552a80d58a53b..7ce19ce17e70c 100644 --- a/tests/baselines/reference/conditionalTypes1.js +++ b/tests/baselines/reference/conditionalTypes1.js @@ -351,6 +351,14 @@ declare function assign(o: T, a: RecursivePartial): void; var a = {o: 1, b: 2, c: [{a: 1, c: '213'}]} assign(a, {o: 2, c: {0: {a: 2, c: '213123'}}}) + +// Repros from #23843 + +type Weird1 = ((a: U) => never) extends + ((a: U) => never) ? never : never; + +type Weird2 = ((a: U) => U) extends + ((a: U) => infer T) ? T : never; //// [conditionalTypes1.js] @@ -715,3 +723,5 @@ declare var a: { c: string; }[]; }; +declare type Weird1 = ((a: U) => never) extends ((a: U) => never) ? never : never; +declare type Weird2 = ((a: U) => U) extends ((a: U) => infer T) ? T : never; diff --git a/tests/baselines/reference/conditionalTypes1.symbols b/tests/baselines/reference/conditionalTypes1.symbols index 19f0ae838909c..e5c6ae3d6c7f7 100644 --- a/tests/baselines/reference/conditionalTypes1.symbols +++ b/tests/baselines/reference/conditionalTypes1.symbols @@ -1371,3 +1371,30 @@ assign(a, {o: 2, c: {0: {a: 2, c: '213123'}}}) >a : Symbol(a, Decl(conditionalTypes1.ts, 351, 25)) >c : Symbol(c, Decl(conditionalTypes1.ts, 351, 30)) +// Repros from #23843 + +type Weird1 = ((a: U) => never) extends +>Weird1 : Symbol(Weird1, Decl(conditionalTypes1.ts, 351, 46)) +>U : Symbol(U, Decl(conditionalTypes1.ts, 355, 16)) +>a : Symbol(a, Decl(conditionalTypes1.ts, 355, 35)) +>U : Symbol(U, Decl(conditionalTypes1.ts, 355, 16)) + + ((a: U) => never) ? never : never; +>U : Symbol(U, Decl(conditionalTypes1.ts, 356, 6)) +>a : Symbol(a, Decl(conditionalTypes1.ts, 356, 22)) +>U : Symbol(U, Decl(conditionalTypes1.ts, 356, 6)) + +type Weird2 = ((a: U) => U) extends +>Weird2 : Symbol(Weird2, Decl(conditionalTypes1.ts, 356, 54)) +>U : Symbol(U, Decl(conditionalTypes1.ts, 358, 16)) +>a : Symbol(a, Decl(conditionalTypes1.ts, 358, 35)) +>U : Symbol(U, Decl(conditionalTypes1.ts, 358, 16)) +>U : Symbol(U, Decl(conditionalTypes1.ts, 358, 16)) + + ((a: U) => infer T) ? T : never; +>U : Symbol(U, Decl(conditionalTypes1.ts, 359, 6)) +>a : Symbol(a, Decl(conditionalTypes1.ts, 359, 22)) +>U : Symbol(U, Decl(conditionalTypes1.ts, 359, 6)) +>T : Symbol(T, Decl(conditionalTypes1.ts, 359, 36)) +>T : Symbol(T, Decl(conditionalTypes1.ts, 359, 36)) + diff --git a/tests/baselines/reference/conditionalTypes1.types b/tests/baselines/reference/conditionalTypes1.types index 00d1965eea420..08fdb4b2f291c 100644 --- a/tests/baselines/reference/conditionalTypes1.types +++ b/tests/baselines/reference/conditionalTypes1.types @@ -1054,3 +1054,21 @@ assign(a, {o: 2, c: {0: {a: 2, c: '213123'}}}) >c : string >'213123' : "213123" +// Repros from #23843 + +type Weird1 = ((a: U) => never) extends +>Weird1 : never +>a : U + + ((a: U) => never) ? never : never; +>true : true +>a : U + +type Weird2 = ((a: U) => U) extends +>Weird2 : boolean +>a : U + + ((a: U) => infer T) ? T : never; +>true : true +>a : U +