-
Notifications
You must be signed in to change notification settings - Fork 12.8k
Improve error message for index signature on generic type when writing #55906
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
Merged
Merged
Changes from all commits
Commits
Show all changes
6 commits
Select commit
Hold shift + click to select a range
f855770
add temporary new error message
gabritto a7e55f9
update error and new test
gabritto 1b27a3c
update other tests
gabritto a724e47
Merge branch 'main' into gabritto/issue47357
gabritto c62a961
remove renamed baseline
gabritto 3d781f5
update tests
gabritto File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
19 changes: 19 additions & 0 deletions
19
tests/baselines/reference/cannotIndexGenericWritingError.errors.txt
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,19 @@ | ||
cannotIndexGenericWritingError.ts(4,5): error TS2862: Type 'T' is generic and can only be indexed for reading. | ||
cannotIndexGenericWritingError.ts(8,5): error TS2862: Type 'T' is generic and can only be indexed for reading. | ||
|
||
|
||
==== cannotIndexGenericWritingError.ts (2 errors) ==== | ||
// From #47357 | ||
|
||
function foo<T extends Record<string | symbol, any>>(target: T, p: string | symbol) { | ||
target[p] = ""; // error | ||
~~~~~~~~~ | ||
!!! error TS2862: Type 'T' is generic and can only be indexed for reading. | ||
} | ||
|
||
function foo2<T extends number[] & { [s: string]: number | string }>(target: T, p: string | number) { | ||
target[p] = 1; // error | ||
~~~~~~~~~ | ||
!!! error TS2862: Type 'T' is generic and can only be indexed for reading. | ||
target[1] = 1; // ok | ||
} |
33 changes: 33 additions & 0 deletions
33
tests/baselines/reference/cannotIndexGenericWritingError.symbols
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,33 @@ | ||
//// [tests/cases/compiler/cannotIndexGenericWritingError.ts] //// | ||
|
||
=== cannotIndexGenericWritingError.ts === | ||
// From #47357 | ||
|
||
function foo<T extends Record<string | symbol, any>>(target: T, p: string | symbol) { | ||
>foo : Symbol(foo, Decl(cannotIndexGenericWritingError.ts, 0, 0)) | ||
>T : Symbol(T, Decl(cannotIndexGenericWritingError.ts, 2, 13)) | ||
>Record : Symbol(Record, Decl(lib.es5.d.ts, --, --)) | ||
>target : Symbol(target, Decl(cannotIndexGenericWritingError.ts, 2, 53)) | ||
>T : Symbol(T, Decl(cannotIndexGenericWritingError.ts, 2, 13)) | ||
>p : Symbol(p, Decl(cannotIndexGenericWritingError.ts, 2, 63)) | ||
|
||
target[p] = ""; // error | ||
>target : Symbol(target, Decl(cannotIndexGenericWritingError.ts, 2, 53)) | ||
>p : Symbol(p, Decl(cannotIndexGenericWritingError.ts, 2, 63)) | ||
} | ||
|
||
function foo2<T extends number[] & { [s: string]: number | string }>(target: T, p: string | number) { | ||
>foo2 : Symbol(foo2, Decl(cannotIndexGenericWritingError.ts, 4, 1)) | ||
>T : Symbol(T, Decl(cannotIndexGenericWritingError.ts, 6, 14)) | ||
>s : Symbol(s, Decl(cannotIndexGenericWritingError.ts, 6, 38)) | ||
>target : Symbol(target, Decl(cannotIndexGenericWritingError.ts, 6, 69)) | ||
>T : Symbol(T, Decl(cannotIndexGenericWritingError.ts, 6, 14)) | ||
>p : Symbol(p, Decl(cannotIndexGenericWritingError.ts, 6, 79)) | ||
|
||
target[p] = 1; // error | ||
>target : Symbol(target, Decl(cannotIndexGenericWritingError.ts, 6, 69)) | ||
>p : Symbol(p, Decl(cannotIndexGenericWritingError.ts, 6, 79)) | ||
|
||
target[1] = 1; // ok | ||
>target : Symbol(target, Decl(cannotIndexGenericWritingError.ts, 6, 69)) | ||
} |
38 changes: 38 additions & 0 deletions
38
tests/baselines/reference/cannotIndexGenericWritingError.types
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,38 @@ | ||
//// [tests/cases/compiler/cannotIndexGenericWritingError.ts] //// | ||
|
||
=== cannotIndexGenericWritingError.ts === | ||
// From #47357 | ||
|
||
function foo<T extends Record<string | symbol, any>>(target: T, p: string | symbol) { | ||
>foo : <T extends Record<string | symbol, any>>(target: T, p: string | symbol) => void | ||
>target : T | ||
>p : string | symbol | ||
|
||
target[p] = ""; // error | ||
>target[p] = "" : "" | ||
>target[p] : any | ||
>target : T | ||
>p : string | symbol | ||
>"" : "" | ||
} | ||
|
||
function foo2<T extends number[] & { [s: string]: number | string }>(target: T, p: string | number) { | ||
>foo2 : <T extends number[] & { [s: string]: string | number; }>(target: T, p: string | number) => void | ||
>s : string | ||
>target : T | ||
>p : string | number | ||
|
||
target[p] = 1; // error | ||
>target[p] = 1 : 1 | ||
>target[p] : any | ||
>target : T | ||
>p : string | number | ||
>1 : 1 | ||
|
||
target[1] = 1; // ok | ||
>target[1] = 1 : 1 | ||
>target[1] : number | ||
>target : T | ||
>1 : 1 | ||
>1 : 1 | ||
} |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,13 @@ | ||
// @strict: true | ||
// @noEmit: true | ||
|
||
// From #47357 | ||
|
||
function foo<T extends Record<string | symbol, any>>(target: T, p: string | symbol) { | ||
target[p] = ""; // error | ||
} | ||
|
||
function foo2<T extends number[] & { [s: string]: number | string }>(target: T, p: string | number) { | ||
target[p] = 1; // error | ||
target[1] = 1; // ok | ||
} |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know we hate the existing message about "T could be instantiated with a different subtype", buuuuuuuuuuuut..what about:
Type {property access} cannot be used to index '{type variable}' because '{type variable}' could be instantiated with an index signature that is a subtype of '{index signature}'.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know if/how adding that explanation helps people... Also, mentioning the index type makes it so you get two almost identical error messages if the index type is a union (e.g.
string | symbol
), which is why Anders suggested above to omit the index type entirely.Why do we hate the existing "T could be instantiated with a different subtype"?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
According to Ryan, we added the elaboration "T could be instantiated with a different subtype" because people didn't understand exactly why they were getting a (general) assignability error, and it helped people at least in the sense they had a more specific error message they could look up to try and understand the rule. I don't feel it's the same here, the message is already specific about
T
being generic. Do you think the extra information here would help people who don't understand the issue already?