Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit 6691408

Browse filesBrowse files
JsonFreemanJsonFreeman
committed
Address PR feedback
1 parent 22f80b9 commit 6691408
Copy full SHA for 6691408

7 files changed

+73-19Lines changed: 73 additions & 19 deletions

File tree

Expand file treeCollapse file tree
Open diff view settings
Filter options
Expand file treeCollapse file tree
Open diff view settings
Collapse file

‎src/compiler/checker.ts‎

Copy file name to clipboardExpand all lines: src/compiler/checker.ts
+50-17Lines changed: 50 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -4763,17 +4763,22 @@ module ts {
47634763

47644764
// For a union type, remove all constituent types that are of the given type kind (when isOfTypeKind is true)
47654765
// or not of the given type kind (when isOfTypeKind is false)
4766-
function removeTypesFromUnionType(type: Type, typeKind: TypeFlags, isOfTypeKind: boolean): Type {
4766+
function removeTypesFromUnionType(type: Type, typeKind: TypeFlags, isOfTypeKind: boolean, allowEmptyUnionResult: boolean): Type {
47674767
if (type.flags & TypeFlags.Union) {
47684768
var types = (<UnionType>type).types;
47694769
if (forEach(types, t => !!(t.flags & typeKind) === isOfTypeKind)) {
47704770
// Above we checked if we have anything to remove, now use the opposite test to do the removal
47714771
var narrowedType = getUnionType(filter(types, t => !(t.flags & typeKind) === isOfTypeKind));
4772-
if (narrowedType !== emptyObjectType) {
4772+
if (allowEmptyUnionResult || narrowedType !== emptyObjectType) {
47734773
return narrowedType;
47744774
}
47754775
}
47764776
}
4777+
else if (allowEmptyUnionResult && !!(type.flags & typeKind) === isOfTypeKind) {
4778+
// Use getUnionType(emptyArray) instead of emptyObjectType in case the way empty union types
4779+
// are represented ever changes.
4780+
return getUnionType(emptyArray);
4781+
}
47774782
return type;
47784783
}
47794784

@@ -4976,20 +4981,21 @@ module ts {
49764981
if (assumeTrue) {
49774982
// Assumed result is true. If check was not for a primitive type, remove all primitive types
49784983
if (!typeInfo) {
4979-
return removeTypesFromUnionType(type, /*typeKind*/ TypeFlags.StringLike | TypeFlags.NumberLike | TypeFlags.Boolean | TypeFlags.ESSymbol, /*isOfTypeKind*/ true);
4984+
return removeTypesFromUnionType(type, /*typeKind*/ TypeFlags.StringLike | TypeFlags.NumberLike | TypeFlags.Boolean | TypeFlags.ESSymbol,
4985+
/*isOfTypeKind*/ true, /*allowEmptyUnionResult*/ false);
49804986
}
49814987
// Check was for a primitive type, return that primitive type if it is a subtype
49824988
if (isTypeSubtypeOf(typeInfo.type, type)) {
49834989
return typeInfo.type;
49844990
}
49854991
// Otherwise, remove all types that aren't of the primitive type kind. This can happen when the type is
49864992
// union of enum types and other types.
4987-
return removeTypesFromUnionType(type, /*typeKind*/ typeInfo.flags, /*isOfTypeKind*/ false);
4993+
return removeTypesFromUnionType(type, /*typeKind*/ typeInfo.flags, /*isOfTypeKind*/ false, /*allowEmptyUnionResult*/ false);
49884994
}
49894995
else {
49904996
// Assumed result is false. If check was for a primitive type, remove that primitive type
49914997
if (typeInfo) {
4992-
return removeTypesFromUnionType(type, /*typeKind*/ typeInfo.flags, /*isOfTypeKind*/ true);
4998+
return removeTypesFromUnionType(type, /*typeKind*/ typeInfo.flags, /*isOfTypeKind*/ true, /*allowEmptyUnionResult*/ false);
49934999
}
49945000
// Otherwise we don't have enough information to do anything.
49955001
return type;
@@ -9027,28 +9033,55 @@ module ts {
90279033
}
90289034
}
90299035

9036+
/**
9037+
* This function does the following steps:
9038+
* 1. Break up arrayOrStringType (possibly a union) into its string constituents and array constituents.
9039+
* 2. Take the element types of the array constituents.
9040+
* 3. Return the union of the element types, and string if there was a string constitutent.
9041+
*
9042+
* For example:
9043+
* string -> string
9044+
* number[] -> number
9045+
* string[] | number[] -> string | number
9046+
* string | number[] -> string | number
9047+
* string | string[] | number[] -> string | number
9048+
*
9049+
* It also errors if:
9050+
* 1. Some constituent is neither a string nor an array.
9051+
* 2. Some constituent is a string and target is less than ES5 (because in ES3 string is not indexable).
9052+
*/
90309053
function checkElementTypeOfArrayOrString(arrayOrStringType: Type, expressionForError: Expression): Type {
90319054
Debug.assert(languageVersion < ScriptTarget.ES6);
9032-
var isJustString = allConstituentTypesHaveKind(arrayOrStringType, TypeFlags.StringLike);
90339055

9034-
// Check isJustString because removeTypesFromUnionType will only remove types if it doesn't result
9035-
// in an emptyObjectType. In this case, we actually do want the emptyObjectType.
9036-
var arrayType = isJustString ? emptyObjectType : removeTypesFromUnionType(arrayOrStringType, TypeFlags.StringLike, /*isTypeOfKind*/ true);
9037-
var hasStringConstituent = arrayOrStringType !== emptyObjectType && arrayOrStringType !== arrayType;
9056+
// After we remove all types that are StringLike, we will know if there was a string constituent
9057+
// based on whether the remaining type is the same as the initial type.
9058+
var arrayType = removeTypesFromUnionType(arrayOrStringType, TypeFlags.StringLike, /*isTypeOfKind*/ true, /*allowEmptyUnionResult*/ true);
9059+
var hasStringConstituent = arrayOrStringType !== arrayType;
90389060

90399061
var reportedError = false;
9040-
if (hasStringConstituent && languageVersion < ScriptTarget.ES5) {
9041-
error(expressionForError, Diagnostics.Using_a_string_in_a_for_of_statement_is_only_supported_in_ECMAScript_5_and_higher);
9042-
reportedError = true;
9043-
}
9062+
if (hasStringConstituent) {
9063+
if (languageVersion < ScriptTarget.ES5) {
9064+
error(expressionForError, Diagnostics.Using_a_string_in_a_for_of_statement_is_only_supported_in_ECMAScript_5_and_higher);
9065+
reportedError = true;
9066+
}
90449067

9045-
if (isJustString) {
9046-
return stringType;
9068+
// Now that we've removed all the StringLike types, if no constituents remain, then the entire
9069+
// arrayOrStringType was a string.
9070+
if (arrayType === emptyObjectType) {
9071+
return stringType;
9072+
}
90479073
}
90489074

90499075
if (!isArrayLikeType(arrayType)) {
90509076
if (!reportedError) {
9051-
error(expressionForError, Diagnostics.Type_0_is_not_an_array_type, typeToString(arrayType));
9077+
// Which error we report depends on whether there was a string constituent. For example,
9078+
// if the input type is number | string, we want to say that number is not an array type.
9079+
// But if the input was just number, we want to say that number is not an array type
9080+
// or a string type.
9081+
var diagnostic = hasStringConstituent
9082+
? Diagnostics.Type_0_is_not_an_array_type
9083+
: Diagnostics.Type_0_is_not_an_array_type_or_a_string_type;
9084+
error(expressionForError, diagnostic, typeToString(arrayType));
90529085
}
90539086
return hasStringConstituent ? stringType : unknownType;
90549087
}
Collapse file

‎src/compiler/diagnosticInformationMap.generated.ts‎

Copy file name to clipboardExpand all lines: src/compiler/diagnosticInformationMap.generated.ts
+1Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -339,6 +339,7 @@ module ts {
339339
Cannot_redeclare_identifier_0_in_catch_clause: { code: 2492, category: DiagnosticCategory.Error, key: "Cannot redeclare identifier '{0}' in catch clause" },
340340
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}'." },
341341
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." },
342+
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." },
342343
Import_declaration_0_is_using_private_name_1: { code: 4000, category: DiagnosticCategory.Error, key: "Import declaration '{0}' is using private name '{1}'." },
343344
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}'." },
344345
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}'." },
Collapse file

‎src/compiler/diagnosticMessages.json‎

Copy file name to clipboardExpand all lines: src/compiler/diagnosticMessages.json
+4Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1347,6 +1347,10 @@
13471347
"category": "Error",
13481348
"code": 2494
13491349
},
1350+
"Type '{0}' is not an array type or a string type.": {
1351+
"category": "Error",
1352+
"code": 2461
1353+
},
13501354

13511355
"Import declaration '{0}' is using private name '{1}'.": {
13521356
"category": "Error",
Collapse file

‎tests/baselines/reference/ES5For-ofTypeCheck10.errors.txt‎

Copy file name to clipboardExpand all lines: tests/baselines/reference/ES5For-ofTypeCheck10.errors.txt
+2-2Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,11 @@
1-
tests/cases/conformance/statements/for-ofStatements/ES5For-ofTypeCheck10.ts(1,15): error TS2461: Type 'StringIterator' is not an array type.
1+
tests/cases/conformance/statements/for-ofStatements/ES5For-ofTypeCheck10.ts(1,15): error TS2461: Type 'StringIterator' is not an array type or a string type.
22
tests/cases/conformance/statements/for-ofStatements/ES5For-ofTypeCheck10.ts(11,6): error TS2304: Cannot find name 'Symbol'.
33

44

55
==== tests/cases/conformance/statements/for-ofStatements/ES5For-ofTypeCheck10.ts (2 errors) ====
66
for (var v of new StringIterator) { }
77
~~~~~~~~~~~~~~~~~~
8-
!!! error TS2461: Type 'StringIterator' is not an array type.
8+
!!! error TS2461: Type 'StringIterator' is not an array type or a string type.
99

1010
// In ES3/5, you cannot for...of over an arbitrary iterable.
1111
class StringIterator {
Collapse file

‎tests/baselines/reference/ES5For-ofTypeCheck12.errors.txt‎

Copy file name to clipboard
+7Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
tests/cases/conformance/statements/for-ofStatements/ES5For-ofTypeCheck12.ts(1,17): error TS2461: Type 'number' is not an array type or a string type.
2+
3+
4+
==== tests/cases/conformance/statements/for-ofStatements/ES5For-ofTypeCheck12.ts (1 errors) ====
5+
for (const v of 0) { }
6+
~
7+
!!! error TS2461: Type 'number' is not an array type or a string type.
Collapse file

‎tests/baselines/reference/ES5For-ofTypeCheck12.js‎

Copy file name to clipboard
+7Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
//// [ES5For-ofTypeCheck12.ts]
2+
for (const v of 0) { }
3+
4+
//// [ES5For-ofTypeCheck12.js]
5+
for (var _i = 0, _a = 0; _i < _a.length; _i++) {
6+
var v = _a[_i];
7+
}
Collapse file

‎tests/cases/conformance/statements/for-ofStatements/ES5For-ofTypeCheck12.ts‎

Copy file name to clipboard
+2Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
//@target: ES5
2+
for (const v of 0) { }

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.