simplify array spread handling

Summary:
When evaluating spreads inside an array literal, we would "finish" the array type with a different element type for each type that was being spread. This effectively created a "union of array types" instead of a single array type, which could be fanned into an exponential blowup, or worse, an infinite loop, by a suitably crafted cycle in the constraint graph.

Now we pin the element type early and thread it through so that we can always "finish" with that element type.

Fixes #4070
Fixes #4370

Reviewed By: samwgoldman

Differential Revision: D7416763

fbshipit-source-id: 5d6c6e020311bef4fd90d95c96abc83325fea80a

src/common/reason.ml

@@ -1025,6 +1025,13 @@ let rec mk_expression_reason = Ast.Expression.(function
 | (loc, _) as x -> mk_reason (RCode (code_desc_of_expression ~wrap:false x)) loc
 )
 
+(* TODO: replace RCustom descriptions with proper descriptions *)
+let unknown_elem_empty_array_desc = RCustom "unknown element type of empty array"
+let inferred_union_elem_array_desc = RCustom
+  "inferred union of array element types \
+   (alternatively, provide an annotation to summarize the array \
+   element type)"
+
 (* Classifies a reason description. These classifications can be used to
  * implement various asthetic behaviors in error messages when we would like to
  * distinguish between different error "classes".

src/common/reason.mli

@@ -237,3 +237,6 @@ val do_patch: string list -> (int * int * string) list -> string
 module ReasonMap : MyMap.S with type key = reason
 
 val mk_expression_reason: Loc.t Ast.Expression.t -> reason
+
+val unknown_elem_empty_array_desc: reason_desc
+val inferred_union_elem_array_desc: reason_desc

src/typing/debug_js.ml

@@ -860,16 +860,11 @@ and _json_of_use_t_impl json_cx t = Hh_json.(
 and json_of_resolve_to json_cx = check_depth json_of_resolve_to_impl json_cx
 and json_of_resolve_to_impl json_cx resolve_to = Hh_json.(JSON_Object (
   match resolve_to with
-  | ResolveSpreadsToTuple (id, tout) -> [
-    "id", JSON_Number (string_of_int id);
-    "t_out", _json_of_t json_cx tout;
-  ]
-  | ResolveSpreadsToArrayLiteral (id, tout) -> [
-    "id", JSON_Number (string_of_int id);
-    "t_out", _json_of_t json_cx tout;
-  ]
-  | ResolveSpreadsToArray (id, tout) -> [
+  | ResolveSpreadsToTuple (id, elem_t, tout)
+  | ResolveSpreadsToArrayLiteral (id, elem_t, tout)
+  | ResolveSpreadsToArray (id, elem_t, tout) -> [
     "id", JSON_Number (string_of_int id);
+    "elem_t", _json_of_t json_cx elem_t;
     "t_out", _json_of_t json_cx tout;
   ]
   | ResolveSpreadsToMultiflowCallFull (id, ft)
@@ -2108,9 +2103,10 @@ and dump_use_t_ (depth, tvars) cx t =
       ~extra:(spf "use_desc=%b, %s" use_desc (use_kid (UseT (use_op, arg))))
   | ResolveSpreadT (use_op, _, {rrt_resolve_to; _;}) ->
       (match rrt_resolve_to with
-      | ResolveSpreadsToTuple (_, tout)
-      | ResolveSpreadsToArrayLiteral (_, tout)
-      | ResolveSpreadsToArray (_, tout)
+      | ResolveSpreadsToTuple (_, elem_t, tout)
+      | ResolveSpreadsToArrayLiteral (_, elem_t, tout)
+      | ResolveSpreadsToArray (_, elem_t, tout) ->
+        p ~extra:(spf "%s, %s, %s" (string_of_use_op use_op) (kid elem_t) (kid tout)) t
       | ResolveSpreadsToMultiflowPartial (_, _, _, tout) ->
         p ~extra:(spf "%s, %s" (string_of_use_op use_op) (kid tout)) t
       | ResolveSpreadsToCallT (_, tin) ->

src/typing/flow_js.ml

@@ -3185,8 +3185,8 @@ let rec __flow cx ((l: Type.t), (u: Type.use_t)) trace =
       (* Any ResolveSpreadsTo* which does some sort of constant folding needs to
        * carry an id around to break the infinite recursion that constant
        * constant folding can trigger *)
-      | ResolveSpreadsToTuple (id, tout)
-      | ResolveSpreadsToArrayLiteral (id, tout) ->
+      | ResolveSpreadsToTuple (id, elem_t, tout)
+      | ResolveSpreadsToArrayLiteral (id, elem_t, tout) ->
         (* You might come across code like
          *
          * for (let x = 1; x < 3; x++) { foo = [...foo, x]; }
@@ -3225,14 +3225,14 @@ let rec __flow cx ((l: Type.t), (u: Type.use_t)) trace =
               rrt_unresolved;
               (* We need a deterministic way to generate a new id. This is fine - not many ids are
                * live at once and a collision is super duper unlikely. *)
-              rrt_resolve_to = ResolveSpreadsToArray (id + 50000, tout);
+              rrt_resolve_to = ResolveSpreadsToArray (id + 50000, elem_t, tout);
             }))
           | _ ->
             (* We've already deconstructed, so there's nothing left to do *)
             ()
         )
 
-      | ResolveSpreadsToArray (id, _) ->
+      | ResolveSpreadsToArray (id, _, _) ->
         let reason_elemt = reason_of_t elemt in
         ConstFoldExpansion.guard id reason_elemt (fun recursion_depth ->
           match recursion_depth with
@@ -9825,8 +9825,11 @@ and multiflow_partial =
         (RRestArray (desc_of_reason reason_op)) reason_op in
 
       let arg_array = Tvar.mk_where cx arg_array_reason (fun tout ->
-        let resolve_to = (ResolveSpreadsToArrayLiteral (mk_id (), tout)) in
-        resolve_spread_list cx ~use_op ~reason_op:arg_array_reason elems resolve_to
+        let reason_op = arg_array_reason in
+        let element_reason = replace_reason_const Reason.inferred_union_elem_array_desc reason_op in
+        let elem_t = Tvar.mk cx element_reason in
+        let resolve_to = (ResolveSpreadsToArrayLiteral (mk_id (), elem_t, tout)) in
+        resolve_spread_list cx ~use_op ~reason_op elems resolve_to
       ) in
       let () =
         let use_op = Frame (FunRestParam {
@@ -9906,7 +9909,7 @@ and finish_resolve_spread_list =
 
   in
 
-  let finish_array cx ?trace ~reason_op ~resolve_to resolved tout =
+  let finish_array cx ?trace ~reason_op ~resolve_to resolved elemt tout =
     (* Did `any` flow to one of the rest parameters? If so, we need to resolve
      * to a type that is both a subtype and supertype of the desired type. *)
     let result = if spread_resolved_to_any resolved
@@ -9952,44 +9955,33 @@ and finish_resolve_spread_list =
       ) TypeExSet.empty elems in
 
       (* composite elem type is an upper bound of all element types *)
-      let elemt =
-        let element_reason =
-          let desc = RCustom (
-            "inferred union of array element types \
-             (alternatively, provide an annotation to summarize the array \
-               element type)") in
-          replace_reason_const desc reason_op
-        in
-        (* Should the element type of the array be the union of its element
-           types?
-
-           No. Instead of using a union, we use an unresolved tvar to
-           represent the least upper bound of each element type. Effectively,
-           this keeps the element type "open," at least locally.[*]
-
-           Using a union pins down the element type prematurely, and moreover,
-           might lead to speculative matching when setting elements or caling
-           contravariant methods (`push`, `concat`, etc.) on the array.
-
-           In any case, using a union doesn't quite work as intended today
-           when the element types themselves could be unresolved tvars. For
-           example, the following code would work even with unions:
-
-           declare var o: { x: number; }
-           var a = ["hey", o.x]; // no error, but is an error if 42 replaces o.x
-           declare var i: number;
-           a[i] = false;
-
-           [*] Eventually, the element type does get pinned down to a union
-           when it is part of the module's exports. In the future we might
-           have to do that pinning more carefully, and using an unresolved
-           tvar instead of a union here doesn't conflict with those plans.
-        *)
-        Tvar.mk_where cx element_reason (fun tvar ->
-          TypeExSet.elements tset |> List.iter (fun t ->
-            flow cx (t, UseT (unknown_use, tvar)))
-        )
-      in
+      (* Should the element type of the array be the union of its element types?
+
+         No. Instead of using a union, we use an unresolved tvar to
+         represent the least upper bound of each element type. Effectively,
+         this keeps the element type "open," at least locally.[*]
+
+         Using a union pins down the element type prematurely, and moreover,
+         might lead to speculative matching when setting elements or caling
+         contravariant methods (`push`, `concat`, etc.) on the array.
+
+         In any case, using a union doesn't quite work as intended today
+         when the element types themselves could be unresolved tvars. For
+         example, the following code would work even with unions:
+
+         declare var o: { x: number; }
+         var a = ["hey", o.x]; // no error, but is an error if 42 replaces o.x
+         declare var i: number;
+         a[i] = false;
+
+         [*] Eventually, the element type does get pinned down to a union
+         when it is part of the module's exports. In the future we might
+         have to do that pinning more carefully, and using an unresolved
+         tvar instead of a union here doesn't conflict with those plans.
+      *)
+      TypeExSet.elements tset |> List.iter (fun t ->
+        flow cx (t, UseT (unknown_use, elemt)));
+
       match tuple_types, resolve_to with
       | _, `Array ->
           DefT (reason_op, ArrT (ArrayAT (elemt, None)))
@@ -10144,12 +10136,12 @@ and finish_resolve_spread_list =
   in
   fun cx ?trace ~use_op ~reason_op resolved resolve_to -> (
     match resolve_to with
-    | ResolveSpreadsToTuple (_, tout)->
-      finish_array cx ?trace ~reason_op ~resolve_to:`Tuple resolved tout
-    | ResolveSpreadsToArrayLiteral (_, tout) ->
-      finish_array cx ?trace ~reason_op ~resolve_to:`Literal resolved tout
-    | ResolveSpreadsToArray (_, tout) ->
-      finish_array cx ?trace ~reason_op ~resolve_to:`Array resolved tout
+    | ResolveSpreadsToTuple (_, elem_t, tout)->
+      finish_array cx ?trace ~reason_op ~resolve_to:`Tuple resolved elem_t tout
+    | ResolveSpreadsToArrayLiteral (_, elem_t, tout) ->
+      finish_array cx ?trace ~reason_op ~resolve_to:`Literal resolved elem_t tout
+    | ResolveSpreadsToArray (_, elem_t, tout) ->
+      finish_array cx ?trace ~reason_op ~resolve_to:`Array resolved elem_t tout
     | ResolveSpreadsToMultiflowPartial (_, ft, call_reason, tout) ->
       finish_multiflow_partial cx ?trace ~use_op ~reason_op ft call_reason resolved tout
     | ResolveSpreadsToMultiflowCallFull (_, ft) ->

src/typing/statement.ml

@@ -2638,18 +2638,18 @@ and expression_ ~is_cond cx loc e = let ex = (loc, e) in Ast.Expression.(match e
     match elements with
     | [] ->
         (* empty array, analogous to object with implicit properties *)
-        let element_reason =
-          let desc = RCustom "unknown element type of empty array" in
-          mk_reason desc loc
-        in
+        let element_reason = mk_reason Reason.unknown_elem_empty_array_desc loc in
         let elemt = Tvar.mk cx element_reason in
         let reason = replace_reason_const REmptyArrayLit reason in
         DefT (reason, ArrT (ArrayAT (elemt, Some [])))
     | elems ->
         let elem_spread_list = expression_or_spread_list cx loc elems in
         Tvar.mk_where cx reason (fun tout ->
-          let resolve_to = (ResolveSpreadsToArrayLiteral (mk_id (), tout)) in
           let reason_op = reason in
+          let element_reason = replace_reason_const Reason.inferred_union_elem_array_desc reason_op in
+          let elem_t = Tvar.mk cx element_reason in
+          let resolve_to = (ResolveSpreadsToArrayLiteral (mk_id (), elem_t, tout)) in
+
           Flow.resolve_spread_list cx ~use_op:unknown_use ~reason_op elem_spread_list resolve_to
         )
     )
@@ -3981,12 +3981,15 @@ and jsx_mk_props cx reason c name attributes children = Ast.JSX.(
     | _ when is_react -> map
     | _ ->
         let arr = Tvar.mk_where cx reason (fun tout ->
+          let reason_op = reason in
+          let element_reason = replace_reason_const Reason.inferred_union_elem_array_desc reason_op in
+          let elem_t = Tvar.mk cx element_reason in
           Flow.resolve_spread_list
             cx
             ~use_op:unknown_use
             ~reason_op:reason
             children
-            (ResolveSpreadsToArrayLiteral (mk_id (), tout))
+            (ResolveSpreadsToArrayLiteral (mk_id (), elem_t, tout))
         ) in
         let p = Field (None, arr, Neutral) in
         SMap.add "children" p map

src/typing/type.ml

@@ -1067,13 +1067,11 @@ module rec TypeTerm : sig
 
   and spread_resolve =
   (* Once we've finished resolving spreads, try to construct a tuple *)
-  | ResolveSpreadsToTuple of int * t
-  (* Once we've finished resolving spreads, try to construct an array with
-   * known element types *)
-  | ResolveSpreadsToArrayLiteral of int * t
-  (* Once we've finished resolving spreads, try to construct a non-tuple array
-   *)
-  | ResolveSpreadsToArray of int * t
+  | ResolveSpreadsToTuple of int * t * t (* elem type, array type *)
+  (* Once we've finished resolving spreads, try to construct an array with known element types *)
+  | ResolveSpreadsToArrayLiteral of int * t * t (* elem type, array type *)
+  (* Once we've finished resolving spreads, try to construct a non-tuple array *)
+  | ResolveSpreadsToArray of int * t * t (* elem type, array type *)
 
   (* Once we've finished resolving spreads for a function's arguments, call the
    * function with those arguments *)

src/typing/type_mapper.ml

@@ -1308,18 +1308,21 @@ class ['a] t = object(self)
 
   method spread_resolve cx map_cx t =
     match t with
-    | ResolveSpreadsToTuple (i, t') ->
-        let t'' = self#type_ cx map_cx t' in
-        if t'' == t' then t
-        else ResolveSpreadsToTuple (i, t'')
-    | ResolveSpreadsToArrayLiteral (i, t') ->
-        let t'' = self#type_ cx map_cx t' in
-        if t'' == t' then t
-        else ResolveSpreadsToArrayLiteral (i, t'')
-    | ResolveSpreadsToArray (i, t') ->
-        let t'' = self#type_ cx map_cx t' in
-        if t'' == t' then t
-        else ResolveSpreadsToArray (i, t'')
+    | ResolveSpreadsToTuple (i, t1', t2') ->
+        let t1'' = self#type_ cx map_cx t1' in
+        let t2'' = self#type_ cx map_cx t2' in
+        if t1'' == t1' && t2'' == t2' then t
+        else ResolveSpreadsToTuple (i, t1'', t2'')
+    | ResolveSpreadsToArrayLiteral (i, t1', t2') ->
+        let t1'' = self#type_ cx map_cx t1' in
+        let t2'' = self#type_ cx map_cx t2' in
+        if t1'' == t1' && t2'' == t2' then t
+        else ResolveSpreadsToArrayLiteral (i, t1'', t2'')
+    | ResolveSpreadsToArray (i, t1', t2') ->
+        let t1'' = self#type_ cx map_cx t1' in
+        let t2'' = self#type_ cx map_cx t2' in
+        if t1'' == t1' && t2'' == t2' then t
+        else ResolveSpreadsToArray (i, t1'', t2'')
     | ResolveSpreadsToMultiflowCallFull (i, funtype) ->
         let funtype' = self#fun_type cx map_cx funtype in
         if funtype' == funtype then t

src/typing/type_visitor.ml

@@ -577,10 +577,13 @@ class ['a] t = object(self)
         self#type_ cx pole_TODO acc t
     ) acc rrt_unresolved in
     let acc = match rrt_resolve_to with
-      | ResolveSpreadsToTuple (_, t)
-      | ResolveSpreadsToArrayLiteral (_, t)
-      | ResolveSpreadsToArray (_, t)
-        -> self#type_ cx pole_TODO acc t
+      | ResolveSpreadsToTuple (_, t1, t2)
+      | ResolveSpreadsToArray (_, t1, t2)
+      | ResolveSpreadsToArrayLiteral (_, t1, t2)
+        ->
+        let acc = self#type_ cx pole_TODO acc t1 in
+        let acc = self#type_ cx pole_TODO acc t2 in
+        acc
       | ResolveSpreadsToMultiflowCallFull (_, fn)
       | ResolveSpreadsToMultiflowSubtypeFull (_, fn)
         -> self#fun_type cx pole_TODO acc fn

tests/rec/array_spread.js

@@ -0,0 +1,18 @@
+// @flow
+
+function foo(xs: Array<any>) {
+  const zs = [];
+  xs
+    .map(
+      x => [],
+    )
+    .map(ys =>
+      ys
+      .map(y => [])
+      .reduce((a, b) => [...a, ...b], []),
+    )
+    .reduce((a, b) => [...a, ...b], [])
+    .forEach(z => {
+        zs.push(z);
+    });
+}

tests/rec/issue-4070.js

@@ -0,0 +1,9 @@
+// @flow
+
+const ys = new Map();
+const y = ys.get('a');
+
+ys.set('a', [...y]);
+ys.set('a', [...y]);
+ys.set('a', [...y]);
+ys.set('a', [...y]);

tests/rec/issue-4370.js

@@ -0,0 +1,16 @@
+// @flow
+export const checkComponent = (obj: any[]): Object[] =>
+  obj.reduce((acc, x) => {
+    if (x === undefined) {
+      return [...acc, {}];
+    }
+
+    if (x === 'hi') {
+      return [...acc, {}];
+    }
+
+    if (x.err) {
+      return [...acc, {}];
+    }
+    return acc;
+  }, []);

tests/rec/issue6155.js

@@ -0,0 +1,22 @@
+// @noflow
+
+type A = {kind: 'a', e: Type};
+type B = {kind: 'b', k: Type, v: Type};
+type C = {kind: 'c'};
+type Type = A | B | C;
+
+type TypeCases<R> = {|
+    a: (A) => R,
+    b: (B) => R,
+    c: (C) => R
+|};
+
+function matcher<R>(cases: TypeCases<R>): (Type) => R {
+    return (type) => cases[type.kind](type);
+}
+
+const f: Type => Array<string> = matcher({
+    a: (a: A) => [...f(a.e)],
+    b: (b: B) => [...f(b.k), ...f(b.v)],
+    c: () =>  ['']
+});