Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@

- Make parser less strict around leading attributes. https://github.com/rescript-lang/rescript/pull/7787
- Dedicated error message for ternary type mismatch. https://github.com/rescript-lang/rescript/pull/7804
- Dedicated error message for passing a braced ident to something expected to be a record. https://github.com/rescript-lang/rescript/pull/7806

#### :house: Internal

Expand Down
33 changes: 32 additions & 1 deletion compiler/ml/error_message_utils.ml
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,7 @@ type type_clash_context =
is_constant: string option;
}
| FunctionArgument of {optional: bool; name: string option}
| BracedIdent
| Statement of type_clash_statement
| ForLoopCondition
| Await
Expand All @@ -128,6 +129,7 @@ let context_to_string = function
| Some IfReturn -> "IfReturn"
| Some TernaryReturn -> "TernaryReturn"
| Some Await -> "Await"
| Some BracedIdent -> "BracedIdent"
| None -> "None"

let fprintf = Format.fprintf
Expand Down Expand Up @@ -198,7 +200,7 @@ let error_expected_type_text ppf type_clash_context =
fprintf ppf
"But you're using @{<info>await@} on this expression, so it is expected \
to be of type:"
| Some MaybeUnwrapOption | None ->
| Some MaybeUnwrapOption | Some BracedIdent | None ->
fprintf ppf "But it's expected to have type:"

let is_record_type ~(extract_concrete_typedecl : extract_concrete_typedecl) ~env
Expand Down Expand Up @@ -546,6 +548,35 @@ let print_extra_type_clash_help ~extract_concrete_typedecl ~env loc ppf
with @{<info>?@} to show you want to pass the option, like: \
@{<info>?%s@}"
(Parser.extract_text_at_loc loc)
| Some BracedIdent, Some (_, ({desc = Tconstr (_, _, _)} as t))
when is_record_type ~extract_concrete_typedecl ~env t ->
fprintf ppf
"@,\
@,\
You might have meant to pass this as a record, but wrote it as a block.@,\
Braces with a single identifier counts as a block, not a record with a \
single (punned) field.@,\
@,\
Possible solutions: @,\
- Return the expected record from the block@,\
- Write out the full record with field and value, like: @{<info>%s@}"
Copy link
Member

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?

Suggested change
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@,\

Copy link
Member Author

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.

(match
Parser.reprint_expr_at_loc
~mapper:(fun e ->
match e.pexp_desc with
| Pexp_ident {txt} ->
Some
{
e with
pexp_desc =
Pexp_record
([{lid = Location.mknoloc txt; opt = false; x = e}], None);
}
| _ -> None)
loc
with
| None -> ""
| Some s -> s)
| _, Some ({Types.desc = Tconstr (p1, _, _)}, {desc = Tvariant row_desc})
when Path.same Predef.path_string p1 -> (
(* Check if we have a string constant that could be a polymorphic variant constructor *)
Expand Down
14 changes: 14 additions & 0 deletions compiler/ml/typecore.ml
Original file line number Diff line number Diff line change
Expand Up @@ -2256,6 +2256,18 @@ let rec type_exp ~context ?recarg env sexp =
*)

and type_expect ~context ?in_function ?recarg env sexp ty_expected =
(* Special errors for braced identifiers passed to records *)
Copy link
Collaborator

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.

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
Copy link
Member Author

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.

let previous_saved_types = Cmt_format.get_saved_types () in
let exp =
Builtin_attributes.warning_scope sexp.pexp_attributes (fun () ->
Expand Down Expand Up @@ -3394,7 +3406,9 @@ and type_label_exp ~call_context create env loc ty_expected
(lid, label, {arg with exp_type = instance env arg.exp_type}, opt)

and type_argument ~context ?recarg env sarg ty_expected' ty_expected =
print_endline "hey1";
let texp = type_expect ~context ?recarg env sarg ty_expected' in
print_endline "hey";
unify_exp ~context env texp ty_expected;
texp

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@

We've found a bug for you!
/.../fixtures/pass_braced_identifier_to_record.res:4:15-25

2 │ let householdId = "12"
3 │
4 │ let ff: xx = {householdId}
5 │

This has type: string
But it's expected to have type: xx

You might have meant to pass this as a record, but wrote it as a block.
Braces with a single identifier counts as a block, not a record with a single (punned) field.

Possible solutions:
- Return the expected record from the block
- Write out the full record with field and value, like: {householdId: householdId}

Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@

We've found a bug for you!
/.../fixtures/pass_braced_identifier_to_record_fn_argument.res:8:15-25

6 │
7 │ let householdId = "12"
8 │ let ff = doX({householdId})
9 │

This has type: string
But it's expected to have type: xx

You might have meant to pass this as a record, but wrote it as a block.
Braces with a single identifier counts as a block, not a record with a single (punned) field.

Possible solutions:
- Return the expected record from the block
- Write out the full record with field and value, like: {householdId: householdId}

Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
type xx = {householdId: string}
let householdId = "12"

let ff: xx = {householdId}
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
type xx = {householdId: string}

let doX = ({householdId}) => {
householdId
}

let householdId = "12"
let ff = doX({householdId})
Loading