Skip to content

Improved error messaging for index signature parameters #20726

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 5 commits into from
Jan 8, 2018
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions src/compiler/checker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25775,6 +25775,16 @@ namespace ts {
return grammarErrorOnNode(parameter.name, Diagnostics.An_index_signature_parameter_must_have_a_type_annotation);
}
if (parameter.type.kind !== SyntaxKind.StringKeyword && parameter.type.kind !== SyntaxKind.NumberKeyword) {
const type = getTypeFromTypeNode(parameter.type);

if (type.flags & TypeFlags.String || type.flags & TypeFlags.Number) {
return grammarErrorOnNode(parameter.name, Diagnostics.An_index_signature_parameter_type_cannot_be_a_type_alias_Use_index_Colon_0_instead, typeToString(type));
}

if (type.flags & TypeFlags.Union && every((<UnionType>type).types, t => !!(t.flags & TypeFlags.StringLiteral))) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

allTypesAssignableToKind

return grammarErrorOnNode(parameter.name, Diagnostics.An_index_signature_parameter_type_cannot_be_a_union_type_Use_K_in_0_instead, symbolName(type.aliasSymbol));
}

return grammarErrorOnNode(parameter.name, Diagnostics.An_index_signature_parameter_type_must_be_string_or_number);
}
if (!node.type) {
Expand Down
8 changes: 8 additions & 0 deletions src/compiler/diagnosticMessages.json
Original file line number Diff line number Diff line change
Expand Up @@ -939,6 +939,14 @@
"category": "Error",
"code": 1335
},
"An index signature parameter type cannot be a type alias. Use '[index: {0}]' instead.": {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider writing '[{0}: {1}]: {2}' instead.

"category": "Error",
"code": 1336
},
"An index signature parameter type cannot be a union type. Use '[K in {0}]' instead.": {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: this won't work if there are any other members in the object type. Maybe we should just write "Consider using a mapped object type instead.

Copy link
Contributor Author

@kujon kujon Dec 19, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is supposed to be thrown only if all members are string literals. How about changing the wording to A union of string literals cannot be used as an index signature parameter type directly. Consider writing '[K in {0}]: {1}' instead.?

Sorry, totally misunderstood that.

"category": "Error",
"code": 1337
},

"Duplicate identifier '{0}'.": {
"category": "Error",
Expand Down
66 changes: 65 additions & 1 deletion tests/baselines/reference/indexerConstraints2.errors.txt
Original file line number Diff line number Diff line change
@@ -1,9 +1,16 @@
tests/cases/compiler/indexerConstraints2.ts(9,5): error TS2413: Numeric index type 'A' is not assignable to string index type 'B'.
tests/cases/compiler/indexerConstraints2.ts(17,5): error TS2413: Numeric index type 'A' is not assignable to string index type 'B'.
tests/cases/compiler/indexerConstraints2.ts(26,5): error TS2413: Numeric index type 'A' is not assignable to string index type 'B'.
tests/cases/compiler/indexerConstraints2.ts(34,6): error TS1336: An index signature parameter type cannot be a type alias. Use '[index: number]' instead.
tests/cases/compiler/indexerConstraints2.ts(40,6): error TS1336: An index signature parameter type cannot be a type alias. Use '[index: string]' instead.
tests/cases/compiler/indexerConstraints2.ts(46,6): error TS1023: An index signature parameter type must be 'string' or 'number'.
tests/cases/compiler/indexerConstraints2.ts(52,6): error TS1337: An index signature parameter type cannot be a union type. Use '[K in IndexableUnion]' instead.
tests/cases/compiler/indexerConstraints2.ts(58,6): error TS1023: An index signature parameter type must be 'string' or 'number'.
tests/cases/compiler/indexerConstraints2.ts(64,6): error TS1023: An index signature parameter type must be 'string' or 'number'.
tests/cases/compiler/indexerConstraints2.ts(70,6): error TS1023: An index signature parameter type must be 'string' or 'number'.


==== tests/cases/compiler/indexerConstraints2.ts (3 errors) ====
==== tests/cases/compiler/indexerConstraints2.ts (10 errors) ====
class A { a: number; }
class B extends A { b: number; }

Expand Down Expand Up @@ -37,4 +44,61 @@ tests/cases/compiler/indexerConstraints2.ts(26,5): error TS2413: Numeric index t
~~~~~~~~~~~~~~~
!!! error TS2413: Numeric index type 'A' is not assignable to string index type 'B'.
[s: string]: B;
}


type AliasedNumber = number;

interface L {
[n: AliasedNumber]: A;
~
!!! error TS1336: An index signature parameter type cannot be a type alias. Use '[index: number]' instead.
}

type AliasedString = string;

