Skip to content

Commit 6691408

Browse files
JsonFreemanJsonFreeman
committed
Address PR feedback
1 parent 22f80b9 commit 6691408

File tree

7 files changed

+73
-19
lines changed

7 files changed

+73
-19
lines changed

‎src/compiler/checker.ts

Lines 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
}

‎src/compiler/diagnosticInformationMap.generated.ts

Lines 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}'." },

‎src/compiler/diagnosticMessages.json

Lines 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",

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

Lines 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 {

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

Lines 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.

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

Lines 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+
}

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

Lines 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)