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

Invalid return value accepted by generic indexed access type with index signature #60700

Closed
mkantor opened this issue Dec 6, 2024 · 10 comments
Closed
Labels
Design Limitation Constraints of the existing architecture prevent this from being fixed

Comments

@mkantor
Copy link
Contributor

mkantor commented Dec 6, 2024

🔎 Search Terms

"return type" "generic" "type parameter" "indexed access" "index signature" "assignable" "constraint"

🕗 Version & Regression Information

  • This is the behavior in every version I tried, and I reviewed the FAQ for entries about generic indexed access

⏯ Playground Link

https://www.typescriptlang.org/play/?ts=5.7.2#code/C4TwDgpgBAShwFcBOA7AzlAvFA3gKCigEMAuKAciPIKgG0BrCEMtYJASxQHMBdFtzlzwBfPHgDGAe3TAoaABaSEAGwAmAIQgBBFAFEkSSUixQAPAGkoEAB7AIKVRlYduAPgAUAfXplzASjI4RFQ0WnMeLFcKSUkwNGoJaVYoACMYgFsTBSU1TR19QyR3SnI-PAB6csJqgD0AfkT0SWUIADplSS53NMl0vyhKqA6uDAAiGLjRgBooFElZUaJRoA

💻 Code

type Returns = {
  a: 'a'
  [key: string]: string
}

const shouldBeAnError = <K extends string>(_k: K): Returns[K] => 'oops'

const boom = shouldBeAnError('a')
//    ^? - const boom: "a"
console.log(boom) // logs "oops", not "a"

🙁 Actual behavior

The invalid return value is allowed.

🙂 Expected behavior

The invalid return value should be flagged as a type error.

Additional information about the issue

Here's another example, though this one is more dubious since the Returns type is nonsensical to begin with.

Similar code without an index signature is correctly flagged, and so is code which uses a non-overlapping index signature. Also the expected error appears when the generic constraint is made more specific.

EDIT: It was brought to my attention in the comments that this doesn't require anything to be generic. Assigning to any indexed access of a type with an index signature can be problematic, even when everything is concrete. I would expect this to error also, for the same reason that this does.

@MartinJohns
Copy link
Contributor

It's a design limitation. Types with indexer and known properties are inherently unsound. For this to work you'd need a type "any string, except ..", which would require negated types.

@mkantor
Copy link
Contributor Author

mkantor commented Dec 6, 2024

For this to work you'd need a type "any string, except ..", which would require negated types.

Can you explain why? If it were just my "dubious" example I'd get it, but there's generally no trouble when all other known properties are assignable to the index signature value type. As far as I can tell this particular problem only arises when there's a type parameter involved.

Here's an example of similar errors being correctly caught by the compiler.

@mkantor
Copy link
Contributor Author

mkantor commented Dec 6, 2024

From my not-very-familiar-with-the-internals perspective, it seems like the issue is that the value 'oops' is being checked against the type Returns[constraint of K] (which is Returns[string]), but what call sites see is the type Returns[K] (which is narrower than what was checked).

@mkantor
Copy link
Contributor Author

mkantor commented Dec 6, 2024

Interestingly, the error is caught when the constraint is more specific:

type Returns = {
  a: 'a'
  [key: string]: string
}

const shouldBeAnError = <K extends 'a' | 'b'>(_k: K): Returns[K] => 'oops'
//                                                                  ^^^^^^
// Type 'string' is not assignable to type 'Returns[K]'.
//   Type '"oops"' is not assignable to type '"a"'.

@MartinJohns
Copy link
Contributor

MartinJohns commented Dec 6, 2024

Can you explain why?

Because the type says any string results in a string property: [key: string]: string

This is perfectly valid code:

const returns: Returns = { a: 'a' }
const keyAsString = 'a' as string
returns[keyAsString] = 'abc'

When the key used to access is typed string, then there's no way for the compiler to know that it may refer to the property a that only allows the string literal "a". There is no way to type it as "any string key, except the key a".