interface M {
[s: AliasedString]: A;
~
!!! error TS1336: An index signature parameter type cannot be a type alias. Use '[index: string]' instead.
}

type AliasedBoolean = boolean;

interface N {
[b: AliasedBoolean]: A;
~
!!! error TS1023: An index signature parameter type must be 'string' or 'number'.
}

type IndexableUnion = "foo" | "bar";

interface O {
[u: IndexableUnion]: A;
~
!!! error TS1337: An index signature parameter type cannot be a union type. Use '[K in IndexableUnion]' instead.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This version of the error message should only appear when the parent is a type alias

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given this suggestion #20726 (comment), is it still applicable?

}

type NonIndexableUnion = boolean | {};

interface P {
[u: NonIndexableUnion]: A;
~
!!! error TS1023: An index signature parameter type must be 'string' or 'number'.
}

type NonIndexableUnion2 = string | number;

interface Q {
[u: NonIndexableUnion2]: A;
~
!!! error TS1023: An index signature parameter type must be 'string' or 'number'.
}

type NonIndexableUnion3 = "foo" | 42;

interface R {
[u: NonIndexableUnion3]: A;
~
!!! error TS1023: An index signature parameter type must be 'string' or 'number'.
}
43 changes: 43 additions & 0 deletions tests/baselines/reference/indexerConstraints2.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,49 @@ class J {
class K extends J {
[n: number]: A;
[s: string]: B;
}


type AliasedNumber = number;

interface L {
[n: AliasedNumber]: A;
}

type AliasedString = string;

interface M {
[s: AliasedString]: A;
}

type AliasedBoolean = boolean;

interface N {
[b: AliasedBoolean]: A;
}

type IndexableUnion = "foo" | "bar";

interface O {
[u: IndexableUnion]: A;
}

type NonIndexableUnion = boolean | {};

interface P {
[u: NonIndexableUnion]: A;
}

type NonIndexableUnion2 = string | number;

interface Q {
[u: NonIndexableUnion2]: A;
}

type NonIndexableUnion3 = "foo" | 42;

interface R {
[u: NonIndexableUnion3]: A;
}

//// [indexerConstraints2.js]
Expand Down
85 changes: 85 additions & 0 deletions tests/baselines/reference/indexerConstraints2.symbols
Original file line number Diff line number Diff line change
Expand Up @@ -62,3 +62,88 @@ class K extends J {
>s : Symbol(s, Decl(indexerConstraints2.ts, 26, 5))
>B : Symbol(B, Decl(indexerConstraints2.ts, 0, 22))
}


type AliasedNumber = number;
>AliasedNumber : Symbol(AliasedNumber, Decl(indexerConstraints2.ts, 27, 1))

interface L {
>L : Symbol(L, Decl(indexerConstraints2.ts, 30, 28))

[n: AliasedNumber]: A;
>n : Symbol(n, Decl(indexerConstraints2.ts, 33, 5))
>AliasedNumber : Symbol(AliasedNumber, Decl(indexerConstraints2.ts, 27, 1))
>A : Symbol(A, Decl(indexerConstraints2.ts, 0, 0))
}

type AliasedString = string;
>AliasedString : Symbol(AliasedString, Decl(indexerConstraints2.ts, 34, 1))

interface M {
>M : Symbol(M, Decl(indexerConstraints2.ts, 36, 28))

[s: AliasedString]: A;
>s : Symbol(s, Decl(indexerConstraints2.ts, 39, 5))
>AliasedString : Symbol(AliasedString, Decl(indexerConstraints2.ts, 34, 1))
>A : Symbol(A, Decl(indexerConstraints2.ts, 0, 0))
}

type AliasedBoolean = boolean;
>AliasedBoolean : Symbol(AliasedBoolean, Decl(indexerConstraints2.ts, 40, 1))

interface N {
>N : Symbol(N, Decl(indexerConstraints2.ts, 42, 30))

[b: AliasedBoolean]: A;
>b : Symbol(b, Decl(indexerConstraints2.ts, 45, 5))
>AliasedBoolean : Symbol(AliasedBoolean, Decl(indexerConstraints2.ts, 40, 1))
>A : Symbol(A, Decl(indexerConstraints2.ts, 0, 0))
}

type IndexableUnion = "foo" | "bar";
>IndexableUnion : Symbol(IndexableUnion, Decl(indexerConstraints2.ts, 46, 1))

interface O {
>O : Symbol(O, Decl(indexerConstraints2.ts, 48, 36))

[u: IndexableUnion]: A;
>u : Symbol(u, Decl(indexerConstraints2.ts, 51, 5))
>IndexableUnion : Symbol(IndexableUnion, Decl(indexerConstraints2.ts, 46, 1))
>A : Symbol(A, Decl(indexerConstraints2.ts, 0, 0))
}

type NonIndexableUnion = boolean | {};
>NonIndexableUnion : Symbol(NonIndexableUnion, Decl(indexerConstraints2.ts, 52, 1))

interface P {
>P : Symbol(P, Decl(indexerConstraints2.ts, 54, 38))

[u: NonIndexableUnion]: A;
>u : Symbol(u, Decl(indexerConstraints2.ts, 57, 5))
>NonIndexableUnion : Symbol(NonIndexableUnion, Decl(indexerConstraints2.ts, 52, 1))
>A : Symbol(A, Decl(indexerConstraints2.ts, 0, 0))
}

type NonIndexableUnion2 = string | number;
>NonIndexableUnion2 : Symbol(NonIndexableUnion2, Decl(indexerConstraints2.ts, 58, 1))

interface Q {
>Q : Symbol(Q, Decl(indexerConstraints2.ts, 60, 42))

[u: NonIndexableUnion2]: A;
>u : Symbol(u, Decl(indexerConstraints2.ts, 63, 5))
>NonIndexableUnion2 : Symbol(NonIndexableUnion2, Decl(indexerConstraints2.ts, 58, 1))
>A : Symbol(A, Decl(indexerConstraints2.ts, 0, 0))
}

type NonIndexableUnion3 = "foo" | 42;
>NonIndexableUnion3 : Symbol(NonIndexableUnion3, Decl(indexerConstraints2.ts, 64, 1))

interface R {
>R : Symbol(R, Decl(indexerConstraints2.ts, 66, 37))

[u: NonIndexableUnion3]: A;
>u : Symbol(u, Decl(indexerConstraints2.ts, 69, 5))
>NonIndexableUnion3 : Symbol(NonIndexableUnion3, Decl(indexerConstraints2.ts, 64, 1))
>A : Symbol(A, Decl(indexerConstraints2.ts, 0, 0))
}
85 changes: 85 additions & 0 deletions tests/baselines/reference/indexerConstraints2.types
Original file line number Diff line number Diff line change
Expand Up @@ -62,3 +62,88 @@ class K extends J {
>s : string
>B : B
}


type AliasedNumber = number;
>AliasedNumber : number

interface L {
>L : L

[n: AliasedNumber]: A;
>n : number
>AliasedNumber : number
>A : A
}

type AliasedString = string;
>AliasedString : string

interface M {
>M : M

[s: AliasedString]: A;
>s : string
>AliasedString : string
>A : A
}

type AliasedBoolean = boolean;
>AliasedBoolean : boolean

interface N {
>N : N

[b: AliasedBoolean]: A;
>b : boolean
>AliasedBoolean : boolean
>A : A
}

type IndexableUnion = "foo" | "bar";
>IndexableUnion : IndexableUnion

interface O {
>O : O

[u: IndexableUnion]: A;
>u : IndexableUnion
>IndexableUnion : IndexableUnion
>A : A
}

type NonIndexableUnion = boolean | {};
>NonIndexableUnion : NonIndexableUnion

interface P {
>P : P

[u: NonIndexableUnion]: A;
>u : NonIndexableUnion
>NonIndexableUnion : NonIndexableUnion
>A : A
}

type NonIndexableUnion2 = string | number;
>NonIndexableUnion2 : NonIndexableUnion2

interface Q {
>Q : Q

[u: NonIndexableUnion2]: A;
>u : NonIndexableUnion2
>NonIndexableUnion2 : NonIndexableUnion2
>A : A
}

type NonIndexableUnion3 = "foo" | 42;
>NonIndexableUnion3 : NonIndexableUnion3

interface R {
>R : R

[u: NonIndexableUnion3]: A;
>u : NonIndexableUnion3
>NonIndexableUnion3 : NonIndexableUnion3
>A : A
}
43 changes: 43 additions & 0 deletions tests/cases/compiler/indexerConstraints2.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,4 +25,47 @@ class J {
class K extends J {
[n: number]: A;
[s: string]: B;
}


type AliasedNumber = number;

interface L {
[n: AliasedNumber]: A;
}

type AliasedString = string;

interface M {
[s: AliasedString]: A;
}

type AliasedBoolean = boolean;

interface N {
[b: AliasedBoolean]: A;
}

type IndexableUnion = "foo" | "bar";

interface O {
[u: IndexableUnion]: A;
}

type NonIndexableUnion = boolean | {};

interface P {
[u: NonIndexableUnion]: A;
}

type NonIndexableUnion2 = string | number;

interface Q {
[u: NonIndexableUnion2]: A;
}

type NonIndexableUnion3 = "foo" | 42;

interface R {
[u: NonIndexableUnion3]: A;
}