Skip to content

Commit 7d3c10e

Browse files
authored
Merge pull request rescript-lang#4664 from rescript-lang/simplify_warning_reporting
start simplify warning reporting
2 parents 93e5abf + eb3cb02 commit 7d3c10e

18 files changed

+269
-1989
lines changed

jscomp/build_tests/super_errors/expected/highlighting4.re.expected

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11

2-
Warning number 3 (configured as error)
2+
Warning number 3
33
/.../fixtures/highlighting4.re:5:10
44

55
3 │ [@deprecated]

jscomp/core/js_implementation.ml

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -12,11 +12,16 @@
1212

1313
(* adapted by rescript from [driver/compile.ml] for convenience *)
1414

15-
open Format
16-
open Typedtree
17-
open Compenv
18-
19-
15+
let module_of_filename outputprefix =
16+
let basename = Filename.basename outputprefix in
17+
let name =
18+
try
19+
let pos = String.index basename '.' in
20+
String.sub basename 0 pos
21+
with Not_found -> basename
22+
in
23+
String.capitalize_ascii name
24+
;;
2025

2126
let fprintf = Format.fprintf
2227

@@ -71,7 +76,7 @@ let after_parsing_sig ppf outputprefix ast =
7176
Warnings.check_fatal()
7277
else
7378
begin
74-
let modulename = module_of_filename ppf !Location.input_name outputprefix in
79+
let modulename = module_of_filename outputprefix in
7580
Lam_compile_env.reset () ;
7681
let initial_env = Res_compmisc.initial_env () in
7782
Env.set_unit_name modulename;
@@ -83,7 +88,7 @@ let after_parsing_sig ppf outputprefix ast =
8388
let sg = tsg.sig_type in
8489
if !Clflags.print_types then
8590
Printtyp.wrap_printing_env initial_env (fun () ->
86-
fprintf std_formatter "%a@."
91+
fprintf Format.std_formatter "%a@."
8792
Printtyp.signature (Typemod.simplify_signature sg));
8893
ignore (Includemod.signatures initial_env sg sg);
8994
Typecore.force_delayed_checks ();

jscomp/super_errors/super_location.ml

Lines changed: 13 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -25,12 +25,8 @@ let print_loc ~normalizedRange ppf (loc : Location.t) =
2525
fprintf ppf "@{<filename>%a@}%a" print_filename loc.loc_start.pos_fname dim_loc normalizedRange
2626
;;
2727