@mkantor
Copy link
Contributor Author

mkantor commented Dec 6, 2024

There is no way to type it as "any string key, except the key a".

My mental model for this kind of compatible index signature (i.e. not the dubious example) doesn't require such a type.

{ [key: string]: string } is a supertype of { a: 'a' }, and I do not expect Type & Supertype to allow an inhabitant that is not assignable to Type. I think your latest example should also be an error.

I would expect these two examples to be reasoned through similarly (OtherType is the only difference):

type OtherType = { a?: string, b?: string }

type Returns = { a: 'a' } & OtherType

declare const returns: Returns
const key = 'a' as keyof Returns
returns[key] = 'abc'
//^^^^^^^^^^
// Type '"abc"' is not assignable to type '"a"'.
type OtherType = { [key: string]: string }

type Returns = { a: 'a' } & OtherType

declare const returns: Returns
const key = 'a' as keyof Returns
returns[key] = 'abc' // no error, but should be?

In both cases the Returns type carries with it the information that the property a must have a value assignable to 'a'. But in the latter case this is not enforced when assigning to property using a key whose type exactly matches the index signature key type. It seems to me that this could accomplished without negated types, especially since it already happens in some cases.

@mkantor
Copy link
Contributor Author

mkantor commented Dec 6, 2024

This example correctly errors too (without any need for negated types):

type OtherType = { [key: string | symbol]: string }

type Returns = { [key: string]: 'a' } & OtherType

declare const returns: Returns
declare const key: keyof Returns
returns[key] = 'abc'
//^^^^^^^^^^
// Type '"abc"' is not assignable to type '"a"'.

@RyanCavanaugh
Copy link
Member

RyanCavanaugh commented Dec 6, 2024

Design limitations that apply in some situations may not apply in others. All these examples are different in very fundamental ways. A type parameter constrained to T is not the same as a value of type T. Same for when there are literals and when there aren't.

The implied algorithm here is that we should iterate K' through keyof Returns, take Returns[K'], and intersect all those types together, since that's what would produce the correct result "a". But that algorithm expands very poorly. Let's say you wrote

type Returns = {
  a: 'a',
  b: 'b',
  [key: string]: string
}

const shouldBeAnError = <K extends string>(_k: K): Returns[K] => ???

Now the only legal return of this function is never, which is obviously not ideal. We'd go from under-correcting to over-correcting beyond the point of salvageability; you'd need a cast, but that cast would be effectively unchecked, since it'd be checking to see if it's assignable from never (which everything is). The current rule at least detects when you massively mess up, but the implied rule wouldn't ever detect any error in practice.

@RyanCavanaugh RyanCavanaugh added the Design Limitation Constraints of the existing architecture prevent this from being fixed label Dec 6, 2024
@mkantor
Copy link
Contributor Author

mkantor commented Dec 6, 2024

Now the only legal return of this function is never

In my opinion, the only legal return should be returns[k], just like it is when the index signature is omitted:

type Returns = {
  a: 'a',
  b: 'b',
}

declare const returns: Returns

const isRightfullyAnError = <K extends keyof Returns>(k: K): Returns[K] => 'a'
const isAllowed = <K extends keyof Returns>(k: K): Returns[K] => returns[k]
type Returns = {
  a: 'a',
  b: 'b',
  [key: string]: string
}

declare const returns: Returns

const shouldBeAnError = <K extends keyof Returns>(k: K): Returns[K] => 'a'
const isAllowed = <K extends keyof Returns>(k: K): Returns[K] => returns[k]

I've since realized this generic example is a specific case of a more general problem though. I'll open another issue covering the general case and close this one. (I'll totally understand if that one is also considered a design limitation.)

@mkantor
Copy link
Contributor Author

mkantor commented Dec 6, 2024

Closing in favor of #60703.

@mkantor mkantor closed this as completed Dec 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Design Limitation Constraints of the existing architecture prevent this from being fixed
Projects
None yet
Development

No branches or pull requests

3 participants