Improve optionality detection in mapped type indexed access substitut…

…ions (#57946)
microsoft · Mar 27, 2024 · e418f8d · e418f8d
1 parent b0d5ae6
commit e418f8d
Show file tree

Hide file tree

Showing 5 changed files with 304 additions and 9 deletions.
diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts
@@ -14131,16 +14131,18 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
         return modifiers & MappedTypeModifiers.ExcludeOptional ? -1 : modifiers & MappedTypeModifiers.IncludeOptional ? 1 : 0;
     }
 
-    function getModifiersTypeOptionality(type: Type): number {
-        return type.flags & TypeFlags.Intersection ? Math.max(...map((type as IntersectionType).types, getModifiersTypeOptionality)) :
-            getObjectFlags(type) & ObjectFlags.Mapped ? getCombinedMappedTypeOptionality(type as MappedType) :
-            0;
-    }
-
     // Return -1, 0, or 1, for stripped, unchanged, or added optionality respectively. When a homomorphic mapped type doesn't
     // modify optionality, recursively consult the optionality of the type being mapped over to see if it strips or adds optionality.
-    function getCombinedMappedTypeOptionality(type: MappedType): number {
-        return getMappedTypeOptionality(type) || getModifiersTypeOptionality(getModifiersTypeFromMappedType(type));
+    // For intersections, return -1 or 1 when all constituents strip or add optionality, otherwise return 0.
+    function getCombinedMappedTypeOptionality(type: Type): number {
+        if (getObjectFlags(type) & ObjectFlags.Mapped) {
+            return getMappedTypeOptionality(type as MappedType) || getCombinedMappedTypeOptionality(getModifiersTypeFromMappedType(type as MappedType));
+        }
+        if (type.flags & TypeFlags.Intersection) {
+            const optionality = getCombinedMappedTypeOptionality((type as IntersectionType).types[0]);
+            return every((type as IntersectionType).types, (t, i) => i === 0 || getCombinedMappedTypeOptionality(t) === optionality) ? optionality : 0;
+        }
+        return 0;
     }
 
     function isPartialMappedType(type: Type) {
@@ -18671,11 +18673,27 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
         return !!(getUnionType([intersectTypes(type1, type2), neverType]).flags & TypeFlags.Never);
     }
 
+    // Given an indexed access on a mapped type of the form { [P in K]: E }[X], return an instantiation of E where P is
+    // replaced with X. Since this simplification doesn't account for mapped type modifiers, add 'undefined' to the
+    // resulting type if the mapped type includes a '?' modifier or if the modifiers type indicates that some properties
+    // are optional. If the modifiers type is generic, conservatively estimate optionality by recursively looking for
+    // mapped types that include '?' modifiers.
     function substituteIndexedMappedType(objectType: MappedType, index: Type) {
         const mapper = createTypeMapper([getTypeParameterFromMappedType(objectType)], [index]);
         const templateMapper = combineTypeMappers(objectType.mapper, mapper);
         const instantiatedTemplateType = instantiateType(getTemplateTypeFromMappedType(objectType.target as MappedType || objectType), templateMapper);
-        return addOptionality(instantiatedTemplateType, /*isProperty*/ true, getCombinedMappedTypeOptionality(objectType) > 0);
+        const isOptional = getMappedTypeOptionality(objectType) > 0 || (isGenericType(objectType) ?
+            getCombinedMappedTypeOptionality(getModifiersTypeFromMappedType(objectType)) > 0 :
+            couldAccessOptionalProperty(objectType, index));
+        return addOptionality(instantiatedTemplateType, /*isProperty*/ true, isOptional);
+    }
+
+    // Return true if an indexed access with the given object and index types could access an optional property.
+    function couldAccessOptionalProperty(objectType: Type, indexType: Type) {
+        const indexConstraint = getBaseConstraintOfType(indexType);
+        return !!indexConstraint && some(getPropertiesOfType(objectType), p =>
+            !!(p.flags & SymbolFlags.Optional) &&
+            isTypeAssignableTo(getLiteralTypeFromProperty(p, TypeFlags.StringOrNumberLiteralOrUnique), indexConstraint));
     }
 
     function getIndexedAccessType(objectType: Type, indexType: Type, accessFlags = AccessFlags.None, accessNode?: ElementAccessExpression | IndexedAccessTypeNode | PropertyName | BindingName | SyntheticExpression, aliasSymbol?: Symbol, aliasTypeArguments?: readonly Type[]): Type {

diff --git a/tests/baselines/reference/mappedTypeIndexedAccessConstraint.errors.txt b/tests/baselines/reference/mappedTypeIndexedAccessConstraint.errors.txt
@@ -72,4 +72,36 @@ mappedTypeIndexedAccessConstraint.ts(53,34): error TS2722: Cannot invoke an obje
 
     const resolveMapper2 = <K extends keyof typeof mapper>(
         key: K, o: MapperArgs<K>) => mapper[key]?.(o)
+
+    // Repro from #57860
+
+    type Obj1 = {
+        a: string;
+        b: number;
+    };
+
+    type Obj2 = {
+        b: number;
+        c: boolean;
+    };
+
+    declare const mapIntersection: {
+        [K in keyof (Partial<Obj1> & Required<Obj2>)]: number;
+    };
+
+    const accessMapped = <K extends keyof Obj2>(key: K) => mapIntersection[key].toString();
+
+    declare const resolved: { a?: number | undefined; b: number; c: number };
+
+    const accessResolved = <K extends keyof Obj2>(key: K) => resolved[key].toString();
+
+    // Additional repro from #57860
+
+    type Foo = {
+        prop: string;
+    }
+
+    function test<K extends keyof Foo>(obj: Pick<Required<Foo> & Partial<Foo>, K>, key: K) {
+        obj[key].length;
+    }
 
diff --git a/tests/baselines/reference/mappedTypeIndexedAccessConstraint.symbols b/tests/baselines/reference/mappedTypeIndexedAccessConstraint.symbols
@@ -225,3 +225,97 @@ const resolveMapper2 = <K extends keyof typeof mapper>(
 >key : Symbol(key, Decl(mappedTypeIndexedAccessConstraint.ts, 54, 55))
 >o : Symbol(o, Decl(mappedTypeIndexedAccessConstraint.ts, 55, 11))
 
+// Repro from #57860
+
+type Obj1 = {
+>Obj1 : Symbol(Obj1, Decl(mappedTypeIndexedAccessConstraint.ts, 55, 49))
+
+    a: string;
+>a : Symbol(a, Decl(mappedTypeIndexedAccessConstraint.ts, 59, 13))
+
+    b: number;
+>b : Symbol(b, Decl(mappedTypeIndexedAccessConstraint.ts, 60, 14))
+
+};
+
+type Obj2 = {
+>Obj2 : Symbol(Obj2, Decl(mappedTypeIndexedAccessConstraint.ts, 62, 2))
+
+    b: number;
+>b : Symbol(b, Decl(mappedTypeIndexedAccessConstraint.ts, 64, 13))
+
+    c: boolean;
+>c : Symbol(c, Decl(mappedTypeIndexedAccessConstraint.ts, 65, 14))
+
+};
+
+declare const mapIntersection: {
+>mapIntersection : Symbol(mapIntersection, Decl(mappedTypeIndexedAccessConstraint.ts, 69, 13))
+
+    [K in keyof (Partial<Obj1> & Required<Obj2>)]: number;
+>K : Symbol(K, Decl(mappedTypeIndexedAccessConstraint.ts, 70, 5))
+>Partial : Symbol(Partial, Decl(lib.es5.d.ts, --, --))
+>Obj1 : Symbol(Obj1, Decl(mappedTypeIndexedAccessConstraint.ts, 55, 49))
+>Required : Symbol(Required, Decl(lib.es5.d.ts, --, --))
+>Obj2 : Symbol(Obj2, Decl(mappedTypeIndexedAccessConstraint.ts, 62, 2))
+
+};
+
+const accessMapped = <K extends keyof Obj2>(key: K) => mapIntersection[key].toString();
+>accessMapped : Symbol(accessMapped, Decl(mappedTypeIndexedAccessConstraint.ts, 73, 5))
+>K : Symbol(K, Decl(mappedTypeIndexedAccessConstraint.ts, 73, 22))
+>Obj2 : Symbol(Obj2, Decl(mappedTypeIndexedAccessConstraint.ts, 62, 2))
+>key : Symbol(key, Decl(mappedTypeIndexedAccessConstraint.ts, 73, 44))
+>K : Symbol(K, Decl(mappedTypeIndexedAccessConstraint.ts, 73, 22))
+>mapIntersection[key].toString : Symbol(Number.toString, Decl(lib.es5.d.ts, --, --))
+>mapIntersection : Symbol(mapIntersection, Decl(mappedTypeIndexedAccessConstraint.ts, 69, 13))
+>key : Symbol(key, Decl(mappedTypeIndexedAccessConstraint.ts, 73, 44))
+>toString : Symbol(Number.toString, Decl(lib.es5.d.ts, --, --))
+
+declare const resolved: { a?: number | undefined; b: number; c: number };
+>resolved : Symbol(resolved, Decl(mappedTypeIndexedAccessConstraint.ts, 75, 13))
+>a : Symbol(a, Decl(mappedTypeIndexedAccessConstraint.ts, 75, 25))
+>b : Symbol(b, Decl(mappedTypeIndexedAccessConstraint.ts, 75, 49))
+>c : Symbol(c, Decl(mappedTypeIndexedAccessConstraint.ts, 75, 60))
+
+const accessResolved = <K extends keyof Obj2>(key: K) => resolved[key].toString();
+>accessResolved : Symbol(accessResolved, Decl(mappedTypeIndexedAccessConstraint.ts, 77, 5))
+>K : Symbol(K, Decl(mappedTypeIndexedAccessConstraint.ts, 77, 24))
+>Obj2 : Symbol(Obj2, Decl(mappedTypeIndexedAccessConstraint.ts, 62, 2))
+>key : Symbol(key, Decl(mappedTypeIndexedAccessConstraint.ts, 77, 46))
+>K : Symbol(K, Decl(mappedTypeIndexedAccessConstraint.ts, 77, 24))
+>resolved[key].toString : Symbol(Number.toString, Decl(lib.es5.d.ts, --, --))
+>resolved : Symbol(resolved, Decl(mappedTypeIndexedAccessConstraint.ts, 75, 13))
+>key : Symbol(key, Decl(mappedTypeIndexedAccessConstraint.ts, 77, 46))
+>toString : Symbol(Number.toString, Decl(lib.es5.d.ts, --, --))
+
+// Additional repro from #57860
+
+type Foo = {
+>Foo : Symbol(Foo, Decl(mappedTypeIndexedAccessConstraint.ts, 77, 82))
+
+    prop: string;
+>prop : Symbol(prop, Decl(mappedTypeIndexedAccessConstraint.ts, 81, 12))
+}
+
+function test<K extends keyof Foo>(obj: Pick<Required<Foo> & Partial<Foo>, K>, key: K) {
+>test : Symbol(test, Decl(mappedTypeIndexedAccessConstraint.ts, 83, 1))
+>K : Symbol(K, Decl(mappedTypeIndexedAccessConstraint.ts, 85, 14))
+>Foo : Symbol(Foo, Decl(mappedTypeIndexedAccessConstraint.ts, 77, 82))
+>obj : Symbol(obj, Decl(mappedTypeIndexedAccessConstraint.ts, 85, 35))
+>Pick : Symbol(Pick, Decl(lib.es5.d.ts, --, --))
+>Required : Symbol(Required, Decl(lib.es5.d.ts, --, --))
+>Foo : Symbol(Foo, Decl(mappedTypeIndexedAccessConstraint.ts, 77, 82))
+>Partial : Symbol(Partial, Decl(lib.es5.d.ts, --, --))
+>Foo : Symbol(Foo, Decl(mappedTypeIndexedAccessConstraint.ts, 77, 82))
+>K : Symbol(K, Decl(mappedTypeIndexedAccessConstraint.ts, 85, 14))
+>key : Symbol(key, Decl(mappedTypeIndexedAccessConstraint.ts, 85, 78))
+>K : Symbol(K, Decl(mappedTypeIndexedAccessConstraint.ts, 85, 14))
+
+    obj[key].length;
+>obj[key].length : Symbol(String.length, Decl(lib.es5.d.ts, --, --))
+>obj : Symbol(obj, Decl(mappedTypeIndexedAccessConstraint.ts, 85, 35))
+>key : Symbol(key, Decl(mappedTypeIndexedAccessConstraint.ts, 85, 78))
+>length : Symbol(String.length, Decl(lib.es5.d.ts, --, --))
+}
+
diff --git a/tests/baselines/reference/mappedTypeIndexedAccessConstraint.types b/tests/baselines/reference/mappedTypeIndexedAccessConstraint.types
@@ -307,3 +307,122 @@ const resolveMapper2 = <K extends keyof typeof mapper>(
 >o : MapperArgs<K>
 >  : ^^^^^^^^^^^^^
 
+// Repro from #57860
+
+type Obj1 = {
+>Obj1 : Obj1
+>     : ^^^^
+
+    a: string;
+>a : string
+>  : ^^^^^^
+
+    b: number;
+>b : number
+>  : ^^^^^^
+
+};
+
+type Obj2 = {
+>Obj2 : Obj2
+>     : ^^^^
+
+    b: number;
+>b : number
+>  : ^^^^^^
+
+    c: boolean;
+>c : boolean
+>  : ^^^^^^^
+
+};
+
+declare const mapIntersection: {
+>mapIntersection : { a?: number | undefined; b: number; c: number; }
+>                : ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+    [K in keyof (Partial<Obj1> & Required<Obj2>)]: number;
+};
+
+const accessMapped = <K extends keyof Obj2>(key: K) => mapIntersection[key].toString();
+>accessMapped : <K extends keyof Obj2>(key: K) => string
+>             : ^ ^^^^^^^^^^^^^^^^^^^^^^^^^^ ^^^^^^^^^^^
+><K extends keyof Obj2>(key: K) => mapIntersection[key].toString() : <K extends keyof Obj2>(key: K) => string
+>                                                                  : ^ ^^^^^^^^^^^^^^^^^^^^^^^^^^ ^^^^^^^^^^^
+>key : K
+>    : ^
+>mapIntersection[key].toString() : string
+>                                : ^^^^^^
+>mapIntersection[key].toString : (radix?: number | undefined) => string
+>                              : ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+>mapIntersection[key] : { a?: number | undefined; b: number; c: number; }[K]
+>                     : ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+>mapIntersection : { a?: number | undefined; b: number; c: number; }
+>                : ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+>key : K
+>    : ^
+>toString : (radix?: number | undefined) => string
+>         : ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+declare const resolved: { a?: number | undefined; b: number; c: number };
+>resolved : { a?: number | undefined; b: number; c: number; }
+>         : ^^^^^^                  ^^^^^      ^^^^^      ^^^
+>a : number | undefined
+>  : ^^^^^^^^^^^^^^^^^^
+>b : number
+>  : ^^^^^^
+>c : number
+>  : ^^^^^^
+
+const accessResolved = <K extends keyof Obj2>(key: K) => resolved[key].toString();
+>accessResolved : <K extends keyof Obj2>(key: K) => string
+>               : ^ ^^^^^^^^^^^^^^^^^^^^^^^^^^ ^^^^^^^^^^^
+><K extends keyof Obj2>(key: K) => resolved[key].toString() : <K extends keyof Obj2>(key: K) => string
+>                                                           : ^ ^^^^^^^^^^^^^^^^^^^^^^^^^^ ^^^^^^^^^^^
+>key : K
+>    : ^
+>resolved[key].toString() : string
+>                         : ^^^^^^
+>resolved[key].toString : (radix?: number | undefined) => string
+>                       : ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+>resolved[key] : { a?: number | undefined; b: number; c: number; }[K]
+>              : ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+>resolved : { a?: number | undefined; b: number; c: number; }
+>         : ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+>key : K
+>    : ^
+>toString : (radix?: number | undefined) => string
+>         : ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+// Additional repro from #57860
+
+type Foo = {
+>Foo : Foo
+>    : ^^^
+
+    prop: string;
+>prop : string
+>     : ^^^^^^
+}
+
+function test<K extends keyof Foo>(obj: Pick<Required<Foo> & Partial<Foo>, K>, key: K) {
+>test : <K extends "prop">(obj: Pick<Required<Foo> & Partial<Foo>, K>, key: K) => void
+>     : ^ ^^^^^^^^^^^^^^^^^^^^^^                                     ^^^^^^^ ^^^^^^^^^
+>obj : Pick<Required<Foo> & Partial<Foo>, K>
+>    : ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+>key : K
+>    : ^
+
+    obj[key].length;
+>obj[key].length : number
+>                : ^^^^^^
+>obj[key] : Pick<Required<Foo> & Partial<Foo>, K>[K]
+>         : ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+>obj : Pick<Required<Foo> & Partial<Foo>, K>
+>    : ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+>key : K
+>    : ^
+>length : number
+>       : ^^^^^^
+}
+
diff --git a/tests/cases/compiler/mappedTypeIndexedAccessConstraint.ts b/tests/cases/compiler/mappedTypeIndexedAccessConstraint.ts
@@ -57,3 +57,35 @@ const resolveMapper1 = <K extends keyof typeof mapper>(
 
 const resolveMapper2 = <K extends keyof typeof mapper>(
     key: K, o: MapperArgs<K>) => mapper[key]?.(o)
+
+// Repro from #57860
+
+type Obj1 = {
+    a: string;
+    b: number;
+};
+
+type Obj2 = {
+    b: number;
+    c: boolean;
+};
+
+declare const mapIntersection: {
+    [K in keyof (Partial<Obj1> & Required<Obj2>)]: number;
+};
+
+const accessMapped = <K extends keyof Obj2>(key: K) => mapIntersection[key].toString();
+
+declare const resolved: { a?: number | undefined; b: number; c: number };
+
+const accessResolved = <K extends keyof Obj2>(key: K) => resolved[key].toString();
+
+// Additional repro from #57860
+
+type Foo = {
+    prop: string;
+}
+
+function test<K extends keyof Foo>(obj: Pick<Required<Foo> & Partial<Foo>, K>, key: K) {
+    obj[key].length;
+}