Skip to content

Commit

Permalink
Merge pull request #12425 from Microsoft/keyofOnlyStrings
Browse files Browse the repository at this point in the history
Change 'keyof T' to always be string-like
  • Loading branch information
ahejlsberg authored Nov 21, 2016
2 parents 4c6b94f + b662a8b commit 77c0540
Show file tree
Hide file tree
Showing 32 changed files with 526 additions and 660 deletions.
30 changes: 9 additions & 21 deletions src/compiler/checker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,6 @@ namespace ts {
const voidType = createIntrinsicType(TypeFlags.Void, "void");
const neverType = createIntrinsicType(TypeFlags.Never, "never");
const silentNeverType = createIntrinsicType(TypeFlags.Never, "never");
const stringOrNumberType = getUnionType([stringType, numberType]);

const emptyObjectType = createAnonymousType(undefined, emptySymbols, emptyArray, emptyArray, undefined, undefined);

Expand Down Expand Up @@ -3189,9 +3188,11 @@ namespace ts {
}
}

// A variable declared in a for..in statement is always of type string
// A variable declared in a for..in statement is of type string, or of type keyof T when the
// right hand expression is of a type parameter type.
if (declaration.parent.parent.kind === SyntaxKind.ForInStatement) {
return stringType;
const indexType = getIndexType(checkNonNullExpression((<ForInStatement>declaration.parent.parent).expression));
return indexType.flags & TypeFlags.Index ? indexType : stringType;
}

