From 5527aa5c3b7cecdcdd55ba777ef3594e9a58b8a2 Mon Sep 17 00:00:00 2001 From: Javier Chavarri Date: Wed, 1 Nov 2023 09:53:39 +0000 Subject: [PATCH 1/5] playground: add test to show broken state with alerts --- .../compile-ml-warning-alert.t | 44 +++++++++++++++++++ 1 file changed, 44 insertions(+) create mode 100644 test/blackbox-tests/melange-playground/compile-ml-warning-alert.t diff --git a/test/blackbox-tests/melange-playground/compile-ml-warning-alert.t b/test/blackbox-tests/melange-playground/compile-ml-warning-alert.t new file mode 100644 index 0000000000..6d43b46529 --- /dev/null +++ b/test/blackbox-tests/melange-playground/compile-ml-warning-alert.t @@ -0,0 +1,44 @@ + $ cat > input.js < require(process.env.DUNE_SOURCEROOT + '/_build/default/bin/jsoo_main.bc.js'); + > require(process.env.DUNE_SOURCEROOT + '/_build/default/bin/melange-cmijs.js'); + > console.log(ocaml.compileML("let t = Js.Vector.length")); + > EOF + + $ node input.js + File "_none_", line 1, characters 8-24: + Alert deprecated: module Js.Vector + Use Belt.Array instead + { + js_code: '// Generated by Melange\n' + + '\n' + + '\n' + + 'function t(prim) {\n' + + ' return prim.length;\n' + + '}\n' + + '\n' + + 'export {\n' + + ' t ,\n' + + '}\n' + + '/* No side effect */\n', + warnings: [], + type_hints: [ + { + start: [Object], + end: [Object], + kind: 'expression', + hint: "'a Js.Vector.t -> int" + }, + { + start: [Object], + end: [Object], + kind: 'pattern_type', + hint: "'a Js.Vector.t -> int" + }, + { + start: [Object], + end: [Object], + kind: 'binding', + hint: "'a Js.Vector.t -> int" + } + ] + } From 23a0e831fb14f53db761d1a1c2fadbad29145004 Mon Sep 17 00:00:00 2001 From: Javier Chavarri Date: Wed, 1 Nov 2023 10:02:39 +0000 Subject: [PATCH 2/5] show alerts in playground --- bin/jsoo_main.ml | 7 ++++--- .../compile-ml-warning-alert.t | 17 +++++++++++++---- 2 files changed, 17 insertions(+), 7 deletions(-) diff --git a/bin/jsoo_main.ml b/bin/jsoo_main.ml index c2fa22807a..cd4a7958e6 100644 --- a/bin/jsoo_main.ml +++ b/bin/jsoo_main.ml @@ -41,11 +41,11 @@ let warnings_collected : Location.report list ref = ref [] (* We need to overload the original warning printer to capture the warnings and not let them go through default printer (which will end up in browser console) *) -let playground_warning_reporter (loc : Location.t) w : Location.report option = +let playground_warning_reporter f (loc : Location.t) w : Location.report option = let mk ~is_error id = if is_error then Location.Report_warning_as_error id else Report_warning id in - match Warnings.report w with + match f w with | `Inactive -> None | `Active { Warnings.id; message; is_error; sub_locs } -> let msg_of_str str ppf = Format.pp_print_string ppf str in @@ -297,7 +297,8 @@ let () = Clflags.binary_annotations := false; Clflags.color := None; - Location.warning_reporter := playground_warning_reporter; + Location.warning_reporter := playground_warning_reporter Warnings.report; + Location.alert_reporter := playground_warning_reporter Warnings.report_alert; (* To add a directory to the load path *) Load_path.add_dir "/static" diff --git a/test/blackbox-tests/melange-playground/compile-ml-warning-alert.t b/test/blackbox-tests/melange-playground/compile-ml-warning-alert.t index 6d43b46529..bad14fd29e 100644 --- a/test/blackbox-tests/melange-playground/compile-ml-warning-alert.t +++ b/test/blackbox-tests/melange-playground/compile-ml-warning-alert.t @@ -5,9 +5,6 @@ > EOF $ node input.js - File "_none_", line 1, characters 8-24: - Alert deprecated: module Js.Vector - Use Belt.Array instead { js_code: '// Generated by Melange\n' + '\n' + @@ -20,7 +17,19 @@ ' t ,\n' + '}\n' + '/* No side effect */\n', - warnings: [], + warnings: [ + { + js_warning_error_msg: 'Line 1, 8:\n' + + ' Warning: deprecated module Js.Vector\n' + + 'Use Belt.Array instead', + row: 0, + column: 8, + endRow: 0, + endColumn: 24, + text: 'module Js.Vector\nUse Belt.Array instead', + type: 'warning' + } + ], type_hints: [ { start: [Object], From 28382bc6cb2f0e1cd3d2f5a8da49d59144e5a951 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javier=20Ch=C3=A1varri?= Date: Thu, 2 Nov 2023 14:06:31 +0100 Subject: [PATCH 3/5] update bin/jsoo_main.ml Co-authored-by: Antonio Nuno Monteiro --- bin/jsoo_main.ml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bin/jsoo_main.ml b/bin/jsoo_main.ml index cd4a7958e6..bfb2723201 100644 --- a/bin/jsoo_main.ml +++ b/bin/jsoo_main.ml @@ -41,7 +41,7 @@ let warnings_collected : Location.report list ref = ref [] (* We need to overload the original warning printer to capture the warnings and not let them go through default printer (which will end up in browser console) *) -let playground_warning_reporter f (loc : Location.t) w : Location.report option = +let playground_warning_reporter ~f (loc : Location.t) w : Location.report option = let mk ~is_error id = if is_error then Location.Report_warning_as_error id else Report_warning id in From 88ca79af7818dad17868a52b622d355dc2d377be Mon Sep 17 00:00:00 2001 From: Javier Chavarri Date: Thu, 2 Nov 2023 13:10:32 +0000 Subject: [PATCH 4/5] fix build --- bin/jsoo_main.ml | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/bin/jsoo_main.ml b/bin/jsoo_main.ml index b9de57ddb2..29caadd02c 100644 --- a/bin/jsoo_main.ml +++ b/bin/jsoo_main.ml @@ -42,7 +42,8 @@ let warnings_collected : Location.report list ref = ref [] (* We need to overload the original warning printer to capture the warnings and not let them go through default printer (which will end up in browser console) *) -let playground_warning_reporter ~f (loc : Location.t) w : Location.report option = +let playground_warning_reporter ~f (loc : Location.t) w : Location.report option + = let mk ~is_error id = if is_error then Location.Report_warning_as_error id else Report_warning id in @@ -259,7 +260,10 @@ let compile = | None -> ( let default = lazy - (Js.obj [| ("js_warning_error_msg", Js.string (Printexc.to_string e)) |]) + (Js.obj + [| + ("js_warning_error_msg", Js.string (Printexc.to_string e)); + |]) in match e with | Warnings.Errors -> ( @@ -296,8 +300,9 @@ let () = Clflags.binary_annotations := false; Clflags.color := None; - Location.warning_reporter := playground_warning_reporter Warnings.report; - Location.alert_reporter := playground_warning_reporter Warnings.report_alert; + Location.warning_reporter := playground_warning_reporter ~f:Warnings.report; + Location.alert_reporter := + playground_warning_reporter ~f:Warnings.report_alert; (* To add a directory to the load path *) Load_path.add_dir "/static" From 35f832735df578de75a69d8db2abb4c7a7409e11 Mon Sep 17 00:00:00 2001 From: Javier Chavarri Date: Thu, 2 Nov 2023 13:23:10 +0000 Subject: [PATCH 5/5] fix alerts handling --- bin/jsoo_main.ml | 23 ++++++++++++------- .../compile-ml-warning-alert.t | 6 ++--- 2 files changed, 17 insertions(+), 12 deletions(-) diff --git a/bin/jsoo_main.ml b/bin/jsoo_main.ml index 29caadd02c..1e4a654542 100644 --- a/bin/jsoo_main.ml +++ b/bin/jsoo_main.ml @@ -42,11 +42,7 @@ let warnings_collected : Location.report list ref = ref [] (* We need to overload the original warning printer to capture the warnings and not let them go through default printer (which will end up in browser console) *) -let playground_warning_reporter ~f (loc : Location.t) w : Location.report option - = - let mk ~is_error id = - if is_error then Location.Report_warning_as_error id else Report_warning id - in +let playground_reporter ~f ~mk (loc : Location.t) w : Location.report option = match f w with | `Inactive -> None | `Active { Warnings.id; message; is_error; sub_locs } -> @@ -62,6 +58,18 @@ let playground_warning_reporter ~f (loc : Location.t) w : Location.report option warnings_collected := { Location.kind; main; sub } :: !warnings_collected; None +let playground_warning_reporter = + let mk ~is_error id = + if is_error then Location.Report_warning_as_error id else Report_warning id + in + playground_reporter ~f:Warnings.report ~mk + +let playground_alert_reporter = + let mk ~is_error id = + if is_error then Location.Report_alert_as_error id else Report_alert id + in + playground_reporter ~f:Warnings.report_alert ~mk + let error_of_exn e = match Location.error_of_exn e with | Some (`Ok e) -> Some e @@ -300,9 +308,8 @@ let () = Clflags.binary_annotations := false; Clflags.color := None; - Location.warning_reporter := playground_warning_reporter ~f:Warnings.report; - Location.alert_reporter := - playground_warning_reporter ~f:Warnings.report_alert; + Location.warning_reporter := playground_warning_reporter; + Location.alert_reporter := playground_alert_reporter; (* To add a directory to the load path *) Load_path.add_dir "/static" diff --git a/test/blackbox-tests/melange-playground/compile-ml-warning-alert.t b/test/blackbox-tests/melange-playground/compile-ml-warning-alert.t index bad14fd29e..2fe02d2f81 100644 --- a/test/blackbox-tests/melange-playground/compile-ml-warning-alert.t +++ b/test/blackbox-tests/melange-playground/compile-ml-warning-alert.t @@ -19,15 +19,13 @@ '/* No side effect */\n', warnings: [ { - js_warning_error_msg: 'Line 1, 8:\n' + - ' Warning: deprecated module Js.Vector\n' + - 'Use Belt.Array instead', + js_warning_error_msg: 'Line 1, 8:\n Alert: deprecated module Js.Vector\nUse Belt.Array instead', row: 0, column: 8, endRow: 0, endColumn: 24, text: 'module Js.Vector\nUse Belt.Array instead', - type: 'warning' + type: 'alert' } ], type_hints: [