Skip to content

Commit

Permalink
[private-method] Type checking private methods
Browse files Browse the repository at this point in the history
Summary:
This diff adds support for type checking private methods.

I added private methods and private static methods into the same class entries that stores type signature of private fields, and then use these type signature information to resolve `GetPrivatePropT` and `PrivateMethodT`. I also ensured that unbinding private methods are also correctly errored on, similar to unbound public methods.

Fixes #6254 #4872 #7877.

Reviewed By: jbrown215

Differential Revision: D30023778

fbshipit-source-id: 5575306de4ac504a92fb8bba221221566b5ac52d
  • Loading branch information
SamChou19815 authored and facebook-github-bot committed Aug 17, 2021
1 parent 5de4ea5 commit 9d7b2a7
Show file tree
Hide file tree
Showing 17 changed files with 496 additions and 61 deletions.
54 changes: 49 additions & 5 deletions src/typing/class_sig.ml
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ module Make (F : Func_sig.S) = struct
(* Multiple function signatures indicates an overloaded method. Note that
function signatures are stored in reverse definition order. *)
methods: func_info Nel.t SMap.t;
private_methods: func_info SMap.t;
getters: func_info SMap.t;
setters: func_info SMap.t;
calls: Type.t list;
Expand All @@ -91,6 +92,7 @@ module Make (F : Func_sig.S) = struct
private_fields = SMap.empty;
proto_fields = SMap.empty;
methods = SMap.empty;
private_methods = SMap.empty;
getters = SMap.empty;
setters = SMap.empty;
calls = [];
Expand Down Expand Up @@ -130,6 +132,13 @@ module Make (F : Func_sig.S) = struct
map_sig (fun s ->
{ s with private_fields = SMap.add name (Some loc, polarity, field) s.private_fields })

let add_private_method ~static name loc fsig ~set_asts ~set_type x =
let func_info = (Some loc, fsig, set_asts, set_type) in
map_sig
~static
(fun s -> { s with private_methods = SMap.add name func_info s.private_methods })
x

