From 215ac06533dbaa448c9f06fa4390a8e619cb0077 Mon Sep 17 00:00:00 2001 From: Daniel Sainati Date: Wed, 9 Dec 2020 16:09:40 -0800 Subject: [PATCH] when promoting primitives on method calls, also promote the method call'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 | 34 +++++++++++++++++-- src/typing/type.ml | 4 +++ .../annotate_exports_empty_array.exp | 6 ++-- tests/arrays/arrays.exp | 16 ++++++++- tests/arrays/map.js | 10 ++++++ .../autofix_empty_array.exp | 2 +- 6 files changed, 64 insertions(+), 8 deletions(-) create mode 100644 tests/arrays/map.js diff --git a/src/typing/flow_js.ml b/src/typing/flow_js.ml index 9c70575862d..1de6efc35a8 100644 --- a/src/typing/flow_js.ml +++ b/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) (*****************************************************) diff --git a/src/typing/type.ml b/src/typing/type.ml index 014e24fca89..79829bf7842 100644 --- a/src/typing/type.ml +++ b/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) -> diff --git a/tests/annotate_exports_empty_array/annotate_exports_empty_array.exp b/tests/annotate_exports_empty_array/annotate_exports_empty_array.exp index 6341c3e8305..88dcf8553cd 100644 --- a/tests/annotate_exports_empty_array/annotate_exports_empty_array.exp +++ b/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 = []; a1.push(1); const a2: Array = []; @@ -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 = []; --- > const a1 = []; 8c8 diff --git a/tests/arrays/arrays.exp b/tests/arrays/arrays.exp index 8b45a2c43da..2ec7d3c7c39 100644 --- a/tests/arrays/arrays.exp +++ b/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, + ^^^^^^ [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 diff --git a/tests/arrays/map.js b/tests/arrays/map.js new file mode 100644 index 00000000000..f891dc6b62e --- /dev/null +++ b/tests/arrays/map.js @@ -0,0 +1,10 @@ +// @flow + +declare var x: ?{ + a: $ReadOnlyArray, +}; + +(x ? x.a : []).map(n => { + if (n !== '') { // number incompatible with string + } +}); diff --git a/tests/autofix_empty_array/autofix_empty_array.exp b/tests/autofix_empty_array/autofix_empty_array.exp index 7bd35f9be1a..2af7650a942 100644 --- a/tests/autofix_empty_array/autofix_empty_array.exp +++ b/tests/autofix_empty_array/autofix_empty_array.exp @@ -2,7 +2,7 @@ const a0: Array = []; -const a1: Array = []; +const a1: Array = []; a1.push(1); const a2: Array = [];