Improve the error message when asserting to a type that is not comparable to the original. #25541
Conversation
src/compiler/checker.ts
Outdated
| @@ -19877,7 +19877,8 @@ namespace ts { | |||
| if (produceDiagnostics && targetType !== errorType) { | |||
| const widenedType = getWidenedType(exprType); | |||
| if (!isTypeComparableTo(targetType, widenedType)) { | |||
| checkTypeComparableTo(exprType, targetType, errNode, Diagnostics.Type_0_cannot_be_converted_to_type_1); | |||
| checkTypeComparableTo(exprType, targetType, errNode, | |||
| Diagnostics.Type_0_cannot_be_converted_directly_to_type_1_because_it_is_neither_a_supertype_nor_a_subtype_of_1_If_this_is_intentional_convert_to_unknown_first); | |||
There was a problem hiding this comment.
It's really not only about supertype vs. subtype: we have these relationships for subtyping, assignability, and comparability). Comparability has some other properties that are technically more lax.
The comparability and assignment compatibility rules differ only in that
- a union type is comparable to a target type if any of its constituents, rather than all of its constituents, are comparable to that target type,
- whether a type is considered weak (3.11.7) has no bearing on whether two types are comparable,
- whether an object type has excess properties (3.11.8) has no bearing on whether two types are comparable,
- an object type with some optional property is comparable to an object type containing a property of the same name if it has a compatible type, regardless of whether that property is optional, and
- when relating any two signatures, each signature is always instantiated using type Any for all type arguments.
So maybe you want something like
Type '{0}' cannot be converted to type '{1}' because neither type sufficiently overlaps with the other.
There was a problem hiding this comment.
@DanielRosenwasser worth reading the discussion in the linked issue IMO
There was a problem hiding this comment.
The comparability and assignment compatibility rules differ only in that [...]
Where is this secret more-up-to-date version of the language spec? Aha, web search is my friend.
There was a problem hiding this comment.
To try to move this forward, I went with the "neither type sufficiently overlaps with the other" wording.
mattmccutchen
force-pushed
the
issue-25539
branch
2 times, most recently
from
1c675dd to
f1fabd8
Compare
mattmccutchen
changed the title
|
@DanielRosenwasser can you take another look please. |
src/compiler/diagnosticMessages.json
Outdated
| @@ -2413,6 +2413,10 @@ | |||
| "category": "Error", | |||
| "code": 2729 | |||
| }, | |||
| "Implicit conversion of a symbol to a string will fail at runtime.": { | |||
There was a problem hiding this comment.
Can you wrap symbol and string in single-quotes so our localization team knows not to translate them? Also, can you add a suggestion at the end like "Consider wrapping this expression in 'String(...)' if this is intentional."
mattmccutchen
force-pushed
the
issue-25539
branch
from
f1fabd8 to
aea6196
Compare
src/compiler/diagnosticMessages.json
Outdated
| @@ -1196,7 +1196,7 @@ | |||
| "category": "Error", | |||
| "code": 2351 | |||
| }, | |||
| "Type '{0}' cannot be converted to type '{1}'.": { | |||
| "Conversion of type '{0}' to type '{1}' may be a mistake because neither type sufficiently overlaps with the other. If it is intended, convert to 'unknown' first.": { | |||
There was a problem hiding this comment.
Final nit: can you make this "If this was intentional, convert the expression to 'unknown' first."
comparable to the original. Also improve the error message for implicit conversion of a symbol to a string in a template literal, which previously shared the error message with type assertions. Fixes microsoft#25539. Addresses microsoft#25870.
mattmccutchen
force-pushed
the
issue-25539
branch
from
aea6196 to
da64479
Compare
|
Thanks! |
Also improve the error message for implicit conversion of a symbol to a string in a template literal, which previously shared the error message with type assertions.
Fixes #25539. Addresses #25870.