when promoting primitives on method calls, also promote the method ca…

…ll's this argument

Summary:
When calling methods on primitive types like arrays, number or strings that are present in their wrapper object, we promote the primitive to an instance of the wrapper class. We do not, however, similarly promote the `this` argument to the method call itself. This did not matter before `this` parameters were more strict than simply `any`, however, in a world where the `this` parameter of a method is the type of the class, we need to correctly promote the primitive `this` argument to an instance of its wrapper object as well.

This also has the nice side effect of improving type inference for array methods, which would infer a union with `any` as a result of the `any`-type of the method's call. By replacing this with the applied instance of the array wrapper class, we can produce a narrower type.

Reviewed By: mvitousek

Differential Revision: D24788676

fbshipit-source-id: 16b7495d3c7b259564c375f942983c3c1ae7d881

src/typing/flow_js.ml

@@ -7197,9 +7197,14 @@ struct
         (**********************)
         (* Array library call *)
         (**********************)
-        | ( DefT (reason, _, ArrT (ArrayAT (t, _))),
-            (GetPropT _ | SetPropT _ | MethodT _ | LookupT _) ) ->
+        | (DefT (reason, _, ArrT (ArrayAT (t, _))), (GetPropT _ | SetPropT _ | LookupT _)) ->
           rec_flow cx trace (get_builtin_typeapp cx ~trace reason "Array" [t], u)
+        | ( DefT (reason, _, ArrT (ArrayAT (t, _))),
+            MethodT (use_op, call_r, lookup_r, propref, action, t_opt) ) ->
+          let arr_t = get_builtin_typeapp cx ~trace reason "Array" [t] in
+          (* Substitute the typeapp for the array primitive in the method call's `this` position *)
+          let action = replace_this_t_in_method_action arr_t action in
+          rec_flow cx trace (arr_t, MethodT (use_op, call_r, lookup_r, propref, action, t_opt))
         (*************************)
         (* Tuple "length" access *)
         (*************************)
@@ -7212,27 +7217,50 @@ struct
           let t = tuple_length reason trust ts in
           rec_flow_t cx trace ~use_op:unknown_use (reposition cx ~trace loc t, OpenT tout)
         | ( DefT (reason, _, ArrT ((TupleAT _ | ROArrayAT _) as arrtype)),
-            (GetPropT _ | SetPropT _ | MethodT _ | LookupT _) ) ->
+            (GetPropT _ | SetPropT _ | LookupT _) ) ->
           let t = elemt_of_arrtype arrtype in
           rec_flow cx trace (get_builtin_typeapp cx ~trace reason "$ReadOnlyArray" [t], u)
