Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix 21732: "in" operator could widden type #39746

Closed
143 changes: 134 additions & 9 deletions src/compiler/checker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -695,7 +695,7 @@ namespace ts {

const tupleTypes = new Map<string, GenericType>();
const unionTypes = new Map<string, UnionType>();
const intersectionTypes = new Map<string, Type>();
const intersectionTypes = new Map<string, IntersectionType>();
const literalTypes = new Map<string, LiteralType>();
const indexedAccessTypes = new Map<string, IndexedAccessType>();
const substitutionTypes = new Map<string, SubstitutionType>();
Expand Down Expand Up @@ -2425,7 +2425,7 @@ namespace ts {
const immediate = resolveExternalModuleName(
node,
getExternalModuleRequireArgument(node) || getExternalModuleImportEqualsDeclarationExpression(node));
const resolved = resolveExternalModuleSymbol(immediate);
const resolved = resolveExternalModuleSymbol(immediate);
markSymbolOfAliasDeclarationIfTypeOnly(node, immediate, resolved, /*overwriteEmpty*/ false);
return resolved;
}
Expand Down Expand Up @@ -3749,6 +3749,11 @@ namespace ts {
members, callSignatures, constructSignatures, stringIndexInfo, numberIndexInfo);
}

function createWidenType(symbol: Symbol | undefined, members: SymbolTable, callSignatures: readonly Signature[], constructSignatures: readonly Signature[], stringIndexInfo: IndexInfo | undefined, numberIndexInfo: IndexInfo | undefined): ResolvedType {
return setStructuredTypeMembers(createObjectType(ObjectFlags.Anonymous | ObjectFlags.WidenedByNarrow, symbol),
members, callSignatures, constructSignatures, stringIndexInfo, numberIndexInfo);
}

