Error for braced single identifier to record (#7806)

* dedicated error for braced ident where record is expected

* tests

* changelog

* cleanup

* message order + trim

* skip potentially expensive check

Author: zth

CHANGELOG.md

@@ -32,6 +32,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
 

compiler/ml/error_message_utils.ml

@@ -63,7 +63,8 @@ end = struct
     match parse_expr_at_loc loc with
     | Some (exp, comments) -> (
       match mapper exp with
-      | Some exp -> Some (!reprint_source (wrap_in_structure exp) comments)
+      | Some exp ->
+        Some (!reprint_source (wrap_in_structure exp) comments |> String.trim)
       | None -> None)
     | None -> None
 end
@@ -105,6 +106,7 @@ type type_clash_context =
       is_constant: string option;
     }
   | FunctionArgument of {optional: bool; name: string option}
+  | BracedIdent
   | Statement of type_clash_statement
   | ForLoopCondition
   | Await
@@ -128,6 +130,7 @@ let context_to_string = function
   | Some IfReturn -> "IfReturn"
   | Some TernaryReturn -> "TernaryReturn"
   | Some Await -> "Await"
+  | Some BracedIdent -> "BracedIdent"
   | None -> "None"
 
 let fprintf = Format.fprintf
@@ -198,7 +201,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
@@ -546,6 +549,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: @,\
+       - Write out the full record with field and value, like: @{<info>%s@}@,\
+       - Return the expected record from the block"
+      (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 *)

compiler/ml/typecore.ml

@@ -2256,6 +2256,17 @@ 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 *)
+  let context =
+    match sexp.pexp_desc with
+    | Pexp_ident _ ->
+      if
+        sexp.pexp_attributes
+        |> List.exists (fun (attr, _) -> attr.txt = "res.braces")
+      then Some Error_message_utils.BracedIdent
+      else context
+    | _ -> context
+  in
   let previous_saved_types = Cmt_format.get_saved_types () in
   let exp =
     Builtin_attributes.warning_scope sexp.pexp_attributes (fun () ->

tests/build_tests/super_errors/expected/array_literal_passed_to_tuple.res.expected

@@ -10,5 +10,4 @@
   This has type: array<'a>
   But it's expected to have type: (string, string)
 
-  - Fix this by passing a tuple instead of an array, like: ("hello", "world")
-
+  - Fix this by passing a tuple instead of an array, like: ("hello", "world")

tests/build_tests/super_errors/expected/object_literal_passed_when_record_expected.res.expected

@@ -13,5 +13,4 @@
   You're passing a ReScript object where a record is expected. Objects are written with quoted keys, and records with unquoted keys.
 
   Possible solutions: 
-  - Rewrite the object to a record, like: {one: true}
-
+  - Rewrite the object to a record, like: {one: true}

tests/build_tests/super_errors/expected/pass_braced_identifier_to_record.res.expected

@@ -0,0 +1,18 @@
+
+  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: 
+  - Write out the full record with field and value, like: {householdId: householdId}
+  - Return the expected record from the block

tests/build_tests/super_errors/expected/pass_braced_identifier_to_record_fn_argument.res.expected

@@ -0,0 +1,18 @@
+
+  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: 
+  - Write out the full record with field and value, like: {householdId: householdId}
+  - Return the expected record from the block

tests/build_tests/super_errors/expected/string_constant_to_polyvariant.res.expected

@@ -11,5 +11,4 @@
   But this function argument is expecting: [#ONE | #TWO]
 
   Possible solutions:
-  - The constant passed matches one of the expected polymorphic variant constructors. Did you mean to pass this as a polymorphic variant? If so, rewrite "ONE" to #ONE
-
+  - The constant passed matches one of the expected polymorphic variant constructors. Did you mean to pass this as a polymorphic variant? If so, rewrite "ONE" to #ONE

tests/build_tests/super_errors/expected/string_constant_to_variant.res.expected

@@ -11,5 +11,4 @@
   But this function argument is expecting: status
 
   Possible solutions:
-  - The constant passed matches the runtime representation of one of the expected variant constructors. Did you mean to pass this as a variant constructor? If so, rewrite "Active" to Active
-
+  - The constant passed matches the runtime representation of one of the expected variant constructors. Did you mean to pass this as a variant constructor? If so, rewrite "Active" to Active

tests/build_tests/super_errors/fixtures/pass_braced_identifier_to_record.res

@@ -0,0 +1,4 @@
+type xx = {householdId: string}
+let householdId = "12"
+
+let ff: xx = {householdId}