Skip to content

Commit 69ccd83

Browse files
committed
add action for automatically unwrapping record field access through option
1 parent c4cf0e6 commit 69ccd83

File tree

4 files changed

+82
-12
lines changed

4 files changed

+82
-12
lines changed

compiler/ml/cmt_utils.ml

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ type action_type =
2424
| PartiallyApplyFunction
2525
| InsertMissingArguments of {missing_args: Asttypes.Noloc.arg_label list}
2626
| ChangeRecordFieldOptional of {optional: bool}
27+
| UnwrapOptionMapRecordField of {field_name: Longident.t}
2728

2829
(* TODO:
2930
- Unused var in patterns (and aliases )*)
@@ -81,6 +82,9 @@ let action_to_string = function
8182
| ChangeRecordFieldOptional {optional} ->
8283
Printf.sprintf "ChangeRecordFieldOptional(%s)"
8384
(if optional then "true" else "false")
85+
| UnwrapOptionMapRecordField {field_name} ->
86+
Printf.sprintf "UnwrapOptionMapRecordField(%s)"
87+
(Longident.flatten field_name |> String.concat ".")
8488

8589
let _add_possible_action : (cmt_action -> unit) ref = ref (fun _ -> ())
8690
let add_possible_action action = !_add_possible_action action

compiler/ml/typetexp.ml

Lines changed: 26 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,11 @@ type error =
4343
| Method_mismatch of string * type_expr * type_expr
4444
| Unbound_value of Longident.t * Location.t
4545
| Unbound_constructor of Longident.t
46-
| Unbound_label of Longident.t * type_expr option
46+
| Unbound_label of {
47+
loc: Location.t;
48+
field_name: Longident.t;
49+
from_type: type_expr option;
50+
}
4751
| Unbound_module of Longident.t
4852
| Unbound_modtype of Longident.t
4953
| Ill_typed_functor_application of Longident.t
@@ -128,8 +132,10 @@ let find_constructor =
128132
let find_all_constructors =
129133
find_component Env.lookup_all_constructors (fun lid ->
130134
Unbound_constructor lid)
131-
let find_all_labels =
132-
find_component Env.lookup_all_labels (fun lid -> Unbound_label (lid, None))
135+
let find_all_labels env loc =
136+
find_component Env.lookup_all_labels
137+
(fun lid -> Unbound_label {loc; field_name = lid; from_type = None})
138+
env loc
133139

134140
let find_value env loc lid =
135141
Env.check_value_name (Longident.last lid) loc;
@@ -168,8 +174,9 @@ let unbound_constructor_error ?from_type env lid =
168174
Unbound_constructor lid)
169175

170176
let unbound_label_error ?from_type env lid =
177+
let lid_with_loc = lid in
171178
narrow_unbound_lid_error env lid.loc lid.txt (fun lid ->
172-
Unbound_label (lid, from_type))
179+
Unbound_label {loc = lid_with_loc.loc; field_name = lid; from_type})
173180

174181
(* Support for first-class modules. *)
175182

@@ -936,10 +943,17 @@ let report_error env ppf = function
936943
= Bar@}.@]@]"
937944
Printtyp.longident lid Printtyp.longident lid Printtyp.longident lid;
938945
spellcheck ppf fold_constructors env lid
939-
| Unbound_label (lid, from_type) ->
946+
| Unbound_label {loc; field_name; from_type} ->
940947
(* modified *)
941948
(match from_type with
942949
| Some {desc = Tconstr (p, _, _)} when Path.same p Predef.path_option ->
950+
Cmt_utils.add_possible_action
951+
{
952+
loc;
953+
action = UnwrapOptionMapRecordField {field_name};
954+
description =
955+
"Unwrap the option first before accessing the record field";
956+
};
943957
(* TODO: Extend for nullable/null? *)
944958
Format.fprintf ppf
945959
"@[<v>You're trying to access the record field @{<info>%a@}, but the \
@@ -951,14 +965,15 @@ let report_error env ppf = function
951965
@{<info>xx->Option.map(field => field.%a)@}@]@,\
952966
@[- Or use @{<info>Option.getOr@} with a default: \
953967
@{<info>xx->Option.getOr(defaultRecord).%a@}@]@]"
954-
Printtyp.longident lid Printtyp.longident lid Printtyp.longident lid
968+
Printtyp.longident field_name Printtyp.longident field_name
969+
Printtyp.longident field_name
955970
| Some {desc = Tconstr (p, _, _)} when Path.same p Predef.path_array ->
956971
Format.fprintf ppf
957972
"@[<v>You're trying to access the record field @{<info>%a@}, but the \
958973
value you're trying to access it on is an @{<info>array@}.@ You need \
959974
to access an individual element of the array if you want to access an \
960975
individual record field.@]"
961-
Printtyp.longident lid
976+
Printtyp.longident field_name
962977
| Some ({desc = Tconstr (_p, _, _)} as t1) ->
963978
Format.fprintf ppf
964979
"@[<v>You're trying to access the record field @{<info>%a@}, but the \
@@ -967,7 +982,7 @@ let report_error env ppf = function
967982
%a@,\n\
968983
@,\
969984
Only records have fields that can be accessed with dot notation.@]"
970-
Printtyp.longident lid Error_message_utils.type_expr t1
985+
Printtyp.longident field_name Error_message_utils.type_expr t1
971986
| None | Some _ ->
972987
Format.fprintf ppf
973988
"@[<v>@{<info>%a@} refers to a record field, but no corresponding \
@@ -978,8 +993,9 @@ let report_error env ppf = function
978993
@{<info>TheModule.%a@}@]@,\
979994
@[- Or specifying the record type explicitly:@ @{<info>let theValue: \
980995
TheModule.theType = {%a: VALUE}@}@]@]"
981-
Printtyp.longident lid Printtyp.longident lid Printtyp.longident lid);
982-
spellcheck ppf fold_labels env lid
996+
Printtyp.longident field_name Printtyp.longident field_name
997+
Printtyp.longident field_name);
998+
spellcheck ppf fold_labels env field_name
983999
| Unbound_modtype lid ->
9841000
fprintf ppf "Unbound module type %a" longident lid;
9851001
spellcheck ppf fold_modtypes env lid

