Skip to content

Commit

Permalink
try making top level unit error message clearer
Browse files Browse the repository at this point in the history
  • Loading branch information
zth committed Sep 18, 2023
1 parent 49bd91f commit bab3cfe
Show file tree
Hide file tree
Showing 8 changed files with 78 additions and 9 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -7,4 +7,10 @@
5 │ f(1, 2)
6 │

Toplevel expression is expected to have unit type.
This function call is at the top level and is expected to return `unit`. But, it's returning `int => int`.

In ReScript, anything at the top level must evaluate to `unit`. You can fix this by assigning the expression to a value, or piping it into the `ignore` function.

Possible solutions:
- Assigning to a value that is then ignored: `let _ = yourFunctionCall()`
- Piping into the built-in ignore function to ignore the result: `yourFunctionCall()->ignore`
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@

Warning number 109 (configured as error)
/.../fixtures/top_level_fn_call_not_unit.res:3:1-18

1 │ let returnsSomething = () => 123
2 │
3 │ returnsSomething()
4 │

This function call is at the top level and is expected to return `unit`. But, it's returning `int`.

In ReScript, anything at the top level must evaluate to `unit`. You can fix this by assigning the expression to a value, or piping it into the `ignore` function.

Possible solutions:
- Assigning to a value that is then ignored: `let _ = yourFunctionCall()`
- Piping into the built-in ignore function to ignore the result: `yourFunctionCall()->ignore`
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@

Warning number 109 (configured as error)
/.../fixtures/top_level_value_not_unit.res:1:1-4

1 │ 1234
2 │

This is at the top level and is expected to return `unit`. But, it's returning `int`.

In ReScript, anything at the top level must evaluate to `unit`. You can fix this by assigning the expression to a value, or piping it into the `ignore` function.

Possible solutions:
- Assigning to a value that is then ignored: `let _ = yourExpression`
- Piping into the built-in ignore function to ignore the result: `yourExpression->ignore`
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
let returnsSomething = () => 123

returnsSomething()
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
1234
25 changes: 21 additions & 4 deletions jscomp/ext/warnings.ml
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@ type loc = {
loc_ghost : bool;
}

type topLevelUnitHelp = FunctionCall | Other

type t =
| Comment_start (* 1 *)
| Comment_not_end (* 2 *)
Expand Down Expand Up @@ -83,7 +85,7 @@ type t =
| Bs_unimplemented_primitive of string (* 106 *)
| Bs_integer_literal_overflow (* 107 *)
| Bs_uninterpreted_delimiters of string (* 108 *)
| Bs_toplevel_expression_unit (* 109 *)
| Bs_toplevel_expression_unit of (string * topLevelUnitHelp) option (* 109 *)

(* If you remove a warning, leave a hole in the numbering. NEVER change
the numbers of existing warnings.
Expand Down Expand Up @@ -148,7 +150,7 @@ let number = function
| Bs_unimplemented_primitive _ -> 106
| Bs_integer_literal_overflow -> 107
| Bs_uninterpreted_delimiters _ -> 108
| Bs_toplevel_expression_unit -> 109
| Bs_toplevel_expression_unit _ -> 109

let last_warning_number = 110

Expand Down Expand Up @@ -490,8 +492,23 @@ let message = function
| Bs_integer_literal_overflow ->
"Integer literal exceeds the range of representable integers of type int"
| Bs_uninterpreted_delimiters s -> "Uninterpreted delimiters " ^ s
| Bs_toplevel_expression_unit ->
"Toplevel expression is expected to have unit type."
| Bs_toplevel_expression_unit help ->
Printf.sprintf "This%sis at the top level and is expected to return `unit`. But, it's returning %s.\n\nIn ReScript, anything at the top level must evaluate to `unit`. You can fix this by assigning the expression to a value, or piping it into the `ignore` function.%s"
(match help with
| Some (_, FunctionCall) -> " function call "
| _ -> " ")

(match help with
| Some (returnType, _) -> Printf.sprintf "`%s`" returnType
| None -> "something that is not `unit`")

(match help with
| Some (_, helpTyp) ->
let helpText = (match helpTyp with
| FunctionCall -> "yourFunctionCall()"
| Other -> "yourExpression") in
Printf.sprintf "\n\nPossible solutions:\n- Assigning to a value that is then ignored: `let _ = %s`\n- Piping into the built-in ignore function to ignore the result: `%s->ignore`" helpText helpText
| _ -> "")

let sub_locs = function
| Deprecated (_, def, use) ->
Expand Down
4 changes: 3 additions & 1 deletion jscomp/ext/warnings.mli
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ type loc = {
loc_ghost : bool;
}

type topLevelUnitHelp = FunctionCall | Other

type t =
| Comment_start (* 1 *)
| Comment_not_end (* 2 *)
Expand Down Expand Up @@ -76,7 +78,7 @@ type t =
| Bs_unimplemented_primitive of string (* 106 *)
| Bs_integer_literal_overflow (* 107 *)
| Bs_uninterpreted_delimiters of string (* 108 *)
| Bs_toplevel_expression_unit (* 109 *)
| Bs_toplevel_expression_unit of (string * topLevelUnitHelp) option (* 109 *)

val parse_options : bool -> string -> unit

Expand Down
16 changes: 13 additions & 3 deletions jscomp/ml/typecore.ml
Original file line number Diff line number Diff line change
Expand Up @@ -3743,11 +3743,21 @@ let type_expression env sexp =
Typetexp.reset_type_variables();
begin_def();
let exp = type_exp env sexp in
if Warnings.is_active Bs_toplevel_expression_unit then
if Warnings.is_active (Bs_toplevel_expression_unit None) then
(try unify env exp.exp_type
(instance_def Predef.type_unit) with
| Unify _
| Tags _ -> Location.prerr_warning sexp.pexp_loc Bs_toplevel_expression_unit);
| Unify _ ->
let buffer = Buffer.create 10 in
let formatter = Format.formatter_of_buffer buffer in
Printtyp.type_expr formatter exp.exp_type;
Format.pp_print_flush formatter ();
let returnType = Buffer.contents buffer in
Location.prerr_warning sexp.pexp_loc (Bs_toplevel_expression_unit (
match sexp.pexp_desc with
| Pexp_apply _ -> Some (returnType, FunctionCall)
| _ -> Some (returnType, Other)
))
| Tags _ -> Location.prerr_warning sexp.pexp_loc (Bs_toplevel_expression_unit None));
end_def();
if not (is_nonexpansive exp) then generalize_expansive env exp.exp_type;
generalize exp.exp_type;
Expand Down

0 comments on commit bab3cfe

Please sign in to comment.