Skip to content

Commit

Permalink
Type Object.values and Object.entries values when called with a d…
Browse files Browse the repository at this point in the history
…ictionary (mixed otherwise)

Summary:
User request from Dec 2021, May 2021, Dec 2020, Apr 2020

For `Object.entries`, keys is typed exactly like it is for `Object.keys`

For both, values is typed as such:
- Dictionaries: the value of the dictionary
- Objects & Instances: `mixed` - this avoids created non-performant union types

Error diff analysis:
- Previously, `Object.entries` returned `string` for the key type, rather than having the same behavior as `Object.keys`. We unsafely allow property read/writes with `string`.
- Error code for suppressions has changed in some places from `incompatible-call` to `incompatible-use`
- Many errors have simply moved (previously error on usage of `mixed`, now erroring on still invalid but more accurate type)

After WWW diff D41527152, errors are only around ~500.

Alternative 1 was to create a new utility type `$DictValues` and type the `Object.values` and `Object.entries` lib defs with this. Cons of this approach are introducing a new lib def, and the fact that `Object.entries` keys definition would differ from `Object.keys`, since `$Keys` and `Object.keys` are implemented differently.

Changelog: [errors] Instead of `mixed`, type the result of `Object.values` and `Object.entries` on a dictionary to be the dictionary values, and `Object.entries` keys to behave like `Object.keys`

Closes #2174, #2221, #4771, #4997, #5838

Reviewed By: jbrown215

Differential Revision: D35710131

fbshipit-source-id: 3163bc02dfc7cb70a5d6c0ba7da574a793672421
  • Loading branch information
