Introduce the "comparable" relation #5517
Renamed the relationship back to "comparable".
- Loading branch information
commit 841789d162e91f6a88e1db411b7bd22ffc6e23f2
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 |
|---|---|---|
|
|
@@ -211,7 +211,7 @@ namespace ts { | |
|
|
||
| let subtypeRelation: Map<RelationComparisonResult> = {}; | ||
| let assignableRelation: Map<RelationComparisonResult> = {}; | ||
| let comparableRelation: Map<RelationComparisonResult> = {}; | ||
| let identityRelation: Map<RelationComparisonResult> = {}; | ||
|
|
||
| // This is for caching the result of getSymbolDisplayBuilder. Do not access directly. | ||
|
|
@@ -4743,8 +4743,8 @@ namespace ts { | |
| * 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); | ||
| } | ||
|
|
||
| function checkTypeSubtypeOf(source: Type, target: Type, errorNode: Node, headMessage?: DiagnosticMessage, containingMessageChain?: DiagnosticMessageChain): boolean { | ||
|
|
@@ -4757,10 +4757,10 @@ namespace ts { | |
|
|
||
| /** | ||
| * 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); | ||
| } | ||
|
|
||
| function isSignatureAssignableTo(source: Signature, target: Signature): boolean { | ||
|
|
@@ -4773,7 +4773,7 @@ namespace ts { | |
| * Checks if 'source' is related to 'target' (e.g.: is a assignable to). | ||
| * @param source The left-hand-side of the relation. | ||
| * @param target The right-hand-side of the relation. | ||
| * @param relation The relation considered. One of 'identityRelation', 'assignableRelation', 'subTypeRelation', or 'comparableRelation'. | ||
| * Used as both to determine which checks are performed and as a cache of previously computed results. | ||
| * @param errorNode The suggested node upon which all errors will be reported, if defined. This may or may not be the actual node used. | ||
| * @param headMessage If the error chain should be prepended by a head message, then headMessage will be used. | ||
|
|
@@ -4798,7 +4798,7 @@ namespace ts { | |
|
|
||
| Debug.assert(relation !== identityRelation || !errorNode, "no error reporting in identity checking"); | ||
|
|
||
| const isAssignableOrComparableRelation = relation === assignableRelation || relation === comparableRelation; | ||
| let result = isRelatedTo(source, target, errorNode !== undefined, headMessage); | ||
| if (overflow) { | ||
| error(errorNode, Diagnostics.Excessive_stack_depth_comparing_types_0_and_1, typeToString(source), typeToString(target)); | ||
|
|
@@ -4877,7 +4877,7 @@ namespace ts { | |
|
|
||
| // Note that the "each" checks must precede the "some" checks to produce the correct results | ||
|
There was a problem hiding this comment. This comment needs to reflect the more complex checks. |
||
| 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 { | ||
|
|
@@ -9410,8 +9410,8 @@ namespace ts { | |
| if (produceDiagnostics && targetType !== unknownType) { | ||
| let widenedType = getWidenedType(exprType); | ||
|
|
||
| if (!isTypeComparableTo(targetType, widenedType)) { | ||
| checkTypeComparableTo(exprType, targetType, node, Diagnostics.Neither_type_0_nor_type_1_is_assignable_to_the_other); | ||
| } | ||
| } | ||
| return targetType; | ||
|
|
@@ -10254,7 +10254,7 @@ namespace ts { | |
| case SyntaxKind.ExclamationEqualsToken: | ||
| case SyntaxKind.EqualsEqualsEqualsToken: | ||
| case SyntaxKind.ExclamationEqualsEqualsToken: | ||
| if (!isTypeComparableTo(leftType, rightType) && !isTypeComparableTo(rightType, leftType)) { | ||
| reportOperatorError(); | ||
| } | ||
| return booleanType; | ||
|
|
@@ -12722,8 +12722,8 @@ namespace ts { | |
| // In a 'switch' statement, each 'case' expression must be of a type that is assignable to or from the type of the 'switch' expression. | ||
|
There was a problem hiding this comment. update 'assignable' to 'comparable' |
||
| let caseType = checkExpression(caseClause.expression); | ||
|
|
||
| if (!isTypeComparableTo(expressionType, caseType)) { | ||
| checkTypeComparableTo(caseType, expressionType, caseClause.expression, /*headMessage*/ undefined); | ||
| } | ||
| } | ||
| forEach(clause.statements, checkSourceElement); | ||
|
|
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