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

Properly defer resolution of mapped types with generic as clauses #51050

Merged
merged 4 commits into from Oct 7, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
40 changes: 31 additions & 9 deletions src/compiler/checker.ts
Expand Up @@ -12073,7 +12073,20 @@ namespace ts {
}

function isGenericMappedType(type: Type): type is MappedType {
return !!(getObjectFlags(type) & ObjectFlags.Mapped) && isGenericIndexType(getConstraintTypeFromMappedType(type as MappedType));
if (getObjectFlags(type) & ObjectFlags.Mapped) {
const constraint = getConstraintTypeFromMappedType(type as MappedType);
if (isGenericIndexType(constraint)) {
return true;
}
// A mapped type is generic if the 'as' clause references generic types other than the iteration type.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
// A mapped type is generic if the as clause references other generic types than the iteration type.
// A mapped type is generic if the 'as' clause references generic types other than the iteration type.

Copy link
Member Author

Choose a reason for hiding this comment

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

Will fix.

// To determine this, we substitute the constraint type (that we now know isn't generic) for the iteration
Copy link
Member

Choose a reason for hiding this comment

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

If you're ever mapping on an empty type, you will end up with never in some cases, which may make the as clause never, making the mapped type not generic. This means that if an interface is intended to be merged with other interfaces, this will return different answers pre-merged vs. post-merged. At the very least, we should have a test for that.

interface Events {
    // empty for now
}

type OnOffEvents<K extends "on" | "off"> = {
    [P in keyof Events as `${K}${P}`]: (event: Events[P]) => void
};

If OnOffEvents is intended to be generic, should we consider just instantiating the mapped type variable with the string literal type "" instead?

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't think any special handling is needed here. isGenericMappedType is supposed to determine whether the given type can possibly be affected by instantiation. When a mapped type maps over an empty object type, no further instantiation will affect the type (regardless of its as clause), so it is fine to say the type is non-generic. You're right that merging declarations may change the outcome here, but that'll change many other things too.

// type and check whether the resulting type is generic.
const nameType = getNameTypeFromMappedType(type as MappedType);
if (nameType && isGenericIndexType(instantiateType(nameType, makeUnaryTypeMapper(getTypeParameterFromMappedType(type as MappedType), constraint)))) {
return true;
}
}
return false;
}

