Uh oh!
There was an error while loading. Please reload this page.
Introduce the "comparable" relation#5517
Uh oh!
There was an error while loading. Please reload this page.
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -5397,6 +5397,10 @@ namespace ts { | ||
| return checkTypeAssignableTo(source, target, /*errorNode*/ undefined); | ||
| } | ||
| /** | ||
| * This is *not* a bi-directional relationship. | ||
| * If one needs to check both directions for comparability, use a second call to this function or 'checkTypeComparableTo'. | ||
There was a problem hiding this comment. All uses do need to check both directions, so probably you should have a version that does that. You might not even need the uni-directional ones except for consistency with the other relations. There was a problem hiding this comment. See this comment | ||
| */ | ||
| function isTypeComparableTo(source: Type, target: Type): boolean { | ||
| return checkTypeComparableTo(source, target, /*errorNode*/ undefined); | ||
| } | ||
| @@ -5409,6 +5413,10 @@ namespace ts { | ||
| return checkTypeRelatedTo(source, target, assignableRelation, errorNode, headMessage, containingMessageChain); | ||
| } | ||
| /** | ||
| * This is *not* a bi-directional relationship. | ||
| * If one needs to check both directions for comparability, use a second call to this function or 'isTypeComparableTo'. | ||
| */ | ||
| function checkTypeComparableTo(source: Type, target: Type, errorNode: Node, headMessage?: DiagnosticMessage, containingMessageChain?: DiagnosticMessageChain): boolean { | ||
There was a problem hiding this comment. nit: This function is hard to read on a portrait (1200 px) monitor with github's generous margins cutting it down to about 700 px. Might not be worth fixing. | ||
| return checkTypeRelatedTo(source, target, comparableRelation, errorNode, headMessage, containingMessageChain); | ||
| } | ||
| @@ -5619,7 +5627,14 @@ namespace ts { | ||
| sourceType = typeToString(source, /*enclosingDeclaration*/ undefined, TypeFormatFlags.UseFullyQualifiedType); | ||
| targetType = typeToString(target, /*enclosingDeclaration*/ undefined, TypeFormatFlags.UseFullyQualifiedType); | ||
| } | ||
| if (!message) { | ||
| message = relation === comparableRelation ? | ||
| Diagnostics.Type_0_is_not_comparable_to_type_1 : | ||
| Diagnostics.Type_0_is_not_assignable_to_type_1; | ||
| } | ||
| reportError(message, sourceType, targetType); | ||
| } | ||
| // Compare two types and return | ||
| @@ -5648,10 +5663,12 @@ namespace ts { | ||
| } | ||
| } | ||
| if (source.flags & TypeFlags.StringLiteral && target === stringType) return Ternary.True; | ||
| if (relation === assignableRelation || relation === comparableRelation) { | ||
| if (isTypeAny(source)) return Ternary.True; | ||
| if (source === numberType && target.flags & TypeFlags.Enum) return Ternary.True; | ||
| } | ||
| if (source.flags & TypeFlags.Boolean && target.flags & TypeFlags.Boolean) { | ||
| return Ternary.True; | ||
| } | ||
| @@ -5674,26 +5691,28 @@ namespace ts { | ||
| const saveErrorInfo = errorInfo; | ||
| // Note that these checks are specifically ordered to produce correct results. | ||
| if (source.flags & TypeFlags.Union) { | ||
| if (relation === comparableRelation) { | ||
| result = someTypeRelatedToType(source as UnionType, target, reportErrors); | ||
There was a problem hiding this comment. Please choose one cast syntax and make at least the changed lines use it, preferably something like all the casts in the function. | ||
| } | ||
| else { | ||
| result = eachTypeRelatedToType(source as UnionType, target, reportErrors); | ||
| } | ||
| if (result) { | ||
| return result; | ||
| } | ||
| } | ||
| else if (target.flags & TypeFlags.Intersection) { | ||
| result = typeRelatedToEachType(source, target as IntersectionType, reportErrors); | ||
| if (result) { | ||
| return result; | ||
| } | ||
| } | ||
| else { | ||
| // It is necessary to try these "some" checks on both sides because there may be nested "each" checks | ||
| // on either side that need to be prioritized. For example, A | B = (A | B) & (C | D) or | ||
| // A & B = (A & B) | (C & D). | ||
| if (source.flags & TypeFlags.Intersection) { | ||
| @@ -5775,8 +5794,8 @@ namespace ts { | ||
| } | ||
| if (source.flags & TypeFlags.Union && target.flags & TypeFlags.Union || | ||
| source.flags & TypeFlags.Intersection && target.flags & TypeFlags.Intersection) { | ||
| if (result = eachTypeRelatedToSomeType(<UnionOrIntersectionType>source, <UnionOrIntersectionType>target, /*reportErrors*/ false)) { | ||
| if (result &= eachTypeRelatedToSomeType(<UnionOrIntersectionType>target, <UnionOrIntersectionType>source, /*reportErrors*/ false)) { | ||
| return result; | ||
| } | ||
| } | ||
| @@ -5827,7 +5846,7 @@ namespace ts { | ||
| return false; | ||
| } | ||
| function eachTypeRelatedToSomeType(source: UnionOrIntersectionType, target: UnionOrIntersectionType, reportErrors: boolean): Ternary { | ||
| let result = Ternary.True; | ||
| const sourceTypes = source.types; | ||
| for (const sourceType of sourceTypes) { | ||
| @@ -11015,7 +11034,7 @@ namespace ts { | ||
| if (produceDiagnostics && targetType !== unknownType) { | ||
| const widenedType = getWidenedType(exprType); | ||
| if (!isTypeComparableTo(targetType, widenedType)) { | ||
| checkTypeComparableTo(exprType, targetType, node, Diagnostics.Type_0_cannot_be_converted_to_type_1); | ||
| } | ||
| } | ||
| return targetType; | ||
| @@ -14458,7 +14477,7 @@ namespace ts { | ||
| if (produceDiagnostics && clause.kind === SyntaxKind.CaseClause) { | ||
| const caseClause = <CaseClause>clause; | ||
| // TypeScript 1.0 spec (April 2014):5.9 | ||
| // In a 'switch' statement, each 'case' expression must be of a type that is comparable | ||
| // to or from the type of the 'switch' expression. | ||
| const caseType = checkExpression(caseClause.expression); | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,8 +1,8 @@ | ||
| tests/cases/conformance/expressions/asOperator/asOperator2.ts(1,9): error TS2352: Type 'number' cannot be converted to type 'string'. | ||
| ==== tests/cases/conformance/expressions/asOperator/asOperator2.ts (1 errors) ==== | ||
| var x = 23 as string; | ||
| ~~~~~~~~~~~~ | ||
| !!! error TS2352: Type 'number' cannot be converted to type 'string'. | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,10 +1,10 @@ | ||
| tests/cases/conformance/expressions/asOperator/asOperatorContextualType.ts(2,9): error TS2352: Type '(v: number) => number' cannot be converted to type '(x: number) => string'. | ||
| Type 'number' is not comparable to type 'string'. | ||
| ==== tests/cases/conformance/expressions/asOperator/asOperatorContextualType.ts (1 errors) ==== | ||
| // should error | ||
| var x = (v => v) as (x: number) => string; | ||
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ | ||
| !!! error TS2352: Type '(v: number) => number' cannot be converted to type '(x: number) => string'. | ||
| !!! error TS2352: Type 'number' is not comparable to type 'string'. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,11 +1,11 @@ | ||
| tests/cases/conformance/expressions/asOperator/asOperatorNames.ts(2,9): error TS2352: Type 'number' cannot be converted to type 'string'. | ||
| ==== tests/cases/conformance/expressions/asOperator/asOperatorNames.ts (1 errors) ==== | ||
| var a = 20; | ||
| var b = a as string; | ||
| ~~~~~~~~~~~ | ||
| !!! error TS2352: Type 'number' cannot be converted to type 'string'. | ||
| var as = "hello"; | ||
| var as1 = as as string; | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,9 +1,9 @@ | ||
| tests/cases/compiler/contextualTyping39.ts(1,11): error TS2352: Type '() => string' cannot be converted to type '() => number'. | ||
| Type 'string' is not comparable to type 'number'. | ||
| ==== tests/cases/compiler/contextualTyping39.ts (1 errors) ==== | ||
| var foo = <{ (): number; }> function() { return "err"; }; | ||
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ | ||
| !!! error TS2352: Type '() => string' cannot be converted to type '() => number'. | ||
| !!! error TS2352: Type 'string' is not comparable to type 'number'. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,9 +1,9 @@ | ||
| tests/cases/compiler/contextualTyping41.ts(1,11): error TS2352: Type '() => string' cannot be converted to type '{ (): number; (i: number): number; }'. | ||
| Type 'string' is not comparable to type 'number'. | ||
| ==== tests/cases/compiler/contextualTyping41.ts (1 errors) ==== | ||
| var foo = <{():number; (i:number):number; }> (function(){return "err";}); | ||
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ | ||
| !!! error TS2352: Type '() => string' cannot be converted to type '{ (): number; (i: number): number; }'. | ||
| !!! error TS2352: Type 'string' is not comparable to type 'number'. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this true of
isTypeAssignableTotoo? Its former usage in this diff indicates 'yes'.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is true of
isTypeAssignableTobutisTypeAssignableTohas a clear direction; on the other hand,isTypeComparableTois vague enough that it warrants a comment. One would think thatAbeing comparable toBimplies thatBis comarable toA, but that's not quite the case.I could have made
isTypeComparableToa bidirectional relation, but I decided not to based on the way in which we use the relation in type assertions. Specifically, with type assertions, we check whether the expression type is comparable to the asserted type and that the asserted type is comparable to the widened type of the expression.Actually, I'm not sure if there is any reason we don't check against the widened type for both. I'll experiment with it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As discussed offline, I think that's a bug. I will change that in a later PR