Skip to content

Commit

Permalink
[flow] Prevent method-unbinding errors when there is a mixed/any hint
Browse files Browse the repository at this point in the history
Summary:
method-unbinding prevents errors by disallowing you from unbinding a method from the `this` required to call it. However, this behavior is incompatible with things like jest.expect:
```
// ERROR, cannot unbind method from foo
expect(foo.method).toBeCalled...
```

We can take advantage of LTI to make this experience significantly better. In this diff, when we see a hint of `mixed` (which cannot be called) or `any` (which indicates an explicit opt-out of safety) we allow a method to be unbound without any errors.

Reviewed By: mvitousek

Differential Revision: D54157530

fbshipit-source-id: 50f32fff815ff6582ed1828a4db5cd91eae21943
  • Loading branch information
jbrown215 authored and facebook-github-bot committed Apr 2, 2024
1 parent 7cf8cbf commit ef4c97f
Show file tree
Hide file tree
Showing 9 changed files with 121 additions and 41 deletions.
12 changes: 10 additions & 2 deletions src/analysis/env_builder/name_def.ml
Original file line number Diff line number Diff line change
Expand Up @@ -579,8 +579,8 @@ let expression_is_definitely_synthesizable ~autocomplete_hooks =
| Ast.Expression.Class _
| Ast.Expression.Import _
| Ast.Expression.JSXFragment _
| Ast.Expression.Member _
| Ast.Expression.MetaProperty _
| Ast.Expression.Member _
| Ast.Expression.OptionalMember _
| Ast.Expression.Sequence _
| Ast.Expression.Super _
Expand Down Expand Up @@ -2696,6 +2696,7 @@ class def_finder ~autocomplete_hooks env_info toplevel_scope =
[]
| _ -> hints
in
let hints_before_synthesizable_check = hints in
let hints =
if expression_is_definitely_synthesizable ~autocomplete_hooks exp then
[]
Expand All @@ -2711,7 +2712,14 @@ class def_finder ~autocomplete_hooks env_info toplevel_scope =
(ExpressionDef { cond_context = cond; expr = exp; hints; chain = false })
| _ -> ()
end;
this#record_hint loc hints;
let () =
match expr with
(* Member expressions are always synthesizable, but we use hints on member expressions to avoid
* method-unbinding errors when the hint is a supertype of a mixed (which would make the method un-callable
*)
| Ast.Expression.Member _ -> this#record_hint loc hints_before_synthesizable_check
| _ -> this#record_hint loc hints
in
match expr with
| Ast.Expression.Array expr -> this#visit_array_expression ~array_hints:hints loc expr
| Ast.Expression.ArrowFunction x ->
Expand Down
1 change: 1 addition & 0 deletions src/typing/annotation_inference.ml
Original file line number Diff line number Diff line change
Expand Up @@ -1051,6 +1051,7 @@ module rec ConsGen : S = struct
~method_accessible:false
~super
~lookup_kind:(Strict reason_instance)
~hint:hint_unavailable
inst
propref
reason_op
Expand Down
9 changes: 8 additions & 1 deletion src/typing/env_resolution.ml
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,14 @@ let rec synthesizable_expression cx ?cond exp =
let expr_reason = mk_expression_reason exp in
let prop_reason = mk_reason (RProperty (Some (OrdinaryName name))) ploc in
let use_op = Op (GetProperty expr_reason) in
Statement.get_prop ~use_op ~cond:None cx expr_reason t (prop_reason, name)
Statement.get_prop
~use_op (* TODO(jmbrown) This feels incorrect *)
~hint:(Type_env.get_hint cx loc)
~cond:None
cx
expr_reason
t
(prop_reason, name)
in
tout
| _ -> expression cx ?cond exp
Expand Down
14 changes: 12 additions & 2 deletions src/typing/flow_js.ml
Original file line number Diff line number Diff line change
Expand Up @@ -3803,7 +3803,15 @@ struct
with
| Some (p, target_kind) ->
let p =
check_method_unbinding cx trace ~use_op ~method_accessible ~reason_op ~propref p
check_method_unbinding
cx
trace
~use_op
~method_accessible
~reason_op
~propref
~hint:hint_unavailable
p
in
(match kind with
| NonstrictReturning (_, Some (id, _)) -> Context.test_prop_hit cx id
Expand Down Expand Up @@ -3932,7 +3940,7 @@ struct
let t = TypeUtil.class_type ?annot_loc:(annot_loc_of_reason r) l in
rec_flow_t cx trace ~use_op:unknown_use (t, OpenT tout)
| ( DefT (reason_instance, InstanceT { super; inst; _ }),
GetPropT { use_op; reason = reason_op; from_annot; id; propref; tout; hint = _ }
GetPropT { use_op; reason = reason_op; id; from_annot; propref; tout; hint }
) ->
let method_accessible = from_annot in
let lookup_action = ReadProp { use_op; obj_t = l; tout } in
Expand All @@ -3956,6 +3964,7 @@ struct
~method_accessible
~super
~lookup_kind
~hint
inst
propref
reason_op
Expand Down Expand Up @@ -4006,6 +4015,7 @@ struct
~method_accessible:true
~super
~lookup_kind
~hint:hint_unavailable
inst
propref
reason_call
Expand Down
56 changes: 46 additions & 10 deletions src/typing/flow_js_utils.ml
Original file line number Diff line number Diff line change
Expand Up @@ -1991,17 +1991,41 @@ let rec unbind_this_method = function
| IntersectionT (r, rep) -> IntersectionT (r, InterRep.map unbind_this_method rep)
| t -> t

let check_method_unbinding cx trace ~use_op ~method_accessible ~reason_op ~propref p =
let check_method_unbinding cx trace ~use_op ~method_accessible ~reason_op ~propref ~hint p =
match p with
| Method { key_loc; type_ = t }
when (not method_accessible)
&& not (Context.allowed_method_unbinding cx (Reason.loc_of_reason reason_op)) ->
let reason_op = reason_of_propref propref in
add_output
cx
~trace
(Error_message.EMethodUnbinding { use_op; reason_op; reason_prop = reason_of_t t });
Method { key_loc; type_ = unbind_this_method t }
let hint_result = (snd hint) reason_op in
let valid_hint_t =
match hint_result with
| HintAvailable (t, _) ->
let t =
match t with
| OpenT (_, id) ->
let (_, constraints) = Context.find_constraints cx id in
(match constraints with
| FullyResolved tvar -> Context.force_fully_resolved_tvar cx tvar
| Resolved t -> t
| Unresolved _ -> t)
| _ -> t
in
(match t with
| DefT (_, MixedT _)
| AnyT _ ->
Some t
| _ -> None)
| _ -> None
in
(match valid_hint_t with
| Some t -> Method { key_loc; type_ = t }
| None ->
let reason_op = reason_of_propref propref in
add_output
cx
~trace
(Error_message.EMethodUnbinding { use_op; reason_op; reason_prop = reason_of_t t });
Method { key_loc; type_ = unbind_this_method t })
| _ -> p

(* e.g. `0`, `-123, `234234` *)
Expand Down Expand Up @@ -2128,11 +2152,23 @@ module GetPropT_kit (F : Get_prop_helper_sig) = struct
| _ -> None

let read_instance_prop
cx trace ~use_op ~instance_t ~id ~method_accessible ~super ~lookup_kind inst propref reason_op
=
cx
trace
~use_op
~instance_t
~id
~method_accessible
~super
~lookup_kind
~hint
inst
propref
reason_op =
match get_instance_prop cx trace ~use_op ~ignore_dicts:true inst propref reason_op with
| Some (p, _target_kind) ->
let p = check_method_unbinding cx trace ~use_op ~method_accessible ~reason_op ~propref p in
let p =
check_method_unbinding cx trace ~use_op ~method_accessible ~reason_op ~propref ~hint p
in
Base.Option.iter id ~f:(Context.test_prop_hit cx);
perform_read_prop_action cx trace use_op propref (Property.type_ p) reason_op None
| None ->
Expand Down
42 changes: 32 additions & 10 deletions src/typing/statement.ml
Original file line number Diff line number Diff line change
Expand Up @@ -3739,7 +3739,14 @@ module Make
| Some t -> t
| None ->
let use_op = Op (GetProperty (mk_expression_reason ex)) in
get_prop ~use_op ~cond cx expr_reason super_t (prop_reason, name)
get_prop
~use_op
~cond
~hint:(Type_env.get_hint cx loc)
cx
expr_reason
super_t
(prop_reason, name)
in
let property = Member.PropertyIdentifier ((ploc, lhs_t), id) in
let ast =
Expand Down Expand Up @@ -4078,7 +4085,14 @@ module Make
let expr_reason = mk_expression_reason ex in
let prop_reason = mk_reason (RProperty (Some (OrdinaryName name))) ploc in
let use_op = Op (GetProperty expr_reason) in
let opt_use = get_prop_opt_use ~cond expr_reason ~use_op (prop_reason, name) in
let opt_use =
get_prop_opt_use
~cond
expr_reason
~use_op
~hint:(Type_env.get_hint cx loc)
(prop_reason, name)
in
let get_mem_t () _ obj_t =
Tvar_resolver.mk_tvar_and_fully_resolve_no_wrap_where cx expr_reason (fun t ->
let use = apply_opt_use opt_use t in
Expand Down Expand Up @@ -5282,7 +5296,7 @@ module Make
let reason = mk_reason (RIdentifier (OrdinaryName "React.Fragment")) expr_loc in
let react = Type_env.var_ref ~lookup_mode:ForValue cx (OrdinaryName "React") expr_loc in
let use_op = Op (GetProperty reason) in
get_prop ~cond:None cx reason ~use_op react (reason, "Fragment")
get_prop ~cond:None cx reason ~use_op ~hint:hint_unavailable react (reason, "Fragment")
in
let (unresolved_params, frag_children) = collapse_children cx frag_children in
let locs = (expr_loc, frag_opening_element, children_loc) in
Expand Down Expand Up @@ -5923,24 +5937,23 @@ module Make
expressions out of `expression`, somewhat like what assignment_lhs does. That
would make everything involving Refinement be in the same place.
*)
and get_prop_opt_use ~cond reason ~use_op (prop_reason, name) =
and get_prop_opt_use ~cond reason ~use_op ~hint (prop_reason, name) =
let id = mk_id () in
let prop_name = OrdinaryName name in
if Base.Option.is_some cond then
OptTestPropT
(use_op, reason, id, mk_named_prop ~reason:prop_reason prop_name, hint_unavailable)
OptTestPropT (use_op, reason, id, mk_named_prop ~reason:prop_reason prop_name, hint)
else
OptGetPropT
{
use_op;
reason;
id = Some id;
propref = mk_named_prop ~reason:prop_reason prop_name;
hint = hint_unavailable;
hint;
}

and get_prop ~cond cx reason ~use_op tobj (prop_reason, name) =
let opt_use = get_prop_opt_use ~cond reason ~use_op (prop_reason, name) in
and get_prop ~cond cx reason ~use_op ~hint tobj (prop_reason, name) =
let opt_use = get_prop_opt_use ~cond reason ~use_op ~hint (prop_reason, name) in
Tvar_resolver.mk_tvar_and_fully_resolve_no_wrap_where cx reason (fun t ->
let get_prop_u = apply_opt_use opt_use t in
Flow.flow cx (tobj, get_prop_u)
Expand Down Expand Up @@ -6504,7 +6517,16 @@ module Make
let expr_reason = mk_expression_reason (loc, expr) in
let prop_reason = mk_reason (RProperty (Some (OrdinaryName name))) ploc in
let use_op = Op (GetProperty expr_reason) in
let tout = get_prop ~use_op ~cond:None cx expr_reason t (prop_reason, name) in
let tout =
get_prop
~use_op
~hint:hint_unavailable
~cond:None
cx
expr_reason
t
(prop_reason, name)
in
( tout,
fun () ->
( (loc, tout),
Expand Down
1 change: 1 addition & 0 deletions src/typing/statement_sig.ml
Original file line number Diff line number Diff line change
Expand Up @@ -269,6 +269,7 @@ module type S = sig
Context.t ->
Reason.t ->
use_op:Type.use_op ->
hint:Type.lazy_hint_t ->
Type.t ->
Reason.t * string ->
Type.t
Expand Down
17 changes: 1 addition & 16 deletions tests/config_file_extensions/config_file_extensions.exp
Original file line number Diff line number Diff line change
Expand Up @@ -15,20 +15,5 @@ References:
^^ [2]


Error ------------------------------------------------------------------------------------------------ test.js.es6:14:19

Cannot get `Object.prototype.toString` because property `toString` [1] cannot be unbound from the context [2] where it
was defined. [method-unbinding]

test.js.es6:14:19
14| (Object.prototype.toString: Function);
^^^^^^^^ [1]

References:
<BUILTINS>/core.js:247:5
247| toString(): string;
^^^^^^^^^^^^^^^^^^ [2]



Found 2 errors
Found 1 error
10 changes: 10 additions & 0 deletions tests/method_unbinding/with_hint.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
declare function expectMixed(x: mixed): void;
declare function expectAny(x: any): void;

class Foo {
method(): number { return 3; }
}

const foo = new Foo();
expectMixed(foo.method); // OK
expectAny(foo.method); // OK

0 comments on commit ef4c97f

Please sign in to comment.