fix(55841) - Indexing with never should produce never
#55842
Conversation
typescript-bot
added
the
For Uncommitted Bug
jfet97
changed the title
never behaving like indexed by number
typescript-bot
added
For Backlog Bug
|
@typescript-bot pack this |
|
Heya @DanielRosenwasser, I've started to run the tarball bundle task on this PR at c556a70. You can monitor the build here. |
|
Heya @DanielRosenwasser, I've started to run the parallelized Definitely Typed test suite on this PR at c556a70. You can monitor the build here. Update: The results are in! |
|
Heya @DanielRosenwasser, I've started to run the regular perf test suite on this PR at c556a70. You can monitor the build here. Update: The results are in! |
|
Heya @DanielRosenwasser, I've started to run the diff-based top-repos suite on this PR at c556a70. You can monitor the build here. Update: The results are in! |
|
@typescript-bot test top300 |
|
Heya @DanielRosenwasser, I've started to run the diff-based top-repos suite on this PR at c556a70. You can monitor the build here. Update: The results are in! |
|
Hey @DanielRosenwasser, I've packed this into an installable tgz. You can install it for testing by referencing it in your and then running There is also a playground for this build and an npm module you can use via |
|
@DanielRosenwasser Here they are:
CompilerComparison Report - baseline..pr
System info unknown
Hosts
Scenarios
tsserverComparison Report - baseline..pr
System info unknown
Hosts
Scenarios
StartupComparison Report - baseline..pr
System info unknown
Hosts
Scenarios
Developer Information: |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
@DanielRosenwasser Here are the results of running the top-repos suite comparing Something interesting changed - please have a look. Details
|
|
Hey @DanielRosenwasser, the results of running the DT tests are ready. |
|
@DanielRosenwasser Here are the results of running the top-repos suite comparing Something interesting changed - please have a look. Details
|
|
Hey @gabritto, I've packed this into an installable tgz. You can install it for testing by referencing it in your and then running There is also a playground for this build and an npm module you can use via |
|
@Andarist We are of the same mind on this, I feel TS is quite inconsistent in these circumstances and I'm not sure this is the right change too. I re-opened this PR just because the correspondent issue has been accepted as a bug. You are right on the fact that this PR is not exactly about arrays and tuples and it is related with index signatures. OTOH I suspect that making
42, of course 😆. |
|
@typescript-bot test top300 |
|
Heya @gabritto, I've started to run the diff-based top-repos suite on this PR at 66159b5. You can monitor the build here. Update: The results are in! |
|
@gabritto Here are the results of running the top-repos suite comparing Everything looks good! |
|
Top tests all pass, but I do find this example kinda bad and it probably should be |
jfet97
changed the title
never behaving like indexed by numbernever should produce never
| @@ -18196,6 +18196,9 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker { | |||
| if (objectType.flags & (TypeFlags.Any | TypeFlags.Never)) { | |||
| return objectType; | |||
| } | |||
| if (indexType === neverType || indexType === implicitNeverType) { | |||
There was a problem hiding this comment.
I'd probably go with this:
| if (indexType === neverType || indexType === implicitNeverType) { | |
| if (indexType.flags & TypeFlags.Never && indexType !== silentNeverType) { |
There was a problem hiding this comment.
In that way we would change the indexing behaviour of unreachableNeverType too. Are you sure? XD
There was a problem hiding this comment.
Am I sure? Not at all. It seems reasonable to treat that the same way as the regular never here but who knows?
|
@gabritto I think now it's fixed and I added the bad cases as tests. What's your take? |
|
@typescript-bot pack this |
|
@typescript-bot run DT |
|
Heya @gabritto, I've started to run the diff-based user code test suite on this PR at 740a74f. You can monitor the build here. Update: The results are in! |
|
Heya @gabritto, I've started to run the parallelized Definitely Typed test suite on this PR at 740a74f. You can monitor the build here. Update: The results are in! |
|
Heya @gabritto, I've started to run the regular perf test suite on this PR at 740a74f. You can monitor the build here. Update: The results are in! |
|
Heya @gabritto, I've started to run the diff-based top-repos suite on this PR at 740a74f. You can monitor the build here. Update: The results are in! |
|
Hey @gabritto, I've packed this into an installable tgz. You can install it for testing by referencing it in your and then running There is also a playground for this build and an npm module you can use via |
|
@gabritto Here are the results of running the user test suite comparing There were infrastructure failures potentially unrelated to your change:
Otherwise... Something interesting changed - please have a look. Details
|
|
@gabritto Here they are:
tscComparison Report - baseline..pr
System info unknown
Hosts
Scenarios
tsserverComparison Report - baseline..pr
System info unknown
Hosts
Scenarios
startupComparison Report - baseline..pr
System info unknown
Hosts
Scenarios
Developer Information: |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Hey @gabritto, the results of running the DT tests are ready. |
|
@gabritto Here are the results of running the top-repos suite comparing Something interesting changed - please have a look. Details
|
|
@gabritto I think this error is right: However, I would say that this PR introduces a sort of breaking change. |
|
About the meaning of indexing with |
Fixes #55841 by doing the check before the one about index signatures.
Closed because: even if
{ a: 42 }[never]is already computed asnever, something likeRecord<string, boolean>[never]isboolean, notnever. So the current behaviour of arrays/tuples is not consistent with the one of object literals, but is consistent with other cases.Re-opened because the correspondent issue has been accepted as a bug.