Check possible undefined on LHS IndexedType for logical OR short-circuit #29794
Conversation
collin5
changed the title
| /** | ||
| * Returns true if any of the items in array satisfies predicate 'if provided' or is truthy | ||
| */ | ||
| export function any<T>(array: ReadonlyArray<T>, predicate?: (i: any) => boolean): boolean { |
There was a problem hiding this comment.
This function is built-in; it's Array#some
| @@ -22763,6 +22763,10 @@ namespace ts { | |||
| getUnionType([extractDefinitelyFalsyTypes(strictNullChecks ? leftType : getBaseTypeOfLiteralType(rightType)), rightType]) : | |||
| leftType; | |||
| case SyntaxKind.BarBarToken: | |||
| leftType = leftType.flags & TypeFlags.IndexedAccess ? | |||
| any(getPropertiesOfType((<IndexedAccessType>leftType).objectType), prop => !!(prop.flags & SymbolFlags.Optional)) ? | |||
There was a problem hiding this comment.
I don't think this is right - checking is any prop in the indexed access's type is optional and then returning never for its type if so seems very wrong - the potential type of the LHS is completely lost here, and it loses the nuance of the individual property accessed. This needs #29317 to be correctly solved - the LHS type should be leftType & not 0 & not null & not undefined & not "", which handles generics appropriately.
There was a problem hiding this comment.
The fact that we don't have more tests this change, as-is, breaks is actually a little disturbing.
|
I'm cleaning up our PR backlog and this one is pretty stale, so I'm going to close it. The issue this fixes, #7719, is closed as fixed by #10357, so I'm not really sure if it's actually fixed. @collin5 you can consider creating a new PR if it's not. @weswigham, you said this probably needs negated types (from #29317) for a real fix, though. Do you still think that? |
Fixes #29642