flow coverage non-termination fix

Summary:
Some bug in our handling of locations in type tables violates an implicit
condition on emitted ranges: pairwise, they should either be nested or disjoint.

This causes the coverage implementation, which relies on that assumption to
split ranges, not terminate on certain inputs.

This diff doesn't fix the actual bug, but instead makes the coverage
implementation more rubust. It also adds a case to the type normalizer to make
declare modules not appear uncovered.

Fixes #2366
Fixes #1962

Reviewed By: gabelevi

Differential Revision: D3796024

fbshipit-source-id: 7e87b10bb4c57ecfa20255ff2e2509406c5db1be

src/commands/coverageCommand.ml

@@ -132,6 +132,35 @@ let rec split_overlapping_ranges accum = Loc.(function
           (head_loc, is_covered1)::accum,
           (loc2, is_covered2)::rest
 
+        else if loc1._end.offset < loc2._end.offset then
+          (* TODO: Given that at this point we also have loc1.start.offset <
+             loc2.start.offset, it means that range 1 and 2 overlap but don't
+             nest. Ideally, this case should never arise: we should be able to
+             guarantee the invariant that ranges (same as "spans" in
+             common/span.ml) are either disjoint or nest. However, some
+             combination of bugs and incompleteness in statement.ml and
+             parser_flow.ml cause this invariant to be violated. So here we are.
+
+             Split range1, range2, and the overlapping part. Consume the first
+             part of range1, which doesn't overlap. Also consume the overlapping
+             part, which we assume to be small enough (usually, 1 token) to not
+             contain any interesting nested stuff (recall that the overlap is a
+             bug, not a feature), and optimistically consider it covered if
+             range1 or range2 is covered (because the alternative is 1-token
+             islands of uncovered stuff).
+          *)
+          let head_loc = { loc1 with
+            _end = { loc1._end with offset = loc2.start.offset - 1 }
+          } in
+          let overlap_loc = { loc1 with
+            start = loc2.start
+          } in
+          let tail_loc = { loc2 with
+            start = { loc2.start with offset = loc1._end.offset + 1 }
+          } in
+          (head_loc, is_covered1)::(overlap_loc, is_covered1 || is_covered2)::accum,
+          (tail_loc, is_covered2)::rest
+
         else
           (* range 2 is in the middle of range 1, so split range 1 and consume
              the first part, which doesn't overlap, and then recurse on

src/typing/type_normalizer.ml

@@ -371,9 +371,20 @@ let rec normalize_type_impl cx ids t = match t with
   | OpenPredT (_, t, _, _) ->
       normalize_type_impl cx ids t
 
+  | ModuleT (_, exporttypes) ->
+    let reason = reason_of_string "module" in
+    let exports_tmap =
+      Context.find_props cx exporttypes.exports_tmap
+      |> SMap.map (normalize_type_impl cx ids)
+      |> Context.make_property_map cx
+    in
+    let cjs_export = match exporttypes.cjs_export with
+      | None -> None
+      | Some t -> Some (normalize_type_impl cx ids t) in
+    ModuleT (reason, { exporttypes with exports_tmap; cjs_export; })
+
   | FunProtoT _
   | ExistsT _
-  | ModuleT (_, _)
   | ExtendsT (_, _, _)
   ->
     (** TODO **)

tests/coverage/.flowconfig

@@ -0,0 +1,7 @@
+[ignore]
+
+[include]
+
+[libs]
+
+[options]

tests/coverage/.testconfig

@@ -0,0 +1 @@
+shell: test.sh

tests/coverage/coverage.exp

@@ -0,0 +1,38 @@
+// check coverage of declare module
+
+declare module foo {
+}
+
+Covered: 100.00% (1 of 1 expressions)
+
+// This file triggers a violation of the "disjoint-or-nested ranges invariant"
+// that we implicitly assume in type-at-pos and coverage implementations. In
+// particular, when unchecked it causes a crash with coverage --color.
+
+declare module foo {
+}
+
+declare module bar {
+}
+
+// TODO
+
+Covered: 100.00% (2 of 2 expressions)
+
+// This file triggers a violation of the "disjoint-or-nested ranges invariant"
+// that we implicitly assume in type-at-pos and coverage implementations. In
+// particular, when unchecked it causes non-termination with coverage --color.
+
+declare module foo {
+}
+
+declare module bar {
+}
+
+// TODO
+
+declare class qux {
+}
+
+Covered: 100.00% (3 of 3 expressions)
+

tests/coverage/crash.js

@@ -0,0 +1,11 @@
+// This file triggers a violation of the "disjoint-or-nested ranges invariant"
+// that we implicitly assume in type-at-pos and coverage implementations. In
+// particular, when unchecked it causes a crash with coverage --color.
+
+declare module foo {
+}
+
+declare module bar {
+}
+
+// TODO

tests/coverage/declare_module.js

@@ -0,0 +1,4 @@
+// check coverage of declare module
+
+declare module foo {
+}

tests/coverage/non-termination.js

@@ -0,0 +1,14 @@
+// This file triggers a violation of the "disjoint-or-nested ranges invariant"
+// that we implicitly assume in type-at-pos and coverage implementations. In
+// particular, when unchecked it causes non-termination with coverage --color.
+
+declare module foo {
+}
+
+declare module bar {
+}
+
+// TODO
+
+declare class qux {
+}

tests/coverage/test.sh

@@ -0,0 +1,12 @@
+#!/bin/sh
+
+FLOW=$1
+
+# coverage of declare module
+$FLOW coverage --color declare_module.js
+
+# should not crash
+$FLOW coverage --color crash.js
+
+# should terminate
+$FLOW coverage --color non-termination.js