From 93ed1ad2b48f038b54bbca9a88c24b0f4dffd528 Mon Sep 17 00:00:00 2001 From: Pedro Castro Date: Sat, 6 Apr 2024 10:51:57 -0300 Subject: [PATCH 1/7] support -warn-error argument --- jscomp/bsb/bsb_ninja_regen.ml | 19 ++++++++++++++++++- jscomp/bsb/bsb_ninja_regen.mli | 1 + jscomp/bsb/bsb_warning.mli | 11 ++++++++++- jscomp/bsb/bsb_world.ml | 1 + jscomp/bsb_exe/rescript_main.ml | 13 +++++++++++++ jscomp/runtime/release.ninja | 6 +++--- 6 files changed, 46 insertions(+), 5 deletions(-) diff --git a/jscomp/bsb/bsb_ninja_regen.ml b/jscomp/bsb/bsb_ninja_regen.ml index 030e98b953..d2953b93ee 100644 --- a/jscomp/bsb/bsb_ninja_regen.ml +++ b/jscomp/bsb/bsb_ninja_regen.ml @@ -30,7 +30,7 @@ let ( // ) = Ext_path.combine return None if we dont need regenerate otherwise return Some info *) -let regenerate_ninja ~(package_kind : Bsb_package_kind.t) ~forced ~per_proj_dir ~warn_legacy_config +let regenerate_ninja ~(package_kind : Bsb_package_kind.t) ~forced ~per_proj_dir ~warn_legacy_config ~warn_as_error : Bsb_config_types.t option = let lib_artifacts_dir = Bsb_config.lib_bs in let lib_bs_dir = per_proj_dir // lib_artifacts_dir in @@ -58,6 +58,23 @@ let regenerate_ninja ~(package_kind : Bsb_package_kind.t) ~forced ~per_proj_dir Bsb_config_parse.interpret_json ~filename:config_filename ~json:config_json ~package_kind ~per_proj_dir in + + let warning = match config.warning with + | None -> ( + match warn_as_error with + | Some e -> Some {Bsb_warning.number = Some e; error = Warn_error_number e} + | None -> None) + | Some {error} as t -> + match (warn_as_error, error) with + | (Some error_str, Warn_error_false) -> + Some {number = Some error_str; error = Warn_error_number error_str} + | (Some error_str, Warn_error_number prev) -> + let new_error = prev ^ error_str in + Some {number = Some new_error; error = Warn_error_number new_error} + | _ -> t + in + + let config = {config with warning = warning} in (* create directory, lib/bs, lib/js, lib/es6 etc *) Bsb_build_util.mkp lib_bs_dir; Bsb_package_specs.list_dirs_by config.package_specs (fun x -> diff --git a/jscomp/bsb/bsb_ninja_regen.mli b/jscomp/bsb/bsb_ninja_regen.mli index b92cf0b115..0af5766e57 100644 --- a/jscomp/bsb/bsb_ninja_regen.mli +++ b/jscomp/bsb/bsb_ninja_regen.mli @@ -27,6 +27,7 @@ val regenerate_ninja : forced:bool -> per_proj_dir:string -> warn_legacy_config:bool -> + warn_as_error:string option -> Bsb_config_types.t option (** Regenerate ninja file by need based on [.bsdeps] return None if we dont need regenerate diff --git a/jscomp/bsb/bsb_warning.mli b/jscomp/bsb/bsb_warning.mli index d5c3ef923d..b04fb6f038 100644 --- a/jscomp/bsb/bsb_warning.mli +++ b/jscomp/bsb/bsb_warning.mli @@ -22,7 +22,16 @@ * along with this program; if not, write to the Free Software * Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA. *) -type t +type warning_error = + | Warn_error_false + (* default [false] to make our changes non-intrusive *) + | Warn_error_true + | Warn_error_number of string + +type t0 = { number : string option; error : warning_error } + +type nonrec t = t0 option + val to_merlin_string : t -> string (** Extra work is need to make merlin happy *) diff --git a/jscomp/bsb/bsb_world.ml b/jscomp/bsb/bsb_world.ml index e5e9dfdc77..760e941eb1 100644 --- a/jscomp/bsb/bsb_world.ml +++ b/jscomp/bsb/bsb_world.ml @@ -71,6 +71,7 @@ let make_world_deps cwd (config : Bsb_config_types.t option) else Dependency { package_specs; jsx; uncurried }) ~per_proj_dir:proj_dir ~forced:false ~warn_legacy_config:false + ~warn_as_error:None in let command = { Bsb_unix.cmd = vendor_ninja; cwd = lib_bs_dir; args } diff --git a/jscomp/bsb_exe/rescript_main.ml b/jscomp/bsb_exe/rescript_main.ml index c8474ee48f..ecc869a7a2 100644 --- a/jscomp/bsb_exe/rescript_main.ml +++ b/jscomp/bsb_exe/rescript_main.ml @@ -30,6 +30,8 @@ let no_deps_mode = ref false let do_install = ref false +let warning_as_error = ref None + let force_regenerate = ref false type spec = Bsb_arg.spec @@ -40,6 +42,8 @@ let unit_set_spec b : spec = Unit (Unit_set b) let string_set_spec s : spec = String (String_set s) +let string_call f: spec = String (String_call f) + let failed_annon ~rev_args = match rev_args with | x :: _ -> Bsb_arg.bad_arg ("Don't know what to do with " ^ x) @@ -132,6 +136,7 @@ let build_subcommand ~start argv argv_len = Always regenerate build.ninja no matter bsconfig.json is changed or \ not" ); ("-no-deps", unit_set_spec no_deps_mode, "*internal* Needed for watcher to build without dependencies on file change"); + ("-warn-error", string_call (fun s -> warning_as_error := Some s), "Enable warnings as error") |] failed_annon; @@ -141,12 +146,18 @@ let build_subcommand ~start argv argv_len = match ninja_args with | [| "-h" |] -> ninja_command_exit ninja_args | _ -> + let warn_as_error = match !warning_as_error with + | Some s -> + Warnings.parse_options true s; + Some s + | None -> None in let config_opt = Bsb_ninja_regen.regenerate_ninja ~package_kind:Toplevel ~per_proj_dir:Bsb_global_paths.cwd ~forced:!force_regenerate ~warn_legacy_config:true + ~warn_as_error in if not !no_deps_mode then Bsb_world.make_world_deps Bsb_global_paths.cwd config_opt ninja_args; if !do_install then install_target (); @@ -180,6 +191,7 @@ let info_subcommand ~start argv = ~per_proj_dir:Bsb_global_paths.cwd ~forced:true ~warn_legacy_config:true + ~warn_as_error:None with | None -> assert false | Some { file_groups = { files } } -> @@ -210,6 +222,7 @@ let () = ~per_proj_dir:Bsb_global_paths.cwd ~forced:false ~warn_legacy_config:true + ~warn_as_error:None in Bsb_world.make_world_deps Bsb_global_paths.cwd config_opt [||]; ninja_command_exit [||]) diff --git a/jscomp/runtime/release.ninja b/jscomp/runtime/release.ninja index 317583d49c..d21ca55b4d 100644 --- a/jscomp/runtime/release.ninja +++ b/jscomp/runtime/release.ninja @@ -25,7 +25,7 @@ o runtime/caml_exceptions.cmj : cc_cmi runtime/caml_exceptions.res | runtime/cam o runtime/caml_exceptions.cmi : cc runtime/caml_exceptions.resi | runtime/bs_stdlib_mini.cmi runtime/js.cmi runtime/js.cmj o runtime/caml_float.cmj : cc_cmi runtime/caml_float.res | runtime/caml_float.cmi runtime/caml_float_extern.cmj o runtime/caml_float.cmi : cc runtime/caml_float.resi | runtime/bs_stdlib_mini.cmi runtime/js.cmi runtime/js.cmj -o runtime/caml_format.cmj : cc_cmi runtime/caml_format.ml | runtime/caml_float.cmj runtime/caml_float_extern.cmj runtime/caml_format.cmi runtime/caml_int64.cmj runtime/caml_int64_extern.cmj runtime/caml_nativeint_extern.cmj runtime/caml_string_extern.cmj +o runtime/caml_format.cmj : cc_cmi runtime/caml_format.ml | runtime/caml.cmj runtime/caml_float.cmj runtime/caml_float_extern.cmj runtime/caml_format.cmi runtime/caml_int64.cmj runtime/caml_int64_extern.cmj runtime/caml_nativeint_extern.cmj runtime/caml_string_extern.cmj o runtime/caml_format.cmi : cc runtime/caml_format.mli | runtime/bs_stdlib_mini.cmi runtime/js.cmi runtime/js.cmj o runtime/caml_hash.cmj : cc_cmi runtime/caml_hash.res | runtime/caml_hash.cmi runtime/caml_hash_primitive.cmj runtime/caml_nativeint_extern.cmj runtime/js.cmj o runtime/caml_hash.cmi : cc runtime/caml_hash.resi | runtime/bs_stdlib_mini.cmi runtime/js.cmi runtime/js.cmj @@ -41,7 +41,7 @@ o runtime/caml_md5.cmj : cc_cmi runtime/caml_md5.res | runtime/caml_array_extern o runtime/caml_md5.cmi : cc runtime/caml_md5.resi | runtime/bs_stdlib_mini.cmi runtime/js.cmi runtime/js.cmj o runtime/caml_module.cmj : cc_cmi runtime/caml_module.res | runtime/caml_array_extern.cmj runtime/caml_module.cmi runtime/caml_obj.cmj o runtime/caml_module.cmi : cc runtime/caml_module.resi | runtime/bs_stdlib_mini.cmi runtime/js.cmi runtime/js.cmj -o runtime/caml_obj.cmj : cc_cmi runtime/caml_obj.res | runtime/caml_array_extern.cmj runtime/caml_obj.cmi runtime/caml_option.cmj runtime/js.cmj +o runtime/caml_obj.cmj : cc_cmi runtime/caml_obj.res | runtime/caml.cmj runtime/caml_array_extern.cmj runtime/caml_obj.cmi runtime/caml_option.cmj runtime/js.cmj o runtime/caml_obj.cmi : cc runtime/caml_obj.resi | runtime/bs_stdlib_mini.cmi runtime/js.cmi runtime/js.cmj o runtime/caml_option.cmj : cc_cmi runtime/caml_option.res | runtime/caml_option.cmi runtime/caml_undefined_extern.cmj runtime/js.cmj o runtime/caml_option.cmi : cc runtime/caml_option.resi | runtime/bs_stdlib_mini.cmi runtime/caml_undefined_extern.cmj runtime/js.cmi runtime/js.cmj @@ -58,7 +58,7 @@ o runtime/caml_bigint_extern.cmi runtime/caml_bigint_extern.cmj : cc runtime/cam o runtime/caml_external_polyfill.cmi runtime/caml_external_polyfill.cmj : cc runtime/caml_external_polyfill.res | runtime/bs_stdlib_mini.cmi runtime/js.cmi runtime/js.cmj o runtime/caml_float_extern.cmi runtime/caml_float_extern.cmj : cc runtime/caml_float_extern.res | runtime/bs_stdlib_mini.cmi runtime/js.cmi runtime/js.cmj o runtime/caml_int64_extern.cmi runtime/caml_int64_extern.cmj : cc runtime/caml_int64_extern.res | runtime/bs_stdlib_mini.cmi runtime/js.cmi runtime/js.cmj -o runtime/caml_js_exceptions.cmi runtime/caml_js_exceptions.cmj : cc runtime/caml_js_exceptions.res | runtime/bs_stdlib_mini.cmi runtime/caml_exceptions.cmj runtime/js.cmi runtime/js.cmj +o runtime/caml_js_exceptions.cmi runtime/caml_js_exceptions.cmj : cc runtime/caml_js_exceptions.res | runtime/bs_stdlib_mini.cmi runtime/caml_exceptions.cmj runtime/caml_option.cmj runtime/js.cmi runtime/js.cmj o runtime/caml_nativeint_extern.cmi runtime/caml_nativeint_extern.cmj : cc runtime/caml_nativeint_extern.res | runtime/bs_stdlib_mini.cmi runtime/js.cmi runtime/js.cmj o runtime/caml_string_extern.cmi runtime/caml_string_extern.cmj : cc runtime/caml_string_extern.res | runtime/bs_stdlib_mini.cmi runtime/js.cmi runtime/js.cmj o runtime/caml_undefined_extern.cmi runtime/caml_undefined_extern.cmj : cc runtime/caml_undefined_extern.res | runtime/bs_stdlib_mini.cmi runtime/js.cmi runtime/js.cmj From 021534a34beba58abe6b68406753405cb5c4eee6 Mon Sep 17 00:00:00 2001 From: Pedro Castro Date: Sat, 6 Apr 2024 11:17:14 -0300 Subject: [PATCH 2/7] add tests --- jscomp/build_tests/build_warn_as_error/input.js | 17 +++++++++++++++++ .../build_warn_as_error/rescript.json | 5 +++++ .../build_warn_as_error/src/Demo.res | 1 + 3 files changed, 23 insertions(+) create mode 100644 jscomp/build_tests/build_warn_as_error/input.js create mode 100644 jscomp/build_tests/build_warn_as_error/rescript.json create mode 100644 jscomp/build_tests/build_warn_as_error/src/Demo.res diff --git a/jscomp/build_tests/build_warn_as_error/input.js b/jscomp/build_tests/build_warn_as_error/input.js new file mode 100644 index 0000000000..878647f9d9 --- /dev/null +++ b/jscomp/build_tests/build_warn_as_error/input.js @@ -0,0 +1,17 @@ +var p = require("child_process"); +var assert = require("assert"); +var rescript_exe = require("../../../scripts/bin_path").rescript_exe; + +var o = p.spawnSync(rescript_exe, ["build", "-warn-error", "+110"], { + encoding: "utf8", + cwd: __dirname, +}); + +var error_message = o.stdout + .split("\n") + .map(s => s.trim()) + .includes("Warning number 110 (configured as error)"); + +if (!error_message) { + assert.fail(o.stdout); +} diff --git a/jscomp/build_tests/build_warn_as_error/rescript.json b/jscomp/build_tests/build_warn_as_error/rescript.json new file mode 100644 index 0000000000..0932ebe394 --- /dev/null +++ b/jscomp/build_tests/build_warn_as_error/rescript.json @@ -0,0 +1,5 @@ +{ + "name": "build_warn_as_error", + "version": "0.1.0", + "sources": ["src"] +} diff --git a/jscomp/build_tests/build_warn_as_error/src/Demo.res b/jscomp/build_tests/build_warn_as_error/src/Demo.res new file mode 100644 index 0000000000..1dabe7324e --- /dev/null +++ b/jscomp/build_tests/build_warn_as_error/src/Demo.res @@ -0,0 +1 @@ +let todo = _ => %todo From 69a531ef9c3e8f2abc386157c325442f65a7d6c0 Mon Sep 17 00:00:00 2001 From: Pedro Castro Date: Sat, 6 Apr 2024 12:00:15 -0300 Subject: [PATCH 3/7] fix cli_help test --- jscomp/build_tests/cli_help/input.js | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/jscomp/build_tests/cli_help/input.js b/jscomp/build_tests/cli_help/input.js index d2ebcde3c7..0aaa598b32 100755 --- a/jscomp/build_tests/cli_help/input.js +++ b/jscomp/build_tests/cli_help/input.js @@ -32,10 +32,11 @@ const buildHelp = "`rescript build -- -h` for Ninja options (internal usage only; unstable)\n" + "\n" + "Options:\n" + - " -w Watch mode\n" + - " -ws [host]:port set up host & port for WebSocket build notifications\n" + - " -verbose Set the output to be verbose\n" + - " -with-deps *deprecated* This is the default behavior now. This option will be removed in a future release\n"; + " -w Watch mode\n" + + " -ws [host]:port set up host & port for WebSocket build notifications\n" + + " -verbose Set the output to be verbose\n" + + " -with-deps *deprecated* This is the default behavior now. This option will be removed in a future release\n" + + " -warn-error Enable warnings as error\n"; const cleanHelp = "Usage: rescript clean \n" + From 96707b3c69b965f7ded3ccfa7e600af823331e2f Mon Sep 17 00:00:00 2001 From: Pedro Castro Date: Sat, 6 Apr 2024 12:19:32 -0300 Subject: [PATCH 4/7] update CHANGELOG.md --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 01a231c23d..464021f88f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,7 @@ #### :rocket: New Feature - Add `%todo` extension for leaving implementation for later. https://github.com/rescript-lang/rescript-compiler/pull/6713 +- Add `-warn-error` argument for generate error on CI. Useful for `%todo` extension. https://github.com/rescript-lang/rescript-compiler/pull/6717 #### :bug: Bug Fix From 1f03d90fa3e212cde3114b76dc88441ff2b9d60d Mon Sep 17 00:00:00 2001 From: Pedro Castro Date: Mon, 8 Apr 2024 22:57:11 -0300 Subject: [PATCH 5/7] printer nice error message --- jscomp/bsb_exe/rescript_main.ml | 4 ++-- jscomp/build_tests/cli_help/input.js | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/jscomp/bsb_exe/rescript_main.ml b/jscomp/bsb_exe/rescript_main.ml index ecc869a7a2..c29f5f2813 100644 --- a/jscomp/bsb_exe/rescript_main.ml +++ b/jscomp/bsb_exe/rescript_main.ml @@ -136,7 +136,7 @@ let build_subcommand ~start argv argv_len = Always regenerate build.ninja no matter bsconfig.json is changed or \ not" ); ("-no-deps", unit_set_spec no_deps_mode, "*internal* Needed for watcher to build without dependencies on file change"); - ("-warn-error", string_call (fun s -> warning_as_error := Some s), "Enable warnings as error") + ("-warn-error", string_call (fun s -> warning_as_error := Some s), "Warning numbers and whether to turn it into error or not") |] failed_annon; @@ -148,7 +148,7 @@ let build_subcommand ~start argv argv_len = | _ -> let warn_as_error = match !warning_as_error with | Some s -> - Warnings.parse_options true s; + let () = try Warnings.parse_options true s with Arg.Bad msg -> Bsb_arg.bad_arg (msg ^ "\n") in Some s | None -> None in let config_opt = diff --git a/jscomp/build_tests/cli_help/input.js b/jscomp/build_tests/cli_help/input.js index 0aaa598b32..bc1a2b414f 100755 --- a/jscomp/build_tests/cli_help/input.js +++ b/jscomp/build_tests/cli_help/input.js @@ -36,7 +36,7 @@ const buildHelp = " -ws [host]:port set up host & port for WebSocket build notifications\n" + " -verbose Set the output to be verbose\n" + " -with-deps *deprecated* This is the default behavior now. This option will be removed in a future release\n" + - " -warn-error Enable warnings as error\n"; + " -warn-error Warning numbers and whether to turn it into error or not\n"; const cleanHelp = "Usage: rescript clean \n" + From bd2a04e2a256b61c425304ffa33b0188efecb8ef Mon Sep 17 00:00:00 2001 From: Pedro Castro Date: Sat, 13 Apr 2024 23:25:55 -0300 Subject: [PATCH 6/7] support pinned-deps --- jscomp/bsb/bsb_world.ml | 4 ++-- jscomp/bsb/bsb_world.mli | 2 +- jscomp/bsb_exe/rescript_main.ml | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/jscomp/bsb/bsb_world.ml b/jscomp/bsb/bsb_world.ml index 760e941eb1..30b776c796 100644 --- a/jscomp/bsb/bsb_world.ml +++ b/jscomp/bsb/bsb_world.ml @@ -26,7 +26,7 @@ let ( // ) = Ext_path.combine let vendor_ninja = Bsb_global_paths.vendor_ninja let make_world_deps cwd (config : Bsb_config_types.t option) - (ninja_args : string array) = + (ninja_args : string array) warn_as_error = let package_specs, jsx, uncurried, pinned_dependencies = match config with | None -> @@ -71,7 +71,7 @@ let make_world_deps cwd (config : Bsb_config_types.t option) else Dependency { package_specs; jsx; uncurried }) ~per_proj_dir:proj_dir ~forced:false ~warn_legacy_config:false - ~warn_as_error:None + ~warn_as_error:(if is_pinned then warn_as_error else None) in let command = { Bsb_unix.cmd = vendor_ninja; cwd = lib_bs_dir; args } diff --git a/jscomp/bsb/bsb_world.mli b/jscomp/bsb/bsb_world.mli index e705a22099..123f63e5a8 100644 --- a/jscomp/bsb/bsb_world.mli +++ b/jscomp/bsb/bsb_world.mli @@ -23,4 +23,4 @@ * Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA. *) val make_world_deps : - string -> Bsb_config_types.t option -> string array -> unit + string -> Bsb_config_types.t option -> string array -> string option -> unit diff --git a/jscomp/bsb_exe/rescript_main.ml b/jscomp/bsb_exe/rescript_main.ml index c29f5f2813..298bb75d8d 100644 --- a/jscomp/bsb_exe/rescript_main.ml +++ b/jscomp/bsb_exe/rescript_main.ml @@ -159,7 +159,7 @@ let build_subcommand ~start argv argv_len = ~warn_legacy_config:true ~warn_as_error in - if not !no_deps_mode then Bsb_world.make_world_deps Bsb_global_paths.cwd config_opt ninja_args; + if not !no_deps_mode then Bsb_world.make_world_deps Bsb_global_paths.cwd config_opt ninja_args warn_as_error; if !do_install then install_target (); ninja_command_exit ninja_args @@ -224,7 +224,7 @@ let () = ~warn_legacy_config:true ~warn_as_error:None in - Bsb_world.make_world_deps Bsb_global_paths.cwd config_opt [||]; + Bsb_world.make_world_deps Bsb_global_paths.cwd config_opt [||] None; ninja_command_exit [||]) else match argv.(1) with From 1c05d6ac43fbdb0de69e1a6e4f7a3c3a9e309d51 Mon Sep 17 00:00:00 2001 From: Pedro Castro Date: Tue, 16 Apr 2024 17:01:54 -0300 Subject: [PATCH 7/7] add examples --- jscomp/bsb_exe/rescript_main.ml | 2 +- jscomp/build_tests/cli_help/input.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/jscomp/bsb_exe/rescript_main.ml b/jscomp/bsb_exe/rescript_main.ml index 298bb75d8d..3e29b33c6e 100644 --- a/jscomp/bsb_exe/rescript_main.ml +++ b/jscomp/bsb_exe/rescript_main.ml @@ -136,7 +136,7 @@ let build_subcommand ~start argv argv_len = Always regenerate build.ninja no matter bsconfig.json is changed or \ not" ); ("-no-deps", unit_set_spec no_deps_mode, "*internal* Needed for watcher to build without dependencies on file change"); - ("-warn-error", string_call (fun s -> warning_as_error := Some s), "Warning numbers and whether to turn it into error or not") + ("-warn-error", string_call (fun s -> warning_as_error := Some s), "Warning numbers and whether to turn them into errors, e.g., \"+8+32-102\"") |] failed_annon; diff --git a/jscomp/build_tests/cli_help/input.js b/jscomp/build_tests/cli_help/input.js index bc1a2b414f..18c4fc7d9b 100755 --- a/jscomp/build_tests/cli_help/input.js +++ b/jscomp/build_tests/cli_help/input.js @@ -36,7 +36,7 @@ const buildHelp = " -ws [host]:port set up host & port for WebSocket build notifications\n" + " -verbose Set the output to be verbose\n" + " -with-deps *deprecated* This is the default behavior now. This option will be removed in a future release\n" + - " -warn-error Warning numbers and whether to turn it into error or not\n"; + ' -warn-error Warning numbers and whether to turn them into errors, e.g., "+8+32-102"\n'; const cleanHelp = "Usage: rescript clean \n" +