Skip to content

Commit dbd0ed8

Browse files
chenglouchenglou
authored
[Super errors] Improve the worst ReasonReact error (#2596)
Specialize the msg for each case.
1 parent 99dcc9b commit dbd0ed8

File tree

3 files changed

+108
-54
lines changed

3 files changed

+108
-54
lines changed

‎jscomp/super_errors/super_reason_react.ml

Lines changed: 47 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -4,29 +4,58 @@
44
and related comments *)
55
open Types
66

7-
let rec deconstruct_component_type t =
7+
let rec drill_through_tlink_and_tsubst t =
88
match t.desc with
9-
| Tconstr (p, types, _) when Path.last p = "componentSpec" -> Some types
109
| Tlink t
11-
| Tsubst t -> deconstruct_component_type t
12-
| _ -> None
10+
| Tsubst t -> drill_through_tlink_and_tsubst t
11+
| t -> t
1312

14-
let type_is_component_spec t =
15-
deconstruct_component_type t <> None
16-
17-
let sig_item_is_component_spec (si: Types.signature_item) = match si with
18-
| Sig_value (_id, value_desc) ->
19-
let typ = value_desc.val_type in
20-
type_is_component_spec typ
21-
| _ ->
22-
false
13+
let is_weak_type_after_drilling t =
14+
match drill_through_tlink_and_tsubst t with
15+
| Tvar _ -> true
16+
| _ -> false
2317

24-
let module_type_is_component_spec (mty : Types.module_type) = match mty with
25-
| Mty_signature sg ->
26-
List.exists sig_item_is_component_spec sg
27-
| _ ->
28-
false
18+
let component_spec_weak_type_variables t =
19+
match drill_through_tlink_and_tsubst t with
20+
(* ReasonReact <=0.3.4 *)
21+
| Tconstr (
22+
Pdot ((Pident {name = "ReasonReact"}), "componentSpec", _),
23+
[state; _initial_state; retained_props; _initial_retained_props; action],
24+
_
25+
) ->
26+
(
27+
state |> is_weak_type_after_drilling,
28+
retained_props |> is_weak_type_after_drilling,
29+
action |> is_weak_type_after_drilling
30+
)
31+
(* future ReasonReact version with retainedProps removed *)
32+
| Tconstr (
33+
Pdot ((Pident {name = "ReasonReact"}), "componentSpec", _),
34+
[state; _initial_state; action],
35+
_
36+
) ->
37+
(
38+
state |> is_weak_type_after_drilling,
39+
false,
40+
action |> is_weak_type_after_drilling
41+
)
42+
| _ -> (false, false, false)
2943

44+
let component_spec_weak_type_variables_in_module_type (mty : Types.module_type) =
45+
match mty with
46+
| Mty_signature signature_values ->
47+
signature_values
48+
|> List.map (function
49+
| Sig_value (_id, value_desc) ->
50+
let typ = value_desc.val_type in
51+
component_spec_weak_type_variables typ
52+
| _ -> (false, false, false)
53+
)
54+
|> List.filter (function
55+
| (false, false, false) -> false
56+
| _ -> true
57+
)
58+
| _ -> []
3059

3160
(* recursively drill down the types (first item is the type alias, if any. Second is the content of the alias) *)
3261
let rec get_to_bottom_of_aliases f = function

‎jscomp/super_errors/super_reason_react.mli

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
1-
val type_is_component_spec: Types.type_expr -> bool
1+
val component_spec_weak_type_variables: Types.type_expr -> bool * bool * bool
22
(** Used by super_typemod when we detect the message "... contains type variables that cannot be generalized" *)
33

4-
val module_type_is_component_spec: Types.module_type -> bool
4+
val component_spec_weak_type_variables_in_module_type: Types.module_type -> (bool * bool * bool) list
55
(** Used by super_typemod when we detect the message "... contains type variables that cannot be generalized" *)
66

77
val state_escape_scope: (Types.type_expr * Types.type_expr) list -> bool

‎jscomp/super_errors/super_typemod.ml

Lines changed: 59 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -2,23 +2,48 @@ open Printtyp
22

33
let fprintf = Format.fprintf
44

