From b9db648afe4f2207a49985aa82b90a485a11a900 Mon Sep 17 00:00:00 2001 From: Sam Goldman Date: Mon, 11 Jun 2018 14:34:54 -0700 Subject: [PATCH] Store object call properties in internal slot Summary: Before this change, callable objects, declared classes, and interfaces all stored the callable signature in a specially-named property `$call`. It's perfectly legal to have a property with that name, however unlikely, so this representation has inherent badness. Furthermore, storing the callable signature in the propmap means that it interacts with prototype lookup, but at runtime a callable signature can not be inherited through the prototype chain. Consider the following program, which is an error at runtime, but which Flow accepts as valid. ``` function f() {} var o = Object.create(f); o(); // TypeError: o is not a function ``` Lastly, users have "figured out" this behavior and have started to actually define objects with an explicit `$call` property instead of using the intended syntax. Sometimes this capability is actually useful, specifically when the desired callable signature is not a function type. Reviewed By: panagosg7 Differential Revision: D8041565 fbshipit-source-id: 0f760cdf82b6e229a3c5db0c2e81a2331c7a2de1 --- src/typing/class_sig.ml | 26 +- src/typing/class_sig.mli | 2 + src/typing/codegen.ml | 2 +- src/typing/flow_js.ml | 293 +++++++++++++++------- src/typing/func_sig.ml | 3 +- src/typing/obj_type.ml | 4 +- src/typing/react_kit.ml | 7 +- src/typing/type.ml | 9 +- src/typing/type_annotation.ml | 15 +- src/typing/type_mapper.ml | 26 +- src/typing/type_visitor.ml | 4 + tests/call_properties/G.js | 5 + tests/call_properties/call_properties.exp | 16 +- 13 files changed, 288 insertions(+), 124 deletions(-) create mode 100644 tests/call_properties/G.js diff --git a/src/typing/class_sig.ml b/src/typing/class_sig.ml index 543457453ee..5d45e8f3b13 100644 --- a/src/typing/class_sig.ml +++ b/src/typing/class_sig.ml @@ -23,6 +23,7 @@ type signature = { methods: (Loc.t option * Func_sig.t) Nel.t SMap.t; getters: (Loc.t option * Func_sig.t) SMap.t; setters: (Loc.t option * Func_sig.t) SMap.t; + calls: Func_sig.t list; } type t = { @@ -62,6 +63,7 @@ let empty id reason tparams tparams_map super = methods = SMap.empty; getters = SMap.empty; setters = SMap.empty; + calls = []; } in let structural, super, ssuper, implements = let open Type in @@ -199,6 +201,11 @@ let append_method ~static name loc fsig x = setters = SMap.remove name s.setters; }) x +let append_call ~static fsig = map_sig ~static (fun s -> { + s with + calls = fsig :: s.calls +}) + let add_getter ~static name loc fsig x = let flat = static || x.structural in map_sig ~static (fun s -> { @@ -245,6 +252,7 @@ let subst_sig cx map s = methods = SMap.map (Nel.map subst_func_sig) s.methods; getters = SMap.map (subst_func_sig) s.getters; setters = SMap.map (subst_func_sig) s.setters; + calls = List.map (Func_sig.subst cx map) s.calls; } let generate_tests cx f x = @@ -343,6 +351,15 @@ let elements cx ~tparams_map ?constructor s = SMap.add name (to_field fld) acc ) s.proto_fields methods in + let call = match List.rev_map (Func_sig.methodtype cx) s.calls with + | [] -> None + | [t] -> Some t + | t0::t1::ts -> + let open Type in + let t = DefT (reason_of_t t0, IntersectionT (InterRep.make t0 t1 ts)) in + Some t + in + (* Only un-initialized fields require annotations, so determine now * (syntactically) which fields have initializers *) let initialized_field_names = SMap.fold (fun x (_, _, field) acc -> @@ -351,7 +368,7 @@ let elements cx ~tparams_map ?constructor s = | Infer _ -> SSet.add x acc ) s.fields SSet.empty in - initialized_field_names, fields, methods + initialized_field_names, fields, methods, call let arg_polarities x = List.fold_left Type.(fun acc tp -> @@ -359,7 +376,7 @@ let arg_polarities x = ) SMap.empty x.tparams let statictype cx ~tparams_map s = - let inited_fields, fields, methods = elements cx ~tparams_map s in + let inited_fields, fields, methods, call = elements cx ~tparams_map s in let props = SMap.union fields methods ~combine:(fun _ _ -> Utils_js.assert_false (Utils_js.spf @@ -369,7 +386,7 @@ let statictype cx ~tparams_map s = (* Statics are not exact, because we allow width subtyping between them. Specifically, given class A and class B extends A, Class <: Class. *) let static = - Obj_type.mk_with_proto cx s.reason ~props s.super + Obj_type.mk_with_proto cx s.reason ~props ?call s.super ~sealed:true ~exact:false in let open Type in @@ -389,7 +406,7 @@ let insttype cx ~tparams_map ~initialized_static_field_names s = let t = DefT (reason_of_t t0, IntersectionT (InterRep.make t0 t1 ts)) in Some (loc0, t) in - let inited_fields, fields, methods = elements cx ~tparams_map ?constructor s.instance in + let inited_fields, fields, methods, call = elements cx ~tparams_map ?constructor s.instance in { Type. class_id = s.id; type_args = SMap.map (fun t -> (Type.reason_of_t t, t)) s.tparams_map; @@ -398,6 +415,7 @@ let insttype cx ~tparams_map ~initialized_static_field_names s = initialized_field_names = inited_fields; initialized_static_field_names; methods_tmap = Context.make_property_map cx methods; + inst_call_t = call; has_unknown_react_mixins = false; structural = s.structural; } diff --git a/src/typing/class_sig.mli b/src/typing/class_sig.mli index 768f5a954ea..f8d84d757af 100644 --- a/src/typing/class_sig.mli +++ b/src/typing/class_sig.mli @@ -77,6 +77,8 @@ val add_method: static:bool -> string -> Loc.t option -> Func_sig.t -> t -> t of a single overloaded method. *) val append_method: static:bool -> string -> Loc.t option -> Func_sig.t -> t -> t +val append_call: static:bool -> Func_sig.t -> t -> t + (** Add getter to signature. *) val add_getter: static:bool -> string -> Loc.t option -> Func_sig.t -> t -> t diff --git a/src/typing/codegen.ml b/src/typing/codegen.ml index 14744cf2ea0..1428759d88c 100644 --- a/src/typing/codegen.ml +++ b/src/typing/codegen.ml @@ -253,7 +253,7 @@ let rec gen_type t env = Type.( add_str "number" env | DefT (_, NumT (Truthy|AnyLiteral)) -> add_str "number" env | DefT (_, NullT) | NullProtoT _ -> add_str "null" env - | DefT (_, ObjT {flags = _; dict_t; props_tmap; proto_t = _;}) -> ( + | DefT (_, ObjT {flags = _; dict_t; call_t = _; props_tmap; proto_t = _;}) -> ( let env = add_str "{" env in (* Generate prop entries *) diff --git a/src/typing/flow_js.ml b/src/typing/flow_js.ml index 8054165fb4d..e2927d5a6fe 100644 --- a/src/typing/flow_js.ml +++ b/src/typing/flow_js.ml @@ -291,14 +291,21 @@ let rec merge_type cx = | _ -> None in + let merge_call = match o1.call_t, o2.call_t with + | None, None -> Some None + | Some _, None -> if o2.flags.exact then Some o1.call_t else None + | None, Some _ -> if o1.flags.exact then Some o2.call_t else None + | Some c1, Some c2 -> Some (Some (create_union (UnionRep.make c1 c2 []))) + in + (* Only merge objects if every property can be merged. *) let should_merge = SMap.for_all (fun _ x -> x) merge_map in (* Don't merge objects with different prototypes. *) let should_merge = should_merge && o1.proto_t = o2.proto_t in - (match should_merge, merge_dict with - | true, Some dict -> + (match should_merge, merge_dict, merge_call with + | true, Some dict, Some call -> let map = SMap.merge (fun _ p1_opt p2_opt -> match p1_opt, p2_opt with (* Merge disjoint+exact objects. *) @@ -318,7 +325,7 @@ let rec merge_type cx = exact = o1.flags.exact && o2.flags.exact; frozen = o1.flags.frozen && o2.flags.frozen; } in - let objtype = mk_objecttype ~flags dict id o1.proto_t in + let objtype = mk_objecttype ~flags ~dict ~call id o1.proto_t in DefT (locationless_reason (RCustom "object"), ObjT objtype) | _ -> create_union (UnionRep.make t1 t2 [])) @@ -928,7 +935,7 @@ module ResolvableTypeJob = struct examples of such bugs yet. Leaving further investigation of this point as future work. *) - | DefT (_, ObjT { props_tmap; dict_t; _ }) -> + | DefT (_, ObjT { props_tmap; dict_t; call_t; _ }) -> let props_tmap = Context.find_props cx props_tmap in let ts = SMap.fold (fun x p ts -> (* avoid resolving types of shadow properties *) @@ -939,6 +946,10 @@ module ResolvableTypeJob = struct | None -> ts | Some { key; value; _ } -> key::value::ts in + let ts = match call_t with + | None -> ts + | Some t -> t::ts + in collect_of_types ?log_unresolved cx reason acc ts | DefT (_, FunT (_, _, { params; return_t; _ })) -> let ts = List.fold_left (fun acc (_, t) -> t::acc) [return_t] params in @@ -953,7 +964,7 @@ module ResolvableTypeJob = struct collect_of_type ?log_unresolved cx reason acc elemt | DefT (_, ArrT EmptyAT) -> acc | DefT (_, InstanceT (static, super, _, - { class_id; type_args; fields_tmap; methods_tmap; _ })) -> + { class_id; type_args; fields_tmap; methods_tmap; inst_call_t; _ })) -> let ts = if class_id = 0 then [] else [super; static] in let ts = SMap.fold (fun _ (_, t) ts -> t::ts) type_args ts in let props_tmap = SMap.union @@ -963,6 +974,10 @@ module ResolvableTypeJob = struct let ts = SMap.fold (fun _ p ts -> Property.fold_t (fun ts t -> t::ts) ts p ) props_tmap ts in + let ts = match inst_call_t with + | None -> ts + | Some t -> t::ts + in collect_of_types ?log_unresolved cx reason acc ts | DefT (_, PolyT (_, t, _)) -> collect_of_type ?log_unresolved cx reason acc t @@ -2912,15 +2927,20 @@ let rec __flow cx ((l: Type.t), (u: Type.use_t)) trace = (* values *) (**********) - | DefT (_, ObjT { props_tmap = tmap; dict_t; flags; _ }), GetValuesT (reason, values) -> + | DefT (_, ObjT o), GetValuesT (reason, values) -> + let { + flags; + proto_t = _; + props_tmap = tmap; + dict_t; + call_t = _; (* call props excluded from values *) + } = o in (* Find all of the props. *) let props = Context.find_props cx tmap in (* Get the read type for all readable properties and discard the rest. *) - let ts = SMap.fold (fun key prop ts -> + let ts = SMap.fold (fun _ prop ts -> match Property.read_t prop with - (* Don't include the type for the internal "$call" property if one - exists. *) - | Some t when key != "$call" -> + | Some t -> let t = if flags.frozen then match t with | DefT (t_reason, StrT (Literal (_, lit))) -> @@ -2935,7 +2955,7 @@ let rec __flow cx ((l: Type.t), (u: Type.use_t)) trace = | _ -> t else t in t :: ts - | _ -> ts + | None -> ts ) props [] in (* If the object has a dictionary value then add that to our types. *) let ts = match dict_t with @@ -2953,11 +2973,8 @@ let rec __flow cx ((l: Type.t), (u: Type.use_t)) trace = match Property.read_t prop with (* We don't want to include the property type if its name is the internal value "$key" because that will be the type for the instance - index and not the value. - - We also don't include the type for the internal "$call" property if - one exists. *) - | Some t when key != "$key" && key != "$call" -> t :: ts + index and not the value. *) + | Some t when key != "$key" -> t :: ts | _ -> ts ) props [] in (* Create a union type from all our selected types. *) @@ -3127,12 +3144,12 @@ let rec __flow cx ((l: Type.t), (u: Type.use_t)) trace = slices, but that approach behaves in nonobvious ways. TODO why? *) | DefT (_, IntersectionT _), - UseT (use_op, DefT (r, ObjT { flags; props_tmap; proto_t; dict_t })) + UseT (use_op, DefT (r, ObjT { flags; props_tmap; proto_t; dict_t; call_t })) when SMap.cardinal (Context.find_props cx props_tmap) > 1 -> iter_real_props cx props_tmap (fun ~is_sentinel:_ x p -> let pmap = SMap.singleton x p in let id = Context.make_property_map cx pmap in - let obj = mk_objecttype ~flags dict_t id dummy_prototype in + let obj = mk_objecttype ~flags ~dict:dict_t ~call:call_t id dummy_prototype in rec_flow cx trace (l, UseT (use_op, DefT (r, ObjT obj))) ); rec_flow cx trace (l, UseT (use_op, proto_t)) @@ -4243,10 +4260,12 @@ let rec __flow cx ((l: Type.t), (u: Type.use_t)) trace = | DefT (lreason, InstanceT (_, super, _, { fields_tmap = lflds; - methods_tmap = lmethods; _ })), + methods_tmap = lmethods; + inst_call_t = lcall; _ })), UseT (use_op, (DefT (ureason, ObjT { props_tmap = uflds; - proto_t = uproto; _ }) as u_deft)) -> + proto_t = uproto; + call_t = ucall; _ }) as u_deft)) -> Type_inference_hooks_js.dispatch_instance_to_obj_hook cx l u_deft; let lflds = @@ -4255,6 +4274,22 @@ let rec __flow cx ((l: Type.t), (u: Type.use_t)) trace = SMap.union fields_tmap methods_tmap in + Option.iter ucall ~f:(fun ucall -> + let prop_name = Some "$call" in + let use_op = Frame (PropertyCompatibility { + prop = prop_name; + lower = lreason; + upper = ureason; + is_sentinel = false; + }, use_op) in + (match lcall with + | Some lcall -> rec_flow cx trace (lcall, UseT (use_op, ucall)) + | None -> + let reason_prop = replace_reason_const (RProperty prop_name) ureason in + add_output cx ~trace (FlowError.EStrictLookupFailed + ((reason_prop, lreason), lreason, prop_name, Some use_op))) + ); + iter_real_props cx uflds (fun ~is_sentinel s up -> let use_op = Frame (PropertyCompatibility { prop = Some s; @@ -4299,37 +4334,36 @@ let rec __flow cx ((l: Type.t), (u: Type.use_t)) trace = BindT (use_op, reason_op, _, _) | CallT (use_op, reason_op, _) ) -> - let tvar = Tvar.mk cx ( - replace_reason (fun desc -> - RCustom (spf "%s used as a function" (string_of_desc desc)) - ) reason - ) in - let strict = match u with - | BindT (_, reason_op, {call_tout; _}, true) -> - (* Pass-through binding an object should not error if the object lacks - a callable property. Instead, we should flow the object to the - output tvar. This nonstrict lookup will unify the object with - `pass`, which flows to the output tvar. *) - let pass = Tvar.mk_where cx reason_op (fun t -> - rec_flow_t cx trace (t, call_tout) - ) in - NonstrictReturning (Some (l, pass), None) - | _ -> Strict reason - in - let action = match u with - | UseT (use_op, (DefT (_, (FunT _ | AnyFunT)) as u_def)) -> - let use_op = Frame (PropertyCompatibility { - prop = Some "$call"; + let prop_name = Some "$call" in + let use_op = match u with + | UseT (_, DefT (_, (FunT _ | AnyFunT))) -> + Frame (PropertyCompatibility { + prop = prop_name; lower = reason; upper = reason_op; is_sentinel = false; - }, use_op) in - LookupProp (use_op, Field (None, u_def, Positive)) + }, use_op) + | _ -> use_op in + let fun_t = match l with + | DefT (_, ObjT {call_t = Some t; _}) -> t + | DefT (_, InstanceT (_, _, _, {inst_call_t = Some t; _})) -> t | _ -> - RWProp (use_op, l, tvar, Read) + (match u with + | BindT (_, _, {call_tout; _}, true) -> + (* Pass-through binding an object should not error if the object lacks + a callable property. Instead, we should flow the object to the + output tvar. *) + rec_flow_t cx trace (l, call_tout) + | _ -> + let reason_prop = replace_reason_const (RProperty prop_name) reason_op in + add_output cx ~trace (FlowError.EStrictLookupFailed + ((reason_prop, reason), reason, prop_name, Some use_op))); + AnyT.why reason_op in - lookup_prop cx trace l reason_op reason strict "$call" action; - rec_flow cx trace (tvar, u) + (match u with + | UseT (_, (DefT (_, (FunT _ | AnyFunT)) as u_def)) -> + rec_flow cx trace (fun_t, UseT (use_op, u_def)) + | _ -> rec_flow cx trace (fun_t, u)) (******************************) (* matching shapes of objects *) @@ -4353,7 +4387,7 @@ let rec __flow cx ((l: Type.t), (u: Type.use_t)) trace = | (ShapeT (o), _) -> rec_flow cx trace (o, u) - | DefT (reason, ObjT { props_tmap = mapr; _ }), UseT (use_op', ShapeT (proto)) -> + | DefT (reason, ObjT { props_tmap = mapr; call_t = None; _ }), UseT (use_op', ShapeT proto) -> (* TODO: ShapeT should have its own reason *) let reason_op = reason_of_t proto in iter_real_props cx mapr (fun ~is_sentinel x p -> @@ -4382,10 +4416,10 @@ let rec __flow cx ((l: Type.t), (u: Type.use_t)) trace = (* Function definitions are incompatible with ShapeT. ShapeT is meant to * match an object type with a subset of the props in the type being * destructured. It would be complicated and confusing to use a function for - * this. Note that ObjTs with a $call property are, however, allowed. + * this. * * This invariant is important for the React setState() type definition. *) - | DefT (_, FunT _), UseT (use_op, ShapeT o) -> + | DefT (_, (FunT _ | ObjT {call_t = Some _; _})), UseT (use_op, ShapeT o) -> add_output cx ~trace (FlowError.EFunctionIncompatibleWithShape (reason_of_t l, reason_of_t o, use_op)) @@ -4582,12 +4616,15 @@ let rec __flow cx ((l: Type.t), (u: Type.use_t)) trace = (* TODO: closure *) (** create new object **) let reason_c = replace_reason_const RNewObject reason_op in - let proto_reason = reason_of_t proto in - let sealed = UnsealedInFile (Loc.source (loc_of_reason proto_reason)) in - let flags = { default_flags with sealed } in - let dict = None in - let pmap = Context.make_property_map cx SMap.empty in - let new_obj = DefT (reason_c, ObjT (mk_objecttype ~flags dict pmap proto)) in + let objtype = + let sealed = UnsealedInFile (Loc.source (loc_of_t proto)) in + let flags = { default_flags with sealed } in + let dict = None in + let call = None in + let pmap = Context.make_property_map cx SMap.empty in + mk_objecttype ~flags ~dict ~call pmap proto + in + let new_obj = DefT (reason_c, ObjT objtype) in (** error if type arguments are provided to non-polymorphic constructor **) Option.iter targs ~f:(fun _ -> add_output cx ~trace FlowError.(ECallTypeArity { @@ -4910,29 +4947,26 @@ let rec __flow cx ((l: Type.t), (u: Type.use_t)) trace = | DefT (lreason, ObjT { props_tmap = mapr; flags; dict_t; _ }), ObjAssignFromT (reason_op, to_obj, t, ObjAssign error_flags) -> - let props_to_skip = ["$call"] in Context.iter_props cx mapr (fun x p -> - if not (List.mem x props_to_skip) then ( - (* move the reason to the call site instead of the definition, so - that it is in the same scope as the Object.assign, so that - strictness rules apply. *) - let reason_prop = - lreason - |> replace_reason (fun desc -> RPropertyOf (x, desc)) - |> repos_reason (loc_of_reason reason_op) - in - match Property.read_t p with - | Some t -> - let propref = Named (reason_prop, x) in - let t = filter_optional cx ~trace reason_prop t in - rec_flow cx trace (to_obj, SetPropT ( - unknown_use, reason_prop, propref, Normal, t, None - )) - | None -> - add_output cx ~trace (FlowError.EPropAccess ( - (reason_prop, reason_op), Some x, Property.polarity p, Read, unknown_use - )) - ) + (* move the reason to the call site instead of the definition, so + that it is in the same scope as the Object.assign, so that + strictness rules apply. *) + let reason_prop = + lreason + |> replace_reason (fun desc -> RPropertyOf (x, desc)) + |> repos_reason (loc_of_reason reason_op) + in + match Property.read_t p with + | Some t -> + let propref = Named (reason_prop, x) in + let t = filter_optional cx ~trace reason_prop t in + rec_flow cx trace (to_obj, SetPropT ( + unknown_use, reason_prop, propref, Normal, t, None + )) + | None -> + add_output cx ~trace (FlowError.EPropAccess ( + (reason_prop, reason_op), Some x, Property.polarity p, Read, unknown_use + )) ); if dict_t <> None then rec_flow_t cx trace (DefT (reason_op, AnyObjT), t) else begin @@ -4946,7 +4980,7 @@ let rec __flow cx ((l: Type.t), (u: Type.use_t)) trace = let fields_pmap = Context.find_props cx fields_tmap in let methods_pmap = Context.find_props cx methods_tmap in let pmap = SMap.union fields_pmap methods_pmap in - let props_to_skip = ["$call"; "$key"; "$value"] in + let props_to_skip = ["$key"; "$value"] in pmap |> SMap.iter (fun x p -> if not (List.mem x props_to_skip) then ( match Property.read_t p with @@ -5690,11 +5724,13 @@ let rec __flow cx ((l: Type.t), (u: Type.use_t)) trace = | DefT (reason_inst, InstanceT (_, super, _, { fields_tmap; methods_tmap; + inst_call_t; structural = true; _; })), ImplementsT (use_op, t) -> - structural_subtype cx trace ~use_op t reason_inst (fields_tmap, methods_tmap); + structural_subtype cx trace ~use_op t reason_inst + (fields_tmap, methods_tmap, inst_call_t); rec_flow cx trace (super, ReposLowerT (reason_inst, false, ImplementsT (use_op, t))) @@ -6209,11 +6245,12 @@ let rec __flow cx ((l: Type.t), (u: Type.use_t)) trace = ExtendsUseT (use_op, _, [], l, DefT (reason_inst, InstanceT (_, super, _, { fields_tmap; methods_tmap; + inst_call_t; structural = true; _; }))) -> structural_subtype cx trace ~use_op l reason_inst - (fields_tmap, methods_tmap); + (fields_tmap, methods_tmap, inst_call_t); rec_flow cx trace (l, UseT (use_op, super)) | DefT (_, NullT), @@ -6497,12 +6534,14 @@ and flow_obj_to_obj cx trace ~use_op (lreason, l_obj) (ureason, u_obj) = let { flags = lflags; dict_t = ldict; + call_t = lcall; props_tmap = lflds; proto_t = lproto; } = l_obj in let { flags = rflags; dict_t = udict; + call_t = ucall; props_tmap = uflds; proto_t = uproto; } = u_obj in @@ -6552,6 +6591,23 @@ and flow_obj_to_obj cx trace ~use_op (lreason, l_obj) (ureason, u_obj) = ) ); + (match ucall with + | Some ucall -> + let prop_name = Some "$call" in + let use_op = Frame (PropertyCompatibility { + prop = prop_name; + lower = lreason; + upper = ureason; + is_sentinel = false; + }, use_op) in + (match lcall with + | Some lcall -> rec_flow cx trace (lcall, UseT (use_op, ucall)) + | None -> + let reason_prop = replace_reason_const (RProperty prop_name) ureason in + add_output cx ~trace (FlowError.EStrictLookupFailed + ((reason_prop, lreason), lreason, prop_name, Some use_op))) + | None -> ()); + (* Properties in u must either exist in l, or match l's indexer. *) iter_real_props cx uflds (fun ~is_sentinel s up -> let reason_prop = replace_reason_const (RProperty (Some s)) ureason in @@ -6613,11 +6669,6 @@ and flow_obj_to_obj cx trace ~use_op (lreason, l_obj) (ureason, u_obj) = robust. Tracked by #11299251. *) if not (Speculation.speculating ()) then Context.set_prop cx lflds s up; - (* Don't lookup $call in the prototype chain. Instead of going through - * LookupT add an EStrictLookupFailed error here. *) - | _ when s = "$call" -> - add_output cx ~trace (FlowError.EStrictLookupFailed - ((reason_prop, lreason), lreason, Some s, Some use_op)) | _ -> (* otherwise, look up the property in the prototype *) let strict = match Obj_type.sealed_in_op ureason lflags.sealed, ldict with @@ -6656,6 +6707,37 @@ and flow_obj_to_obj cx trace ~use_op (lreason, l_obj) (ureason, u_obj) = in let up = Field (None, value, dict_polarity) in if lit + then + match Property.read_t lp, Property.read_t up with + | Some lt, Some ut -> rec_flow cx trace (lt, UseT (use_op, ut)) + | _ -> () + else + let reason_prop = replace_reason_const (RProperty (Some s)) lreason in + let propref = Named (reason_prop, s) in + rec_flow_p cx trace ~use_op lreason ureason propref (lp, up))); + + (* Previously, call properties were stored in the props map, and were + checked against dictionary upper bounds. This is wrong, but useful for + distinguishing between thunk-like types found in graphql-js. + + Now that call properties are stored separately, it is particularly + egregious to emit this constraint. This only serves to maintain buggy + behavior, which should be fixed, and this code removed. *) + (match lcall, ucall with + | Some lcall, None -> + let s = "$call" in + let use_op = Frame (PropertyCompatibility { + prop = Some s; + lower = lreason; + upper = ureason; + is_sentinel = false; + }, use_op) in + let lp = match lcall with + | DefT (_, OptionalT t) -> Field (None, t, Positive) + | _ -> Field (None, lcall, Positive) + in + let up = Field (None, value, dict_polarity) in + if lit then match Property.read_t lp, Property.read_t up with | Some lt, Some ut -> rec_flow cx trace (lt, UseT (use_op, ut)) @@ -6664,7 +6746,8 @@ and flow_obj_to_obj cx trace ~use_op (lreason, l_obj) (ureason, u_obj) = let reason_prop = replace_reason_const (RProperty (Some s)) lreason in let propref = Named (reason_prop, s) in rec_flow_p cx trace ~use_op lreason ureason propref (lp, up) - ))); + | _ -> ()); + ); rec_flow cx trace (uproto, ReposUseT (ureason, false, use_op, DefT (lreason, ObjT l_obj))) @@ -6699,7 +6782,6 @@ and is_function_prototype = function * implied by an object indexer type *) and is_dictionary_exempt = function | x when is_object_prototype_method x -> true - | "$call" -> true | _ -> false (* common case checking a function as an object *) @@ -6719,7 +6801,6 @@ and quick_error_fun_as_obj cx trace ~use_op reason statics reason_o props = in not ( optional || - x = "$call" || is_function_prototype x || SMap.mem x statics_own_props ) @@ -6978,12 +7059,12 @@ and flow_type_args cx trace ~use_op lreason ureason pmap tmap1 tmap2 = | Neutral -> rec_unify cx trace ~use_op t1 t2) ) -and inherited_method x = x <> "constructor" && x <> "$call" +and inherited_method x = x <> "constructor" (* dispatch checks to verify that lower satisfies the structural requirements given in the tuple. *) and structural_subtype cx trace ?(use_op=unknown_use) lower reason_struct - (fields_pmap, methods_pmap) = + (fields_pmap, methods_pmap, call_t) = let lreason = reason_of_t lower in let fields_pmap = Context.find_props cx fields_pmap in let methods_pmap = Context.find_props cx methods_pmap in @@ -7033,6 +7114,26 @@ and structural_subtype cx trace ?(use_op=unknown_use) lower reason_struct LookupT (reason_struct, Strict lreason, [], propref, LookupProp (use_op, p))) ); + call_t |> Option.iter ~f:(fun ut -> + let prop_name = Some "$call" in + let use_op = Frame (PropertyCompatibility { + prop = prop_name; + lower = lreason; + upper = reason_struct; + is_sentinel = false; + }, use_op) in + match lower with + | DefT (_, ObjT {call_t = Some lt; _}) -> + rec_flow cx trace (lt, UseT (use_op, ut)) + | DefT (_, InstanceT (_, _, _, {inst_call_t = Some lt; _})) -> + rec_flow cx trace (lt, UseT (use_op, ut)) + | _ -> + let reason_prop = replace_reason (fun desc -> + RPropertyOf ("$call", desc) + ) reason_struct in + add_output cx ~trace (FlowError.EStrictLookupFailed + ((reason_prop, lreason), lreason, prop_name, Some use_op)) + ); and check_super cx trace ~use_op lreason ureason t x p = let use_op = Frame (PropertyCompatibility { @@ -11091,7 +11192,8 @@ and object_kit = in { sealed = Sealed; frozen = false; exact } in - let t = DefT (r, ObjT (mk_objecttype ~flags dict id proto)) in + let call = None in + let t = DefT (r, ObjT (mk_objecttype ~flags ~dict ~call id proto)) in (* Wrap the final type in an `ExactT` if we have an exact flag *) if flags.exact then ExactT (reason, t) else t in @@ -11290,7 +11392,8 @@ and object_kit = } in let id = Context.make_property_map cx props in let proto = ObjProtoT r1 in - let t = DefT (r1, ObjT (mk_objecttype ~flags dict id proto)) in + let call = None in + let t = DefT (r1, ObjT (mk_objecttype ~flags ~dict ~call id proto)) in (* Wrap the final type in an `ExactT` if we have an exact flag *) if flags.exact then ExactT (r1, t) else t in @@ -11325,9 +11428,10 @@ and object_kit = let props = SMap.map (fun (t, _) -> Field (None, t, polarity)) props in let dict = Option.map dict (fun dict -> { dict with dict_polarity = polarity }) in + let call = None in let id = Context.make_property_map cx props in let proto = ObjProtoT reason in - let t = DefT (r, ObjT (mk_objecttype ~flags dict id proto)) in + let t = DefT (r, ObjT (mk_objecttype ~flags ~dict ~call id proto)) in if flags.exact then ExactT (reason, t) else t in @@ -11442,10 +11546,11 @@ and object_kit = } in props, dict, flags in + let call = None in (* Finish creating our props object. *) let id = Context.make_property_map cx props in let proto = ObjProtoT reason in - let t = DefT (reason, ObjT (mk_objecttype ~flags dict id proto)) in + let t = DefT (reason, ObjT (mk_objecttype ~flags ~dict ~call id proto)) in if flags.exact then ExactT (reason, t) else t in diff --git a/src/typing/func_sig.ml b/src/typing/func_sig.ml index 0e31326bc1b..69e47f073bc 100644 --- a/src/typing/func_sig.ml +++ b/src/typing/func_sig.ml @@ -84,9 +84,8 @@ let generate_tests cx f x = let functiontype cx this_t {reason; kind; tparams; fparams; return_t; _} = let knot = Tvar.mk cx reason in let static = - let props = SMap.singleton "$call" (Method (None, knot)) in let proto = FunProtoT reason in - Obj_type.mk_with_proto cx reason ~props proto + Obj_type.mk_with_proto cx reason ~call:knot proto in let prototype = let reason = replace_reason_const RPrototype reason in diff --git a/src/typing/obj_type.ml b/src/typing/obj_type.ml index c29b7231905..6ee3b8d4b1f 100644 --- a/src/typing/obj_type.ml +++ b/src/typing/obj_type.ml @@ -8,14 +8,14 @@ open Type let mk_with_proto cx reason - ?(sealed=false) ?(exact=true) ?(frozen=false) ?dict ?(props=SMap.empty) proto = + ?(sealed=false) ?(exact=true) ?(frozen=false) ?dict ?call ?(props=SMap.empty) proto = let sealed = if sealed then Sealed else UnsealedInFile (Loc.source (Reason.loc_of_reason reason)) in let flags = { sealed; exact; frozen } in let pmap = Context.make_property_map cx props in - DefT (reason, ObjT (mk_objecttype ~flags dict pmap proto)) + DefT (reason, ObjT (mk_objecttype ~flags ~dict ~call pmap proto)) let mk cx reason = mk_with_proto cx reason (ObjProtoT reason) diff --git a/src/typing/react_kit.ml b/src/typing/react_kit.ml index ec80472f4c8..409829a95d4 100644 --- a/src/typing/react_kit.ml +++ b/src/typing/react_kit.ml @@ -196,7 +196,7 @@ let run cx trace ~use_op reason_op l u rec_flow_t cx trace (component, component_function tin) (* Stateless functional components, again. This time for callable `ObjT`s. *) - | DefT (_, ObjT { props_tmap = id; _ }) when Context.find_props cx id |> SMap.mem "$call" -> + | DefT (_, ObjT { call_t = Some _; _ }) -> (* This direction works because function arguments are flowed in the * opposite direction. *) rec_flow_t cx trace (component, component_function tin) @@ -228,7 +228,7 @@ let run cx trace ~use_op reason_op l u rec_flow_t cx trace (component_function ~with_return_t:false tout, component) (* Stateless functional components, again. This time for callable `ObjT`s. *) - | DefT (_, ObjT { props_tmap = id; _ }) when Context.find_props cx id |> SMap.mem "$call" -> + | DefT (_, ObjT { call_t = Some _; _ }) -> (* This direction works because function arguments are flowed in the * opposite direction. *) rec_flow_t cx trace (component_function ~with_return_t:false tout, component) @@ -488,7 +488,7 @@ let run cx trace ~use_op reason_op l u rec_flow_t cx trace (VoidT.make (replace_reason_const RVoid r), tout) (* Stateless functional components, again. This time for callable `ObjT`s. *) - | DefT (r, ObjT { props_tmap = id; _ }) when Context.find_props cx id |> SMap.mem "$call" -> + | DefT (r, ObjT { call_t = Some _; _ }) -> rec_flow_t cx trace (VoidT.make (replace_reason_const RVoid r), tout) (* Intrinsic components. *) @@ -964,6 +964,7 @@ let run cx trace ~use_op reason_op l u initialized_field_names = SSet.empty; initialized_static_field_names = SSet.empty; methods_tmap = Context.make_property_map cx SMap.empty; + inst_call_t = None; has_unknown_react_mixins = spec.unknown_mixins <> []; structural = false; } in diff --git a/src/typing/type.ml b/src/typing/type.ml index 05cf175207a..0e9355ab48a 100644 --- a/src/typing/type.ml +++ b/src/typing/type.ml @@ -787,6 +787,7 @@ module rec TypeTerm : sig dict_t: dicttype option; props_tmap: Properties.id; proto_t: prototype; + call_t: t option; } (* Object.assign(target, source1, ...source2) first resolves target then the @@ -915,6 +916,7 @@ module rec TypeTerm : sig initialized_field_names: SSet.t; initialized_static_field_names: SSet.t; methods_tmap: Properties.id; + inst_call_t: t option; has_unknown_react_mixins: bool; structural: bool; } @@ -3287,11 +3289,12 @@ let default_flags = { frozen = false; } -let mk_objecttype ?(flags=default_flags) dict pmap proto = { +let mk_objecttype ?(flags=default_flags) ~dict ~call pmap proto = { flags; - dict_t = dict; + proto_t = proto; props_tmap = pmap; - proto_t = proto + dict_t = dict; + call_t = call; } let apply_opt_funcalltype (this, targs, args, clos, strict) t_out = { diff --git a/src/typing/type_annotation.ml b/src/typing/type_annotation.ml index 28daf06f4e5..8fa7ed02c85 100644 --- a/src/typing/type_annotation.ml +++ b/src/typing/type_annotation.ml @@ -675,17 +675,14 @@ let rec convert cx tparams_map = Ast.Type.(function | _ -> false ) properties in let mk_object ~exact (call_props, dict, props_map, proto) = - let props_map = match List.rev call_props with - | [] -> props_map - | [t] -> - let p = Field (None, t, Positive) in - SMap.add "$call" p props_map + let call = match List.rev call_props with + | [] -> None + | [t] -> Some t | t0::t1::ts -> let callable_reason = mk_reason (RCustom "callable object type") loc in let rep = InterRep.make t0 t1 ts in let t = DefT (callable_reason, IntersectionT rep) in - let p = Field (None, t, Positive) in - SMap.add "$call" p props_map + Some t in (* Use the same reason for proto and the ObjT so we can walk the proto chain and use the root proto reason to build an error. *) @@ -712,7 +709,7 @@ let rec convert cx tparams_map = Ast.Type.(function frozen = false } in DefT (mk_reason reason_desc loc, - ObjT (mk_objecttype ~flags dict pmap proto)) + ObjT (mk_objecttype ~flags ~dict ~call pmap proto)) in let property loc prop props proto = match prop with @@ -1083,7 +1080,7 @@ and add_interface_properties cx tparams_map properties s = List.fold_left Ast.Type.Object.(fun x -> function | CallProperty (loc, { CallProperty.value = (_, func); static }) -> let fsig = mk_func_sig cx tparams_map loc func in - append_method ~static "$call" None fsig x + append_call ~static fsig x | Indexer (loc, { Indexer.static; _ }) when mem_field ~static "$key" x -> Flow.add_output cx Flow_error.(EUnsupportedSyntax (loc, MultipleIndexers)); diff --git a/src/typing/type_mapper.ml b/src/typing/type_mapper.ml index cb9d22b62b9..3677cffb53c 100644 --- a/src/typing/type_mapper.ml +++ b/src/typing/type_mapper.ml @@ -298,6 +298,7 @@ class ['a] t = object(self) initialized_field_names; initialized_static_field_names; methods_tmap; + inst_call_t; has_unknown_react_mixins; structural } = i in @@ -317,7 +318,13 @@ class ['a] t = object(self) let methods_tmap' = if m_tmap == m_tmap' then methods_tmap else Context.make_property_map cx m_tmap' in - if type_args == type_args' && methods_tmap == methods_tmap' && fields_tmap == fields_tmap' + let inst_call_t' = OptionUtils.ident_map (self#type_ cx map_cx) inst_call_t in + if ( + type_args == type_args' && + methods_tmap == methods_tmap' && + fields_tmap == fields_tmap' && + inst_call_t = inst_call_t' + ) then i else { class_id; @@ -327,6 +334,7 @@ class ['a] t = object(self) initialized_field_names; initialized_static_field_names; methods_tmap = methods_tmap'; + inst_call_t = inst_call_t'; has_unknown_react_mixins; structural; } @@ -425,16 +433,24 @@ class ['a] t = object(self) if exps == exps' then id else Context.make_export_map cx exps' - method obj_type cx map_cx ({ flags; dict_t; props_tmap; proto_t} as t) = + method obj_type cx map_cx t = + let { flags; dict_t; props_tmap; proto_t; call_t } = t in let dict_t' = OptionUtils.ident_map (self#dict_type cx map_cx) dict_t in let p_tmap = Context.find_props cx props_tmap in let p_tmap' = SMap.ident_map (Property.ident_map_t (self#type_ cx map_cx)) p_tmap in let props_tmap' = if p_tmap == p_tmap' then props_tmap else Context.make_property_map cx p_tmap' in let proto_t' = self#type_ cx map_cx proto_t in - if dict_t' == dict_t && props_tmap' == props_tmap && proto_t' == proto_t then t - else - { flags; dict_t = dict_t'; props_tmap = props_tmap'; proto_t = proto_t' } + let call_t' = OptionUtils.ident_map (self#type_ cx map_cx) call_t in + if dict_t' == dict_t && props_tmap' == props_tmap && proto_t' == proto_t && call_t' == call_t + then t + else { + flags; + dict_t = dict_t'; + props_tmap = props_tmap'; + proto_t = proto_t'; + call_t = call_t'; + } method dict_type cx map_cx ({dict_name; key; value; dict_polarity} as t) = let key' = self#type_ cx map_cx key in diff --git a/src/typing/type_visitor.ml b/src/typing/type_visitor.ml index a77159337f8..34463bb74e5 100644 --- a/src/typing/type_visitor.ml +++ b/src/typing/type_visitor.ml @@ -711,11 +711,13 @@ class ['a] t = object(self) dict_t; props_tmap; proto_t; + call_t; flags = _; } = o in let acc = self#opt (self#dict_type cx pole) acc dict_t in let acc = self#props cx pole acc props_tmap in let acc = self#type_ cx pole acc proto_t in + let acc = self#opt (self#type_ cx pole) acc call_t in acc method private arr_type cx pole acc = function @@ -736,6 +738,7 @@ class ['a] t = object(self) type_args; fields_tmap; methods_tmap; + inst_call_t; class_id = _; initialized_field_names = _; initialized_static_field_names = _; @@ -748,6 +751,7 @@ class ['a] t = object(self) ) type_args acc in let acc = self#props cx pole acc fields_tmap in let acc = self#props cx pole acc methods_tmap in + let acc = self#opt (self#type_ cx pole) acc inst_call_t in acc method private export_types cx pole acc e = diff --git a/tests/call_properties/G.js b/tests/call_properties/G.js new file mode 100644 index 00000000000..aca5340302d --- /dev/null +++ b/tests/call_properties/G.js @@ -0,0 +1,5 @@ +// Callable properties are not inherited + +function f() {} +var o = Object.create(f); +o(); // error: o is not callable diff --git a/tests/call_properties/call_properties.exp b/tests/call_properties/call_properties.exp index 749dfa12793..a3284068dc6 100644 --- a/tests/call_properties/call_properties.exp +++ b/tests/call_properties/call_properties.exp @@ -289,6 +289,20 @@ References: ^^^^^^ [1] +Error --------------------------------------------------------------------------------------------------------- G.js:5:1 + +Cannot call `o` because a callable signature is missing in `Object.create` [1]. + + G.js:5:1 + 5| o(); // error: o is not callable + ^^^ + +References: + G.js:4:9 + 4| var o = Object.create(f); + ^^^^^^^^^^^^^^^^ [1] + + Error --------------------------------------------------------------------------------------------------- use_ops.js:4:2 Cannot cast `a` to `B` because a callable signature is missing in object type [1] but exists in function type [2] in @@ -308,7 +322,7 @@ References: -Found 19 errors +Found 20 errors Only showing the most relevant union/intersection branches. To see all branches, re-run Flow with --show-all-branches