let public_fields_of_signature ~static s =
(if static then
s.static
Expand Down Expand Up @@ -303,6 +312,7 @@ module Make (F : Func_sig.S) = struct

let iter_methods_with_name f s =
SMap.iter (f %> Nel.iter) s.methods;
SMap.iter f s.private_methods;
SMap.iter f s.getters;
SMap.iter f s.setters

Expand All @@ -324,6 +334,7 @@ module Make (F : Func_sig.S) = struct
private_fields = SMap.map (subst_field cx map) s.private_fields;
proto_fields = SMap.map (subst_field cx map) s.proto_fields;
methods = SMap.map (Nel.map subst_func_sig) s.methods;
private_methods = SMap.map subst_func_sig s.private_methods;
getters = SMap.map subst_func_sig s.getters;
setters = SMap.map subst_func_sig s.setters;
calls = Base.List.map ~f:(Flow.subst cx map) s.calls;
Expand Down Expand Up @@ -363,6 +374,19 @@ module Make (F : Func_sig.S) = struct

let this_or_mixed loc = this_t %> Base.Option.value ~default:(Type.dummy_this loc)

let this_or_mixed_of_t ~static x =
let loc =
if static then
aloc_of_reason x.static.reason
else
aloc_of_reason x.instance.reason
in
let this_t = this_or_mixed loc x in
if static then
TypeUtil.class_type this_t
else
this_t

let tparams_with_this tparams this_tp =
(* Use the loc for the original tparams, or just the loc for the this type if there are no
* tparams *)
Expand Down Expand Up @@ -403,8 +427,15 @@ module Make (F : Func_sig.S) = struct
in
Type.Field (loc, t, polarity)

let to_prop_map cx =
SMap.map to_field %> NameUtils.namemap_of_smap %> Context.generate_property_map cx
let to_method cx this_default (loc, fsig, _, _) =
Type.Method (loc, F.methodtype cx this_default fsig)

let to_prop_map to_prop_converter cx =
SMap.map to_prop_converter %> NameUtils.namemap_of_smap %> Context.generate_property_map cx

let fields_to_prop_map = to_prop_map to_field

let methods_to_prop_map ~cx ~this_default = to_prop_map (to_method cx this_default) cx

let elements cx ~this ?constructor s super =
(* To determine the default `this` parameter for a method without `this` annotation, we
Expand Down Expand Up @@ -444,6 +475,12 @@ module Make (F : Func_sig.S) = struct
(loc0, IntersectionT (reason_of_t t0, InterRep.make t0 t1 ts)))
s.methods
in
let () =
SMap.iter
(fun _name (loc, x, _, set_type) ->
Base.Option.iter loc ~f:(fun _loc -> set_type (F.methodtype cx (this_default x) x)))
s.private_methods
in
(* Re-add the constructor as a method. *)
let methods =
match constructor with
Expand Down Expand Up @@ -863,7 +900,8 @@ module Make (F : Func_sig.S) = struct
(poly t_inner, poly t_outer)

(* Processes the bodies of instance and static class members. *)
let toplevels cx ~decls ~stmts ~expr ~private_property_map x =
let toplevels cx ~decls ~stmts ~expr ~private_property_map ~instance_this_type ~static_this_type x
=
let open Type in
Env.in_lex_scope (fun () ->
let new_entry ?(state = Scope.State.Initialized) t =
Expand Down Expand Up @@ -928,8 +966,14 @@ module Make (F : Func_sig.S) = struct
let instance_this_recipe = this_recipe instance_this_default in
let static_this_recipe = this_recipe static_this_default in

(* Bind private fields to the environment *)
Env.bind_class cx x.id private_property_map (to_prop_map cx x.static.private_fields);
(* Bind private fields and methods to the environment *)
Env.bind_class
cx
x.id
private_property_map
(fields_to_prop_map cx x.static.private_fields)
(methods_to_prop_map ~cx ~this_default:instance_this_type x.instance.private_methods)
(methods_to_prop_map ~cx ~this_default:static_this_type x.static.private_methods);

x
|> with_sig ~static:true (fun s ->
Expand Down
10 changes: 9 additions & 1 deletion src/typing/class_sig_intf.ml
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,10 @@ module type S = sig
(** Add private field to signature. *)
val add_private_field : string -> ALoc.t -> Polarity.t -> field -> static:bool -> t -> t

(** Add private method to signature. *)
val add_private_method :
static:bool -> string -> ALoc.t -> func_sig -> set_asts:set_asts -> set_type:set_type -> t -> t

(* Access public fields of signature *)
val public_fields_of_signature : static:bool -> t -> field' SMap.t

Expand Down Expand Up @@ -159,7 +163,9 @@ module type S = sig
Type.t -> (* self *)
Context.t -> Reason.t -> Type.typeparams -> Type.typeparam * Type.t

val to_prop_map : Context.t -> field' SMap.t -> Type.Properties.id
val this_or_mixed_of_t : static:bool -> t -> Type.t

val fields_to_prop_map : Context.t -> field' SMap.t -> Type.Properties.id

(** 1. Manipulation *)

Expand Down Expand Up @@ -193,6 +199,8 @@ module type S = sig
(ALoc.t, ALoc.t) Flow_ast.Expression.t ->
(ALoc.t, ALoc.t * Type.t) Flow_ast.Expression.t) ->
private_property_map:Type.Properties.id ->
instance_this_type:Type.t ->
static_this_type:Type.t ->
t ->
unit

Expand Down
15 changes: 13 additions & 2 deletions src/typing/env.ml
Original file line number Diff line number Diff line change
Expand Up @@ -483,11 +483,22 @@ let bind_entry cx name entry loc =
if not (is_excluded name) then loop !scopes

(* bind class entry *)
let bind_class cx class_id class_private_fields class_private_static_fields =
let bind_class
cx
class_id
class_private_fields
class_private_static_fields
class_private_methods
class_private_static_methods =
bind_entry
cx
(internal_name "class")
(Entry.new_class class_id class_private_fields class_private_static_fields)
(Entry.new_class
class_id
class_private_fields
class_private_static_fields
class_private_methods
class_private_static_methods)
ALoc.none

(* bind var entry *)
Expand Down
9 changes: 8 additions & 1 deletion src/typing/env.mli
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,14 @@ val promote_non_const :
Entry.non_const_specialization ->
Loc_collections.ALocSet.t option * Entry.non_const_specialization

val bind_class : Context.t -> ALoc.id -> Type.Properties.id -> Type.Properties.id -> unit
val bind_class :
Context.t ->
ALoc.id ->
Type.Properties.id ->
Type.Properties.id ->
Type.Properties.id ->
Type.Properties.id ->
unit

val bind_var : ?state:State.t -> Context.t -> string -> Type.annotated_or_inferred -> ALoc.t -> unit

Expand Down
88 changes: 74 additions & 14 deletions src/typing/flow_js.ml
Original file line number Diff line number Diff line change
Expand Up @@ -1700,7 +1700,8 @@ struct
instantiate_this_class cx trace reason_tapp tc this (Upper u)
| (TypeAppT _, ReposLowerT (reason, use_desc, u)) ->
rec_flow cx trace (reposition_reason cx ~trace reason ~use_desc l, u)
| (TypeAppT (reason_tapp, use_op, c, ts), MethodT (_, _, _, _, _, _)) ->
| (TypeAppT (reason_tapp, use_op, c, ts), MethodT (_, _, _, _, _, _))
| (TypeAppT (reason_tapp, use_op, c, ts), PrivateMethodT (_, _, _, _, _, _, _, _)) ->
let reason_op = reason_of_use_t u in
let t = mk_typeapp_instance cx ~trace ~use_op ~reason_op ~reason_tapp ~cache:[] c ts in
rec_flow cx trace (t, u)
Expand Down Expand Up @@ -3825,6 +3826,7 @@ struct
GetPrivatePropT (use_op, reason_op, prop_name, scopes, static, tout) ) ->
get_private_prop
~cx
~allow_method_access:false
~trace
~l
~reason_c
Expand Down Expand Up @@ -3886,10 +3888,20 @@ struct
PrivateMethodT
(use_op, reason_op, reason_lookup, prop_name, scopes, static, method_action, prop_t)
) ->
(* BoundTs from private methods are not on the InstanceT due to scoping rules,
so we need to substitute those BoundTs when the method is called. *)
let scopes = Subst.subst_class_bindings cx (SMap.singleton "this" l) scopes in
let tvar = Tvar.mk_no_wrap cx reason_lookup in
let funt = OpenT (reason_lookup, tvar) in
let l =
if static then
TypeUtil.class_type l
else
l
in
get_private_prop
~cx
~allow_method_access:true
~trace
~l
~reason_c
Expand Down Expand Up @@ -5378,6 +5390,21 @@ struct
trace
( position_generic_bound reason bound,
MethodT (use_op, call_r, lookup_r, propref, action, t_opt) )
| ( GenericT { reason; bound; _ },
PrivateMethodT (use_op, call_r, lookup_r, prop_name, scopes, static, action, prop_t) )
->
let action =
match action with
| CallM mct -> CallM { mct with meth_generic_this = Some l }
| ChainM (r1, r2, t, mct, tout) ->
ChainM (r1, r2, t, { mct with meth_generic_this = Some l }, tout)
in
rec_flow
cx
trace
( position_generic_bound reason bound,
PrivateMethodT (use_op, call_r, lookup_r, prop_name, scopes, static, action, prop_t)
)
| (GenericT { reason; bound; _ }, _) ->
rec_flow cx trace (position_generic_bound reason bound, u)
(***************)
Expand Down Expand Up @@ -6158,7 +6185,8 @@ struct
distribute_union_intersection ()
else
wait_for_concrete_bound ()
| MethodT _ ->
| MethodT _
| PrivateMethodT _ ->
if is_concrete bound && not (is_literal_type bound) then
distribute_union_intersection ()
else
Expand Down Expand Up @@ -7535,7 +7563,18 @@ struct
map

