From 297d856bc88d0addb9090963cb00cfa67b7dc6f6 Mon Sep 17 00:00:00 2001 From: Gabriel Nordeborn Date: Mon, 28 Jul 2025 22:15:54 +0200 Subject: [PATCH 1/3] improve error message for trying to do dot access for a record field on something that is an option/array --- compiler/ml/typecore.ml | 11 ++-- compiler/ml/typetexp.ml | 65 ++++++++++++++----- compiler/ml/typetexp.mli | 8 ++- .../access_record_field_on_array.res.expected | 11 ++++ ...access_record_field_on_option.res.expected | 15 +++++ .../fixtures/access_record_field_on_array.res | 12 ++++ .../access_record_field_on_option.res | 12 ++++ 7 files changed, 110 insertions(+), 24 deletions(-) create mode 100644 tests/build_tests/super_errors/expected/access_record_field_on_array.res.expected create mode 100644 tests/build_tests/super_errors/expected/access_record_field_on_option.res.expected create mode 100644 tests/build_tests/super_errors/fixtures/access_record_field_on_array.res create mode 100644 tests/build_tests/super_errors/fixtures/access_record_field_on_option.res 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..b796119eb3 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 \ + thing 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 \ + thing 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..a6ab9c63a6 --- /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 thing 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..056766cf06 --- /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 thing 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 From 57c028951960d7493bb7d5b9484a97daf100b58e Mon Sep 17 00:00:00 2001 From: Gabriel Nordeborn Date: Mon, 28 Jul 2025 22:17:57 +0200 Subject: [PATCH 2/3] changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) 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 From ed7c2fb7287ea15a9bb35fd45cbc6eff72193984 Mon Sep 17 00:00:00 2001 From: Gabriel Nordeborn Date: Tue, 29 Jul 2025 14:31:09 +0200 Subject: [PATCH 3/3] update text --- compiler/ml/typetexp.ml | 4 ++-- .../expected/access_record_field_on_array.res.expected | 2 +- .../expected/access_record_field_on_option.res.expected | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/compiler/ml/typetexp.ml b/compiler/ml/typetexp.ml index b796119eb3..53758e26c1 100644 --- a/compiler/ml/typetexp.ml +++ b/compiler/ml/typetexp.ml @@ -918,7 +918,7 @@ let report_error env ppf = function (* TODO: Extend for nullable/null? *) Format.fprintf ppf "@[You're trying to access the record field @{%a@}, but the \ - thing you're trying to access it on is an @{option@}.@ You need \ + 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:@,\ @@ -930,7 +930,7 @@ let report_error env ppf = function | 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 \ - thing you're trying to access it on is an @{array@}.@ You need \ + 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 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 index a6ab9c63a6..56dbdea906 100644 --- 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 @@ -7,5 +7,5 @@ 12 │ let f = X.x.c.d 13 │ - You're trying to access the record field d, but the thing you're trying to access it on is an array. + 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 index 056766cf06..aaf398ff7d 100644 --- 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 @@ -7,7 +7,7 @@ 12 │ let f = X.x.c.d 13 │ - You're trying to access the record field d, but the thing you're trying to access it on is an option. + 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: