diff --git a/CHANGELOG.md b/CHANGELOG.md index 896e9282ff..1277c90abe 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/compiler/ml/typecore.ml b/compiler/ml/typecore.ml index bce1b55c60..ff3671b681 100644 --- a/compiler/ml/typecore.ml +++ b/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) diff --git a/compiler/ml/typetexp.ml b/compiler/ml/typetexp.ml index 942d81d663..53758e26c1 100644 --- a/compiler/ml/typetexp.ml +++ b/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 - "@[@{%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:@ \ - @{TheModule.%a@}@]@,\ - @[- Or specifying the record type explicitly:@ @{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 + "@[You're trying to access the record field @{%a@}, 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.@,\ + @\n\ + Possible solutions:@,\ + @[- Use @{Option.map@} to transform the option: \ + @{xx->Option.map(field => field.%a)@}@]@,\ + @[- Or use @{Option.getOr@} with a default: \ + @{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 + "@[You're trying to access the record field @{%a@}, 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.@]" + Printtyp.longident lid + | Some ({desc = Tconstr (_p, _, _)} as t1) -> + Format.fprintf ppf + "@[You're trying to access the record field @{%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 + "@[@{%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:@ \ + @{TheModule.%a@}@]@,\ + @[- Or specifying the record type explicitly:@ @{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; diff --git a/compiler/ml/typetexp.mli b/compiler/ml/typetexp.mli index f09c26c58e..19f7fa46b9 100644 --- a/compiler/ml/typetexp.mli +++ b/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 diff --git a/tests/build_tests/super_errors/expected/access_record_field_on_array.res.expected b/tests/build_tests/super_errors/expected/access_record_field_on_array.res.expected new file mode 100644 index 0000000000..56dbdea906 --- /dev/null +++ b/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. \ No newline at end of file diff --git a/tests/build_tests/super_errors/expected/access_record_field_on_option.res.expected b/tests/build_tests/super_errors/expected/access_record_field_on_option.res.expected new file mode 100644 index 0000000000..aaf398ff7d --- /dev/null +++ b/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 \ No newline at end of file diff --git a/tests/build_tests/super_errors/fixtures/access_record_field_on_array.res b/tests/build_tests/super_errors/fixtures/access_record_field_on_array.res new file mode 100644 index 0000000000..d49ac75ac9 --- /dev/null +++ b/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, + } + + let x = {a: 1, b: 2, c: [{d: 3}]} +} + +let f = X.x.c.d diff --git a/tests/build_tests/super_errors/fixtures/access_record_field_on_option.res b/tests/build_tests/super_errors/fixtures/access_record_field_on_option.res new file mode 100644 index 0000000000..9d71abf6d5 --- /dev/null +++ b/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, + } + + let x = {a: 1, b: 2, c: Some({d: 3})} +} + +let f = X.x.c.d