compiler/ml/typetexp.mli

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,11 @@ type error =
5252
| Method_mismatch of string * type_expr * type_expr
5353
| Unbound_value of Longident.t * Location.t
5454
| Unbound_constructor of Longident.t
55-
| Unbound_label of Longident.t * type_expr option
55+
| Unbound_label of {
56+
loc: Location.t;
57+
field_name: Longident.t;
58+
from_type: type_expr option;
59+
}
5660
| Unbound_module of Longident.t
5761
| Unbound_modtype of Longident.t
5862
| Ill_typed_functor_application of Longident.t

tools/src/tools.ml

Lines changed: 47 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1293,6 +1293,24 @@ module ExtractCodeblocks = struct
12931293
|> Protocol.array)
12941294
end
12951295

1296+
module TemplateUtils = struct
1297+
let get_expr source =
1298+
let {Res_driver.parsetree; invalid} =
1299+
Res_driver.parse_implementation_from_source ~for_printer:true
1300+
~display_filename:"<generated>" ~source
1301+
in
1302+
if invalid then Error "Could not parse expression"
1303+
else
1304+
match parsetree with
1305+
| [{pstr_desc = Pstr_eval (e, _)}] -> Ok e
1306+
| _ -> Error "Expected a record expression"
1307+
1308+
let get_expr_exn source =
1309+
match get_expr source with
1310+
| Ok e -> e
1311+
| Error e -> failwith e
1312+
end
1313+
12961314
module Actions = struct
12971315
let change_record_field_optional (record_el : _ Parsetree.record_element)
12981316
target_loc actions =
@@ -1635,6 +1653,32 @@ module Actions = struct
16351653
else
16361654
(* Other cases when the loc is on something else in the expr *)
16371655
match (expr.pexp_desc, action.action) with
1656+
| ( Pexp_field (e, {loc}),
1657+
UnwrapOptionMapRecordField {field_name} )
1658+
when action.loc = loc ->
1659+
Some
1660+
{
1661+
expr with
1662+
pexp_desc =
1663+
Pexp_apply
1664+
{
1665+
funct =
1666+
Ast_helper.Exp.ident
1667+
(Location.mknoloc (Longident.Lident "->"));
1668+
partial = false;
1669+
transformed_jsx = false;
1670+
args =
1671+
[
1672+
(Nolabel, e);
1673+
( Nolabel,
1674+
TemplateUtils.get_expr_exn
1675+
(Printf.sprintf
1676+
"Option.map(v => v.%s)"
1677+
(Longident.flatten field_name
1678+
|> String.concat ".")) );
1679+
];
1680+
};
1681+
}
16381682
| ( Pexp_apply ({funct; args} as apply),
16391683
InsertMissingArguments {missing_args} )
16401684
when funct.pexp_loc = action.loc ->
@@ -1833,7 +1877,9 @@ module Actions = struct
18331877
| InsertMissingArguments _ ->
18341878
List.mem "InsertMissingArguments" filter
18351879
| ChangeRecordFieldOptional _ ->
1836-
List.mem "ChangeRecordFieldOptional" filter)
1880+
List.mem "ChangeRecordFieldOptional" filter
1881+
| UnwrapOptionMapRecordField _ ->
1882+
List.mem "UnwrapOptionMapRecordField" filter)
18371883
in
18381884
match applyActionsToFile path possible_actions with
18391885
| Ok applied ->

0 commit comments

Comments
 (0)