Fixed an issue with unsound indexed access on unknown type param#49729
Fixed an issue with unsound indexed access on unknown type param#49729Andarist wants to merge 1 commit intomicrosoft:mainfrom
Conversation
|
This PR doesn't have any linked issues. Please open an issue that references this PR. From there we can discuss and prioritise. |
| !!! error TS2531: Object is possibly 'null'. | ||
| } | ||
|
|
||
| test(null, 'foo') |
There was a problem hiding this comment.
I've included those calls as there was a temporary regression related to this, you can check out here. I've figured out that this might be worth it to add here as part of the regression tests
| } | ||
| const facts = getTypeFacts(type); | ||
| if (facts & TypeFacts.IsUndefinedOrNull) { | ||
| if (facts & TypeFacts.IsUndefinedOrNull || type.flags & (TypeFlags.Intersection | TypeFlags.Instantiable) && (getBaseConstraintOfType(type) || unknownType) === unknownType) { |
There was a problem hiding this comment.
I'm not super sure if this is the best check, I've based this on the logic from getTypeFacts:
https://github.dev/microsoft/TypeScript/blob/b379e7fc791fc8646d9d255adce5127a6ab83099/src/compiler/checker.ts#L23562-L23564
In fact, I think this case probably should report a similar error like the one introduced here:
https://github.com/microsoft/TypeScript/pull/49119/files#diff-d9ab6589e714c71e657f601cf30ff51dfc607fc98419bf72e04f6b0fa92cc4b8R18815-R18817
Note that this logic has been further adjusted in:
https://github.com/microsoft/TypeScript/pull/49481/files#diff-d9ab6589e714c71e657f601cf30ff51dfc607fc98419bf72e04f6b0fa92cc4b8R18795
cc @ahejlsberg
|
I've checked a good chunk of the failures and I think they are all sound - those places should fail. Perhaps some of them could report different errors though or some errors could be skipped for brevity on the assumption that there is another related error reported for the given piece of code. |
|
Closed as it turns out that perhaps allowing such indexed access might be desirable: #49696 (comment) |
given the changed behavior from #49119 those indexed accesses within those generic functions should error as mentioned by @RyanCavanaugh here: #49696 (comment)