function forEachSymbolTableInScope<T>(enclosingDeclaration: Node | undefined, callback: (symbolTable: SymbolTable) => T): T {
let result: T;
for (let location = enclosingDeclaration; location; location = location.parent) {
Expand Down Expand Up @@ -13037,6 +13042,33 @@ namespace ts {
return type;
}

// This function assumes the constituent type list is sorted and deduplicated.
function getIntersectionTypeFromSortedList(types: Type[], objectFlags: ObjectFlags, aliasSymbol?: Symbol, aliasTypeArguments?: readonly Type[]): Type {
if (types.length === 0) {
return neverType;
}
if (types.length === 1) {
return types[0];
}
const id = getTypeListId(types);
let type = intersectionTypes.get(id);
if (!type) {
type = <IntersectionType>createType(TypeFlags.Intersection);
intersectionTypes.set(id, type);
type.objectFlags = objectFlags | getPropagatingFlagsOfTypes(types, /*excludeKinds*/ TypeFlags.Nullable);
type.types = types;
/*
Note: This is the alias symbol (or lack thereof) that we see when we first encounter this union type.
For aliases of identical unions, eg `type T = A | B; type U = A | B`, the symbol of the first alias encountered is the aliasSymbol.
(In the language service, the order may depend on the order in which a user takes actions, such as hovering over symbols.)
It's important that we create equivalent union types only once, so that's an unfortunate side effect.
*/
type.aliasSymbol = aliasSymbol;
type.aliasTypeArguments = aliasTypeArguments;
}
return type;
}

function getTypeFromUnionTypeNode(node: UnionTypeNode): Type {
const links = getNodeLinks(node);
if (!links.resolvedType) {
Expand Down Expand Up @@ -13238,7 +13270,7 @@ namespace ts {
return typeSet[0];
}
const id = getTypeListId(typeSet);
let result = intersectionTypes.get(id);
let result: Type | undefined = intersectionTypes.get(id);
if (!result) {
if (includes & TypeFlags.Union) {
if (intersectUnionsOfPrimitiveTypes(typeSet)) {
Expand Down Expand Up @@ -13271,7 +13303,7 @@ namespace ts {
else {
result = createIntersectionType(typeSet, aliasSymbol, aliasTypeArguments);
}
intersectionTypes.set(id, result);
intersectionTypes.set(id, <IntersectionType>result);
}
return result;
}
Expand Down Expand Up @@ -21564,14 +21596,107 @@ namespace ts {
return !assumeTrue;
}

function narrowByInKeyword(type: Type, literal: LiteralExpression, assumeTrue: boolean) {
if (type.flags & (TypeFlags.Union | TypeFlags.Object)
function widdenTypeWithSymbol(type: Type, newSymbol: Symbol): Type {
// If type is this/any/unknown, it could not be widden.
if ((type.flags & TypeFlags.AnyOrUnknown) || isThisTypeParameter(type)) {
return type;
}
const propName = newSymbol.escapedName;
const members = createSymbolTable();
members.set(propName, newSymbol);
const newObjType = createWidenType(/* symbol */ undefined, members, emptyArray, emptyArray, /* stringIndexInfo */ undefined, /* numberIndexInfo */ undefined);

// if `type` is never, just return the new anonymous object type.
if (type.flags & TypeFlags.Never) {
return newObjType;
}

// if type is intersection, we might have added type into it, and we just need to add into this type again rather than a new one.
// else add a new anonymous object type which contains the type and widden the origional type with it.

if (isIntersectionType(type)) {
// try to get the first Anonymous Object type to add new type to it.
const widenedType: Type | undefined = type.types.find(t => isObjectType(t) && t.objectFlags & ObjectFlags.WidenedByNarrow);
if (widenedType && isObjectType(widenedType)) {
const typeWithOutWiden = filterIntersectionType(type, t => t !== widenedType);

const members = createSymbolTable();
members.set(propName, newSymbol);
if (widenedType.members) {
mergeSymbolTable(members, widenedType.members);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This recursively merges the symbol table deeply (so overlapping symbols within the table get combined, and have a merge pointer at the new, combined symbol). You don't want that. Rather than most of this logic, you should probably just use getSpreadType, rather than everything after the // if type is intersection comment, since it already combines object types where appropriate, since you guarantee you're adding a new property in the second object anyway. (Re)Using it should also allow you to cut out a lot of the other structural changes around intersection construction in this PR, too.

}
newObjType.members = members;
newObjType.properties = getNamedMembers(members);
return createIntersectionType([typeWithOutWiden, newObjType]);
}
}
return createIntersectionType([type, newObjType]);

// this function is almost like `filterType`, expect that the `type` is Intersection rather than Union.
// maybe we should advanced `filterType`, but I do not know whether it would be too far.
function filterIntersectionType(type: Type, f: (t: Type) => boolean): Type {
if (type.flags & TypeFlags.Intersection) {
const types = (<IntersectionType>type).types;
const filtered = filter(types, f);

return filtered === types ? type : getIntersectionTypeFromSortedList(filtered, (<IntersectionType>type).objectFlags);
}
return type.flags & TypeFlags.Never || f(type) ? type : neverType;
}

// I would be very glad to create a helper file like `nodeTests.ts` if feedback positive review.
function isIntersectionType(type: Type): type is IntersectionType {
return !!(type.flags & TypeFlags.Intersection);
}

function isObjectType(type: Type): type is ObjectType {
return !!(type.flags & TypeFlags.Object);
}
}

function narrowOrWiddenTypeByInKeyword(type: Type, literal: LiteralExpression, assumeTrue: boolean) {
const propName = escapeLeadingUnderscores(literal.text);
const addSymbol = createSymbol(SymbolFlags.Property, propName);
addSymbol.type = unknownType;

if ((type.flags & (TypeFlags.Union | TypeFlags.Object)
|| isThisTypeParameter(type)
|| type.flags & TypeFlags.Intersection && every((type as IntersectionType).types, t => t.symbol !== globalThisSymbol)) {
const propName = escapeLeadingUnderscores(literal.text);
|| type.flags & TypeFlags.Intersection && every((type as IntersectionType).types, t => t.symbol !== globalThisSymbol)) && isSomeDirectSubtypeContainsPropName(type, propName)) {
return filterType(type, t => isTypePresencePossible(t, propName, assumeTrue));
}
// only widden property when the type does not contain string-index/propName in any of the constituents.
else if (assumeTrue && !isSomeDirectSubtypeContainsPropName(type, propName) && !getIndexInfoOfType(type, IndexKind.String)) {
return widdenTypeWithSymbol(type, addSymbol);
}
return type;

// This function is almost like function `getPropertyOfType`, except when type.flags contains `UnionOrIntersection`
// it would return the property rather than undefiend even when property is partial.
function isSomeDirectSubtypeContainsPropName(type: Type, name: __String): Symbol | undefined {
type = getReducedApparentType(type);
if (type.flags & TypeFlags.Object) {
const resolved = resolveStructuredTypeMembers(<ObjectType>type);
const symbol = resolved.members.get(name);
if (symbol && symbolIsValue(symbol)) {
return symbol;
}
const functionType = resolved === anyFunctionType ? globalFunctionType :
resolved.callSignatures.length ? globalCallableFunctionType :
resolved.constructSignatures.length ? globalNewableFunctionType :
undefined;
if (functionType) {
const symbol = getPropertyOfObjectType(functionType, name);
if (symbol) {
return symbol;
}
}
return getPropertyOfObjectType(globalObjectType, name);
}
if (type.flags & TypeFlags.UnionOrIntersection) {
return getUnionOrIntersectionProperty(<UnionOrIntersectionType>type, name);
}
return undefined;
}
}

function narrowTypeByBinaryExpression(type: Type, expr: BinaryExpression, assumeTrue: boolean): Type {
Expand Down Expand Up @@ -21626,7 +21751,7 @@ namespace ts {
case SyntaxKind.InKeyword:
const target = getReferenceCandidate(expr.right);
if (isStringLiteralLike(expr.left) && isMatchingReference(reference, target)) {
return narrowByInKeyword(type, expr.left, assumeTrue);
return narrowOrWiddenTypeByInKeyword(type, expr.left, assumeTrue);
}
break;
case SyntaxKind.CommaToken:
Expand Down
4 changes: 4 additions & 0 deletions src/compiler/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4991,6 +4991,7 @@ namespace ts {
FreshLiteral = 1 << 15, // Fresh object literal
ArrayLiteral = 1 << 16, // Originates in an array literal
ObjectRestType = 1 << 17, // Originates in object rest declaration

/* @internal */
PrimitiveUnion = 1 << 18, // Union of only primitive types
/* @internal */
Expand Down Expand Up @@ -5019,6 +5020,9 @@ namespace ts {
IsNeverIntersection = 1 << 29, // Intersection reduces to never
/* @internal */
IsClassInstanceClone = 1 << 30, // Type is a clone of a class instance type

/* @internal */
WidenedByNarrow = 1 << 31, // in keyword could widen type, this mark it as the widen part.
ClassOrInterface = Class | Interface,
/* @internal */
RequiresWidening = ContainsWideningType | ContainsObjectOrArrayLiteral,
Expand Down
10 changes: 5 additions & 5 deletions tests/baselines/reference/conditionalTypeDoesntSpinForever.types
Original file line number Diff line number Diff line change
Expand Up @@ -212,12 +212,12 @@ export enum PubSubRecordIsStoredInRedisAsA {
>buildPubSubRecordType(Object.assign({}, soFar, {identifier: instance as TYPE}) as SO_FAR & {identifier: TYPE}) : BuildPubSubRecordType<SO_FAR & { identifier: TYPE; }>
>buildPubSubRecordType : <SO_FAR>(soFar: SO_FAR) => BuildPubSubRecordType<SO_FAR>
>Object.assign({}, soFar, {identifier: instance as TYPE}) as SO_FAR & {identifier: TYPE} : SO_FAR & { identifier: TYPE; }
>Object.assign({}, soFar, {identifier: instance as TYPE}) : SO_FAR & { identifier: TYPE; }
>Object.assign({}, soFar, {identifier: instance as TYPE}) : SO_FAR & { record: unknown; } & { identifier: TYPE; }
>Object.assign : { <T, U>(target: T, source: U): T & U; <T, U, V>(target: T, source1: U, source2: V): T & U & V; <T, U, V, W>(target: T, source1: U, source2: V, source3: W): T & U & V & W; (target: object, ...sources: any[]): any; }
>Object : ObjectConstructor
>assign : { <T, U>(target: T, source: U): T & U; <T, U, V>(target: T, source1: U, source2: V): T & U & V; <T, U, V, W>(target: T, source1: U, source2: V, source3: W): T & U & V & W; (target: object, ...sources: any[]): any; }
>{} : {}
>soFar : SO_FAR
>soFar : SO_FAR & { record: unknown; }
>{identifier: instance as TYPE} : { identifier: TYPE; }
>identifier : TYPE
>instance as TYPE : TYPE
Expand Down Expand Up @@ -389,13 +389,13 @@ export enum PubSubRecordIsStoredInRedisAsA {
>soFar : SO_FAR
>"object" in soFar : boolean
>"object" : "object"
>soFar : SO_FAR
>soFar : SO_FAR & { identifier: unknown; }
>"maxMsToWaitBeforePublishing" in soFar : boolean
>"maxMsToWaitBeforePublishing" : "maxMsToWaitBeforePublishing"
>soFar : SO_FAR
>soFar : SO_FAR & { object: unknown; identifier: unknown; }
>"PubSubRecordIsStoredInRedisAsA" in soFar : boolean
>"PubSubRecordIsStoredInRedisAsA" : "PubSubRecordIsStoredInRedisAsA"
>soFar : SO_FAR
>soFar : SO_FAR & { maxMsToWaitBeforePublishing: unknown; object: unknown; identifier: unknown; }
>{} : {}
>{ type: soFar, fields: () => new Set(Object.keys(soFar) as (keyof SO_FAR)[]), hasField: (fieldName: string | number | symbol) => fieldName in soFar } : { type: SO_FAR; fields: () => Set<keyof SO_FAR>; hasField: (fieldName: string | number | symbol) => boolean; }

Expand Down
8 changes: 4 additions & 4 deletions tests/baselines/reference/inKeywordTypeguard.errors.txt
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@ tests/cases/compiler/inKeywordTypeguard.ts(16,11): error TS2339: Property 'a' do
tests/cases/compiler/inKeywordTypeguard.ts(27,11): error TS2339: Property 'b' does not exist on type 'AWithOptionalProp | BWithOptionalProp'.
Property 'b' does not exist on type 'AWithOptionalProp'.
tests/cases/compiler/inKeywordTypeguard.ts(42,11): error TS2339: Property 'b' does not exist on type 'AWithMethod'.
tests/cases/compiler/inKeywordTypeguard.ts(49,11): error TS2339: Property 'a' does not exist on type 'never'.
tests/cases/compiler/inKeywordTypeguard.ts(50,11): error TS2339: Property 'b' does not exist on type 'never'.
tests/cases/compiler/inKeywordTypeguard.ts(49,11): error TS2339: Property 'a' does not exist on type '(AWithMethod | BWithMethod) & { c: unknown; }'.
tests/cases/compiler/inKeywordTypeguard.ts(50,11): error TS2339: Property 'b' does not exist on type '(AWithMethod | BWithMethod) & { c: unknown; }'.
tests/cases/compiler/inKeywordTypeguard.ts(52,11): error TS2339: Property 'a' does not exist on type 'AWithMethod | BWithMethod'.
Property 'a' does not exist on type 'BWithMethod'.
tests/cases/compiler/inKeywordTypeguard.ts(53,11): error TS2339: Property 'b' does not exist on type 'AWithMethod | BWithMethod'.
Expand Down Expand Up @@ -85,10 +85,10 @@ tests/cases/compiler/inKeywordTypeguard.ts(94,26): error TS2339: Property 'a' do
if ("c" in x) {
x.a();
~
!!! error TS2339: Property 'a' does not exist on type 'never'.
!!! error TS2339: Property 'a' does not exist on type '(AWithMethod | BWithMethod) & { c: unknown; }'.
x.b();
~
!!! error TS2339: Property 'b' does not exist on type 'never'.
!!! error TS2339: Property 'b' does not exist on type '(AWithMethod | BWithMethod) & { c: unknown; }'.
} else {
x.a();
~
Expand Down
4 changes: 2 additions & 2 deletions tests/baselines/reference/inKeywordTypeguard.types
Original file line number Diff line number Diff line change
Expand Up @@ -146,13 +146,13 @@ function negativeTestClassesWithMemberMissingInBothClasses(x: AWithMethod | BWit
x.a();
>x.a() : any
>x.a : any
>x : never
>x : (AWithMethod | BWithMethod) & { c: unknown; }
>a : any

x.b();
>x.b() : any
>x.b : any
>x : never
>x : (AWithMethod | BWithMethod) & { c: unknown; }
>b : any

} else {
Expand Down