improve error message for trying to do dot access on option/array (#7732)

* improve error message for trying to do dot access for a record field on something that is an option/array

* changelog

* update text

Author: zth

CHANGELOG.md

@@ -18,6 +18,7 @@
 - Show deprecation warnings for `bs-dependencies` etc. for local dependencies only. https://github.com/rescript-lang/rescript/pull/7724
 - Add check for minimum required node version. https://github.com/rescript-lang/rescript/pull/7723
 - Use more optional args in stdlib and deprecate some functions. https://github.com/rescript-lang/rescript/pull/7730
+- Improve error message for when trying to do dot access on an option/array. https://github.com/rescript-lang/rescript/pull/7732
 
 #### :bug: Bug fix
 

compiler/ml/typecore.ml

@@ -822,7 +822,8 @@ module NameChoice (Name : sig
   val get_descrs : Env.type_descriptions -> t list
 
   val unsafe_do_not_use__add_with_name : t -> string -> t
-  val unbound_name_error : Env.t -> Longident.t loc -> 'a
+  val unbound_name_error :
+    ?from_type:type_expr -> Env.t -> Longident.t loc -> 'a
 end) =
 struct
   open Name
@@ -881,7 +882,7 @@ struct
     List.find check_type lbls
 
   let disambiguate ?(warn = Location.prerr_warning) ?(check_lk = fun _ _ -> ())
-      ?scope lid env opath lbls =
+      ?(from_type : Types.type_expr option) ?scope lid env opath lbls =
     let scope =
       match scope with
       | None -> lbls
@@ -891,7 +892,7 @@ struct
       match opath with
       | None -> (
         match lbls with
-        | [] -> unbound_name_error env lid
+        | [] -> unbound_name_error ?from_type env lid
         | (lbl, use) :: rest ->
           use ();
           let paths = ambiguous_types env lbl rest in
@@ -910,7 +911,7 @@ struct
             check_lk tpath lbl;
             lbl
           with Not_found ->
-            if lbls = [] then unbound_name_error env lid
+            if lbls = [] then unbound_name_error ?from_type env lid
             else
               let tp = (tpath0, expand_path env tpath) in
               let tpl =
@@ -3311,7 +3312,7 @@ and type_label_access env srecord lid =
   let labels = Typetexp.find_all_labels env lid.loc lid.txt in
   let label =
     wrap_disambiguate "This expression has" ty_exp
-      (Label.disambiguate lid env opath)
+      (Label.disambiguate ~from_type:ty_exp lid env opath)
       labels
   in
   (record, label, opath)

compiler/ml/typetexp.ml

@@ -43,7 +43,7 @@ type error =
   | Method_mismatch of string * type_expr * type_expr
   | Unbound_value of Longident.t
   | Unbound_constructor of Longident.t
-  | Unbound_label of Longident.t
+  | Unbound_label of Longident.t * type_expr option
   | Unbound_module of Longident.t
   | Unbound_modtype of Longident.t
   | Ill_typed_functor_application of Longident.t
@@ -129,7 +129,7 @@ let find_all_constructors =
   find_component Env.lookup_all_constructors (fun lid ->
       Unbound_constructor lid)
 let find_all_labels =
-  find_component Env.lookup_all_labels (fun lid -> Unbound_label lid)
+  find_component Env.lookup_all_labels (fun lid -> Unbound_label (lid, None))
 
 let find_value env loc lid =
   Env.check_value_name (Longident.last lid) loc;
@@ -160,12 +160,14 @@ let find_modtype env loc lid =
   Builtin_attributes.check_deprecated loc decl.mtd_attributes (Path.name path);
   r
 
-let unbound_constructor_error env lid =
+let unbound_constructor_error ?from_type env lid =
+  ignore from_type;
   narrow_unbound_lid_error env lid.loc lid.txt (fun lid ->
       Unbound_constructor lid)
 
-let unbound_label_error env lid =
-  narrow_unbound_lid_error env lid.loc lid.txt (fun lid -> Unbound_label lid)
+let unbound_label_error ?from_type env lid =
+  narrow_unbound_lid_error env lid.loc lid.txt (fun lid ->
+      Unbound_label (lid, from_type))
 
 (* Support for first-class modules. *)
 
@@ -909,18 +911,49 @@ let report_error env ppf = function
        = Bar@}.@]@]"
       Printtyp.longident lid Printtyp.longident lid Printtyp.longident lid;
     spellcheck ppf fold_constructors env lid
-  | Unbound_label lid ->
+  | Unbound_label (lid, from_type) ->
     (* modified *)