and get_private_prop
~cx ~trace ~l ~reason_c ~instance ~use_op ~reason_op ~prop_name ~scopes ~static ~tout =
~cx
~allow_method_access
~trace
~l
~reason_c
~instance
~use_op
~reason_op
~prop_name
~scopes
~static
~tout =
match scopes with
| [] ->
add_output
Expand All @@ -7546,6 +7585,7 @@ struct
if not (ALoc.equal_id scope.class_binding_id instance.class_id) then
get_private_prop
~cx
~allow_method_access
~trace
~l
~reason_c
Expand All @@ -7557,23 +7597,43 @@ struct
~static
~tout
else
let map =
let x = OrdinaryName prop_name in
let perform_lookup_action p =
let action = ReadProp { use_op; obj_t = l; tout } in
let propref = Named (reason_op, x) in
perform_lookup_action cx trace propref p PropertyMapProperty reason_c reason_op action
in
let field_maps =
if static then
scope.class_private_static_fields
else
scope.class_private_fields
in
let x = OrdinaryName prop_name in
(match NameUtils.Map.find_opt x (Context.find_props cx map) with
(match NameUtils.Map.find_opt x (Context.find_props cx field_maps) with
| Some p -> perform_lookup_action p
| None ->
add_output
cx
~trace
(Error_message.EPrivateLookupFailed ((reason_op, reason_c), x, use_op))
| Some p ->
let action = ReadProp { use_op; obj_t = l; tout } in
let propref = Named (reason_op, x) in
perform_lookup_action cx trace propref p PropertyMapProperty reason_c reason_op action)
let method_maps =
if static then
scope.class_private_static_methods
else
scope.class_private_methods
in
(match NameUtils.Map.find_opt x (Context.find_props cx method_maps) with
| Some p ->
(if not allow_method_access then
match p with
| Method (_, t) ->
add_output
cx
~trace
(Error_message.EMethodUnbinding { use_op; reason_op; reason_prop = reason_of_t t })
| _ -> ());
perform_lookup_action p
| None ->
add_output
cx
~trace
(Error_message.EPrivateLookupFailed ((reason_op, reason_c), x, use_op))))

