Solve the for-in / indexer mess

[RyanCavanaugh]

/cc Vladimir Matveev (@vladima) Anders Hejlsberg (@ahejlsberg) Bill Ticehurst (@billti)

Motivating Examples

var arr = [1, 2, 3];

This should be an error (k is always a string):

for(var k in arr) {
  if(k === 0) { ... }
}

This should not be an error, even under no implicit any:

for(var k in arr) {
  if(arr[k] === 0) { ... }
}

This should somehow? not be an error, even though it involves a coercion we would normally disallow

for(var k in arr) {
  if (k > 0) { ... }
}

[DanielRosenwasser]

If there was some way of communicating that your string type was actually the domain of strings whose text was numeric (i.e. ToString(ToNumber(text) === text), that would possibly go a long way here.

[mhegazy]

I had an offline discussion with Anders Hejlsberg (@ahejlsberg) about this. I think we have a proposal. given:

for (var x in a) {...}

• if type of a has a numeric index signature, type of x is NumericString
• otherwise, typeof x is string

Where NumericString is an builtin type that can not be named, and is only given to for..in indexers. It is identical to string for all type system operations except that it is treated as number when indexing. e.g.:

var a = [1, 2, 3];
for (var x in a) {
    y = a[x];  // y is number
    z = x + 1 // z is string;
}

[ahejlsberg]

An alternative is to check whether the expression used to index an array is exactly an identifier declared as a for-in variable for an array that has a numeric indexer. This avoids having a new type and, importantly, avoids having to "erase" that type when it becomes in inferred type of a variable. I will try to work on it and put up a PR.

[DanielRosenwasser]

You mean if it only has a numeric index signature? Or even if it has a string index signature as well?

[mhegazy]

Daniel Rosenwasser (@DanielRosenwasser) you are correct. it has to have only a numeric index signature.

[jeffreymorlan]

An object can satisfy a numeric index signature type and still have other non-numeric properties:

var obj: {[n: number]: string};
obj = { 0: 'a', 1: 'b', 2: 'c', length: 3 };
for (var k in obj) {
    console.log(k, obj[k].toUpperCase()); // This throws when k === 'length'
}

Even on a plain array, for/in may include properties that have been added to Array.prototype.

[ahejlsberg]

jeffreymorlan Perhaps a better check is to see if the object being for-in'ed is array-like (specifically, if it is assignable to any[]). That is the same check we apply to a spread expression.

[jeffreymorlan]

One library I'm using returns array-like objects that inherit from Array.prototype but add additional (enumerable) methods that will show up in for/in, making it unsafe to use the for/in keys as indexes.

There are so many ways that for/in on array-likes may return properties other than array indexes...

• extra properties on the array object itself
• extra properties on prototype inheriting from Array
• some library "conveniently" adding extra properties onto Array.prototype itself (MooTools is an infamous example of this)
• old JS implementations where .length was enumerable

...not to mention the other surprising behaviors (indexes are strings instead of numbers, indexes not guaranteed to be in numeric order pre-ES6), that for/in on an array-like should just be considered a bug. At any rate it shouldn't be given special treatment from the type system to pretend that it's safe.

[ahejlsberg]

jeffreymorlan Completely agree for-in is a messed up construct--but we don't get to change that. The objective is to provide the best possible type checking of for-in as it exists, and unquestionably it is a common pattern to use for-in with arrays and expect indexing with the for-in variable to produce the elements of the array. I agree that there are situations where non-numeric property names may appear because applications or libraries have added extra properties to arrays, but we have to balance that against existing (and more common) code that just uses plain JavaScript arrays. The operating principle here is to find the best possible compromise. I think the changes in #6379 bring us closer to that.

[jeffreymorlan]

it is a common pattern to use for-in with arrays and expect indexing with the for-in variable to produce the elements of the array

It's a common mistake; all the more reason not to allow it in TS, so people can find places where they have made that mistake and replace it with the correct for/of, .forEach, or good-old-fashioned for (var i = 0; i < arr.length; i++).

Monkey patching of Array.prototype isn't just something obscure that you'll never see in the real world (unfortunately). According to http://w3techs.com/technologies/overview/javascript_library/all MooTools is used by 3.8% of websites, and Prototype by 2.2%. It's a controversial practice; I don't like it myself, but many people take the moderate position that it's fine for polyfilling newer methods like .filter for older browsers - which still breaks for/in. On the other hand, for/in on arrays is nearly universally castigated; google for "for in array" and results # 1, 3, 4, and 7 are all about why you shouldn't do it.

Even TypeScript itself used to add properties to Array.prototype that would globally break for/in on arrays. From TypeScript 1.3 src/services/syntax/syntaxList.ts:

module TypeScript.Syntax {
    ...
    Array.prototype.kind = function () { ... }
    Array.prototype.separatorCount = function (): number { ... }
    Array.prototype.separatorAt = function (index: number): ISyntaxToken { ... }
    ...
}