From 66fa9f6cd793f7df210799b7d665d05bdfa9ade1 Mon Sep 17 00:00:00 2001 From: Wesley Wigham Date: Wed, 21 Feb 2018 12:51:26 -0800 Subject: [PATCH] Just map type variables to constraints at certain positions for narrowing so that we do not map primitives (#21384) * Use a limited version of getApparentType that doesnt map primitives * Reuse [most of] getBaseConstraintOfType, since it does the needed behaviors * Move new function next to the very similar function --- src/compiler/checker.ts | 34 +++++++++++----- ...ameterExtendingStringAssignableToString.js | 18 +++++++++ ...rExtendingStringAssignableToString.symbols | 32 +++++++++++++++ ...terExtendingStringAssignableToString.types | 40 +++++++++++++++++++ ...ameterExtendingStringAssignableToString.ts | 9 +++++ 5 files changed, 122 insertions(+), 11 deletions(-) create mode 100644 tests/baselines/reference/nonNullParameterExtendingStringAssignableToString.js create mode 100644 tests/baselines/reference/nonNullParameterExtendingStringAssignableToString.symbols create mode 100644 tests/baselines/reference/nonNullParameterExtendingStringAssignableToString.types create mode 100644 tests/cases/compiler/nonNullParameterExtendingStringAssignableToString.ts diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index 00353c1c391..b177c0c9e04 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -4015,7 +4015,7 @@ namespace ts { parentType = getNonNullableType(parentType); } const propType = getTypeOfPropertyOfType(parentType, text); - const declaredType = propType && getApparentTypeForLocation(propType, declaration.name); + const declaredType = propType && getConstraintForLocation(propType, declaration.name); type = declaredType && getFlowTypeOfReference(declaration, declaredType) || isNumericLiteralName(text) && getIndexTypeOfType(parentType, IndexKind.Number) || getIndexTypeOfType(parentType, IndexKind.String); @@ -6138,17 +6138,29 @@ namespace ts { return getConstraintOfDistributiveConditionalType(type) || getDefaultConstraintOfConditionalType(type); } - function getBaseConstraintOfType(type: Type): Type { + function getBaseConstraintOfInstantiableNonPrimitiveUnionOrIntersection(type: Type) { if (type.flags & (TypeFlags.InstantiableNonPrimitive | TypeFlags.UnionOrIntersection)) { const constraint = getResolvedBaseConstraint(type); if (constraint !== noConstraintType && constraint !== circularConstraintType) { return constraint; } } - else if (type.flags & TypeFlags.Index) { + } + + function getBaseConstraintOfType(type: Type): Type { + const constraint = getBaseConstraintOfInstantiableNonPrimitiveUnionOrIntersection(type); + if (!constraint && type.flags & TypeFlags.Index) { return stringType; } - return undefined; + return constraint; + } + + /** + * This is similar to `getBaseConstraintOfType` except it returns the input type if there's no base constraint, instead of `undefined` + * It also doesn't map indexes to `string`, as where this is used this would be unneeded (and likely undesirable) + */ + function getBaseConstraintOrType(type: Type) { + return getBaseConstraintOfType(type) || type; } function hasNonCircularBaseConstraint(type: InstantiableType): boolean { @@ -11935,7 +11947,7 @@ namespace ts { function getFlowCacheKey(node: Node): string | undefined { if (node.kind === SyntaxKind.Identifier) { const symbol = getResolvedSymbol(node); - return symbol !== unknownSymbol ? (isApparentTypePosition(node) ? "@" : "") + getSymbolId(symbol) : undefined; + return symbol !== unknownSymbol ? (isConstraintPosition(node) ? "@" : "") + getSymbolId(symbol) : undefined; } if (node.kind === SyntaxKind.ThisKeyword) { return "0"; @@ -13297,7 +13309,7 @@ namespace ts { return annotationIncludesUndefined ? getTypeWithFacts(declaredType, TypeFacts.NEUndefined) : declaredType; } - function isApparentTypePosition(node: Node) { + function isConstraintPosition(node: Node) { const parent = node.parent; return parent.kind === SyntaxKind.PropertyAccessExpression || parent.kind === SyntaxKind.CallExpression && (parent).expression === node || @@ -13310,13 +13322,13 @@ namespace ts { return type.flags & TypeFlags.InstantiableNonPrimitive && maybeTypeOfKind(getBaseConstraintOfType(type) || emptyObjectType, TypeFlags.Nullable); } - function getApparentTypeForLocation(type: Type, node: Node) { + function getConstraintForLocation(type: Type, node: Node) { // When a node is the left hand expression of a property access, element access, or call expression, // and the type of the node includes type variables with constraints that are nullable, we fetch the // apparent type of the node *before* performing control flow analysis such that narrowings apply to // the constraint type. - if (isApparentTypePosition(node) && forEachType(type, typeHasNullableConstraint)) { - return mapType(getWidenedType(type), getApparentType); + if (isConstraintPosition(node) && forEachType(type, typeHasNullableConstraint)) { + return mapType(getWidenedType(type), getBaseConstraintOrType); } return type; } @@ -13404,7 +13416,7 @@ namespace ts { checkCollisionWithCapturedNewTargetVariable(node, node); checkNestedBlockScopedBinding(node, symbol); - const type = getApparentTypeForLocation(getTypeOfSymbol(localOrExportSymbol), node); + const type = getConstraintForLocation(getTypeOfSymbol(localOrExportSymbol), node); const assignmentKind = getAssignmentTargetKind(node); if (assignmentKind) { @@ -16024,7 +16036,7 @@ namespace ts { return unknownType; } } - propType = getApparentTypeForLocation(getTypeOfSymbol(prop), node); + propType = getConstraintForLocation(getTypeOfSymbol(prop), node); } // Only compute control flow type if this is a property access expression that isn't an // assignment target, and the referenced property was declared as a variable, property, diff --git a/tests/baselines/reference/nonNullParameterExtendingStringAssignableToString.js b/tests/baselines/reference/nonNullParameterExtendingStringAssignableToString.js new file mode 100644 index 00000000000..c28a33c1a61 --- /dev/null +++ b/tests/baselines/reference/nonNullParameterExtendingStringAssignableToString.js @@ -0,0 +1,18 @@ +//// [nonNullParameterExtendingStringAssignableToString.ts] +declare function foo(p: string): void; + +function fn(one: T, two: U) { + let three = Boolean() ? one : two; + foo(one!); + foo(two!); + foo(three!); // this line is the important one +} + +//// [nonNullParameterExtendingStringAssignableToString.js] +"use strict"; +function fn(one, two) { + var three = Boolean() ? one : two; + foo(one); + foo(two); + foo(three); // this line is the important one +} diff --git a/tests/baselines/reference/nonNullParameterExtendingStringAssignableToString.symbols b/tests/baselines/reference/nonNullParameterExtendingStringAssignableToString.symbols new file mode 100644 index 00000000000..e351d25ac31 --- /dev/null +++ b/tests/baselines/reference/nonNullParameterExtendingStringAssignableToString.symbols @@ -0,0 +1,32 @@ +=== tests/cases/compiler/nonNullParameterExtendingStringAssignableToString.ts === +declare function foo(p: string): void; +>foo : Symbol(foo, Decl(nonNullParameterExtendingStringAssignableToString.ts, 0, 0)) +>p : Symbol(p, Decl(nonNullParameterExtendingStringAssignableToString.ts, 0, 21)) + +function fn(one: T, two: U) { +>fn : Symbol(fn, Decl(nonNullParameterExtendingStringAssignableToString.ts, 0, 38)) +>T : Symbol(T, Decl(nonNullParameterExtendingStringAssignableToString.ts, 2, 12)) +>U : Symbol(U, Decl(nonNullParameterExtendingStringAssignableToString.ts, 2, 41)) +>one : Symbol(one, Decl(nonNullParameterExtendingStringAssignableToString.ts, 2, 60)) +>T : Symbol(T, Decl(nonNullParameterExtendingStringAssignableToString.ts, 2, 12)) +>two : Symbol(two, Decl(nonNullParameterExtendingStringAssignableToString.ts, 2, 67)) +>U : Symbol(U, Decl(nonNullParameterExtendingStringAssignableToString.ts, 2, 41)) + + let three = Boolean() ? one : two; +>three : Symbol(three, Decl(nonNullParameterExtendingStringAssignableToString.ts, 3, 7)) +>Boolean : Symbol(Boolean, Decl(lib.d.ts, --, --), Decl(lib.d.ts, --, --)) +>one : Symbol(one, Decl(nonNullParameterExtendingStringAssignableToString.ts, 2, 60)) +>two : Symbol(two, Decl(nonNullParameterExtendingStringAssignableToString.ts, 2, 67)) + + foo(one!); +>foo : Symbol(foo, Decl(nonNullParameterExtendingStringAssignableToString.ts, 0, 0)) +>one : Symbol(one, Decl(nonNullParameterExtendingStringAssignableToString.ts, 2, 60)) + + foo(two!); +>foo : Symbol(foo, Decl(nonNullParameterExtendingStringAssignableToString.ts, 0, 0)) +>two : Symbol(two, Decl(nonNullParameterExtendingStringAssignableToString.ts, 2, 67)) + + foo(three!); // this line is the important one +>foo : Symbol(foo, Decl(nonNullParameterExtendingStringAssignableToString.ts, 0, 0)) +>three : Symbol(three, Decl(nonNullParameterExtendingStringAssignableToString.ts, 3, 7)) +} diff --git a/tests/baselines/reference/nonNullParameterExtendingStringAssignableToString.types b/tests/baselines/reference/nonNullParameterExtendingStringAssignableToString.types new file mode 100644 index 00000000000..9ab1ef1f9ac --- /dev/null +++ b/tests/baselines/reference/nonNullParameterExtendingStringAssignableToString.types @@ -0,0 +1,40 @@ +=== tests/cases/compiler/nonNullParameterExtendingStringAssignableToString.ts === +declare function foo(p: string): void; +>foo : (p: string) => void +>p : string + +function fn(one: T, two: U) { +>fn : (one: T, two: U) => void +>T : T +>U : U +>one : T +>T : T +>two : U +>U : U + + let three = Boolean() ? one : two; +>three : T | U +>Boolean() ? one : two : T | U +>Boolean() : boolean +>Boolean : BooleanConstructor +>one : T +>two : U + + foo(one!); +>foo(one!) : void +>foo : (p: string) => void +>one! : string +>one : string | undefined + + foo(two!); +>foo(two!) : void +>foo : (p: string) => void +>two! : U +>two : U + + foo(three!); // this line is the important one +>foo(three!) : void +>foo : (p: string) => void +>three! : string +>three : string +} diff --git a/tests/cases/compiler/nonNullParameterExtendingStringAssignableToString.ts b/tests/cases/compiler/nonNullParameterExtendingStringAssignableToString.ts new file mode 100644 index 00000000000..e16bf7c13ca --- /dev/null +++ b/tests/cases/compiler/nonNullParameterExtendingStringAssignableToString.ts @@ -0,0 +1,9 @@ +// @strict: true +declare function foo(p: string): void; + +function fn(one: T, two: U) { + let three = Boolean() ? one : two; + foo(one!); + foo(two!); + foo(three!); // this line is the important one +} \ No newline at end of file