and match_prop
cx
Expand Down
6 changes: 4 additions & 2 deletions src/typing/generics/scoped_tvar_finder.ml
Original file line number Diff line number Diff line change
Expand Up @@ -229,8 +229,10 @@ class finder cx =
Signature_utils.is_munged_property_string label && Context.should_munge_underscores cx
in
(Utils_js.spf "property `%s`" label, is_munged)
| Ast.Expression.Object.Property.PrivateName _ ->
failwith "class_private_field should have been called instead"
| Ast.Expression.Object.Property.PrivateName private_name ->
let _ = this#private_name private_name in
let (_, { Ast.PrivateName.id = (_, { Ast.Identifier.name; _ }); _ }) = private_name in
(Utils_js.spf "private property `%s`" name, false)
| Ast.Expression.Object.Property.Computed _ -> ("computed property", false)

method! class_private_field field =
Expand Down
16 changes: 14 additions & 2 deletions src/typing/scope.ml
Original file line number Diff line number Diff line change
Expand Up @@ -114,8 +114,20 @@ module Entry = struct
| Class of Type.class_binding

(* constructors *)
let new_class class_binding_id class_private_fields class_private_static_fields =
Class { Type.class_binding_id; Type.class_private_fields; Type.class_private_static_fields }
let new_class
class_binding_id
class_private_fields
class_private_static_fields
class_private_methods
class_private_static_methods =
Class
{
Type.class_binding_id;
class_private_fields;
class_private_static_fields;
class_private_methods;
class_private_static_methods;
}

let new_value kind state specific ?closure_writes general value_declare_loc =
Value
Expand Down
Loading

0 comments on commit 9d7b2a7

Please sign in to comment.