Handle 'keyof' for generic tuple types #39218
Conversation
|
@DanielRosenwasser Would be good if we can get this into the 4.0 beta. |
| @@ -12647,6 +12642,11 @@ namespace ts { | |||
| /*readonly*/ false, target.labeledElementDeclarations && target.labeledElementDeclarations.slice(index, endIndex)); | |||
| } | |||
|
|
|||
| function getKnownKeysOfTupleType(type: TupleTypeReference) { | |||
| return getUnionType(append(arrayOf(type.target.fixedLength, i => getLiteralType("" + i)), | |||
There was a problem hiding this comment.
If the tuple has a rest, shouldn't the keys contain number?
There was a problem hiding this comment.
The keys already contain number because all array types have an index signature.
There was a problem hiding this comment.
Hm. I question if we should be OK with 2 being assignable to keyof [...T] where T extends [] | [object]. But... I guess it's inline with how we already handle the keys of tuples?
| @@ -10195,7 +10195,8 @@ namespace ts { | |||
| return type; | |||
| } | |||
| if (type.flags & TypeFlags.Index) { | |||
| return getIndexType(getApparentType((<IndexType>type).type)); | |||
| const t = (<IndexType>type).type; | |||
| return isTupleType(t) ? getKnownKeysOfTupleType(t) : getIndexType(getApparentType(t)); | |||
There was a problem hiding this comment.
What about the keyof T in {[K in keyof T]: any} where T extends [...Some, ...Variadic, ...Tuple]? That will fall back to getIndexType (since T isn't a tuple type), which seems to still be lacking a specialized implementation for variadic tuples? Or is the getLiteralTypeFromProperties fallback sufficient? If so, why would we need to call out isTupleType here?
There was a problem hiding this comment.
The apparent type of a tuple type is simply the tuple type itself, so we need a special case for (generic) tuple types here as we'd otherwise just create the same type again. I'm not quite sure what you're asking about the mapped type. It's homomorphic, which means we'll just iterate over getPropertiesOfType for T, which works just fine for a type parameter constrained to a generic tuple type.
There was a problem hiding this comment.
Ok, but what if we have a non-homomorphic keyof T? (Eg, because it appears via instantiation). The apparent type of the tuple being itself is fine, so long as getIndexType handles it. If it does, then this is redundant. If it does not, then this doesn't handle the case where the apparent type resolves to a tuple type.
There was a problem hiding this comment.
Yeah, seems better to do the check after getApparentType. Although it is surprisingly hard to construct an example where this fails because we typically call getApparentType on a mapped type before resolving members and that turns a MappedType<keyof T> into MappedType<keyof [...Some, ...Variadic]> before we even get to this code. But anyways, I'll make the change.
|
@weswigham Should be good to go now. |
|
Does this fix/relate to #27995? |
|
@treybrisbane No. |
# Conflicts: # tests/baselines/reference/variadicTuples1.errors.txt # tests/baselines/reference/variadicTuples1.symbols
* upstream/master: LEGO: check in for master to temporary branch. Preserve newlines between try/catch/finally, if/else, do/while (microsoft#39280) not narrow static property without type annotation in constructor. (microsoft#39252) Switch to ES Map/Set internally (microsoft#33771) fix(38840): omit completions for a spread like argument in a function definition (microsoft#38897) fix(38785): include in NavigationBar child items from default exported functions (microsoft#38915) LEGO: check in for master to temporary branch. LEGO: check in for master to temporary branch. Avoid effect of element access expression (microsoft#39174) Update typescript-eslint to 3.4.1-alpha.1 (microsoft#39260) Handle 'keyof' for generic tuple types (microsoft#39218) Disable unsound T[K] rule in subtype relations (microsoft#39249) LEGO: check in for master to temporary branch. Upgrade typescript-eslint version (microsoft#39242) Handle recursive type references up to a certain level of expansion in inference (microsoft#38011) Do not consider binding patterns in contextual types for return type inference where all the signature type parameters have defaults (microsoft#39081) LEGO: check in for master to temporary branch. # Conflicts: # src/compiler/program.ts # src/compiler/types.ts
This PR implements support for
keyof Twith generic tuple types that was missing in #39094.