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

Wildly unpredictable behavior for resolving number-type index expressions #57296

Closed
rotu opened this issue Feb 5, 2024 · 5 comments
Closed
Assignees
Labels
Working as Intended The behavior described is the intended behavior; this is not a bug

Comments

@rotu
Copy link

rotu commented Feb 5, 2024

πŸ”Ž Search Terms

numeric index property access

πŸ•— Version & Regression Information

  • This is the behavior in every version I tried, and I reviewed the FAQ for entries about index signatures

⏯ Playground Link

https://www.typescriptlang.org/dev/bug-workbench/?target=9#code/PTAEAEBcEMCcHMCmkBcBRAygJgAxawFAAmiAxgDZyKikD2AdgM6Si2gqgDeBovoA2gA8UzWAEt68ALooA5KInxZAH1n0ArgFsARolgq1WhZNkAaHnyEoNOvTMO39qm8aXm+A4QAMAJJxu6sAC+XvYukOImBEEEdEwsAA4AjKAAvKz8SVIEIB4AegD8sQzMoAlYaRmySbLZuXyFxfFlAMyVtPyyDIi1OWANRXGlCQAs7Z04gji99byNQ4kArOOyAJL0AGYSYpAAnjP9c4MliQBs476cLTg49CF1h6DzJ2UA7CtJWC0A+iOLpwd8sdmgkABzjT4-P6nB5AprDACcKwAtDVYQN4YkkjgVgBqNF9OELMpJFLpDqXfgw+6EgZAA

πŸ’» Code

// @target:ES2022
declare const o : {
    [x:string]:'string'|'number'|'numstring',
    [x:number]:'number'|'numstring',
    [x:`${number}`]:'numstring'
}
const p1 = o[1]
//    ^?
const p2 = o['1']
//    ^?
const p3 = o['one']
//    ^?
const p4 = o['0x0']
//    ^?
const p5 = o['Infinity']
//    ^?
const p6 = o[`${300n}`]
//    ^?
const p7 = o['123_456']
//    ^?
const p8 = o[123_456]
//    ^?
const p9 = o['-1']
//    ^?
const p10 = o['+1']
//    ^?
const p11 = o[`${[6]}`]
//    ^?

πŸ™ Actual behavior

The inferred types of these index expressions are very unpredictable.

The indexes '1','0x0',`${300n}`, -1, +1 are inferred as `${number}`, and NOT as 'number'.
The indexes 1, 'Infinity', 123_456, are inferred as number
The indexes 'one', '123_456', '${[6]}' are inferred as string (these are understandable and not a problem)

πŸ™‚ Expected behavior

I expect:

  1. number and ${number} to catch the same cases, since (I believe) they express the same intent as object keys. Likely they should be considered a duplicate index signature in the type declaration.
  2. '1' and 1 to be treated the same since they retrieve the identical property.
  3. +1 and 0x0 to be inferred as non-numeric string, since (I think) the ${number} is supposed to be "the strings that a number may coerce to", not "the strings which are valid numeric literals". (though it's curious that strings containing binary, octal, and hex literals are accepted by the template string but NOT digit separators '123_456')
  4. Possibly ${300n} to be inferred as a non-numeric string. It's odd that it is treated as the string literal "300" in the index but a non-literal string when initializing a const variable.

Additional information about the issue

Originally mentioned here.

This can surely be split into multiple issues.

@RyanCavanaugh RyanCavanaugh added the Needs Investigation This issue needs a team member to investigate its status. label Feb 5, 2024
@ahejlsberg
Copy link
Member

ahejlsberg commented Feb 22, 2024

I wouldn't say our behavior is wildly unpredictable, but it is perhaps a bit surprising in some cases. These are the rules:

  • A string index signature matches any type that is assignable to string or number.
  • A number index signature matches any type that is assignable to number, the type `${number}`, and any string literal type with a value s for which (+s).toString() === s is true (i.e. numeric strings that "round trip").
  • A `${number}` index signature matches any non-empty string literal type with a value s for which +s produces a finite number.
  • When multiple index signatures are applicable, their element types are intersected.

Aspects of these rules that are somewhat surprising:

  • A number index signature requires matching numeric strings to "round-trip", but a `${number}` index signature does not. The round-trip requirement is intentional to reflect JavaScript numeric indexing semantics.
  • A number index signature matches the string literal types "Infinity", "-Infinity", and "NaN" (just like it matches the string literal types "0" and "123"). Again, this is intentional to reflect JavaScript's numeric indexing semantics.
  • A `${number}` index signature doesn't match "Infinity", "-Infinity", or "NaN" because we require the resulting number to be finite. This doesn't seem unreasonable.
  • A number index signature matches the template literal type `${number}`, in effect making `${number}` index signatures subsets of number index signatures. This is debatable since `${number}` doesn't come with the round-trip requirement, but it isn't entirely unreasonable (e.g. it makes types with `${number}` index signatures satisfy types with number index signatures).

Overall, it's not clear to me that anything needs to change here.

@ahejlsberg ahejlsberg added Working as Intended The behavior described is the intended behavior; this is not a bug and removed Needs Investigation This issue needs a team member to investigate its status. labels Feb 22, 2024
@rotu
Copy link
Author

rotu commented Feb 22, 2024

Wow. That makes so much more sense when you explain it like that!

The round-trip issue isn't a big problem, but did hinder me trying to grok the actual behavior.

The only thing I think is an important bug is that ${number} has the wrong semantics:

  1. It's surprising. " 1 \r \n \t " is assignable to ${number} even though it probably is not what's intended.
  2. It's backwards by analogy with other types. ${boolean}, ${null}, ${undefined} all mean "the string values which boolean/null/undefined may become when interpolated into a string", not "values that may coerce to boolean/null/undefined". These other types also forbid leading/trailing whitespace.

A ${number} index signature matches any string literal type with a value s for which +s produces a finite number.

The implementation doesn't quite match the description you gave. "" is not assignable to ${number} even though +"" evaluates to the number 0. I'm guessing you meant to say the strings that JSON.parse would turn into numbers. This would also explain why leading/trailing whitespace is tolerated.

@RyanCavanaugh
Copy link
Member

It's surprising. " 1 \r \n \t " is assignable to ${number} even though it probably is not what's intended.

I think most people's mental model is that if they have ${number} in a template string, it means they're gonna call parseInt/parseFloat/+n on it, all of which happily eat whitespace.

The reverse interpretation - that ${number} only accept strings which can come from numbers - has undesirable consequences like forbidding "2.0" (since no number toStrings to it).

@rotu
Copy link
Author

rotu commented Feb 22, 2024

I think most people's mental model is that if they have ${number} in a template string, it means they're gonna call parseInt/parseFloat/+n on it, all of which happily eat whitespace.

The reverse interpretation - that ${number} only accept strings which can come from numbers - has undesirable consequences like forbidding "2.0" (since no number toStrings to it).

That doesn't match my mental model, but perhaps I'm in the minority. I would expect that if something is number-parseable, you have already parsed it!

Anyway, thank you for explaining. This is clearly at least somewhat by design and I think my issues are covered by #46109 and #57404. Though the docs definitely could clarify what ${number} means!

@rotu rotu closed this as completed Feb 22, 2024
@ahejlsberg
Copy link
Member

Regarding

A ${number} index signature matches any string literal type with a value s for which +s produces a finite number.

It should say "non-empty string literal type". I've modified the original comment to reflect this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Working as Intended The behavior described is the intended behavior; this is not a bug
Projects
None yet
Development

No branches or pull requests

3 participants