From 6691408147bb6e773fed3ef116c3fd30ee6c64e4 Mon Sep 17 00:00:00 2001 From: Jason Freeman Date: Thu, 12 Mar 2015 14:56:58 -0700 Subject: [PATCH] Address PR feedback --- src/compiler/checker.ts | 67 ++++++++++++++----- .../diagnosticInformationMap.generated.ts | 1 + src/compiler/diagnosticMessages.json | 4 ++ .../reference/ES5For-ofTypeCheck10.errors.txt | 4 +- .../reference/ES5For-ofTypeCheck12.errors.txt | 7 ++ .../reference/ES5For-ofTypeCheck12.js | 7 ++ .../for-ofStatements/ES5For-ofTypeCheck12.ts | 2 + 7 files changed, 73 insertions(+), 19 deletions(-) create mode 100644 tests/baselines/reference/ES5For-ofTypeCheck12.errors.txt create mode 100644 tests/baselines/reference/ES5For-ofTypeCheck12.js create mode 100644 tests/cases/conformance/statements/for-ofStatements/ES5For-ofTypeCheck12.ts diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index 75c85844286..1500a47f197 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -4763,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 = (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; } @@ -4976,7 +4981,8 @@ 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)) { @@ -4984,12 +4990,12 @@ module ts { } // 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; @@ -9027,28 +9033,55 @@ 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); - var isJustString = allConstituentTypesHaveKind(arrayOrStringType, TypeFlags.StringLike); - // Check isJustString because removeTypesFromUnionType will only remove types if it doesn't result - // in an emptyObjectType. In this case, we actually do want the emptyObjectType. - var arrayType = isJustString ? emptyObjectType : removeTypesFromUnionType(arrayOrStringType, TypeFlags.StringLike, /*isTypeOfKind*/ true); - var hasStringConstituent = arrayOrStringType !== emptyObjectType && arrayOrStringType !== arrayType; + // 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 && languageVersion < ScriptTarget.ES5) { - error(expressionForError, Diagnostics.Using_a_string_in_a_for_of_statement_is_only_supported_in_ECMAScript_5_and_higher); - reportedError = true; - } + 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; + } - if (isJustString) { - return stringType; + // 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) { - error(expressionForError, Diagnostics.Type_0_is_not_an_array_type, typeToString(arrayType)); + // 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; } diff --git a/src/compiler/diagnosticInformationMap.generated.ts b/src/compiler/diagnosticInformationMap.generated.ts index ef8a3936bc3..d40fcd25ce0 100644 --- a/src/compiler/diagnosticInformationMap.generated.ts +++ b/src/compiler/diagnosticInformationMap.generated.ts @@ -339,6 +339,7 @@ module ts { 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}'." }, diff --git a/src/compiler/diagnosticMessages.json b/src/compiler/diagnosticMessages.json index efeb54d93fc..c4121e92251 100644 --- a/src/compiler/diagnosticMessages.json +++ b/src/compiler/diagnosticMessages.json @@ -1347,6 +1347,10 @@ "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", diff --git a/tests/baselines/reference/ES5For-ofTypeCheck10.errors.txt b/tests/baselines/reference/ES5For-ofTypeCheck10.errors.txt index 8725fa62471..9551d0467e9 100644 --- a/tests/baselines/reference/ES5For-ofTypeCheck10.errors.txt +++ b/tests/baselines/reference/ES5For-ofTypeCheck10.errors.txt @@ -1,11 +1,11 @@ -tests/cases/conformance/statements/for-ofStatements/ES5For-ofTypeCheck10.ts(1,15): error TS2461: Type 'StringIterator' is not an array type. +tests/cases/conformance/statements/for-ofStatements/ES5For-ofTypeCheck10.ts(1,15): error TS2461: Type 'StringIterator' is not an array type or a string type. tests/cases/conformance/statements/for-ofStatements/ES5For-ofTypeCheck10.ts(11,6): error TS2304: Cannot find name 'Symbol'. ==== tests/cases/conformance/statements/for-ofStatements/ES5For-ofTypeCheck10.ts (2 errors) ==== for (var v of new StringIterator) { } ~~~~~~~~~~~~~~~~~~ -!!! error TS2461: Type 'StringIterator' is not an array type. +!!! error TS2461: Type 'StringIterator' is not an array type or a string type. // In ES3/5, you cannot for...of over an arbitrary iterable. class StringIterator { diff --git a/tests/baselines/reference/ES5For-ofTypeCheck12.errors.txt b/tests/baselines/reference/ES5For-ofTypeCheck12.errors.txt new file mode 100644 index 00000000000..b726aaad6c7 --- /dev/null +++ b/tests/baselines/reference/ES5For-ofTypeCheck12.errors.txt @@ -0,0 +1,7 @@ +tests/cases/conformance/statements/for-ofStatements/ES5For-ofTypeCheck12.ts(1,17): error TS2461: Type 'number' is not an array type or a string type. + + +==== tests/cases/conformance/statements/for-ofStatements/ES5For-ofTypeCheck12.ts (1 errors) ==== + for (const v of 0) { } + ~ +!!! error TS2461: Type 'number' is not an array type or a string type. \ No newline at end of file diff --git a/tests/baselines/reference/ES5For-ofTypeCheck12.js b/tests/baselines/reference/ES5For-ofTypeCheck12.js new file mode 100644 index 00000000000..6004990f6dc --- /dev/null +++ b/tests/baselines/reference/ES5For-ofTypeCheck12.js @@ -0,0 +1,7 @@ +//// [ES5For-ofTypeCheck12.ts] +for (const v of 0) { } + +//// [ES5For-ofTypeCheck12.js] +for (var _i = 0, _a = 0; _i < _a.length; _i++) { + var v = _a[_i]; +} diff --git a/tests/cases/conformance/statements/for-ofStatements/ES5For-ofTypeCheck12.ts b/tests/cases/conformance/statements/for-ofStatements/ES5For-ofTypeCheck12.ts new file mode 100644 index 00000000000..2d220dc5f26 --- /dev/null +++ b/tests/cases/conformance/statements/for-ofStatements/ES5For-ofTypeCheck12.ts @@ -0,0 +1,2 @@ +//@target: ES5 +for (const v of 0) { } \ No newline at end of file