if (declaration.parent.parent.kind === SyntaxKind.ForOfStatement) {
Expand Down Expand Up @@ -4660,7 +4661,6 @@ namespace ts {
t.flags & TypeFlags.NumberLike ? globalNumberType :
t.flags & TypeFlags.BooleanLike ? globalBooleanType :
t.flags & TypeFlags.ESSymbol ? getGlobalESSymbolType() :
t.flags & TypeFlags.Index ? stringOrNumberType :
t;
}

Expand Down Expand Up @@ -5893,8 +5893,7 @@ namespace ts {
function getIndexType(type: Type): Type {
return type.flags & TypeFlags.TypeParameter ? getIndexTypeForTypeParameter(<TypeParameter>type) :
getObjectFlags(type) & ObjectFlags.Mapped ? getConstraintTypeFromMappedType(<MappedType>type) :
type.flags & TypeFlags.Any || getIndexInfoOfType(type, IndexKind.String) ? stringOrNumberType :
getIndexInfoOfType(type, IndexKind.Number) ? getUnionType([numberType, getLiteralTypeFromPropertyNames(type)]) :
type.flags & TypeFlags.Any || getIndexInfoOfType(type, IndexKind.String) ? stringType :
getLiteralTypeFromPropertyNames(type);
}

Expand Down Expand Up @@ -6013,10 +6012,9 @@ namespace ts {
return indexedAccessTypes[id] || (indexedAccessTypes[id] = createIndexedAccessType(objectType, indexType));
}
const apparentObjectType = getApparentType(objectType);
const apparentIndexType = indexType.flags & TypeFlags.Index ? stringOrNumberType : indexType;
if (apparentIndexType.flags & TypeFlags.Union && !(apparentIndexType.flags & TypeFlags.Primitive)) {
if (indexType.flags & TypeFlags.Union && !(indexType.flags & TypeFlags.Primitive)) {
const propTypes: Type[] = [];
for (const t of (<UnionType>apparentIndexType).types) {
for (const t of (<UnionType>indexType).types) {
const propType = getPropertyTypeForIndexType(apparentObjectType, t, accessNode, /*cacheSymbol*/ false);
if (propType === unknownType) {
return unknownType;
Expand All @@ -6025,7 +6023,7 @@ namespace ts {
}
return getUnionType(propTypes);
}
return getPropertyTypeForIndexType(apparentObjectType, apparentIndexType, accessNode, /*cacheSymbol*/ true);
return getPropertyTypeForIndexType(apparentObjectType, indexType, accessNode, /*cacheSymbol*/ true);
}

function getTypeFromIndexedAccessTypeNode(node: IndexedAccessTypeNode) {
Expand Down Expand Up @@ -7097,16 +7095,6 @@ namespace ts {

if (isSimpleTypeRelatedTo(source, target, relation, reportErrors ? reportError : undefined)) return Ternary.True;

if (source.flags & TypeFlags.Index) {
// A keyof T is related to a union type containing both string and number
const related = relation === comparableRelation ?
maybeTypeOfKind(target, TypeFlags.String | TypeFlags.Number) :
maybeTypeOfKind(target, TypeFlags.String) && maybeTypeOfKind(target, TypeFlags.Number);
if (related) {
return Ternary.True;
}
}

if (getObjectFlags(source) & ObjectFlags.ObjectLiteral && source.flags & TypeFlags.FreshLiteral) {
if (hasExcessProperties(<FreshObjectLiteralType>source, target, reportErrors)) {
if (reportErrors) {
Expand Down Expand Up @@ -15627,7 +15615,7 @@ namespace ts {
const type = <MappedType>getTypeFromMappedTypeNode(node);
const constraintType = getConstraintTypeFromMappedType(type);
const keyType = constraintType.flags & TypeFlags.TypeParameter ? getApparentTypeOfTypeParameter(<TypeParameter>constraintType) : constraintType;
checkTypeAssignableTo(keyType, stringOrNumberType, node.typeParameter.constraint);
checkTypeAssignableTo(keyType, stringType, node.typeParameter.constraint);
}

function isPrivateWithinAmbient(node: Node): boolean {
Expand Down
2 changes: 1 addition & 1 deletion src/compiler/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2790,7 +2790,7 @@ namespace ts {
Intrinsic = Any | String | Number | Boolean | BooleanLiteral | ESSymbol | Void | Undefined | Null | Never,
/* @internal */
Primitive = String | Number | Boolean | Enum | ESSymbol | Void | Undefined | Null | Literal,
StringLike = String | StringLiteral,
StringLike = String | StringLiteral | Index,
NumberLike = Number | NumberLiteral | Enum | EnumLiteral,
BooleanLike = Boolean | BooleanLiteral,
EnumLike = Enum | EnumLiteral,
Expand Down
2 changes: 1 addition & 1 deletion src/lib/es5.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1367,7 +1367,7 @@ type Pick<T, K extends keyof T> = {
/**
* Construct a type with a set of properties K of type T
*/
type Record<K extends string | number, T> = {
type Record<K extends string, T> = {
[P in K]: T;
}

Expand Down
8 changes: 7 additions & 1 deletion tests/baselines/reference/for-inStatements.errors.txt
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
tests/cases/conformance/statements/for-inStatements/for-inStatements.ts(33,18): error TS2403: Subsequent variable declarations must have the same type. Variable 'x' must be of type 'string', but here has type 'keyof this'.
tests/cases/conformance/statements/for-inStatements/for-inStatements.ts(50,18): error TS2403: Subsequent variable declarations must have the same type. Variable 'x' must be of type 'string', but here has type 'keyof this'.
tests/cases/conformance/statements/for-inStatements/for-inStatements.ts(79,15): error TS2407: The right-hand side of a 'for...in' statement must be of type 'any', an object type or a type parameter.


==== tests/cases/conformance/statements/for-inStatements/for-inStatements.ts (1 errors) ====
==== tests/cases/conformance/statements/for-inStatements/for-inStatements.ts (3 errors) ====
var aString: string;
for (aString in {}) { }

Expand Down Expand Up @@ -35,6 +37,8 @@ tests/cases/conformance/statements/for-inStatements/for-inStatements.ts(79,15):
for (var x in this.biz()) { }
for (var x in this.biz) { }
for (var x in this) { }
~
!!! error TS2403: Subsequent variable declarations must have the same type. Variable 'x' must be of type 'string', but here has type 'keyof this'.
return null;
}

Expand All @@ -52,6 +56,8 @@ tests/cases/conformance/statements/for-inStatements/for-inStatements.ts(79,15):
for (var x in this.biz()) { }
for (var x in this.biz) { }
for (var x in this) { }
~
!!! error TS2403: Subsequent variable declarations must have the same type. Variable 'x' must be of type 'string', but here has type 'keyof this'.

for (var x in super.biz) { }
for (var x in super.biz()) { }
Expand Down
8 changes: 7 additions & 1 deletion tests/baselines/reference/for-inStatementsInvalid.errors.txt
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,15 @@ tests/cases/conformance/statements/for-inStatements/for-inStatementsInvalid.ts(1
tests/cases/conformance/statements/for-inStatements/for-inStatementsInvalid.ts(20,15): error TS2407: The right-hand side of a 'for...in' statement must be of type 'any', an object type or a type parameter.
tests/cases/conformance/statements/for-inStatements/for-inStatementsInvalid.ts(22,15): error TS2407: The right-hand side of a 'for...in' statement must be of type 'any', an object type or a type parameter.
tests/cases/conformance/statements/for-inStatements/for-inStatementsInvalid.ts(29,23): error TS2407: The right-hand side of a 'for...in' statement must be of type 'any', an object type or a type parameter.
tests/cases/conformance/statements/for-inStatements/for-inStatementsInvalid.ts(31,18): error TS2403: Subsequent variable declarations must have the same type. Variable 'x' must be of type 'string', but here has type 'keyof this'.
tests/cases/conformance/statements/for-inStatements/for-inStatementsInvalid.ts(38,23): error TS2407: The right-hand side of a 'for...in' statement must be of type 'any', an object type or a type parameter.
tests/cases/conformance/statements/for-inStatements/for-inStatementsInvalid.ts(46,23): error TS2407: The right-hand side of a 'for...in' statement must be of type 'any', an object type or a type parameter.
tests/cases/conformance/statements/for-inStatements/for-inStatementsInvalid.ts(48,18): error TS2403: Subsequent variable declarations must have the same type. Variable 'x' must be of type 'string', but here has type 'keyof this'.
tests/cases/conformance/statements/for-inStatements/for-inStatementsInvalid.ts(51,23): error TS2407: The right-hand side of a 'for...in' statement must be of type 'any', an object type or a type parameter.
tests/cases/conformance/statements/for-inStatements/for-inStatementsInvalid.ts(62,15): error TS2407: The right-hand side of a 'for...in' statement must be of type 'any', an object type or a type parameter.


==== tests/cases/conformance/statements/for-inStatements/for-inStatementsInvalid.ts (15 errors) ====
==== tests/cases/conformance/statements/for-inStatements/for-inStatementsInvalid.ts (17 errors) ====
var aNumber: number;
for (aNumber in {}) { }
~~~~~~~
Expand Down Expand Up @@ -69,6 +71,8 @@ tests/cases/conformance/statements/for-inStatements/for-inStatementsInvalid.ts(6
!!! error TS2407: The right-hand side of a 'for...in' statement must be of type 'any', an object type or a type parameter.
for (var x in this.biz) { }
for (var x in this) { }
~
!!! error TS2403: Subsequent variable declarations must have the same type. Variable 'x' must be of type 'string', but here has type 'keyof this'.
return null;
}

Expand All @@ -90,6 +94,8 @@ tests/cases/conformance/statements/for-inStatements/for-inStatementsInvalid.ts(6
!!! error TS2407: The right-hand side of a 'for...in' statement must be of type 'any', an object type or a type parameter.
for (var x in this.biz) { }
for (var x in this) { }
~
!!! error TS2403: Subsequent variable declarations must have the same type. Variable 'x' must be of type 'string', but here has type 'keyof this'.

for (var x in super.biz) { }
for (var x in super.biz()) { }
Expand Down
2 changes: 1 addition & 1 deletion tests/baselines/reference/forInStatement3.types
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ function F<T>() {
>T : T

for (var a in expr) {
>a : string
>a : keyof T
>expr : T
}
}
2 changes: 1 addition & 1 deletion tests/baselines/reference/implicitAnyInCatch.types
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ class C {
>temp : () => void

for (var x in this) {
>x : string
>x : keyof this
>this : this
}
}
Expand Down
2 changes: 1 addition & 1 deletion tests/baselines/reference/inOperatorWithGeneric.types
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ class C<T> {
>T : T

for (var p in x) {
>p : string
>p : keyof T
>x : T
}
}
Expand Down
60 changes: 26 additions & 34 deletions tests/baselines/reference/keyofAndIndexedAccess.js
Original file line number Diff line number Diff line change
Expand Up @@ -95,22 +95,18 @@ function f10(shape: Shape) {

function f11(a: Shape[]) {
let len = getProperty(a, "length"); // number
let shape = getProperty(a, 1000); // Shape
setProperty(a, 1000, getProperty(a, 1001));
setProperty(a, "length", len);
}

function f12(t: [Shape, boolean]) {
let len = getProperty(t, "length");
let s1 = getProperty(t, 0); // Shape
let s2 = getProperty(t, "0"); // Shape
let b1 = getProperty(t, 1); // boolean
let b2 = getProperty(t, "1"); // boolean
let x1 = getProperty(t, 2); // Shape | boolean
}

function f13(foo: any, bar: any) {
let x = getProperty(foo, "x"); // any
let y = getProperty(foo, 100); // any
let y = getProperty(foo, "100"); // any
let z = getProperty(foo, bar); // any
}

Expand Down Expand Up @@ -181,20 +177,14 @@ function f40(c: C) {
let z: Z = c["z"];
}

function f50<T>(k: keyof T, s: string, n: number) {
function f50<T>(k: keyof T, s: string) {
const x1 = s as keyof T;
const x2 = n as keyof T;
const x3 = k as string;
const x4 = k as number;
const x5 = k as string | number;
const x2 = k as string;
}

function f51<T, K extends keyof T>(k: K, s: string, n: number) {
function f51<T, K extends keyof T>(k: K, s: string) {
const x1 = s as keyof T;
const x2 = n as keyof T;
const x3 = k as string;
const x4 = k as number;
const x5 = k as string | number;
const x2 = k as string;
}

function f52<T>(obj: { [x: string]: boolean }, k: keyof T, s: string, n: number) {
Expand All @@ -221,6 +211,12 @@ function f55<T, K extends keyof T>(obj: T, key: K) {
const b = "foo" in obj[key];
}

function f60<T>(source: T, target: T) {
for (let k in source) {
target[k] = source[k];
}
}

// Repros from #12011

class Base {
Expand Down Expand Up @@ -297,20 +293,16 @@ function f10(shape) {
}
function f11(a) {
var len = getProperty(a, "length"); // number
var shape = getProperty(a, 1000); // Shape
setProperty(a, 1000, getProperty(a, 1001));
setProperty(a, "length", len);
}
function f12(t) {
var len = getProperty(t, "length");
var s1 = getProperty(t, 0); // Shape
var s2 = getProperty(t, "0"); // Shape
var b1 = getProperty(t, 1); // boolean
var b2 = getProperty(t, "1"); // boolean
var x1 = getProperty(t, 2); // Shape | boolean
}
function f13(foo, bar) {
var x = getProperty(foo, "x"); // any
var y = getProperty(foo, 100); // any
var y = getProperty(foo, "100"); // any
var z = getProperty(foo, bar); // any
}
var Component = (function () {
Expand Down Expand Up @@ -369,19 +361,13 @@ function f40(c) {
var y = c["y"];
var z = c["z"];
}
function f50(k, s, n) {
function f50(k, s) {
var x1 = s;
var x2 = n;
var x3 = k;
var x4 = k;
var x5 = k;
var x2 = k;
}
function f51(k, s, n) {
function f51(k, s) {
var x1 = s;
var x2 = n;
var x3 = k;
var x4 = k;
var x5 = k;
var x2 = k;
}
function f52(obj, k, s, n) {
var x1 = obj[s];
Expand All @@ -403,6 +389,11 @@ function f55(obj, key) {
}
var b = "foo" in obj[key];
}
function f60(source, target) {
for (var k in source) {
target[k] = source[k];
}
}
// Repros from #12011
var Base = (function () {
function Base() {
Expand Down Expand Up @@ -528,8 +519,8 @@ declare class C {
private z;
}
declare function f40(c: C): void;
declare function f50<T>(k: keyof T, s: string, n: number): void;
declare function f51<T, K extends keyof T>(k: K, s: string, n: number): void;
declare function f50<T>(k: keyof T, s: string): void;
declare function f51<T, K extends keyof T>(k: K, s: string): void;
declare function f52<T>(obj: {
[x: string]: boolean;
}, k: keyof T, s: string, n: number): void;
Expand All @@ -538,6 +529,7 @@ declare function f53<T, K extends keyof T>(obj: {
}, k: K, s: string, n: number): void;
declare function f54<T>(obj: T, key: keyof T): void;
declare function f55<T, K extends keyof T>(obj: T, key: K): void;
declare function f60<T>(source: T, target: T): void;
declare class Base {
get<K extends keyof this>(prop: K): this[K];
set<K extends keyof this>(prop: K, value: this[K]): void;
Expand Down
Loading

0 comments on commit 77c0540

Please sign in to comment.