28-
let print ~message_kind intro ppf (loc : Location.t) =
29-
begin match message_kind with
30-
| `warning -> fprintf ppf "@[@{<info>%s@}@]@," intro
31-
| `warning_as_error -> fprintf ppf "@[@{<error>%s@} (configured as error) @]@," intro
32-
| `error -> fprintf ppf "@[@{<error>%s@}@]@," intro
33-
end;
28+
let print intro ppf (loc : Location.t) =
29+
fprintf ppf "@[@{<error>%s@}@]@," intro;
3430
(* ocaml's reported line/col numbering is horrible and super error-prone
3531
when being handled programmatically (or humanly for that matter. If you're
3632
an ocaml contributor reading this: who the heck reads the character count
@@ -64,7 +60,7 @@ let print ~message_kind intro ppf (loc : Location.t) =
6460
branch might not be reached (aka no inline file content display) so
6561
we don't wanna end up with two line breaks in the the consequent *)
6662
fprintf ppf "@,%a"
67-
(Super_misc.print_file ~is_warning:(message_kind=`warning) ~lines ~range)
63+
(Super_misc.print_file ~lines ~range)
6864
()
6965
with
7066
(* this might happen if the file is e.g. "", "_none_" or any of the fake file name placeholders.
@@ -75,37 +71,23 @@ let print ~message_kind intro ppf (loc : Location.t) =
7571

7672
(* taken from https://github.com/rescript-lang/ocaml/blob/d4144647d1bf9bc7dc3aadc24c25a7efa3a67915/parsing/location.ml#L380 *)
7773
(* This is the error report entry point. We'll replace the default reporter with this one. *)
78-
let rec super_error_reporter ppf ({Location.loc; msg; sub; if_highlight} as err) =
79-
let highlighted =
80-
if if_highlight <> "" then
81-
let rec collect_locs locs {Location.loc; sub; if_highlight = _; _} =
82-
List.fold_left collect_locs (loc :: locs) sub
83-
in
84-
let locs = collect_locs [] err in
85-
Location.highlight_locations ppf locs
86-
else
87-
false
88-
in
89-
if highlighted then
90-
Format.pp_print_string ppf if_highlight
91-
else begin
92-
setup_colors ();
93-
(* open a vertical box. Everything in our message is indented 2 spaces *)
94-
Format.fprintf ppf "@[<v 2>@,%a@,%s@,@]" (print ~message_kind:`error "We've found a bug for you!") loc msg;
95-
List.iter (Format.fprintf ppf "@,@[%a@]" super_error_reporter) sub;
74+
let rec super_error_reporter ppf ({loc; msg; sub} : Location.error) =
75+
setup_colors ();
76+
(* open a vertical box. Everything in our message is indented 2 spaces *)
77+
Format.fprintf ppf "@[<v 2>@,%a@,%s@,@]" (print "We've found a bug for you!") loc msg;
78+
List.iter (Format.fprintf ppf "@,@[%a@]" super_error_reporter) sub
9679
(* no need to flush here; location's report_exception (which uses this ultimately) flushes *)
97-
end
80+
9881

9982
(* extracted from https://github.com/rescript-lang/ocaml/blob/d4144647d1bf9bc7dc3aadc24c25a7efa3a67915/parsing/location.ml#L299 *)
10083
(* This is the warning report entry point. We'll replace the default printer with this one *)
10184
let super_warning_printer loc ppf w =
10285
match Warnings.report w with
10386
| `Inactive -> ()
104-
| `Active { Warnings. number = _; message = _; is_error; sub_locs = _} ->
105-
setup_colors ();
106-
let message_kind = if is_error then `warning_as_error else `warning in
107-
Format.fprintf ppf "@[<v 2>@,%a@,%s@,@]"
108-
(print ~message_kind ("Warning number " ^ (Warnings.number w |> string_of_int)))
87+
| `Active { Warnings. number = _; message = _; sub_locs = _} ->
88+
setup_colors ();
89+
Format.fprintf ppf "@[<v 2>@,%a@,%s@,@]@."
90+
(print ("Warning number " ^ (Warnings.number w |> string_of_int)))
10991
loc
11092
(Super_warnings.message w);
11193
(* at this point, you can display sub_locs too, from e.g. https://github.com/ocaml/ocaml/commit/f6d53cc38f87c67fbf49109f5fb79a0334bab17a
Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
2+
3+
val error_of_printer :
4+
Location.t ->
5+
(Format.formatter -> 'a -> unit) ->
6+
'a ->
7+
Location.error
8+
9+
val error_of_printer_file :
10+
(Format.formatter -> 'a -> unit) ->
11+
'a ->
12+
Location.error
13+
14+
15+
val setup : unit -> unit

jscomp/super_errors/super_misc.ml

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ type current_printed_line_status =
3636
(* Range coordinates all 1-indexed, like for editors. Otherwise this code
3737
would have way too many off-by-one errors *)
3838
let print_file
39-
~is_warning
39+
4040
(* start_line_start_char inclusive, end_line_end_char exclusive *)
4141
~range:((start_line, start_line_start_char), (end_line, end_line_end_char))
4242
~lines
@@ -69,10 +69,10 @@ ppf
6969
| Some n -> n
7070
in
7171
(* coloring *)
72-
let highlighted_line_number : _ format = if is_warning then "@{<info>%s@}%a" else "@{<error>%s@}%a" in
72+
let highlighted_line_number : _ format = "@{<error>%s@}%a" in
7373

7474
let print_char_maybe_highlight ~begin_highlight_line ~end_highlight_line ch =
75-
let highlighted_open_tag: _ format = if is_warning then "@{<info>" else "@{<error>" in
75+
let highlighted_open_tag: _ format = "@{<error>" in
7676
if begin_highlight_line then fprintf ppf highlighted_open_tag;
7777
fprintf ppf "%c@," ch;
7878
if end_highlight_line then fprintf ppf "@}"

jscomp/super_errors/super_misc.mli

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
11
(** Range coordinates all 1-indexed, like for editors. Otherwise this code
22
would have way too many off-by-one errors *)
3-
val print_file: is_warning:bool -> range:(int * int) * (int * int) -> lines:string array -> Format.formatter -> unit -> unit
3+
val print_file:
4+
range:(int * int) * (int * int) ->
5+
lines:string array -> Format.formatter -> unit -> unit

jscomp/super_errors/super_pparse.ml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,9 @@
1-
let fprintf = Format.fprintf
1+
22

33
(* taken from https://github.com/rescript-lang/ocaml/blob/d4144647d1bf9bc7dc3aadc24c25a7efa3a67915/driver/pparse.ml#L170 *)
44
(* modified branches are commented *)
55
let report_error ppf () =
6-
fprintf ppf
6+
Format.fprintf ppf
77
"@[<v>@{<info>There's been an error running Reason's parser on a file.@}@,\
88
If the message doesn't help, check for errors slightly above.@,\
99
@[Please file an issue on@ github.com/facebook/reason.@ Thanks!@]@]"

0 commit comments

Comments
 (0)