-    Format.fprintf ppf
-      "@[<v>@{<info>%a@} refers to a record field, but no corresponding record \
-       type is in scope.@,\
-       @,\
-       If it's defined in another module or file, bring it into scope by:@,\
-       @[- Prefixing the field name with the module name:@ \
-       @{<info>TheModule.%a@}@]@,\
-       @[- Or specifying the record type explicitly:@ @{<info>let theValue: \
-       TheModule.theType = {%a: VALUE}@}@]@]"
-      Printtyp.longident lid Printtyp.longident lid Printtyp.longident lid;
+    (match from_type with
+    | Some {desc = Tconstr (p, _, _)} when Path.same p Predef.path_option ->
+      (* TODO: Extend for nullable/null? *)
+      Format.fprintf ppf
+        "@[<v>You're trying to access the record field @{<info>%a@}, but the \
+         value you're trying to access it on is an @{<info>option@}.@ You need \
+         to unwrap the option first before accessing the record field.@,\
+         @\n\
+         Possible solutions:@,\
+         @[- Use @{<info>Option.map@} to transform the option: \
+         @{<info>xx->Option.map(field => field.%a)@}@]@,\
+         @[- Or use @{<info>Option.getOr@} with a default: \
+         @{<info>xx->Option.getOr(defaultRecord).%a@}@]@]"
+        Printtyp.longident lid Printtyp.longident lid Printtyp.longident lid
+    | Some {desc = Tconstr (p, _, _)} when Path.same p Predef.path_array ->
+      Format.fprintf ppf
+        "@[<v>You're trying to access the record field @{<info>%a@}, but the \
+         value you're trying to access it on is an @{<info>array@}.@ You need \
+         to access an individual element of the array if you want to access an \
+         individual record field.@]"
+        Printtyp.longident lid
+    | Some ({desc = Tconstr (_p, _, _)} as t1) ->
+      Format.fprintf ppf
+        "@[<v>You're trying to access the record field @{<info>%a@}, but the \
+         thing you're trying to access it on is not a record. @,\n\
+         The type of the thing you're trying to access it on is:@,\n\
+         %a@,\n\
+         @,\
+         Only records have fields that can be accessed with dot notation.@]"
+        Printtyp.longident lid Error_message_utils.type_expr t1
+    | None | Some _ ->
+      Format.fprintf ppf
+        "@[<v>@{<info>%a@} refers to a record field, but no corresponding \
+         record type is in scope.@,\
+         @,\
+         If it's defined in another module or file, bring it into scope by:@,\
+         @[- Prefixing the field name with the module name:@ \
+         @{<info>TheModule.%a@}@]@,\
+         @[- Or specifying the record type explicitly:@ @{<info>let theValue: \
+         TheModule.theType = {%a: VALUE}@}@]@]"
+        Printtyp.longident lid Printtyp.longident lid Printtyp.longident lid);
     spellcheck ppf fold_labels env lid
   | Unbound_modtype lid ->
     fprintf ppf "Unbound module type %a" longident lid;

compiler/ml/typetexp.mli

@@ -52,7 +52,7 @@ type error =
   | Method_mismatch of string * type_expr * type_expr
   | Unbound_value of Longident.t
   | Unbound_constructor of Longident.t
-  | Unbound_label of Longident.t
+  | Unbound_label of Longident.t * type_expr option
   | Unbound_module of Longident.t
   | Unbound_modtype of Longident.t
   | Ill_typed_functor_application of Longident.t
@@ -96,5 +96,7 @@ val lookup_module : ?load:bool -> Env.t -> Location.t -> Longident.t -> Path.t
 val find_modtype :
   Env.t -> Location.t -> Longident.t -> Path.t * modtype_declaration
 
-val unbound_constructor_error : Env.t -> Longident.t Location.loc -> 'a
-val unbound_label_error : Env.t -> Longident.t Location.loc -> 'a
+val unbound_constructor_error :
+  ?from_type:type_expr -> Env.t -> Longident.t Location.loc -> 'a
+val unbound_label_error :
+  ?from_type:type_expr -> Env.t -> Longident.t Location.loc -> 'a

tests/build_tests/super_errors/expected/access_record_field_on_array.res.expected

@@ -0,0 +1,11 @@
+
+  We've found a bug for you!
+  /.../fixtures/access_record_field_on_array.res:12:15
+
+  10 │ }
+  11 │ 
+  12 │ let f = X.x.c.d
+  13 │ 
+
+  You're trying to access the record field d, but the value you're trying to access it on is an array.
+  You need to access an individual element of the array if you want to access an individual record field.

tests/build_tests/super_errors/expected/access_record_field_on_option.res.expected

@@ -0,0 +1,15 @@
+
+  We've found a bug for you!
+  /.../fixtures/access_record_field_on_option.res:12:15
+
+  10 │ }
+  11 │ 
+  12 │ let f = X.x.c.d
+  13 │ 
+
+  You're trying to access the record field d, but the value you're trying to access it on is an option.
+  You need to unwrap the option first before accessing the record field.
+  
+  Possible solutions:
+  - Use Option.map to transform the option: xx->Option.map(field => field.d)
+  - Or use Option.getOr with a default: xx->Option.getOr(defaultRecord).d

tests/build_tests/super_errors/fixtures/access_record_field_on_array.res

@@ -0,0 +1,12 @@
+module X = {
+  type y = {d: int}
+  type x = {
+    a: int,
+    b: int,
+    c: array<y>,
+  }
+
+  let x = {a: 1, b: 2, c: [{d: 3}]}
+}
+
+let f = X.x.c.d

tests/build_tests/super_errors/fixtures/access_record_field_on_option.res

@@ -0,0 +1,12 @@
+module X = {
+  type y = {d: int}
+  type x = {
+    a: int,
+    b: int,
+    c: option<y>,
+  }
+
+  let x = {a: 1, b: 2, c: Some({d: 3})}
+}
+
+let f = X.x.c.d