Skip to content

Type checking 'for...of' in ES3/5 #2308

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 9 commits into from
Mar 12, 2015
116 changes: 89 additions & 27 deletions src/compiler/checker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1918,7 +1918,11 @@ module ts {
return anyType;
}
if (declaration.parent.parent.kind === SyntaxKind.ForOfStatement) {
return getTypeForVariableDeclarationInForOfStatement(<ForOfStatement>declaration.parent.parent);
// checkRightHandSideOfForOf will return undefined if the for-of expression type was
// missing properties/signatures required to get its iteratedType (like
// [Symbol.iterator] or next). This may be because we accessed properties from anyType,
// or it may have led to an error inside getIteratedType.
return checkRightHandSideOfForOf((<ForOfStatement>declaration.parent.parent).expression) || anyType;
}
if (isBindingPattern(declaration.parent)) {
return getTypeForBindingElement(<BindingElement>declaration);
Expand Down Expand Up @@ -4759,17 +4763,22 @@ module ts {

// For a union type, remove all constituent types that are of the given type kind (when isOfTypeKind is true)
// or not of the given type kind (when isOfTypeKind is false)
function removeTypesFromUnionType(type: Type, typeKind: TypeFlags, isOfTypeKind: boolean): Type {
function removeTypesFromUnionType(type: Type, typeKind: TypeFlags, isOfTypeKind: boolean, allowEmptyUnionResult: boolean): Type {
if (type.flags & TypeFlags.Union) {
var types = (<UnionType>type).types;
if (forEach(types, t => !!(t.flags & typeKind) === isOfTypeKind)) {
// Above we checked if we have anything to remove, now use the opposite test to do the removal
var narrowedType = getUnionType(filter(types, t => !(t.flags & typeKind) === isOfTypeKind));
if (narrowedType !== emptyObjectType) {
if (allowEmptyUnionResult || narrowedType !== emptyObjectType) {
return narrowedType;
}
}
}
else if (allowEmptyUnionResult && !!(type.flags & typeKind) === isOfTypeKind) {
// Use getUnionType(emptyArray) instead of emptyObjectType in case the way empty union types
// are represented ever changes.
return getUnionType(emptyArray);
}
return type;
}

Expand Down Expand Up @@ -4972,20 +4981,21 @@ module ts {
if (assumeTrue) {
// Assumed result is true. If check was not for a primitive type, remove all primitive types
if (!typeInfo) {
return removeTypesFromUnionType(type, /*typeKind*/ TypeFlags.StringLike | TypeFlags.NumberLike | TypeFlags.Boolean | TypeFlags.ESSymbol, /*isOfTypeKind*/ true);
return removeTypesFromUnionType(type, /*typeKind*/ TypeFlags.StringLike | TypeFlags.NumberLike | TypeFlags.Boolean | TypeFlags.ESSymbol,
/*isOfTypeKind*/ true, /*allowEmptyUnionResult*/ false);
}
// Check was for a primitive type, return that primitive type if it is a subtype
if (isTypeSubtypeOf(typeInfo.type, type)) {
return typeInfo.type;
}
// Otherwise, remove all types that aren't of the primitive type kind. This can happen when the type is
// union of enum types and other types.
return removeTypesFromUnionType(type, /*typeKind*/ typeInfo.flags, /*isOfTypeKind*/ false);
return removeTypesFromUnionType(type, /*typeKind*/ typeInfo.flags, /*isOfTypeKind*/ false, /*allowEmptyUnionResult*/ false);
}
else {
// Assumed result is false. If check was for a primitive type, remove that primitive type
if (typeInfo) {
return removeTypesFromUnionType(type, /*typeKind*/ typeInfo.flags, /*isOfTypeKind*/ true);
return removeTypesFromUnionType(type, /*typeKind*/ typeInfo.flags, /*isOfTypeKind*/ true, /*allowEmptyUnionResult*/ false);
}
// Otherwise we don't have enough information to do anything.
return type;
Expand Down Expand Up @@ -8826,25 +8836,19 @@ module ts {
}

function checkForOfStatement(node: ForOfStatement): void {
if (languageVersion < ScriptTarget.ES6) {
grammarErrorOnFirstToken(node, Diagnostics.for_of_statements_are_only_available_when_targeting_ECMAScript_6_or_higher);
return;
}

checkGrammarForInOrForOfStatement(node)

// Check the LHS and RHS
// If the LHS is a declaration, just check it as a variable declaration, which will in turn check the RHS
// via getTypeForVariableDeclarationInForOfStatement.
// via checkRightHandSideOfForOf.
// If the LHS is an expression, check the LHS, as a destructuring assignment or as a reference.
// Then check that the RHS is assignable to it.
if (node.initializer.kind === SyntaxKind.VariableDeclarationList) {
checkForInOrForOfVariableDeclaration(node);
}
else {
var varExpr = <Expression>node.initializer;
var rightType = checkExpression(node.expression);
var iteratedType = checkIteratedType(rightType, node.expression);
var iteratedType = checkRightHandSideOfForOf(node.expression);

// There may be a destructuring assignment on the left side
if (varExpr.kind === SyntaxKind.ArrayLiteralExpression || varExpr.kind === SyntaxKind.ObjectLiteralExpression) {
Expand Down Expand Up @@ -8926,18 +8930,11 @@ module ts {
}
}

function getTypeForVariableDeclarationInForOfStatement(forOfStatement: ForOfStatement): Type {
// Temporarily return 'any' below ES6
if (languageVersion < ScriptTarget.ES6) {
return anyType;
}

// iteratedType will be undefined if the for-of expression type was missing properties/signatures
// required to get its iteratedType (like [Symbol.iterator] or next). This may be
// because we accessed properties from anyType, or it may have led to an error inside
// getIteratedType.
var expressionType = getTypeOfExpression(forOfStatement.expression);
return checkIteratedType(expressionType, forOfStatement.expression) || anyType;
function checkRightHandSideOfForOf(rhsExpression: Expression): Type {
var expressionType = getTypeOfExpression(rhsExpression);
return languageVersion >= ScriptTarget.ES6
? checkIteratedType(expressionType, rhsExpression)
: checkElementTypeOfArrayOrString(expressionType, rhsExpression);
}

/**
Expand Down Expand Up @@ -9036,6 +9033,72 @@ module ts {
}
}

/**
* This function does the following steps:
* 1. Break up arrayOrStringType (possibly a union) into its string constituents and array constituents.
* 2. Take the element types of the array constituents.
* 3. Return the union of the element types, and string if there was a string constitutent.
*
* For example:
* string -> string
* number[] -> number
* string[] | number[] -> string | number
* string | number[] -> string | number
* string | string[] | number[] -> string | number
*
* It also errors if:
* 1. Some constituent is neither a string nor an array.
* 2. Some constituent is a string and target is less than ES5 (because in ES3 string is not indexable).
*/
function checkElementTypeOfArrayOrString(arrayOrStringType: Type, expressionForError: Expression): Type {
Debug.assert(languageVersion < ScriptTarget.ES6);

// After we remove all types that are StringLike, we will know if there was a string constituent
// based on whether the remaining type is the same as the initial type.
var arrayType = removeTypesFromUnionType(arrayOrStringType, TypeFlags.StringLike, /*isTypeOfKind*/ true, /*allowEmptyUnionResult*/ true);
var hasStringConstituent = arrayOrStringType !== arrayType;

var reportedError = false;
if (hasStringConstituent) {
if (languageVersion < ScriptTarget.ES5) {
error(expressionForError, Diagnostics.Using_a_string_in_a_for_of_statement_is_only_supported_in_ECMAScript_5_and_higher);
reportedError = true;
}

// Now that we've removed all the StringLike types, if no constituents remain, then the entire
// arrayOrStringType was a string.
if (arrayType === emptyObjectType) {
return stringType;
}
}

if (!isArrayLikeType(arrayType)) {
if (!reportedError) {
// Which error we report depends on whether there was a string constituent. For example,
// if the input type is number | string, we want to say that number is not an array type.
// But if the input was just number, we want to say that number is not an array type
// or a string type.
var diagnostic = hasStringConstituent
? Diagnostics.Type_0_is_not_an_array_type
: Diagnostics.Type_0_is_not_an_array_type_or_a_string_type;
error(expressionForError, diagnostic, typeToString(arrayType));
}
return hasStringConstituent ? stringType : unknownType;
}

var arrayElementType = getIndexTypeOfType(arrayType, IndexKind.Number) || unknownType;
if (hasStringConstituent) {
// This is just an optimization for the case where arrayOrStringType is string | string[]
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you add, at the top of this function, the types of types you want this fnction to be able to handle, and what final type will be returned?

Note: this function never seems to return 'undefined'. But i thought you used || earlier to default to hte 'any' type of you got 'undefined' back from this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah I will add a summary of what the result should be, as well as some examples.

if (arrayElementType.flags & TypeFlags.StringLike) {
return stringType;
}

return getUnionType([arrayElementType, stringType]);
}

return arrayElementType;
}

function checkBreakOrContinueStatement(node: BreakOrContinueStatement) {
// Grammar checking
checkGrammarStatementInAmbientContext(node) || checkGrammarBreakOrContinueStatement(node);
Expand Down Expand Up @@ -10980,7 +11043,6 @@ module ts {
}

function isUnknownIdentifier(location: Node, name: string): boolean {
// Do not call resolveName on a synthesized node!
Debug.assert(!nodeIsSynthesized(location), "isUnknownIdentifier called with a synthesized location");
return !resolveName(location, name, SymbolFlags.Value, /*nodeNotFoundMessage*/ undefined, /*nameArg*/ undefined) &&
!hasProperty(getGeneratedNamesForSourceFile(getSourceFile(location)), name);
Expand Down
3 changes: 2 additions & 1 deletion src/compiler/diagnosticInformationMap.generated.ts
Original file line number Diff line number Diff line change
Expand Up @@ -327,7 +327,6 @@ module ts {
Property_0_does_not_exist_on_const_enum_1: { code: 2479, category: DiagnosticCategory.Error, key: "Property '{0}' does not exist on 'const' enum '{1}'." },
let_is_not_allowed_to_be_used_as_a_name_in_let_or_const_declarations: { code: 2480, category: DiagnosticCategory.Error, key: "'let' is not allowed to be used as a name in 'let' or 'const' declarations." },
Cannot_initialize_outer_scoped_variable_0_in_the_same_scope_as_block_scoped_declaration_1: { code: 2481, category: DiagnosticCategory.Error, key: "Cannot initialize outer scoped variable '{0}' in the same scope as block scoped declaration '{1}'." },
for_of_statements_are_only_available_when_targeting_ECMAScript_6_or_higher: { code: 2482, category: DiagnosticCategory.Error, key: "'for...of' statements are only available when targeting ECMAScript 6 or higher." },
The_left_hand_side_of_a_for_of_statement_cannot_use_a_type_annotation: { code: 2483, category: DiagnosticCategory.Error, key: "The left-hand side of a 'for...of' statement cannot use a type annotation." },
Export_declaration_conflicts_with_exported_declaration_of_0: { code: 2484, category: DiagnosticCategory.Error, key: "Export declaration conflicts with exported declaration of '{0}'" },
The_left_hand_side_of_a_for_of_statement_cannot_be_a_previously_defined_constant: { code: 2485, category: DiagnosticCategory.Error, key: "The left-hand side of a 'for...of' statement cannot be a previously defined constant." },
Expand All @@ -339,6 +338,8 @@ module ts {
The_left_hand_side_of_a_for_in_statement_cannot_be_a_destructuring_pattern: { code: 2491, category: DiagnosticCategory.Error, key: "The left-hand side of a 'for...in' statement cannot be a destructuring pattern." },
Cannot_redeclare_identifier_0_in_catch_clause: { code: 2492, category: DiagnosticCategory.Error, key: "Cannot redeclare identifier '{0}' in catch clause" },
Tuple_type_0_with_length_1_cannot_be_assigned_to_tuple_with_length_2: { code: 2493, category: DiagnosticCategory.Error, key: "Tuple type '{0}' with length '{1}' cannot be assigned to tuple with length '{2}'." },
Using_a_string_in_a_for_of_statement_is_only_supported_in_ECMAScript_5_and_higher: { code: 2494, category: DiagnosticCategory.Error, key: "Using a string in a 'for...of' statement is only supported in ECMAScript 5 and higher." },
Type_0_is_not_an_array_type_or_a_string_type: { code: 2461, category: DiagnosticCategory.Error, key: "Type '{0}' is not an array type or a string type." },
Import_declaration_0_is_using_private_name_1: { code: 4000, category: DiagnosticCategory.Error, key: "Import declaration '{0}' is using private name '{1}'." },
Type_parameter_0_of_exported_class_has_or_is_using_private_name_1: { code: 4002, category: DiagnosticCategory.Error, key: "Type parameter '{0}' of exported class has or is using private name '{1}'." },
Type_parameter_0_of_exported_interface_has_or_is_using_private_name_1: { code: 4004, category: DiagnosticCategory.Error, key: "Type parameter '{0}' of exported interface has or is using private name '{1}'." },
Expand Down
12 changes: 8 additions & 4 deletions src/compiler/diagnosticMessages.json
Original file line number Diff line number Diff line change
Expand Up @@ -1299,10 +1299,6 @@
"category": "Error",
"code": 2481
},
"'for...of' statements are only available when targeting ECMAScript 6 or higher.": {
"category": "Error",
"code": 2482
},
"The left-hand side of a 'for...of' statement cannot use a type annotation.": {
"category": "Error",
"code": 2483
Expand Down Expand Up @@ -1347,6 +1343,14 @@
"category": "Error",
"code": 2493
},
"Using a string in a 'for...of' statement is only supported in ECMAScript 5 and higher.": {
"category": "Error",
"code": 2494
},
"Type '{0}' is not an array type or a string type.": {
"category": "Error",
"code": 2461
},

"Import declaration '{0}' is using private name '{1}'.": {
"category": "Error",
Expand Down
7 changes: 7 additions & 0 deletions tests/baselines/reference/ES3For-ofTypeCheck1.errors.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
tests/cases/conformance/statements/for-ofStatements/ES3For-ofTypeCheck1.ts(1,15): error TS2494: Using a string in a 'for...of' statement is only supported in ECMAScript 5 and higher.


==== tests/cases/conformance/statements/for-ofStatements/ES3For-ofTypeCheck1.ts (1 errors) ====
for (var v of "") { }
~~
!!! error TS2494: Using a string in a 'for...of' statement is only supported in ECMAScript 5 and higher.
7 changes: 7 additions & 0 deletions tests/baselines/reference/ES3For-ofTypeCheck1.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
//// [ES3For-ofTypeCheck1.ts]
for (var v of "") { }

//// [ES3For-ofTypeCheck1.js]
for (var _i = 0, _a = ""; _i < _a.length; _i++) {
var v = _a[_i];
}
9 changes: 9 additions & 0 deletions tests/baselines/reference/ES3For-ofTypeCheck2.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
//// [ES3For-ofTypeCheck2.ts]
for (var v of [true]) { }

//// [ES3For-ofTypeCheck2.js]
for (var _i = 0, _a = [
true
]; _i < _a.length; _i++) {
var v = _a[_i];
}
5 changes: 5 additions & 0 deletions tests/baselines/reference/ES3For-ofTypeCheck2.types
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
=== tests/cases/conformance/statements/for-ofStatements/ES3For-ofTypeCheck2.ts ===
for (var v of [true]) { }
>v : boolean
>[true] : boolean[]

8 changes: 8 additions & 0 deletions tests/baselines/reference/ES3For-ofTypeCheck4.errors.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
tests/cases/conformance/statements/for-ofStatements/ES3For-ofTypeCheck4.ts(2,17): error TS2494: Using a string in a 'for...of' statement is only supported in ECMAScript 5 and higher.


==== tests/cases/conformance/statements/for-ofStatements/ES3For-ofTypeCheck4.ts (1 errors) ====
var union: string | string[];
for (const v of union) { }
~~~~~
!!! error TS2494: Using a string in a 'for...of' statement is only supported in ECMAScript 5 and higher.
9 changes: 9 additions & 0 deletions tests/baselines/reference/ES3For-ofTypeCheck4.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
//// [ES3For-ofTypeCheck4.ts]
var union: string | string[];
for (const v of union) { }

//// [ES3For-ofTypeCheck4.js]
var union;
for (var _i = 0; _i < union.length; _i++) {
var v = union[_i];
}
9 changes: 9 additions & 0 deletions tests/baselines/reference/ES3For-ofTypeCheck6.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
//// [ES3For-ofTypeCheck6.ts]
var union: string[] | number[];
for (var v of union) { }

//// [ES3For-ofTypeCheck6.js]
var union;
for (var _i = 0; _i < union.length; _i++) {
var v = union[_i];
}
8 changes: 8 additions & 0 deletions tests/baselines/reference/ES3For-ofTypeCheck6.types
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
=== tests/cases/conformance/statements/for-ofStatements/ES3For-ofTypeCheck6.ts ===
var union: string[] | number[];
>union : string[] | number[]

for (var v of union) { }
>v : string | number
>union : string[] | number[]

6 changes: 3 additions & 3 deletions tests/baselines/reference/ES5For-of1.errors.txt
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
tests/cases/conformance/statements/for-ofStatements/ES5For-of1.ts(1,1): error TS2482: 'for...of' statements are only available when targeting ECMAScript 6 or higher.
tests/cases/conformance/statements/for-ofStatements/ES5For-of1.ts(2,5): error TS2304: Cannot find name 'console'.


==== tests/cases/conformance/statements/for-ofStatements/ES5For-of1.ts (1 errors) ====
for (var v of ['a', 'b', 'c']) {
~~~
!!! error TS2482: 'for...of' statements are only available when targeting ECMAScript 6 or higher.
console.log(v);
~~~~~~~
!!! error TS2304: Cannot find name 'console'.
}
13 changes: 0 additions & 13 deletions tests/baselines/reference/ES5For-of10.errors.txt

This file was deleted.

29 changes: 29 additions & 0 deletions tests/baselines/reference/ES5For-of10.types
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
=== tests/cases/conformance/statements/for-ofStatements/ES5For-of10.ts ===
function foo() {
>foo : () => { x: number; }

return { x: 0 };
>{ x: 0 } : { x: number; }
>x : number
}
for (foo().x of []) {
>foo().x : number
>foo() : { x: number; }
>foo : () => { x: number; }
>x : number
>[] : undefined[]

for (foo().x of [])
>foo().x : number
>foo() : { x: number; }
>foo : () => { x: number; }
>x : number
>[] : undefined[]

var p = foo().x;
>p : number
>foo().x : number
>foo() : { x: number; }
>foo : () => { x: number; }
>x : number
}
8 changes: 0 additions & 8 deletions tests/baselines/reference/ES5For-of11.errors.txt

This file was deleted.

8 changes: 8 additions & 0 deletions tests/baselines/reference/ES5For-of11.types
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
=== tests/cases/conformance/statements/for-ofStatements/ES5For-of11.ts ===
var v;
>v : any

for (v of []) { }
>v : any
>[] : undefined[]

Loading