Start work on improving subtype error messages #7404
Conversation
zth
force-pushed
the
better-subtype-error-messages
branch
2 times, most recently
from
55476b9 to
1093261
Compare
| issues: Record_coercion.record_field_subtype_violation list; | ||
| } | ||
|
|
||
| exception Subtype of type_pairs * type_pairs * subtype_context option |
There was a problem hiding this comment.
Adding subtype_context is the real change here. It tries to track the context a subtype issue was found in, so better error messages can be presented.
| | Tarrow (l1, t1, u1, _, _), Tarrow (l2, t2, u2, _, _) | ||
| when Asttypes.Noloc.same_arg_label l1 l2 -> | ||
| let cstrs = subtype_rec env ((t2, t1) :: trace) t2 t1 cstrs in | ||
| subtype_rec env ((u1, u2) :: trace) u1 u2 cstrs | ||
| | Ttuple tl1, Ttuple tl2 -> subtype_list env trace tl1 tl2 cstrs | ||
| | Ttuple tl1, Ttuple tl2 -> | ||
| (* TODO(subtype-errors) Tuple as context *) |
There was a problem hiding this comment.
Can leave these TODO:s in here since I'll do another pass on this to extend it more soonish.
rescript
@rescript/darwin-x64
@rescript/darwin-arm64
@rescript/linux-arm64
@rescript/win32-x64
@rescript/linux-x64
commit: |
zth
changed the title
This reverts commit 55476b9.
zth
force-pushed
the
better-subtype-error-messages
branch
from
fcffc7e to
e3a2b95
Compare
zth
requested review from
cristianoc,
cknitt and
tsnobip
and removed request for
cristianoc
|
@cristianoc would you mind having a look at the general approach here, to make sure it's good enough? @tsnobip @cknitt would you mind looking at the error messages, like you have in the previous PRs? 🙏 |
| The record [1;33mx[0m cannot be coerced to the record [1;33my[0m because: | ||
| - The field [1;33mx[0m is optional in record [1;33mx[0m, but is not optional in record [1;33my[0m |
There was a problem hiding this comment.
the error message is very detailed and insightful, nice! That reminds me that the other way around is not yet possible unfortunately, you can't coerce to a type that has more optional fields than the original type.
module Foo = {
type t = {foo: int}
}
module Bar = {
type t = {foo: int, bar?: int}
}
let foo = {Foo.foo: 1}
let bar = (foo :> Bar.t) // errorThere was a problem hiding this comment.
Would that be reasonable to support?
There was a problem hiding this comment.
definitely low-priority but I once had the need for such a thing and I can't think of any argument against it.
There was a problem hiding this comment.
@cristianoc you see any issues with supporting it?
| Type x is not a subtype of y | ||
|
|
||
| The record [1;33mx[0m cannot be coerced to the record [1;33my[0m because: | ||
| - Field [1;33mx[0m runtime representation is configured to be [1;33m"z"[0m (via the @as attribute) in record [1;33my[0m, but in record [1;33mx[0m it is configured to be [1;33m"w"[0m (via the @as attribute). Runtime representations must match. |
This improves the subtype error messages to make them understandable/actionable where relevant.
In this first iteration I've focused on adding optional context to each subtype error. We use that to, where possible, provide better subtype errors, that are actionable.
This does not cover all cases, and there's plenty of follow ups (some listed below). But, it's a start, and validates the concept that we can do this this way.
Follow ups
Will put these in a separate issue to be tracked
Then there's the larger follow up of tracking where in a nested subtype check an error is happening. Example: Records in records, where a record further down the type structure is not a subtype.