Corrected ES5 for-in destructuring binding emit

[JoshuaKGoldberg]

Although it's not permitted in the checker to use an array or object binding pattern ("destructuring") as the iterating variable in a for..in loop, TypeScript can still output the syntax. This corrects the emitter to correctly destructure the variables in the body of the loop. Previously they were incorrectly being bound in the iteration statement.

I'm not particularly satisfied with how this code shaped up. There's some duplication in the convertForInStatementForLoop* functions, but just small enough of it that I didn't extract into a shared function... does anyone see a cleaner approach?

Fixes #2272

[rbuckton]

I don't have an issue with these changes in general, but one of the reasons this is an error is that we don't correctly downlevel iterator destructuring for strings in ES5, as an ES2015 string iterator yields "code points" rather than "code units", so there would be differences in runtime behavior for ES2015 and ES5 for these two emits:

// --target ES2015
for (var [a, b] in { "🐛": 1 }) console.log(a, b);
// prints: 🐛 undefined

// --target ES5
for (var _a in { "🐛": 1 }) {
    var a = _a[0], b = _a[1];
    console.log(a, b);
}
// prints: � �

I can bring this up in a design meeting, but I'm somewhat wary of introducing a change that:

• Only affects code we've labeled as an error
• Produces incorrect results for unicode surrogate pairs.

[rbuckton]

Suggested change

	function convertForStatementHead(node: ForInStatement | ForOfStatement, boundValue: Expression, convertedLoopBodyStatements: Statement[]) {
	function convertForInOrForOfStatementHead(node: ForInStatement | ForOfStatement, boundValue: Expression, convertedLoopBodyStatements: Statement[]) {

Only because a for statement is different from a for-in or for-of statement and its better to avoid confusion.

[rbuckton]

You could possibly simplify this by using a 3rd function that optionally converts the iteration statement, or make the conversion function something that is passed in as an argument (and falls back to visitEachChild(node, visitor, context) when not converting).

[rbuckton]

We discussed this in the design meeting today, and while we appreciate the effort that went into putting together this PR, unfortunately we will not be taking this change. There are several factors involved:

• The downlevel emit for ES5 is not compliant with ES2015 semantics with respect to surrogate pairs.
• Supporting true ES2015 string iteration via a helper would overcomplicate the existing helpers and add a fairly large amount of extra code at the top of any file using array destructuring downlevel.
• We already report an error for this, and it is not the only case where we may emit invalid JavaScript in the presence of a reported error (which is one of the reasons --noEmitOnError exists as a compiler option).

However, since this is legal ES2015, we may consider removing the error for --target ES2015 and higher.