Skip to content

Commit e418f8d

Browse files
ahejlsbergahejlsberg
authored
Improve optionality detection in mapped type indexed access substitutions (#57946)
1 parent b0d5ae6 commit e418f8d

5 files changed

+304
-9
lines changed

‎src/compiler/checker.ts

Lines changed: 27 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -14131,16 +14131,18 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
1413114131
return modifiers & MappedTypeModifiers.ExcludeOptional ? -1 : modifiers & MappedTypeModifiers.IncludeOptional ? 1 : 0;
1413214132
}
1413314133

14134-
function getModifiersTypeOptionality(type: Type): number {
14135-
return type.flags & TypeFlags.Intersection ? Math.max(...map((type as IntersectionType).types, getModifiersTypeOptionality)) :
14136-
getObjectFlags(type) & ObjectFlags.Mapped ? getCombinedMappedTypeOptionality(type as MappedType) :
14137-
0;
14138-
}
14139-
1414014134
// Return -1, 0, or 1, for stripped, unchanged, or added optionality respectively. When a homomorphic mapped type doesn't
1414114135
// modify optionality, recursively consult the optionality of the type being mapped over to see if it strips or adds optionality.
14142-
function getCombinedMappedTypeOptionality(type: MappedType): number {
14143-
return getMappedTypeOptionality(type) || getModifiersTypeOptionality(getModifiersTypeFromMappedType(type));
14136+
// For intersections, return -1 or 1 when all constituents strip or add optionality, otherwise return 0.
14137+
function getCombinedMappedTypeOptionality(type: Type): number {
14138+
if (getObjectFlags(type) & ObjectFlags.Mapped) {
14139+
return getMappedTypeOptionality(type as MappedType) || getCombinedMappedTypeOptionality(getModifiersTypeFromMappedType(type as MappedType));
14140+
}
14141+
if (type.flags & TypeFlags.Intersection) {
14142+
const optionality = getCombinedMappedTypeOptionality((type as IntersectionType).types[0]);
14143+
return every((type as IntersectionType).types, (t, i) => i === 0 || getCombinedMappedTypeOptionality(t) === optionality) ? optionality : 0;
14144+
}
14145+
return 0;
1414414146
}
1414514147

1414614148
function isPartialMappedType(type: Type) {
@@ -18671,11 +18673,27 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
1867118673
return !!(getUnionType([intersectTypes(type1, type2), neverType]).flags & TypeFlags.Never);
1867218674
}
1867318675

18676+
// Given an indexed access on a mapped type of the form { [P in K]: E }[X], return an instantiation of E where P is
18677+
// replaced with X. Since this simplification doesn't account for mapped type modifiers, add 'undefined' to the
18678+
// resulting type if the mapped type includes a '?' modifier or if the modifiers type indicates that some properties
18679+
// are optional. If the modifiers type is generic, conservatively estimate optionality by recursively looking for
18680+
// mapped types that include '?' modifiers.
1867418681
function substituteIndexedMappedType(objectType: MappedType, index: Type) {
1867518682
const mapper = createTypeMapper([getTypeParameterFromMappedType(objectType)], [index]);
1867618683
const templateMapper = combineTypeMappers(objectType.mapper, mapper);
1867718684
const instantiatedTemplateType = instantiateType(getTemplateTypeFromMappedType(objectType.target as MappedType || objectType), templateMapper);
18678-
return addOptionality(instantiatedTemplateType, /*isProperty*/ true, getCombinedMappedTypeOptionality(objectType) > 0);
18685+
const isOptional = getMappedTypeOptionality(objectType) > 0 || (isGenericType(objectType) ?
18686+
getCombinedMappedTypeOptionality(getModifiersTypeFromMappedType(objectType)) > 0 :
18687+
couldAccessOptionalProperty(objectType, index));
18688+
return addOptionality(instantiatedTemplateType, /*isProperty*/ true, isOptional);
18689+
}
18690+
18691+
// Return true if an indexed access with the given object and index types could access an optional property.
18692+
function couldAccessOptionalProperty(objectType: Type, indexType: Type) {
18693+
const indexConstraint = getBaseConstraintOfType(indexType);
18694+
return !!indexConstraint && some(getPropertiesOfType(objectType), p =>
18695+
!!(p.flags & SymbolFlags.Optional) &&
18696+
isTypeAssignableTo(getLiteralTypeFromProperty(p, TypeFlags.StringOrNumberLiteralOrUnique), indexConstraint));
1867918697
}
1868018698

1868118699
function getIndexedAccessType(objectType: Type, indexType: Type, accessFlags = AccessFlags.None, accessNode?: ElementAccessExpression | IndexedAccessTypeNode | PropertyName | BindingName | SyntheticExpression, aliasSymbol?: Symbol, aliasTypeArguments?: readonly Type[]): Type {

‎tests/baselines/reference/mappedTypeIndexedAccessConstraint.errors.txt

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,4 +72,36 @@ mappedTypeIndexedAccessConstraint.ts(53,34): error TS2722: Cannot invoke an obje
7272

7373
const resolveMapper2 = <K extends keyof typeof mapper>(
7474
key: K, o: MapperArgs<K>) => mapper[key]?.(o)
75+
76+
// Repro from #57860
77+
78+
type Obj1 = {
79+
a: string;
80+
b: number;
81+
};
82+
83+
type Obj2 = {
84+
b: number;
85+
c: boolean;
86+
};
87+
88+
declare const mapIntersection: {
89+
[K in keyof (Partial<Obj1> & Required<Obj2>)]: number;
90+
};
91+
92+
const accessMapped = <K extends keyof Obj2>(key: K) => mapIntersection[key].toString();
93+
94+
declare const resolved: { a?: number | undefined; b: number; c: number };
95+
96+
const accessResolved = <K extends keyof Obj2>(key: K) => resolved[key].toString();
97+
98+
// Additional repro from #57860
99+
100+
type Foo = {
101+
prop: string;
102+
}
103+
104+
function test<K extends keyof Foo>(obj: Pick<Required<Foo> & Partial<Foo>, K>, key: K) {
105+
obj[key].length;
106+
}
75107

‎tests/baselines/reference/mappedTypeIndexedAccessConstraint.symbols

Lines changed: 94 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -225,3 +225,97 @@ const resolveMapper2 = <K extends keyof typeof mapper>(
225225
>key : Symbol(key, Decl(mappedTypeIndexedAccessConstraint.ts, 54, 55))
226226
>o : Symbol(o, Decl(mappedTypeIndexedAccessConstraint.ts, 55, 11))
227227

228+
// Repro from #57860
229+
230+
type Obj1 = {
231+
>Obj1 : Symbol(Obj1, Decl(mappedTypeIndexedAccessConstraint.ts, 55, 49))
232+
233+
a: string;
234+
>a : Symbol(a, Decl(mappedTypeIndexedAccessConstraint.ts, 59, 13))
235+
236+
b: number;
237+
>b : Symbol(b, Decl(mappedTypeIndexedAccessConstraint.ts, 60, 14))
238+
239+
};
240+
241+
type Obj2 = {
242+
>Obj2 : Symbol(Obj2, Decl(mappedTypeIndexedAccessConstraint.ts, 62, 2))
243+
244+
b: number;
245+
>b : Symbol(b, Decl(mappedTypeIndexedAccessConstraint.ts, 64, 13))
246+
247+
c: boolean;
248+
>c : Symbol(c, Decl(mappedTypeIndexedAccessConstraint.ts, 65, 14))
249+
250+
};
251+
252+
declare const mapIntersection: {
253+
>mapIntersection : Symbol(mapIntersection, Decl(mappedTypeIndexedAccessConstraint.ts, 69, 13))
254+
255+
[K in keyof (Partial<Obj1> & Required<Obj2>)]: number;
256+
>K : Symbol(K, Decl(mappedTypeIndexedAccessConstraint.ts, 70, 5))
257+
>Partial : Symbol(Partial, Decl(lib.es5.d.ts, --, --))
258+
>Obj1 : Symbol(Obj1, Decl(mappedTypeIndexedAccessConstraint.ts, 55, 49))
259+
>Required : Symbol(Required, Decl(lib.es5.d.ts, --, --))
260+
>Obj2 : Symbol(Obj2, Decl(mappedTypeIndexedAccessConstraint.ts, 62, 2))
261+
262+
};
263+
264+
const accessMapped = <K extends keyof Obj2>(key: K) => mapIntersection[key].toString();
265+
>accessMapped : Symbol(accessMapped, Decl(mappedTypeIndexedAccessConstraint.ts, 73, 5))
266+
>K : Symbol(K, Decl(mappedTypeIndexedAccessConstraint.ts, 73, 22))
267+
>Obj2 : Symbol(Obj2, Decl(mappedTypeIndexedAccessConstraint.ts, 62, 2))
268+
>key : Symbol(key, Decl(mappedTypeIndexedAccessConstraint.ts, 73, 44))
269+
>K : Symbol(K, Decl(mappedTypeIndexedAccessConstraint.ts, 73, 22))
270+
>mapIntersection[key].toString : Symbol(Number.toString, Decl(lib.es5.d.ts, --, --))
271+
>mapIntersection : Symbol(mapIntersection, Decl(mappedTypeIndexedAccessConstraint.ts, 69, 13))
272+
>key : Symbol(key, Decl(mappedTypeIndexedAccessConstraint.ts, 73, 44))
273+
>toString : Symbol(Number.toString, Decl(lib.es5.d.ts, --, --))
274+
275+
declare const resolved: { a?: number | undefined; b: number; c: number };
276+
>resolved : Symbol(resolved, Decl(mappedTypeIndexedAccessConstraint.ts, 75, 13))
277+
>a : Symbol(a, Decl(mappedTypeIndexedAccessConstraint.ts, 75, 25))
278+
>b : Symbol(b, Decl(mappedTypeIndexedAccessConstraint.ts, 75, 49))
279+
>c : Symbol(c, Decl(mappedTypeIndexedAccessConstraint.ts, 75, 60))
280+
281+
const accessResolved = <K extends keyof Obj2>(key: K) => resolved[key].toString();
282+
>accessResolved : Symbol(accessResolved, Decl(mappedTypeIndexedAccessConstraint.ts, 77, 5))
283+
>K : Symbol(K, Decl(mappedTypeIndexedAccessConstraint.ts, 77, 24))
284+
>Obj2 : Symbol(Obj2, Decl(mappedTypeIndexedAccessConstraint.ts, 62, 2))
285+
>key : Symbol(key, Decl(mappedTypeIndexedAccessConstraint.ts, 77, 46))
286+
>K : Symbol(K, Decl(mappedTypeIndexedAccessConstraint.ts, 77, 24))
287+
>resolved[key].toString : Symbol(Number.toString, Decl(lib.es5.d.ts, --, --))
288+
>resolved : Symbol(resolved, Decl(mappedTypeIndexedAccessConstraint.ts, 75, 13))
289+
>key : Symbol(key, Decl(mappedTypeIndexedAccessConstraint.ts, 77, 46))
290+
>toString : Symbol(Number.toString, Decl(lib.es5.d.ts, --, --))
291+
292+
// Additional repro from #57860
293+
294+
type Foo = {
295+
>Foo : Symbol(Foo, Decl(mappedTypeIndexedAccessConstraint.ts, 77, 82))
296+
297+
prop: string;
298+
>prop : Symbol(prop, Decl(mappedTypeIndexedAccessConstraint.ts, 81, 12))
299+
}
300+
301+
function test<K extends keyof Foo>(obj: Pick<Required<Foo> & Partial<Foo>, K>, key: K) {
302+
>test : Symbol(test, Decl(mappedTypeIndexedAccessConstraint.ts, 83, 1))
303+
>K : Symbol(K, Decl(mappedTypeIndexedAccessConstraint.ts, 85, 14))
304+
>Foo : Symbol(Foo, Decl(mappedTypeIndexedAccessConstraint.ts, 77, 82))
305+
>obj : Symbol(obj, Decl(mappedTypeIndexedAccessConstraint.ts, 85, 35))
306+
>Pick : Symbol(Pick, Decl(lib.es5.d.ts, --, --))
307+
>Required : Symbol(Required, Decl(lib.es5.d.ts, --, --))
308+
>Foo : Symbol(Foo, Decl(mappedTypeIndexedAccessConstraint.ts, 77, 82))
309+
>Partial : Symbol(Partial, Decl(lib.es5.d.ts, --, --))
310+
>Foo : Symbol(Foo, Decl(mappedTypeIndexedAccessConstraint.ts, 77, 82))
311+
>K : Symbol(K, Decl(mappedTypeIndexedAccessConstraint.ts, 85, 14))
312+
>key : Symbol(key, Decl(mappedTypeIndexedAccessConstraint.ts, 85, 78))
313+
>K : Symbol(K, Decl(mappedTypeIndexedAccessConstraint.ts, 85, 14))
314+
315+
obj[key].length;
316+
>obj[key].length : Symbol(String.length, Decl(lib.es5.d.ts, --, --))
317+
>obj : Symbol(obj, Decl(mappedTypeIndexedAccessConstraint.ts, 85, 35))
318+
>key : Symbol(key, Decl(mappedTypeIndexedAccessConstraint.ts, 85, 78))
319+
>length : Symbol(String.length, Decl(lib.es5.d.ts, --, --))
320+
}
321+

‎tests/baselines/reference/mappedTypeIndexedAccessConstraint.types

Lines changed: 119 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -307,3 +307,122 @@ const resolveMapper2 = <K extends keyof typeof mapper>(
307307
>o : MapperArgs<K>
308308
> : ^^^^^^^^^^^^^
309309

310+
// Repro from #57860
311+
312+
type Obj1 = {
313+
>Obj1 : Obj1
314+
> : ^^^^
315+
316+
a: string;
317+
>a : string
318+
> : ^^^^^^
319+
320+
b: number;
321+
>b : number
322+
> : ^^^^^^
323+
324+
};
325+
326+
type Obj2 = {
327+
>Obj2 : Obj2
328+
> : ^^^^
329+
330+
b: number;
331+
>b : number
332+
> : ^^^^^^
333+
334+
c: boolean;
335+
>c : boolean
336+
> : ^^^^^^^
337+
338+
};
339+
340+
declare const mapIntersection: {
341+
>mapIntersection : { a?: number | undefined; b: number; c: number; }
342+
> : ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
343+
344+
[K in keyof (Partial<Obj1> & Required<Obj2>)]: number;
345+
};
346+
347+
const accessMapped = <K extends keyof Obj2>(key: K) => mapIntersection[key].toString();
348+
>accessMapped : <K extends keyof Obj2>(key: K) => string
349+
> : ^ ^^^^^^^^^^^^^^^^^^^^^^^^^^ ^^^^^^^^^^^
350+
><K extends keyof Obj2>(key: K) => mapIntersection[key].toString() : <K extends keyof Obj2>(key: K) => string
351+
> : ^ ^^^^^^^^^^^^^^^^^^^^^^^^^^ ^^^^^^^^^^^
352+
>key : K
353+
> : ^
354+
>mapIntersection[key].toString() : string
355+
> : ^^^^^^
356+
>mapIntersection[key].toString : (radix?: number | undefined) => string
357+
> : ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
358+
>mapIntersection[key] : { a?: number | undefined; b: number; c: number; }[K]
359+
> : ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
360+
>mapIntersection : { a?: number | undefined; b: number; c: number; }
361+
> : ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
362+
>key : K
363+
> : ^
364+
>toString : (radix?: number | undefined) => string
365+
> : ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
366+
367+
declare const resolved: { a?: number | undefined; b: number; c: number };
368+
>resolved : { a?: number | undefined; b: number; c: number; }
369+
> : ^^^^^^ ^^^^^ ^^^^^ ^^^
370+
>a : number | undefined
371+
> : ^^^^^^^^^^^^^^^^^^
372+
>b : number
373+
> : ^^^^^^
374+
>c : number
375+
> : ^^^^^^
376+
377+
const accessResolved = <K extends keyof Obj2>(key: K) => resolved[key].toString();
378+
>accessResolved : <K extends keyof Obj2>(key: K) => string
379+
> : ^ ^^^^^^^^^^^^^^^^^^^^^^^^^^ ^^^^^^^^^^^
380+
><K extends keyof Obj2>(key: K) => resolved[key].toString() : <K extends keyof Obj2>(key: K) => string
381+
> : ^ ^^^^^^^^^^^^^^^^^^^^^^^^^^ ^^^^^^^^^^^
382+
>key : K
383+
> : ^
384+
>resolved[key].toString() : string
385+
> : ^^^^^^
386+
>resolved[key].toString : (radix?: number | undefined) => string
387+
> : ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
388+
>resolved[key] : { a?: number | undefined; b: number; c: number; }[K]
389+
> : ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
390+
>resolved : { a?: number | undefined; b: number; c: number; }
391+
> : ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
392+
>key : K
393+
> : ^
394+
>toString : (radix?: number | undefined) => string
395+
> : ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
396+
397+
// Additional repro from #57860
398+
399+
type Foo = {
400+
>Foo : Foo
401+
> : ^^^
402+
403+
prop: string;
404+
>prop : string
405+
> : ^^^^^^
406+
}
407+
408+
function test<K extends keyof Foo>(obj: Pick<Required<Foo> & Partial<Foo>, K>, key: K) {
409+
>test : <K extends "prop">(obj: Pick<Required<Foo> & Partial<Foo>, K>, key: K) => void
410+
> : ^ ^^^^^^^^^^^^^^^^^^^^^^ ^^^^^^^ ^^^^^^^^^
411+
>obj : Pick<Required<Foo> & Partial<Foo>, K>
412+
> : ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
413+
>key : K
414+
> : ^
415+
416+
obj[key].length;
417+
>obj[key].length : number
418+
> : ^^^^^^
419+
>obj[key] : Pick<Required<Foo> & Partial<Foo>, K>[K]
420+
> : ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
421+
>obj : Pick<Required<Foo> & Partial<Foo>, K>
422+
> : ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
423+
>key : K
424+
> : ^
425+
>length : number
426+
> : ^^^^^^
427+
}
428+

‎tests/cases/compiler/mappedTypeIndexedAccessConstraint.ts

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,3 +57,35 @@ const resolveMapper1 = <K extends keyof typeof mapper>(
5757

5858
const resolveMapper2 = <K extends keyof typeof mapper>(
5959
key: K, o: MapperArgs<K>) => mapper[key]?.(o)
60+
61+
// Repro from #57860
62+
63+
type Obj1 = {
64+
a: string;
65+
b: number;
66+
};
67+
68+
type Obj2 = {
69+
b: number;
70+
c: boolean;
71+
};
72+
73+
declare const mapIntersection: {
74+
[K in keyof (Partial<Obj1> & Required<Obj2>)]: number;
75+
};
76+
77+
const accessMapped = <K extends keyof Obj2>(key: K) => mapIntersection[key].toString();
78+
79+
declare const resolved: { a?: number | undefined; b: number; c: number };
80+
81+
const accessResolved = <K extends keyof Obj2>(key: K) => resolved[key].toString();
82+
83+
// Additional repro from #57860
84+
85+
type Foo = {
86+
prop: string;
87+
}
88+
89+
function test<K extends keyof Foo>(obj: Pick<Required<Foo> & Partial<Foo>, K>, key: K) {
90+
obj[key].length;
91+
}

0 commit comments

Comments
 (0)