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.

Sign up for GitHub

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

Improve relationship checking with keyof GenericMappedType as the source type #56246

Closed
wants to merge 7 commits into from
Closed

Improve relationship checking with keyof GenericMappedType as the source type #56246

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
b9a01cb
Instantiate `nameType` of filtering mapped types when creating index …
Andarist Oct 28, 2023
e13ad38
just always instantiate the name type
Andarist Oct 28, 2023
880d4af
Merge remote-tracking branch 'origin/main' into instantiate-name-type…
Andarist Dec 26, 2023
4c69959
Revert `nameType` intantiation
Andarist Dec 27, 2023
585091e
Improve relationship checking
Andarist Dec 27, 2023
ac23642
Reuse the logic in `getGenericMappedTypeKeys`
Andarist Dec 28, 2023
3d1fe73
Merge remote-tracking branch 'origin/main' into instantiate-name-type…
Andarist Nov 25, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 12 additions & 6 deletions src/compiler/checker.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17449,7 +17449,7 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
* reduction in the constraintType) when possible.
* @param noIndexSignatures Indicates if _string_ index signatures should be elided. (other index signatures are always reported)
*/
function getIndexTypeForMappedType(type: MappedType, indexFlags: IndexFlags) {
function getIndexTypeForMappedType(type: MappedType, indexFlags: IndexFlags): Type {
const typeParameter = getTypeParameterFromMappedType(type);
const constraintType = getConstraintTypeFromMappedType(type);
const nameType = getNameTypeFromMappedType(type.target as MappedType || type);
Expand All @@ -17468,6 +17468,12 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
const modifiersType = getApparentType(getModifiersTypeFromMappedType(type)); // The 'T' in 'keyof T'
forEachMappedTypePropertyKeyTypeAndIndexSignatureKeyType(modifiersType, TypeFlags.StringOrNumberLiteralOrUnique, !!(indexFlags & IndexFlags.StringsOnly), addMemberForKeyType);
}
else if (nameType) {
const modifiersIndex = getIndexType(getModifiersTypeFromMappedType(type), indexFlags, UnionReduction.None);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

UnionReduction.None was added here to keep this assignment allowed.

I think that without it the union reduction was forgetting about 'str' since it's a subtype of string. string is a valid input K but it is meant to be filtered away by DistributiveNonIndex. So with the union reduction on, I have ended up removing 'str' entirely - breaking the test case.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm somewhat unsure - but maybe the union reduction could still mess this up when another union gets created manually with that as one of its members? Maybe I should explore a different fix that would do the same~ instantiation but later on, when comparing some constraints?

Copy link
Member

Choose a reason for hiding this comment

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

UnionReduction.None is, in some ways, a hack to make contextual typing work a little bit better (and avoid the perf overhead of subtype reduction we don't need to bother doing). The union members could, very easily, evaporate later on down the line.

IMO, it'd be better to leave this case as producing a generic keyof type (especially since this nameType could still just be filtering a homomorphic mapped type), and improve constraint-following logic inside the generic-keyof-over-mapped-type case in structuredTypeRelatedToWorker - specifically, we already have a generic-index-type-target case that does the right thing (tm) - that probably just needs to be brought up and situationally applied when the source is a generic index, too. Since our only special case for keyof on the source side right now is

            else if (sourceFlags & TypeFlags.Index) {
                if (result = isRelatedTo(keyofConstraintType, target, RecursionFlags.Source, reportErrors)) {
                    return result;
                }
            }

it could probably use some more specifics, since it's definitely true that generic keyofs (over mapped types) can have constraints more specific than string | number | symbol.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you for the advice! I now reused the mentioned logic for keyof GenericMappedType as the target type so now the whole fix is within structuredTypeRelatedToWorker

Copy link
Member

Choose a reason for hiding this comment

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

Hah, I never linked it here, but after I looked over this, I was assigned another issue that tracked back to the same thing, so I ended up writing #56742, which basically scooped this change. Do you want to sync this and at least add the new test cases? In #56742 I used much more specific carve-outs of which keyof types this applies to than the very general check here (and actually use a slightly different source key type).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

#56742 didn't fix #56239 , see the nightly playground here. I'll investigate what's the difference between our approaches in the coming days and report back.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The reason this wasn't fixed is that keyof { [K in keyof TActors as K & string]: { src: K; logic: TActors[K]; }; } wasn't treated as a deferred index type (although I'd argue that it clearly is deferred). So the logic added by #56742 couldn't kick in.

I recently changed that in #60528 and I think that essentially supersedes this PR so I just merged it in there now to keep the tests.

const mapper = makeUnaryTypeMapper(getTypeParameterFromMappedType(type), modifiersIndex);
const nameMapper = combineTypeMappers(type.mapper, mapper);
return instantiateType(nameType, nameMapper);
}
else {
// we have a generic index and a homomorphic mapping (but a distributive key remapping) - we need to defer the whole `keyof whatever` for later
// since it's not safe to resolve the shape of modifier type
Expand Down Expand Up @@ -17554,13 +17560,13 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
return !!(keyType.flags & include || keyType.flags & TypeFlags.Intersection && some((keyType as IntersectionType).types, t => isKeyTypeIncluded(t, include)));
}

function getLiteralTypeFromProperties(type: Type, include: TypeFlags, includeOrigin: boolean) {
function getLiteralTypeFromProperties(type: Type, include: TypeFlags, includeOrigin: boolean, unionReduction = UnionReduction.Literal) {
const origin = includeOrigin && (getObjectFlags(type) & (ObjectFlags.ClassOrInterface | ObjectFlags.Reference) || type.aliasSymbol) ? createOriginIndexType(type) : undefined;
const propertyTypes = map(getPropertiesOfType(type), prop => getLiteralTypeFromProperty(prop, include));
const indexKeyTypes = map(getIndexInfosOfType(type), info =>
info !== enumNumberIndexInfo && isKeyTypeIncluded(info.keyType, include) ?
info.keyType === stringType && include & TypeFlags.Number ? stringOrNumberType : info.keyType : neverType);
return getUnionType(concatenate(propertyTypes, indexKeyTypes), UnionReduction.Literal, /*aliasSymbol*/ undefined, /*aliasTypeArguments*/ undefined, origin);
return getUnionType(concatenate(propertyTypes, indexKeyTypes), unionReduction, /*aliasSymbol*/ undefined, /*aliasTypeArguments*/ undefined, origin);
}

function shouldDeferIndexType(type: Type, indexFlags = IndexFlags.None) {
Expand All @@ -17571,16 +17577,16 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
type.flags & TypeFlags.Intersection && maybeTypeOfKind(type, TypeFlags.Instantiable) && some((type as IntersectionType).types, isEmptyAnonymousObjectType));
}

function getIndexType(type: Type, indexFlags = defaultIndexFlags): Type {
function getIndexType(type: Type, indexFlags = defaultIndexFlags, unionReduction?: UnionReduction): Type {
type = getReducedType(type);
return shouldDeferIndexType(type, indexFlags) ? getIndexTypeForGenericType(type as InstantiableType | UnionOrIntersectionType, indexFlags) :
type.flags & TypeFlags.Union ? getIntersectionType(map((type as UnionType).types, t => getIndexType(t, indexFlags))) :
type.flags & TypeFlags.Intersection ? getUnionType(map((type as IntersectionType).types, t => getIndexType(t, indexFlags))) :
type.flags & TypeFlags.Intersection ? getUnionType(map((type as IntersectionType).types, t => getIndexType(t, indexFlags, unionReduction)), unionReduction) :
getObjectFlags(type) & ObjectFlags.Mapped ? getIndexTypeForMappedType(type as MappedType, indexFlags) :
type === wildcardType ? wildcardType :
type.flags & TypeFlags.Unknown ? neverType :
type.flags & (TypeFlags.Any | TypeFlags.Never) ? keyofConstraintType :
getLiteralTypeFromProperties(type, (indexFlags & IndexFlags.NoIndexSignatures ? TypeFlags.StringLiteral : TypeFlags.StringLike) | (indexFlags & IndexFlags.StringsOnly ? 0 : TypeFlags.NumberLike | TypeFlags.ESSymbolLike), indexFlags === defaultIndexFlags);
getLiteralTypeFromProperties(type, (indexFlags & IndexFlags.NoIndexSignatures ? TypeFlags.StringLiteral : TypeFlags.StringLike) | (indexFlags & IndexFlags.StringsOnly ? 0 : TypeFlags.NumberLike | TypeFlags.ESSymbolLike), indexFlags === defaultIndexFlags, unionReduction);
}

function getExtractStringType(type: Type) {
Expand Down
8 changes: 4 additions & 4 deletions tests/baselines/reference/keyRemappingKeyofResult.types
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -156,19 +156,19 @@ function g<T>() {
// no string index signature, right?

type Oops = keyof Remapped;
>Oops : keyof { [K in keyof ({ [k: string]: any; str: any; [sym]: any; } & T) as K extends unknown ? {} extends Record<K, any> ? never : K : never]: any; }
>Oops : unique symbol | "str" | (keyof T extends unknown ? {} extends Record<keyof T, any> ? never : keyof T : never)

let x: Oops;
>x : keyof { [K in keyof ({ [k: string]: any; str: any; [sym]: any; } & T) as K extends unknown ? {} extends Record<K, any> ? never : K : never]: any; }
>x : unique symbol | "str" | (keyof T extends unknown ? {} extends Record<keyof T, any> ? never : keyof T : never)

x = sym;
>x = sym : unique symbol
>x : keyof { [K in keyof ({ [k: string]: any; str: any; [sym]: any; } & T) as K extends unknown ? {} extends Record<K, any> ? never : K : never]: any; }
>x : unique symbol | "str" | (keyof T extends unknown ? {} extends Record<keyof T, any> ? never : keyof T : never)
>sym : unique symbol

x = "str";
>x = "str" : "str"
>x : keyof { [K in keyof ({ [k: string]: any; str: any; [sym]: any; } & T) as K extends unknown ? {} extends Record<K, any> ? never : K : never]: any; }
>x : unique symbol | "str" | (keyof T extends unknown ? {} extends Record<keyof T, any> ? never : keyof T : never)
>"str" : "str"
}

Expand Down
84 changes: 84 additions & 0 deletions tests/baselines/reference/keyRemappingKeyofResult2.symbols
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,84 @@
//// [tests/cases/compiler/keyRemappingKeyofResult2.ts] ////

=== keyRemappingKeyofResult2.ts ===
// https://github.com/microsoft/TypeScript/issues/56239

type Values<T> = T[keyof T];
>Values : Symbol(Values, Decl(keyRemappingKeyofResult2.ts, 0, 0))
>T : Symbol(T, Decl(keyRemappingKeyofResult2.ts, 2, 12))
>T : Symbol(T, Decl(keyRemappingKeyofResult2.ts, 2, 12))
>T : Symbol(T, Decl(keyRemappingKeyofResult2.ts, 2, 12))

type ProvidedActor = {
>ProvidedActor : Symbol(ProvidedActor, Decl(keyRemappingKeyofResult2.ts, 2, 28))

src: string;
>src : Symbol(src, Decl(keyRemappingKeyofResult2.ts, 4, 22))

logic: unknown;
>logic : Symbol(logic, Decl(keyRemappingKeyofResult2.ts, 5, 14))

};

interface StateMachineConfig<TActors extends ProvidedActor> {
>StateMachineConfig : Symbol(StateMachineConfig, Decl(keyRemappingKeyofResult2.ts, 7, 2))
>TActors : Symbol(TActors, Decl(keyRemappingKeyofResult2.ts, 9, 29))
>ProvidedActor : Symbol(ProvidedActor, Decl(keyRemappingKeyofResult2.ts, 2, 28))

invoke: {
>invoke : Symbol(StateMachineConfig.invoke, Decl(keyRemappingKeyofResult2.ts, 9, 61))

src: TActors["src"];
>src : Symbol(src, Decl(keyRemappingKeyofResult2.ts, 10, 11))
>TActors : Symbol(TActors, Decl(keyRemappingKeyofResult2.ts, 9, 29))

};
}

declare function setup<TActors extends Record<string, unknown>>(_: {
>setup : Symbol(setup, Decl(keyRemappingKeyofResult2.ts, 13, 1))
>TActors : Symbol(TActors, Decl(keyRemappingKeyofResult2.ts, 15, 23))
>Record : Symbol(Record, Decl(lib.es5.d.ts, --, --))
>_ : Symbol(_, Decl(keyRemappingKeyofResult2.ts, 15, 64))

actors: {
>actors : Symbol(actors, Decl(keyRemappingKeyofResult2.ts, 15, 68))

[K in keyof TActors]: TActors[K];
>K : Symbol(K, Decl(keyRemappingKeyofResult2.ts, 17, 5))
>TActors : Symbol(TActors, Decl(keyRemappingKeyofResult2.ts, 15, 23))
>TActors : Symbol(TActors, Decl(keyRemappingKeyofResult2.ts, 15, 23))
>K : Symbol(K, Decl(keyRemappingKeyofResult2.ts, 17, 5))

};
}): {
createMachine: (
>createMachine : Symbol(createMachine, Decl(keyRemappingKeyofResult2.ts, 19, 5))

config: StateMachineConfig<
>config : Symbol(config, Decl(keyRemappingKeyofResult2.ts, 20, 18))
>StateMachineConfig : Symbol(StateMachineConfig, Decl(keyRemappingKeyofResult2.ts, 7, 2))

Values<{
>Values : Symbol(Values, Decl(keyRemappingKeyofResult2.ts, 0, 0))

[K in keyof TActors as K & string]: {
>K : Symbol(K, Decl(keyRemappingKeyofResult2.ts, 23, 9))
>TActors : Symbol(TActors, Decl(keyRemappingKeyofResult2.ts, 15, 23))
>K : Symbol(K, Decl(keyRemappingKeyofResult2.ts, 23, 9))

src: K;
>src : Symbol(src, Decl(keyRemappingKeyofResult2.ts, 23, 45))
>K : Symbol(K, Decl(keyRemappingKeyofResult2.ts, 23, 9))

logic: TActors[K];
>logic : Symbol(logic, Decl(keyRemappingKeyofResult2.ts, 24, 17))
>TActors : Symbol(TActors, Decl(keyRemappingKeyofResult2.ts, 15, 23))
>K : Symbol(K, Decl(keyRemappingKeyofResult2.ts, 23, 9))

};
}>
>,
) => void;
};

59 changes: 59 additions & 0 deletions tests/baselines/reference/keyRemappingKeyofResult2.types
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
//// [tests/cases/compiler/keyRemappingKeyofResult2.ts] ////

=== keyRemappingKeyofResult2.ts ===
// https://github.com/microsoft/TypeScript/issues/56239

type Values<T> = T[keyof T];
>Values : Values<T>

type ProvidedActor = {
>ProvidedActor : { src: string; logic: unknown; }

src: string;
>src : string

logic: unknown;
>logic : unknown

};

interface StateMachineConfig<TActors extends ProvidedActor> {
invoke: {
>invoke : { src: TActors["src"]; }

src: TActors["src"];
>src : TActors["src"]

};
}

declare function setup<TActors extends Record<string, unknown>>(_: {
>setup : <TActors extends Record<string, unknown>>(_: { actors: { [K in keyof TActors]: TActors[K]; }; }) => { createMachine: (config: StateMachineConfig<Values<{ [K in keyof TActors as K & string]: { src: K; logic: TActors[K]; }; }>>) => void; }
>_ : { actors: { [K in keyof TActors]: TActors[K]; }; }

actors: {
>actors : { [K in keyof TActors]: TActors[K]; }

[K in keyof TActors]: TActors[K];
};
}): {
createMachine: (
>createMachine : (config: StateMachineConfig<Values<{ [K in keyof TActors as K & string]: { src: K; logic: TActors[K]; }; }>>) => void

config: StateMachineConfig<
>config : StateMachineConfig<Values<{ [K in keyof TActors as K & string]: { src: K; logic: TActors[K]; }; }>>

Values<{
[K in keyof TActors as K & string]: {
src: K;
>src : K

logic: TActors[K];
>logic : TActors[K]

};
}>
>,
) => void;
};

12 changes: 6 additions & 6 deletions tests/baselines/reference/mappedTypeAsClauses.types
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -262,25 +262,25 @@ type NameMap = { 'a': 'x', 'b': 'y', 'c': 'z' };
// Distributive, will be simplified

type TS0<T> = keyof { [P in keyof T as keyof Record<P, number>]: string };
>TS0 : keyof { [P in keyof T as P]: string; }
>TS0 : keyof T

type TS1<T> = keyof { [P in keyof T as Extract<P, 'a' | 'b' | 'c'>]: string };
>TS1 : keyof { [P in keyof T as Extract<P, "a" | "b" | "c">]: string; }
>TS1 : Extract<keyof T, "a" | "b" | "c">

type TS2<T> = keyof { [P in keyof T as P & ('a' | 'b' | 'c')]: string };
>TS2 : keyof { [P in keyof T as P & ("a" | "b" | "c")]: string; }
>TS2 : keyof T & ("a" | "b" | "c")

type TS3<T> = keyof { [P in keyof T as Exclude<P, 'a' | 'b' | 'c'>]: string };
>TS3 : keyof { [P in keyof T as Exclude<P, "a" | "b" | "c">]: string; }
>TS3 : Exclude<keyof T, "a" | "b" | "c">

type TS4<T> = keyof { [P in keyof T as NameMap[P & keyof NameMap]]: string };
>TS4 : keyof { [P in keyof T as NameMap[P & keyof NameMap]]: string; }
>TS4 : NameMap[keyof T & keyof NameMap]

type TS5<T> = keyof { [P in keyof T & keyof NameMap as NameMap[P]]: string };
>TS5 : NameMap[keyof T & "a"] | NameMap[keyof T & "b"] | NameMap[keyof T & "c"]

type TS6<T, U, V> = keyof { [ K in keyof T as V & (K extends U ? K : never)]: string };
>TS6 : keyof { [K in keyof T as V & (K extends U ? K : never)]: string; }
>TS6 : V & (keyof T extends U ? U & keyof T : never)

// Non-distributive, won't be simplified

Expand Down
34 changes: 34 additions & 0 deletions tests/cases/compiler/keyRemappingKeyofResult2.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
// @strict: true
// @noEmit: true

// https://github.com/microsoft/TypeScript/issues/56239

type Values<T> = T[keyof T];

type ProvidedActor = {
src: string;
logic: unknown;
};

interface StateMachineConfig<TActors extends ProvidedActor> {
invoke: {
src: TActors["src"];
};
}

declare function setup<TActors extends Record<string, unknown>>(_: {
actors: {
[K in keyof TActors]: TActors[K];
};
}): {
createMachine: (
config: StateMachineConfig<
Values<{
[K in keyof TActors as K & string]: {
src: K;
logic: TActors[K];
};
}>
>,
) => void;
};
Loading