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

anything is assignable to T[P] where T extends null/undefined/void/object/never #15371

Closed
zpdDG4gta8XKpMCd opened this issue Apr 25, 2017 · 6 comments · Fixed by #17870 or #17912
Closed
Assignees
Labels
Bug A bug in TypeScript Fixed A PR has been merged for this issue

Comments

@zpdDG4gta8XKpMCd
Copy link

zpdDG4gta8XKpMCd commented Apr 25, 2017

case 1

export function test<T extends object, P extends keyof T>(
    whatever: T, name: P
): T[P] {
    const result = Math.random() > 0.5 ? whatever[name] : 'Hello!'; // <-- expected T[P] | string, actual T[P]
    return result; // <-- returning unsound result
}

case 2

export function oneOrAnother<A, B>(one: A, another: B): A | B {
    return Math.random() > 0.5 ? one : another;
}

export function test<T extends object, P extends keyof T>(
    whatever: T, name: P
): T[P] {
    const result = oneOrAnother(whatever[name], 'Hello!'); // <-- result is clearly string | T[P] as expected
    return result; // <-- expected a type error, actual returning string | T[P] instead of T[P] no problem
}
@MrAndersen1
Copy link

You probably already found this out but these issues arise from extending object.
What one gains from extending object and why these issues arise when you do it is beyond me however.

@mhegazy mhegazy added the Bug A bug in TypeScript label May 5, 2017
@mhegazy mhegazy added this to the TypeScript 2.4 milestone May 5, 2017
@sandersn sandersn added this to Not started in Rolling Work Tracking May 16, 2017
@DanielRosenwasser DanielRosenwasser modified the milestones: TypeScript 2.5.1, TypeScript 2.5 Aug 4, 2017
@sandersn sandersn moved this from Not started to In Progress in Rolling Work Tracking Aug 16, 2017
@sandersn sandersn changed the title unsound value picking from mapped types T[P] | string should not be assignable to T[P] Aug 16, 2017
@sandersn sandersn changed the title T[P] | string should not be assignable to T[P] T[P] | string should not be assignable to T[P] where T extends object Aug 16, 2017
@sandersn
Copy link
Member

sandersn commented Aug 16, 2017

Here's a smaller repro:

function test<T extends object, P extends keyof T>(s: string, tp: T[P]): void {
    tp = s; // should error
}

@sandersn sandersn changed the title T[P] | string should not be assignable to T[P] where T extends object string should not be assignable to T[P] where T has a type constraint Aug 16, 2017
@sandersn sandersn changed the title string should not be assignable to T[P] where T has a type constraint string should not be assignable to T[P] where T has a primitive type constraint Aug 16, 2017
@mhegazy mhegazy modified the milestones: TypeScript 2.6, TypeScript 2.5.1 Aug 16, 2017
@sandersn sandersn changed the title string should not be assignable to T[P] where T has a primitive type constraint anything is assignable to T[P] where T extends null/undefined/void/object/never Aug 16, 2017
@sandersn
Copy link
Member

To check assignability of string to T[P], the compiler checks whether string is assignable to the constraint of T[P]. But getConstraintOfIndexedAccess results in anyType:

  • the type constraint of T[P] is object[string]
  • But object doesn't have a string index signature.
  • so getPropertyTypeForIndexType returns anyType.

So string is assignable to T[P] because the apparent type of T[P] is any, which is incorrect. I think getConstraintOfIndexedAccess is incorrect but I'm not yet sure what the fix is.

sandersn added a commit that referenced this issue Aug 17, 2017
1. `T[K]` now correctly produces `number` when
   `K extends string, T extends Record<K, number>`.
2. `T[K]` no longer allows any type to be assigned to it when
   `T extends object, K extends keyof T`.

Previously both of these cases failed in
getConstraintOfIndexedAccessType because both bases followed `K`'s base
constraint to `string` and then incorrectly produced `any` for types
(like `object`) with no string index signature. In (1), this produced an
error in checkBinaryLikeExpression`. In (2), this failed to produce an
error in `checkTypeRelatedTo`.
@sandersn
Copy link
Member

Fix is up at #17870.

@sandersn sandersn added the Fixed A PR has been merged for this issue label Aug 17, 2017
@sandersn sandersn moved this from In Progress to In Review in Rolling Work Tracking Aug 17, 2017
@sandersn sandersn reopened this Aug 18, 2017
sandersn added a commit that referenced this issue Aug 18, 2017
1. `T[K]` now correctly produces `number` when
   `K extends string, T extends Record<K, number>`.

2. `T[K]` no longer allows any type to be assigned to it when
   `T extends object, K extends keyof T`.

Previously both of these cases failed in
getConstraintOfIndexedAccessType because both bases followed `K`'s base
constraint to `string` and then incorrectly produced `any` for types
(like `object`) with no string index signature. In (1), this produced an
error in checkBinaryLikeExpression`. In (2), this failed to produce an
error in `checkTypeRelatedTo`.
@sandersn
Copy link
Member

The previous PR got merged by mistake and is now reverted; #17912 is the PR with the fix now.

@zpdDG4gta8XKpMCd
Copy link
Author

looks like this pr might be the reason to #12215 (comment)

@microsoft microsoft locked and limited conversation to collaborators Jul 3, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Bug A bug in TypeScript Fixed A PR has been merged for this issue
Projects
5 participants