5-
6-
let pp_component_type_not_generalizable_pre ppf =
7-
fprintf ppf "@[<v>\
8-
@[@{<info>Is this a ReasonReact reducerComponent or component with retained props?@}@ \
9-
If so, you can fix it by doing one of these:@]@,\
10-
@[- Defining the component's `make` function@]@,\
11-
@[- Annotating the state once with a type, wherever it's used (e.g. render)@]@,\
12-
@[- Annotating actions (in e.g. reducer)@]@,\
13-
@[- Annotating retained props, if any@]@,@,\
14-
@[@{<info>Here's the original error message@}@]\
15-
@]@,"
16-
17-
let pp_component_type_not_generalizable_post ppf () =
18-
fprintf ppf
19-
"@[This happens when the type system senses there's a mutation/side-effect,@ in combination with a polymorphic value.@,\
20-
Using or annotating that value usually solves it.@ \
21-
More info:@ https://realworldocaml.org/v1/en/html/imperative-programming-1.html#side-effects-and-weak-polymorphism@]"
5+
let non_generalizable_msg ppf ~result ~is_module print_fallback_msg =
6+
let contain_vs_be =
7+
if is_module then "This module seems to contain" else "This seems to be" in
8+
match result with
9+
| (_, true, _) :: _ ->
10+
fprintf ppf "@[<v>\
11+
@{<info>%s a ReasonReact reducerComponentWithRetainedProps?@}@ \
12+
The retained props feature is deprecated.@ \
13+
Please use a regular @{<info>reducerComponent@} and keep the props you want to retain in state.\
14+
@]"
15+
contain_vs_be
16+
| (true, _, _) :: _->
17+
fprintf ppf "@[<v>\
18+
@[\
19+
@{<info>%s a ReasonReact reducerComponent?@}@ \
20+
We don't have@ all@ the@ type@ info@ for@ its@ @{<info>state@}.@ \
21+
Make sure you've done the following: @]@,@,\
22+
@[- Define the component `make` function@]@,\
23+
@[- Define `reducer` in that `make` body@]@,\
24+
@[- Annotate reducer's second parameter (state) with the desired state type\
25+
@]\
26+
@]"
27+
contain_vs_be
28+
| (_, _, true) :: _->
29+
fprintf ppf "@[<v>\
30+
@[\
31+
@{<info>%s a ReasonReact reducerComponent?@}@ \
32+
We don't have@ all@ the@ type@ info@ for@ its@ @{<info>action@}.@ \
33+
Make sure you've done the following: @]@,@,\
34+
@[- Define the component `make` function@]@,\
35+
@[- Define `reducer` in that `make` body@]@,\
36+
@[- Annotate reducer's first parameter (action) with the desired action type\
37+
@]\
38+
@]"
39+
contain_vs_be
40+
| _ ->
41+
fprintf ppf
42+
"%a@,@,\
43+
@[This happens when the type system senses there's a mutation/side-effect,@ in combination with a polymorphic value.@,\
44+
@{<info>Using or annotating that value usually solves it.@}@ \
45+
More info:@ https://realworldocaml.org/v1/en/html/imperative-programming-1.html#side-effects-and-weak-polymorphism@]"
46+
print_fallback_msg ()
2247

2348
(* taken from https://github.com/BuckleScript/ocaml/blob/d4144647d1bf9bc7dc3aadc24c25a7efa3a67915/typing/typemod.ml#L1754 *)
2449
(* modified branches are commented *)
@@ -57,14 +82,14 @@ let report_error ppf = Typemod.(function
5782
| Non_generalizable typ ->
5883
(* modified *)
5984
fprintf ppf "@[<v>";
60-
if Super_reason_react.type_is_component_spec typ then begin
61-
pp_component_type_not_generalizable_pre ppf
62-
end;
63-
fprintf ppf
64-
"@[This expression's type contains type variables that can't be generalized:@,@{<error>%a@}@]@,@,\
65-
%a"
66-
type_scheme typ
67-
pp_component_type_not_generalizable_post ();
85+
non_generalizable_msg
86+
ppf
87+
~result:([Super_reason_react.component_spec_weak_type_variables typ])
88+
~is_module:false
89+
(fun ppf () ->
90+
fprintf ppf
91+
"@[This expression's type contains type variables that can't be generalized:@,@{<error>%a@}@]"
92+
type_scheme typ);
6893
fprintf ppf "@]"
6994
| Non_generalizable_class (id, desc) ->
7095
fprintf ppf
@@ -74,15 +99,15 @@ let report_error ppf = Typemod.(function
7499
| Non_generalizable_module mty ->
75100
(* modified *)
76101
fprintf ppf "@[<v>";
77-
if Super_reason_react.module_type_is_component_spec mty then begin
78-
pp_component_type_not_generalizable_pre ppf
79-
end;
80-
fprintf ppf
81-
"@[The type of this module contains type variables that cannot be generalized:@,@{<error>%a@}@]@,@,\
82-
%a"
83-
modtype mty
84-
pp_component_type_not_generalizable_post ();
85-
fprintf ppf "@]"
102+
non_generalizable_msg
103+
ppf
104+
~result:(Super_reason_react.component_spec_weak_type_variables_in_module_type mty)
105+
~is_module:true
106+
(fun ppf () ->
107+
fprintf ppf
108+
"@[The type of this module contains type variables that cannot be generalized:@,@{<error>%a@}@]"
109+
modtype mty);
110+
fprintf ppf "@]"
86111
| Implementation_is_required intf_name ->
87112
fprintf ppf
88113
"@[The interface %a@ declares values, not just types.@ \

0 commit comments

Comments
 (0)