diff --git a/CHANGELOG.md b/CHANGELOG.md index 51d816a9fa..8a88a8c76c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -26,6 +26,7 @@ #### :nail_care: Polish - A little performance improvement for JSX V4 runtime helper by removing one object allocation for components with key prop. https://github.com/rescript-lang/rescript-compiler/pull/6376 +- The error message for "toplevel expressions should evaluate to unit" has been revamped and improved. https://github.com/rescript-lang/rescript-compiler/pull/6407 # 11.0.0-rc.3 diff --git a/jscomp/build_tests/super_errors/expected/partial_app.res.expected b/jscomp/build_tests/super_errors/expected/partial_app.res.expected index 857ffbc5f5..99971bc90b 100644 --- a/jscomp/build_tests/super_errors/expected/partial_app.res.expected +++ b/jscomp/build_tests/super_errors/expected/partial_app.res.expected @@ -7,4 +7,10 @@ 5 │ f(1, 2) 6 │ - Toplevel expression is expected to have unit type. \ No newline at end of file + 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` \ No newline at end of file diff --git a/jscomp/build_tests/super_errors/expected/top_level_fn_call_not_unit.res.expected b/jscomp/build_tests/super_errors/expected/top_level_fn_call_not_unit.res.expected new file mode 100644 index 0000000000..5480aeda58 --- /dev/null +++ b/jscomp/build_tests/super_errors/expected/top_level_fn_call_not_unit.res.expected @@ -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` \ No newline at end of file diff --git a/jscomp/build_tests/super_errors/expected/top_level_value_not_unit.res.expected b/jscomp/build_tests/super_errors/expected/top_level_value_not_unit.res.expected new file mode 100644 index 0000000000..449338322a --- /dev/null +++ b/jscomp/build_tests/super_errors/expected/top_level_value_not_unit.res.expected @@ -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` \ No newline at end of file diff --git a/jscomp/build_tests/super_errors/fixtures/top_level_fn_call_not_unit.res b/jscomp/build_tests/super_errors/fixtures/top_level_fn_call_not_unit.res new file mode 100644 index 0000000000..25fae76b0d --- /dev/null +++ b/jscomp/build_tests/super_errors/fixtures/top_level_fn_call_not_unit.res @@ -0,0 +1,3 @@ +let returnsSomething = () => 123 + +returnsSomething() diff --git a/jscomp/build_tests/super_errors/fixtures/top_level_value_not_unit.res b/jscomp/build_tests/super_errors/fixtures/top_level_value_not_unit.res new file mode 100644 index 0000000000..81c545efeb --- /dev/null +++ b/jscomp/build_tests/super_errors/fixtures/top_level_value_not_unit.res @@ -0,0 +1 @@ +1234 diff --git a/jscomp/ext/warnings.ml b/jscomp/ext/warnings.ml index 35f8e42ee8..64807763eb 100644 --- a/jscomp/ext/warnings.ml +++ b/jscomp/ext/warnings.ml @@ -26,6 +26,8 @@ type loc = { loc_ghost : bool; } +type topLevelUnitHelp = FunctionCall | Other + type t = | Comment_start (* 1 *) | Comment_not_end (* 2 *) @@ -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. @@ -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 @@ -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\n 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.%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\n Possible 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) -> diff --git a/jscomp/ext/warnings.mli b/jscomp/ext/warnings.mli index 4e36bc00e5..d5d8445c20 100644 --- a/jscomp/ext/warnings.mli +++ b/jscomp/ext/warnings.mli @@ -19,6 +19,8 @@ type loc = { loc_ghost : bool; } +type topLevelUnitHelp = FunctionCall | Other + type t = | Comment_start (* 1 *) | Comment_not_end (* 2 *) @@ -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 diff --git a/jscomp/ml/typecore.ml b/jscomp/ml/typecore.ml index 3573e7819a..2c1e471ce7 100644 --- a/jscomp/ml/typecore.ml +++ b/jscomp/ml/typecore.ml @@ -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;