+        | ( DefT (reason, _, ArrT ((TupleAT _ | ROArrayAT _) as arrtype)),
+            MethodT (use_op, call_r, lookup_r, propref, action, t_opt) ) ->
+          let t = elemt_of_arrtype arrtype in
+          let arr_t = get_builtin_typeapp cx ~trace reason "$ReadOnlyArray" [t] in
+          (* Substitute the typeapp for the array primitive in the method call's `this` position *)
+          let action = replace_this_t_in_method_action arr_t action in
+          rec_flow cx trace (arr_t, MethodT (use_op, call_r, lookup_r, propref, action, t_opt))
         (***********************)
         (* String library call *)
         (***********************)
+        | (DefT (reason, _, StrT _), MethodT (use_op, call_r, lookup_r, propref, action, t_opt)) ->
+          let promoted = get_builtin_type cx ~trace reason "String" in
+          let action = replace_this_t_in_method_action promoted action in
+          rec_flow cx trace (promoted, MethodT (use_op, call_r, lookup_r, propref, action, t_opt))
         | (DefT (reason, _, StrT _), u) when primitive_promoting_use_t u ->
           rec_flow cx trace (get_builtin_type cx ~trace reason "String", u)
         (***********************)
         (* Number library call *)
         (***********************)
+        | (DefT (reason, _, NumT _), MethodT (use_op, call_r, lookup_r, propref, action, t_opt)) ->
+          let promoted = get_builtin_type cx ~trace reason "Number" in
+          let action = replace_this_t_in_method_action promoted action in
+          rec_flow cx trace (promoted, MethodT (use_op, call_r, lookup_r, propref, action, t_opt))
         | (DefT (reason, _, NumT _), u) when primitive_promoting_use_t u ->
           rec_flow cx trace (get_builtin_type cx ~trace reason "Number", u)
         (***********************)
         (* Boolean library call *)
         (***********************)
+        | (DefT (reason, _, BoolT _), MethodT (use_op, call_r, lookup_r, propref, action, t_opt)) ->
+          let promoted = get_builtin_type cx ~trace reason "Boolean" in
+          let action = replace_this_t_in_method_action promoted action in
+          rec_flow cx trace (promoted, MethodT (use_op, call_r, lookup_r, propref, action, t_opt))
         | (DefT (reason, _, BoolT _), u) when primitive_promoting_use_t u ->
           rec_flow cx trace (get_builtin_type cx ~trace reason "Boolean", u)
         (***********************)
         (* Symbol library call *)
         (***********************)
+        | (DefT (reason, _, SymbolT), MethodT (use_op, call_r, lookup_r, propref, action, t_opt)) ->
+          let promoted = get_builtin_type cx ~trace reason "Symbol" in
+          let action = replace_this_t_in_method_action promoted action in
+          rec_flow cx trace (promoted, MethodT (use_op, call_r, lookup_r, propref, action, t_opt))
         | (DefT (reason, _, SymbolT), u) when primitive_promoting_use_t u ->
           rec_flow cx trace (get_builtin_type cx ~trace reason "Symbol", u)
         (*****************************************************)

src/typing/type.ml

@@ -2827,6 +2827,10 @@ let primitive_promoting_use_t = function
   (* TODO: enumerate all use types *)
   | _ -> false
 
+let replace_this_t_in_method_action call_this_t = function
+  | CallM fct -> CallM { fct with call_this_t }
+  | ChainM (r1, r2, t, fct, tout) -> ChainM (r1, r2, t, { fct with call_this_t }, tout)
+
 let rec fold_use_op f1 f2 = function
   | Op root -> f1 root
   | Frame (frame, use_op) ->

tests/annotate_exports_empty_array/annotate_exports_empty_array.exp

@@ -6,7 +6,7 @@
 
 const a0: Array<$FlowFixMeEmpty> = [];
 
-const a1: Array<$FlowFixMe | number> = [];
+const a1: Array<number> = [];
 a1.push(1);
 
 const a2: Array<number> = [];
@@ -22,7 +22,7 @@ module.exports = { a0, a1, a2, a3 };
 Stats:
   Files changed:                            1
   Number of annotations added:              3
-  Total size of annotations:               10
+  Total size of annotations:                8
   Number of sig. ver. errors:               4
   Number of annotations required:           4
   Number of annotations skipped:            0
@@ -56,7 +56,7 @@ Warnings:
 ---
 > const a0 = [];
 5c5
-< const a1: Array<$FlowFixMe | number> = [];
+< const a1: Array<number> = [];
 ---
 > const a1 = [];
 8c8

tests/arrays/arrays.exp

@@ -69,6 +69,20 @@ References:
                           ^^^^^^^^^^^^ [2]
 
 
+Error ------------------------------------------------------------------------------------------------------ map.js:8:13
+
+empty string [1] is incompatible with number [2]. [incompatible-type]
+
+   map.js:8:13
+   8|   if (n !== '') { // number incompatible with string
+                  ^^ [1]
+
+References:
+   map.js:4:21
+   4|   a: $ReadOnlyArray<number>,
+                          ^^^^^^ [2]
+
+
 Error ---------------------------------------------------------------------------------------------- numeric_elem.js:6:1
 
 Cannot assign `0` to `arr[day]` because `Date` [1] is not an array index. [incompatible-use]
@@ -113,4 +127,4 @@ References:
 
 
 
-Found 7 errors
+Found 8 errors

tests/arrays/map.js

@@ -0,0 +1,10 @@
+// @flow
+
+declare var x: ?{
+  a: $ReadOnlyArray<number>,
+};
+
+(x ? x.a : []).map(n => {
+  if (n !== '') { // number incompatible with string
+  }
+});

tests/autofix_empty_array/autofix_empty_array.exp

@@ -2,7 +2,7 @@
 
 const a0: Array<empty> = [];
 
-const a1: Array<any | number> = [];
+const a1: Array<number> = [];
 a1.push(1);
 
 const a2: Array<number> = [];