Fix flow coverage's error behavior

Summary:
Similarly to the check-contents test, the coverage test was
also using `2>&1` to record stderr. This is dangerous, since status
messages can show up on stderr.

Looking more into `flow coverage`, it's actually doing some silly
things:

1. Not properly respecting `--quiet`
2. Exiting with exit code 0 on `--json --color` or `--json --debug`
3. Only checking for invalid flag combinations after connecting to the server
4. Not documenting which flags can't be used together.

Reviewed By: mroch

Differential Revision: D6277951

fbshipit-source-id: 1f97c76d2bdc358cbe3dafc068cc1e4b3aeab0db

src/commands/coverageCommand.ml

@@ -29,9 +29,11 @@ let spec = {
  |> root_flag
  |> from_flag
  |> flag "--color" no_arg
- ~doc:"Print the file with colors showing which parts have unknown types"
+ ~doc:("Print the file with colors showing which parts have unknown types. " ^
+ "Cannot be used with --json or --pretty")
  |> flag "--debug" no_arg
- ~doc:"Print debugging info about each range in the file to stderr"
+ ~doc:("Print debugging info about each range in the file to stderr. " ^
+ "Cannot be used with --json or --pretty")
  |> flag "--path" (optional string)
  ~doc:"Specify (fake) path to file when reading data from stdin"
  |> flag "--respect-pragma" no_arg ~doc:"" (* deprecated *)
@@ -175,42 +177,36 @@ let rec split_overlapping_ranges accum = Loc.(function
 )
 
 let handle_response ~json ~pretty ~color ~debug (types : (Loc.t * bool) list) content =
- if color && json then
- prerr_endline "Error: --color and --json flags cannot be used together"
- else if debug && json then
- prerr_endline "Error: --debug and --json flags cannot be used together"
- else (
- if debug then List.iter debug_range types;
-
- begin if color then
- let types = split_overlapping_ranges [] types |> List.rev in
- let colors, _ = colorize_file content 0 [] types in
- Tty.cprint (List.rev colors);
- print_endline ""
- end;
-
- let covered, total = List.fold_left accum_coverage (0, 0) types in
- let percent = if total = 0 then 100. else (float_of_int covered /. float_of_int total) *. 100. in
-
- if json then
- let uncovered_locs = types
- |> List.filter (fun (_, is_covered) -> not is_covered)
- |> List.map (fun (loc, _) -> loc)
- in
- let open Hh_json in
- JSON_Object [
- "expressions", JSON_Object [
- "covered_count", int_ covered;
- "uncovered_count", int_ (total - covered);
- "uncovered_locs", JSON_Array (uncovered_locs |> List.map Reason.json_of_loc);
- ];
- ]
- |> json_to_string ~pretty
- |> print_endline
- else
- Utils_js.print_endlinef
- "Covered: %0.2f%% (%d of %d expressions)\n" percent covered total
- )
+ if debug then List.iter debug_range types;
+
+ begin if color then
+ let types = split_overlapping_ranges [] types |> List.rev in
+ let colors, _ = colorize_file content 0 [] types in
+ Tty.cprint (List.rev colors);
+ print_endline ""
+ end;
+
+ let covered, total = List.fold_left accum_coverage (0, 0) types in
+ let percent = if total = 0 then 100. else (float_of_int covered /. float_of_int total) *. 100. in
+
+ if json then
+ let uncovered_locs = types
+ |> List.filter (fun (_, is_covered) -> not is_covered)
+ |> List.map (fun (loc, _) -> loc)
+ in
+ let open Hh_json in
+ JSON_Object [
+ "expressions", JSON_Object [
+ "covered_count", int_ covered;
+ "uncovered_count", int_ (total - covered);
+ "uncovered_locs", JSON_Array (uncovered_locs |> List.map Reason.json_of_loc);
+ ];
+ ]
+ |> json_to_string ~pretty
+ |> print_endline
+ else
+ Utils_js.print_endlinef
+ "Covered: %0.2f%% (%d of %d expressions)\n" percent covered total
 
 let main
  option_values json pretty root from color debug path respect_pragma
@@ -223,7 +219,7 @@ let main
  | None -> File_input.path_of_file_input file
  ) in
 
- if not json && all && respect_pragma then prerr_endline
+ if not option_values.quiet && all && respect_pragma then prerr_endline
  "Warning: --all and --respect-pragma cannot be used together. --all wins.";
 
  (* TODO: --respect-pragma is deprecated. We will soon flip the default. As a
@@ -236,6 +232,11 @@ let main
  (* pretty implies json *)
  let json = json || pretty in
 
+ if color && json
+ then raise (CommandSpec.Failed_to_parse ("--color", "Can't be used with json flags"));
+ if debug && json
+ then raise (CommandSpec.Failed_to_parse ("--debug", "Can't be used with json flags"));
+
  let request = ServerProt.COVERAGE (file, all) in
  let response: ServerProt.coverage_response =
  connect_and_make_request option_values root request in

tests/coverage/coverage.exp

@@ -42,6 +42,5 @@ Covered: 75.00% (3 of 4 expressions)
 
 Covered: 0.00% (0 of 4 expressions)
 
-Warning: --all and --respect-pragma cannot be used together. --all wins.
 Covered: 75.00% (3 of 4 expressions)
 

tests/coverage/test.sh

@@ -21,4 +21,4 @@ assert_ok "$FLOW" coverage --all no_pragma.js
 assert_ok "$FLOW" coverage --respect-pragma no_pragma.js
 
 # --all wins (and assumes @flow weak)
-assert_ok "$FLOW" coverage --respect-pragma --all no_pragma.js 2>&1
+assert_ok "$FLOW" coverage --respect-pragma --all no_pragma.js