function resolveStructuredTypeMembers(type: StructuredType): ResolvedType {
Expand Down Expand Up @@ -15638,8 +15651,14 @@ namespace ts {
return getStringLiteralType(text);
}
newTexts.push(text);
if (every(newTexts, t => t === "") && every(newTypes, t => !!(t.flags & TypeFlags.String))) {
return stringType;
if (every(newTexts, t => t === "")) {
if (every(newTypes, t => !!(t.flags & TypeFlags.String))) {
return stringType;
}
// Normalize `${Mapping<xxx>}` into Mapping<xxx>
if (newTypes.length === 1 && isPatternLiteralType(newTypes[0])) {
return newTypes[0];
}
Comment on lines +15658 to +15661
Copy link
Member

Choose a reason for hiding this comment

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

Is this correct for if xxx becomes number?

Copy link
Member Author

Choose a reason for hiding this comment

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

The string mapping types are all constrained to string, so xxx can't be number. It could however be `${number}`.

}
const id = `${getTypeListId(newTypes)}|${map(newTexts, t => t.length).join(",")}|${newTexts.join("")}`;
let type = templateLiteralTypes.get(id);
Expand Down Expand Up @@ -15698,11 +15717,13 @@ namespace ts {

function getStringMappingType(symbol: Symbol, type: Type): Type {
return type.flags & (TypeFlags.Union | TypeFlags.Never) ? mapType(type, t => getStringMappingType(symbol, t)) :
// Mapping<Mapping<T>> === Mapping<T>
type.flags & TypeFlags.StringMapping && symbol === type.symbol ? type :
isGenericIndexType(type) || isPatternLiteralPlaceholderType(type) ? getStringMappingTypeForGenericType(symbol, isPatternLiteralPlaceholderType(type) && !(type.flags & TypeFlags.StringMapping) ? getTemplateLiteralType(["", ""], [type]) : type) :
type.flags & TypeFlags.StringLiteral ? getStringLiteralType(applyStringMapping(symbol, (type as StringLiteralType).value)) :
type.flags & TypeFlags.TemplateLiteral ? getTemplateLiteralType(...applyTemplateStringMapping(symbol, (type as TemplateLiteralType).texts, (type as TemplateLiteralType).types)) :
// Mapping<Mapping<T>> === Mapping<T>
type.flags & TypeFlags.StringMapping && symbol === type.symbol ? type :
type.flags & (TypeFlags.Any | TypeFlags.String || type.flags & TypeFlags.StringMapping) || isGenericIndexType(type) ? getStringMappingTypeForGenericType(symbol, type) :
// This handles Mapping<`${number}`> and Mapping<`${bigint}`>
isPatternLiteralPlaceholderType(type) ? getStringMappingTypeForGenericType(symbol, getTemplateLiteralType(["", ""], [type])) :
type;
}

Expand Down Expand Up @@ -16000,11 +16021,12 @@ namespace ts {
}

function isPatternLiteralPlaceholderType(type: Type): boolean {
return !!(type.flags & (TypeFlags.Any | TypeFlags.String | TypeFlags.Number | TypeFlags.BigInt)) || !!(type.flags & TypeFlags.StringMapping && isPatternLiteralPlaceholderType((type as StringMappingType).type));
return !!(type.flags & (TypeFlags.Any | TypeFlags.String | TypeFlags.Number | TypeFlags.BigInt)) || isPatternLiteralType(type);
}

function isPatternLiteralType(type: Type) {
return !!(type.flags & TypeFlags.TemplateLiteral) && every((type as TemplateLiteralType).types, isPatternLiteralPlaceholderType);
return !!(type.flags & TypeFlags.TemplateLiteral) && every((type as TemplateLiteralType).types, isPatternLiteralPlaceholderType) ||
!!(type.flags & TypeFlags.StringMapping) && isPatternLiteralPlaceholderType((type as StringMappingType).type);
}

function isGenericType(type: Type): boolean {
Expand Down Expand Up @@ -22559,7 +22581,7 @@ namespace ts {
}

function isMemberOfStringMapping(source: Type, target: Type): boolean {
if (target.flags & (TypeFlags.String | TypeFlags.AnyOrUnknown)) {
if (target.flags & (TypeFlags.String | TypeFlags.Any)) {
return true;
}
if (target.flags & TypeFlags.TemplateLiteral) {
Expand Down
48 changes: 48 additions & 0 deletions tests/baselines/reference/genericMappedTypeAsClause.errors.txt
@@ -0,0 +1,48 @@
tests/cases/compiler/genericMappedTypeAsClause.ts(11,36): error TS2322: Type 'string' is not assignable to type 'number'.
tests/cases/compiler/genericMappedTypeAsClause.ts(14,11): error TS2322: Type 'number' is not assignable to type 'MappedModel<T>'.
tests/cases/compiler/genericMappedTypeAsClause.ts(15,11): error TS2322: Type 'string' is not assignable to type 'MappedModel<T>'.
tests/cases/compiler/genericMappedTypeAsClause.ts(16,11): error TS2322: Type 'number[]' is not assignable to type 'MappedModel<T>'.
tests/cases/compiler/genericMappedTypeAsClause.ts(17,11): error TS2322: Type 'boolean' is not assignable to type 'MappedModel<T>'.
tests/cases/compiler/genericMappedTypeAsClause.ts(18,34): error TS2322: Type '{ a: string; b: number; }' is not assignable to type 'MappedModel<T>'.
Object literal may only specify known properties, and 'a' does not exist in type 'MappedModel<T>'.
tests/cases/compiler/genericMappedTypeAsClause.ts(19,11): error TS2322: Type 'undefined' is not assignable to type 'MappedModel<T>'.


==== tests/cases/compiler/genericMappedTypeAsClause.ts (7 errors) ====
type Model = {
a: string;
b: number;
};

type MappedModel<Suffix extends string> = {
[K in keyof Model as `${K}${Suffix}`]: Model[K];
};

const foo1: MappedModel<'Foo'> = { aFoo: 'test', bFoo: 42 };
const foo2: MappedModel<'Foo'> = { bFoo: 'bar' }; // Error
~~~~
!!! error TS2322: Type 'string' is not assignable to type 'number'.
!!! related TS6500 tests/cases/compiler/genericMappedTypeAsClause.ts:6:43: The expected type comes from property 'bFoo' which is declared here on type 'MappedModel<"Foo">'

function f1<T extends string>() {
const x1: MappedModel<T> = 42; // Error
~~
!!! error TS2322: Type 'number' is not assignable to type 'MappedModel<T>'.
const x2: MappedModel<T> = 'test'; // Error
~~
!!! error TS2322: Type 'string' is not assignable to type 'MappedModel<T>'.
const x3: MappedModel<T> = [1, 2, 3]; // Error
~~
!!! error TS2322: Type 'number[]' is not assignable to type 'MappedModel<T>'.
const x4: MappedModel<T> = false; // Error
~~
!!! error TS2322: Type 'boolean' is not assignable to type 'MappedModel<T>'.
const x5: MappedModel<T> = { a: 'bar', b: 42 }; // Error
~~~~~~~~
!!! error TS2322: Type '{ a: string; b: number; }' is not assignable to type 'MappedModel<T>'.
!!! error TS2322: Object literal may only specify known properties, and 'a' does not exist in type 'MappedModel<T>'.
const x6: MappedModel<T> = undefined; // Error
~~
!!! error TS2322: Type 'undefined' is not assignable to type 'MappedModel<T>'.
}

35 changes: 35 additions & 0 deletions tests/baselines/reference/genericMappedTypeAsClause.js
@@ -0,0 +1,35 @@
//// [genericMappedTypeAsClause.ts]
type Model = {
a: string;
b: number;
};

type MappedModel<Suffix extends string> = {
[K in keyof Model as `${K}${Suffix}`]: Model[K];
};

const foo1: MappedModel<'Foo'> = { aFoo: 'test', bFoo: 42 };
const foo2: MappedModel<'Foo'> = { bFoo: 'bar' }; // Error

function f1<T extends string>() {
const x1: MappedModel<T> = 42; // Error
const x2: MappedModel<T> = 'test'; // Error
const x3: MappedModel<T> = [1, 2, 3]; // Error
const x4: MappedModel<T> = false; // Error
const x5: MappedModel<T> = { a: 'bar', b: 42 }; // Error
const x6: MappedModel<T> = undefined; // Error
}


//// [genericMappedTypeAsClause.js]
"use strict";
var foo1 = { aFoo: 'test', bFoo: 42 };
var foo2 = { bFoo: 'bar' }; // Error
function f1() {
var x1 = 42; // Error
var x2 = 'test'; // Error
var x3 = [1, 2, 3]; // Error
var x4 = false; // Error
var x5 = { a: 'bar', b: 42 }; // Error
var x6 = undefined; // Error
}
75 changes: 75 additions & 0 deletions tests/baselines/reference/genericMappedTypeAsClause.symbols
@@ -0,0 +1,75 @@
=== tests/cases/compiler/genericMappedTypeAsClause.ts ===
type Model = {
>Model : Symbol(Model, Decl(genericMappedTypeAsClause.ts, 0, 0))

a: string;
>a : Symbol(a, Decl(genericMappedTypeAsClause.ts, 0, 14))

b: number;
>b : Symbol(b, Decl(genericMappedTypeAsClause.ts, 1, 14))

};

type MappedModel<Suffix extends string> = {
>MappedModel : Symbol(MappedModel, Decl(genericMappedTypeAsClause.ts, 3, 2))
>Suffix : Symbol(Suffix, Decl(genericMappedTypeAsClause.ts, 5, 17))

[K in keyof Model as `${K}${Suffix}`]: Model[K];
>K : Symbol(K, Decl(genericMappedTypeAsClause.ts, 6, 5))
>Model : Symbol(Model, Decl(genericMappedTypeAsClause.ts, 0, 0))
>K : Symbol(K, Decl(genericMappedTypeAsClause.ts, 6, 5))
>Suffix : Symbol(Suffix, Decl(genericMappedTypeAsClause.ts, 5, 17))
>Model : Symbol(Model, Decl(genericMappedTypeAsClause.ts, 0, 0))
>K : Symbol(K, Decl(genericMappedTypeAsClause.ts, 6, 5))

};

const foo1: MappedModel<'Foo'> = { aFoo: 'test', bFoo: 42 };
>foo1 : Symbol(foo1, Decl(genericMappedTypeAsClause.ts, 9, 5))
>MappedModel : Symbol(MappedModel, Decl(genericMappedTypeAsClause.ts, 3, 2))
>aFoo : Symbol(aFoo, Decl(genericMappedTypeAsClause.ts, 9, 34))
>bFoo : Symbol(bFoo, Decl(genericMappedTypeAsClause.ts, 9, 48))

const foo2: MappedModel<'Foo'> = { bFoo: 'bar' }; // Error
>foo2 : Symbol(foo2, Decl(genericMappedTypeAsClause.ts, 10, 5))
>MappedModel : Symbol(MappedModel, Decl(genericMappedTypeAsClause.ts, 3, 2))
>bFoo : Symbol(bFoo, Decl(genericMappedTypeAsClause.ts, 10, 34))

function f1<T extends string>() {
>f1 : Symbol(f1, Decl(genericMappedTypeAsClause.ts, 10, 49))
>T : Symbol(T, Decl(genericMappedTypeAsClause.ts, 12, 12))

const x1: MappedModel<T> = 42; // Error
>x1 : Symbol(x1, Decl(genericMappedTypeAsClause.ts, 13, 9))
>MappedModel : Symbol(MappedModel, Decl(genericMappedTypeAsClause.ts, 3, 2))
>T : Symbol(T, Decl(genericMappedTypeAsClause.ts, 12, 12))

const x2: MappedModel<T> = 'test'; // Error
>x2 : Symbol(x2, Decl(genericMappedTypeAsClause.ts, 14, 9))
>MappedModel : Symbol(MappedModel, Decl(genericMappedTypeAsClause.ts, 3, 2))
>T : Symbol(T, Decl(genericMappedTypeAsClause.ts, 12, 12))

const x3: MappedModel<T> = [1, 2, 3]; // Error
>x3 : Symbol(x3, Decl(genericMappedTypeAsClause.ts, 15, 9))
>MappedModel : Symbol(MappedModel, Decl(genericMappedTypeAsClause.ts, 3, 2))
>T : Symbol(T, Decl(genericMappedTypeAsClause.ts, 12, 12))

const x4: MappedModel<T> = false; // Error
>x4 : Symbol(x4, Decl(genericMappedTypeAsClause.ts, 16, 9))
>MappedModel : Symbol(MappedModel, Decl(genericMappedTypeAsClause.ts, 3, 2))
>T : Symbol(T, Decl(genericMappedTypeAsClause.ts, 12, 12))

const x5: MappedModel<T> = { a: 'bar', b: 42 }; // Error
>x5 : Symbol(x5, Decl(genericMappedTypeAsClause.ts, 17, 9))
>MappedModel : Symbol(MappedModel, Decl(genericMappedTypeAsClause.ts, 3, 2))
>T : Symbol(T, Decl(genericMappedTypeAsClause.ts, 12, 12))
>a : Symbol(a, Decl(genericMappedTypeAsClause.ts, 17, 32))
>b : Symbol(b, Decl(genericMappedTypeAsClause.ts, 17, 42))

const x6: MappedModel<T> = undefined; // Error
>x6 : Symbol(x6, Decl(genericMappedTypeAsClause.ts, 18, 9))
>MappedModel : Symbol(MappedModel, Decl(genericMappedTypeAsClause.ts, 3, 2))
>T : Symbol(T, Decl(genericMappedTypeAsClause.ts, 12, 12))
>undefined : Symbol(undefined)
}

67 changes: 67 additions & 0 deletions tests/baselines/reference/genericMappedTypeAsClause.types
@@ -0,0 +1,67 @@
=== tests/cases/compiler/genericMappedTypeAsClause.ts ===
type Model = {
>Model : { a: string; b: number; }

a: string;
>a : string

b: number;
>b : number

};

type MappedModel<Suffix extends string> = {
>MappedModel : MappedModel<Suffix>

[K in keyof Model as `${K}${Suffix}`]: Model[K];
};

const foo1: MappedModel<'Foo'> = { aFoo: 'test', bFoo: 42 };
>foo1 : MappedModel<"Foo">
>{ aFoo: 'test', bFoo: 42 } : { aFoo: string; bFoo: number; }
>aFoo : string
>'test' : "test"
>bFoo : number
>42 : 42

const foo2: MappedModel<'Foo'> = { bFoo: 'bar' }; // Error
>foo2 : MappedModel<"Foo">
>{ bFoo: 'bar' } : { bFoo: string; }
>bFoo : string
>'bar' : "bar"

function f1<T extends string>() {
>f1 : <T extends string>() => void

const x1: MappedModel<T> = 42; // Error
>x1 : MappedModel<T>
>42 : 42

const x2: MappedModel<T> = 'test'; // Error
>x2 : MappedModel<T>
>'test' : "test"

const x3: MappedModel<T> = [1, 2, 3]; // Error
>x3 : MappedModel<T>
>[1, 2, 3] : number[]
>1 : 1
>2 : 2
>3 : 3

const x4: MappedModel<T> = false; // Error
>x4 : MappedModel<T>
>false : false

const x5: MappedModel<T> = { a: 'bar', b: 42 }; // Error
>x5 : MappedModel<T>
>{ a: 'bar', b: 42 } : { a: string; b: number; }
>a : string
>'bar' : "bar"
>b : number
>42 : 42

const x6: MappedModel<T> = undefined; // Error
>x6 : MappedModel<T>
>undefined : undefined
}

12 changes: 6 additions & 6 deletions tests/baselines/reference/intrinsicTypes.types
Expand Up @@ -9,7 +9,7 @@ type TU3 = Uppercase<string>; // Uppercase<string>
>TU3 : Uppercase<string>

type TU4 = Uppercase<any>; // Uppercase<`${any}`>
>TU4 : Uppercase<`${any}`>
>TU4 : Uppercase<any>

type TU5 = Uppercase<never>; // never
>TU5 : never
Expand All @@ -27,7 +27,7 @@ type TL3 = Lowercase<string>; // Lowercase<string>
>TL3 : Lowercase<string>

type TL4 = Lowercase<any>; // Lowercase<`${any}`>
>TL4 : Lowercase<`${any}`>
>TL4 : Lowercase<any>

type TL5 = Lowercase<never>; // never
>TL5 : never
Expand All @@ -45,7 +45,7 @@ type TC3 = Capitalize<string>; // Capitalize<string>
>TC3 : Capitalize<string>

type TC4 = Capitalize<any>; // Capitalize<`${any}`>
>TC4 : Capitalize<`${any}`>
>TC4 : Capitalize<any>

type TC5 = Capitalize<never>; // never
>TC5 : never
Expand All @@ -63,7 +63,7 @@ type TN3 = Uncapitalize<string>; // Uncapitalize<string>
>TN3 : Uncapitalize<string>

type TN4 = Uncapitalize<any>; // Uncapitalize<`${any}`>
>TN4 : Uncapitalize<`${any}`>
>TN4 : Uncapitalize<any>

type TN5 = Uncapitalize<never>; // never
>TN5 : never
Expand All @@ -72,13 +72,13 @@ type TN6 = Uncapitalize<42>; // Error
>TN6 : 42

type TX1<S extends string> = Uppercase<`aB${S}`>;
>TX1 : Uppercase<`aB${S}`>
>TX1 : `AB${Uppercase<S>}`

type TX2 = TX1<'xYz'>; // "ABXYZ"
>TX2 : "ABXYZ"

type TX3<S extends string> = Lowercase<`aB${S}`>;
>TX3 : Lowercase<`aB${S}`>
>TX3 : `ab${Lowercase<S>}`

type TX4 = TX3<'xYz'>; // "abxyz"
>TX4 : "abxyz"
Expand Down
2 changes: 1 addition & 1 deletion tests/baselines/reference/mappedTypeAsClauses.types
Expand Up @@ -61,7 +61,7 @@ type TD1 = DoubleProp<{ a: string, b: number }>; // { a1: string, a2: string, b
>b : number

type TD2 = keyof TD1; // 'a1' | 'a2' | 'b1' | 'b2'
>TD2 : "a1" | "a2" | "b1" | "b2"
>TD2 : "a1" | "b1" | "a2" | "b2"

type TD3<U> = keyof DoubleProp<U>; // `${keyof U & string}1` | `${keyof U & string}2`
>TD3 : `${keyof U & string}1` | `${keyof U & string}2`
Expand Down
2 changes: 1 addition & 1 deletion tests/baselines/reference/numericStringLiteralTypes.types
Expand Up @@ -12,7 +12,7 @@ type T3<T extends string> = string & `${T}`; // `${T}
>T3 : `${T}`

type T4<T extends string> = string & `${Capitalize<`${T}`>}`; // `${Capitalize<T>}`
>T4 : `${Capitalize<`${T}`>}`
>T4 : `${Capitalize<T>}`

function f1(a: boolean[], x: `${number}`) {
>f1 : (a: boolean[], x: `${number}`) => void
Expand Down