-
Notifications
You must be signed in to change notification settings - Fork 471
Error for braced single identifier to record #7806
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
f620b1c
to
1f24505
Compare
(* Special errors for braced identifiers passed to records *) | ||
let context = | ||
match sexp.pexp_desc with | ||
| Pexp_ident _ -> | ||
if | ||
sexp.pexp_attributes | ||
|> List.exists (fun (attr, _) -> attr.txt = "res.braces") | ||
&& is_record_type ~extract_concrete_typedecl ~env ty_expected | ||
then Some Error_message_utils.BracedIdent | ||
else context | ||
| _ -> context | ||
in |
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.
@cristianoc I've tried to guard this lookup as much as possible, but do you think it's worth seeing if this processing could be moved to only happen if there's an actual type error? I guess I could catch and reraise.
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.
Great improvement.
Makes me wonder if we should also write this somewhere on https://rescript-lang.org/docs/manual/v12.0.0/record#tips--tricks
compiler/ml/error_message_utils.ml
Outdated
Possible solutions: @,\ | ||
- Return the expected record from the block@,\ | ||
- Write out the full record with field and value, like: @{<info>%s@}" |
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.
I think writing out the record fully without punning will be the most common fix, so maybe we should show it first in the list of possible solutions?
Possible solutions: @,\ | |
- Return the expected record from the block@,\ | |
- Write out the full record with field and value, like: @{<info>%s@}" | |
Possible solutions: @,\ | |
- Write out the full record with field and value, like: @{<info>%s@}" | |
- Return the expected record from the block@,\ |
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.
@mediremi good point. Done in the latest commit.
rescript
@rescript/darwin-arm64
@rescript/darwin-x64
@rescript/linux-arm64
@rescript/linux-x64
@rescript/runtime
@rescript/win32-x64
commit: |
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.
Sweet
*) | ||
and type_expect ~context ?in_function ?recarg env sexp ty_expected = | ||
(* Special errors for braced identifiers passed to records *) |
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.
Use example pattern, or else "braced identifier" is a bit difficult to understand for the comment reader.
|
||
Possible solutions: | ||
- Write out the full record with field and value, like: [1;33m{householdId: householdId}[0m | ||
- Return the expected record from the block No newline at end of file |
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.
See if an llm (simple, non-thinking) understands this message and is able to fix the code.
Or else, try rephrasing.
This is a pretty common gotcha -
{ident}
is counted as a block with an ident, not a record expr with a single field.