gkz authored and facebook-github-bot committed Dec 5, 2022
1 parent 297c286 commit b9aae51
Show file tree
Hide file tree
Showing 11 changed files with 475 additions and 104 deletions.
1 change: 1 addition & 0 deletions src/typing/debug_js.ml
Original file line number Diff line number Diff line change
Expand Up @@ -775,6 +775,7 @@ and dump_use_t_ (depth, tvars) cx t =
p ~extra:(spf "%s, (%s, %s)" (kid ix) (string_of_reason preason) (tvar ptvar)) t
| GetKeysT _ -> p t
| GetValuesT _ -> p t
| GetDictValuesT _ -> p t
| MatchPropT (use_op, _, prop, (preason, ptvar))
| GetPropT (use_op, _, _, prop, (preason, ptvar)) ->
p
Expand Down
1 change: 1 addition & 0 deletions src/typing/default_resolve.ml
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,7 @@ let rec default_resolve_touts ~flow cx loc u =
| ArrRestT (_, _, _, t) ->
resolve t
| BecomeT { t; _ } -> resolve t
| GetDictValuesT (_, use) -> default_resolve_touts ~flow cx loc use
| GetKeysT (_, use) -> default_resolve_touts ~flow cx loc use
| HasOwnPropT _ -> ()
| GetValuesT (_, t) -> resolve t
Expand Down
23 changes: 23 additions & 0 deletions src/typing/flow_js.ml
Original file line number Diff line number Diff line change
Expand Up @@ -1565,6 +1565,28 @@ struct
(* Any will always be ok *)
| (AnyT (_, src), GetValuesT (reason, values)) ->
rec_flow_t ~use_op:unknown_use cx trace (AnyT.why src reason, values)
(***********************************************)
(* Values of a dictionary - `mixed` otherwise. *)
(***********************************************)
| ( DefT
( _,
_,
ObjT
{ flags = { obj_kind = Indexed { value; dict_polarity; _ }; _ }; props_tmap; _ }
),
GetDictValuesT (_, result)
)
when Context.find_props cx props_tmap |> NameUtils.Map.is_empty
&& Polarity.compat (dict_polarity, Polarity.Positive) ->
rec_flow cx trace (value, result)
(* Temporarily allow Arrays, to split up error diff. *)
| (DefT (_, _, ArrT _), GetDictValuesT (reason, result))
| (DefT (_, _, ObjT _), GetDictValuesT (reason, result))
| (DefT (_, _, InstanceT _), GetDictValuesT (reason, result)) ->
rec_flow cx trace (MixedT.why reason (bogus_trust ()), result)
(* Any will always be ok *)
| (AnyT (_, src), GetDictValuesT (reason, result)) ->
rec_flow cx trace (AnyT.why src reason, result)
(*******************************************)
(* Refinement based on function predicates *)
(*******************************************)
Expand Down Expand Up @@ -6164,6 +6186,7 @@ struct
| GetProtoT _
| GetStaticsT _
| GetValuesT _
| GetDictValuesT _
| GuardT _
| FilterOptionalT _
| FilterMaybeT _
Expand Down
2 changes: 2 additions & 0 deletions src/typing/flow_js_utils.ml
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,7 @@ let object_like_op = function
| GetKeysT _
| HasOwnPropT _
| GetValuesT _
| GetDictValuesT _
| ObjAssignToT _
| ObjAssignFromT _
| ObjRestT _
Expand Down Expand Up @@ -411,6 +412,7 @@ let error_message_kind_of_upper = function
Error_message.IncompatibleHasOwnPropT (aloc_of_reason r, Some name)
| HasOwnPropT (_, r, _) -> Error_message.IncompatibleHasOwnPropT (aloc_of_reason r, None)
| GetValuesT _ -> Error_message.IncompatibleGetValuesT
| GetDictValuesT _ -> Error_message.IncompatibleGetValuesT
| UnaryArithT _ -> Error_message.IncompatibleUnaryArithT
| MapTypeT (_, _, (ObjectMap _ | ObjectMapi _ | ObjectMapConst _ | ObjectKeyMirror), _) ->
Error_message.IncompatibleMapTypeTObject
Expand Down
1 change: 1 addition & 0 deletions src/typing/implicit_instantiation.ml
Original file line number Diff line number Diff line change
Expand Up @@ -257,6 +257,7 @@ struct
| OptionalIndexedAccessT _
| GetKeysT _
| GetValuesT _
| GetDictValuesT _
(* Import-export related upper bounds won't appear during implicit instantiation. *)
| CJSRequireT _
| ImportModuleNsT _
Expand Down
59 changes: 42 additions & 17 deletions src/typing/statement.ml
Original file line number Diff line number Diff line change
Expand Up @@ -5785,6 +5785,26 @@ module Make
}
)
in
let get_keys ~arr_reason obj_t =
Tvar.mk_where cx arr_reason (fun tvar ->
let keys_reason =
update_desc_reason
(fun desc -> RCustom (spf "element of %s" (string_of_desc desc)))
reason
in
Flow.flow cx (obj_t, GetKeysT (keys_reason, UseT (use_op, tvar)))
)
in
let get_values ~arr_reason obj_t =
Tvar.mk_where cx arr_reason (fun tvar ->
let values_reason =
update_desc_reason
(fun desc -> RCustom (spf "element of %s" (string_of_desc desc)))
reason
in
Flow.flow cx (obj_t, GetDictValuesT (values_reason, UseT (use_op, tvar)))
)
in
match (m, targs, args) with
| ("create", None, (args_loc, { ArgList.arguments = [Expression e]; comments })) ->
let (((_, e_t), _) as e_ast) = expression cx e in
Expand Down Expand Up @@ -5871,23 +5891,28 @@ module Make
) ->
let arr_reason = mk_reason RArrayType loc in
let (((_, o), _) as e_ast) = expression cx e in
( DefT
( arr_reason,
bogus_trust (),
ArrT
(ArrayAT
( Tvar.mk_where cx arr_reason (fun tvar ->
let keys_reason =
update_desc_reason
(fun desc -> RCustom (spf "element of %s" (string_of_desc desc)))
reason
in
Flow.flow cx (o, GetKeysT (keys_reason, UseT (use_op, tvar)))
),
None
)
)
),
let keys_t = get_keys ~arr_reason o in
( DefT (arr_reason, bogus_trust (), ArrT (ArrayAT (keys_t, None))),
None,
(args_loc, { ArgList.arguments = [Expression e_ast]; comments })
)
| ("values", None, (args_loc, { ArgList.arguments = [Expression e]; comments })) ->
let arr_reason = mk_reason RArrayType loc in
let (((_, o), _) as e_ast) = expression cx e in
( DefT (arr_reason, bogus_trust (), ArrT (ArrayAT (get_values ~arr_reason o, None))),
None,
(args_loc, { ArgList.arguments = [Expression e_ast]; comments })
)
| ("entries", None, (args_loc, { ArgList.arguments = [Expression e]; comments })) ->
let arr_reason = mk_reason RArrayType loc in
let (((_, o), _) as e_ast) = expression cx e in
let keys_t = get_keys ~arr_reason o in
let values_t = get_values ~arr_reason o in
let elem_t = UnionT (mk_reason RTupleElement loc, UnionRep.make keys_t values_t []) in
let entry_t =
DefT (mk_reason RTupleType loc, bogus_trust (), ArrT (TupleAT (elem_t, [keys_t; values_t])))
in
( DefT (arr_reason, bogus_trust (), ArrT (ArrayAT (entry_t, None))),
None,
(args_loc, { ArgList.arguments = [Expression e_ast]; comments })
)
Expand Down
3 changes: 3 additions & 0 deletions src/typing/type.ml
Original file line number Diff line number Diff line change
Expand Up @@ -665,6 +665,8 @@ module rec TypeTerm : sig
| HasOwnPropT of use_op * reason * t (* The incoming string that we want to check against *)
(* Values *)
| GetValuesT of reason * t
(* Values of a dictionary, `mixed` otherwise. *)
| GetDictValuesT of reason * use_t
(* Element access *)
| ElemT of use_op * reason * t * elem_action
(* exact ops *)
Expand Down Expand Up @@ -3686,6 +3688,7 @@ let string_of_use_ctor = function
| GetElemT _ -> "GetElemT"
| GetKeysT _ -> "GetKeysT"
| GetValuesT _ -> "GetValuesT"
| GetDictValuesT _ -> "GetDictValuesT"
| GetPropT _ -> "GetPropT"
| GetPrivatePropT _ -> "GetPrivatePropT"
| GetProtoT _ -> "GetProtoT"
Expand Down
3 changes: 3 additions & 0 deletions src/typing/typeUtil.ml
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,7 @@ and reason_of_use_t = function
| GetElemT (_, reason, _, _, _) -> reason
| GetKeysT (reason, _) -> reason
| GetValuesT (reason, _) -> reason
| GetDictValuesT (reason, _) -> reason
| GetPropT (_, reason, _, _, _) -> reason
| GetPrivatePropT (_, reason, _, _, _, _) -> reason
| GetProtoT (reason, _) -> reason
Expand Down Expand Up @@ -266,6 +267,7 @@ and mod_reason_of_use_t f = function
| GetElemT (use_op, reason, annot, it, et) -> GetElemT (use_op, f reason, annot, it, et)
| GetKeysT (reason, t) -> GetKeysT (f reason, t)
| GetValuesT (reason, t) -> GetValuesT (f reason, t)
| GetDictValuesT (reason, t) -> GetDictValuesT (f reason, t)
| GetPropT (use_op, reason, id, n, t) -> GetPropT (use_op, f reason, id, n, t)
| GetPrivatePropT (use_op, reason, name, bindings, static, t) ->
GetPrivatePropT (use_op, f reason, name, bindings, static, t)
Expand Down Expand Up @@ -418,6 +420,7 @@ let rec util_use_op_of_use_t :
| ArrRestT (op, r, i, t) -> util op (fun op -> ArrRestT (op, r, i, t))
| HasOwnPropT (op, r, t) -> util op (fun op -> HasOwnPropT (op, r, t))
| GetKeysT (r, u2) -> nested_util u2 (fun u2 -> GetKeysT (r, u2))
| GetDictValuesT (r, u2) -> nested_util u2 (fun u2 -> GetDictValuesT (r, u2))
| ElemT (op, r, t, a) -> util op (fun op -> ElemT (op, r, t, a))
| ObjKitT (op, r, x, y, t) -> util op (fun op -> ObjKitT (op, r, x, y, t))
| ReactKitT (op, r, t) -> util op (fun op -> ReactKitT (op, r, t))
Expand Down
Loading

0 comments on commit b9aae51

Please sign in to comment.