Skip to content

Commit 608b318

Browse files
mheiberfacebook-github-bot
authored andcommitted
refactor Tast_env.get_member to be explicit
Summary: ## What Rather than using a unlabeled boolean to decide which function to call, break get_member into two functions. ## Why - safer (harder to mix up) - a step toward consistency in the codebase ## Context This came up in diff review for another diff https://www.internalfb.com/diff/D78688689?dst_version_fbid=1237203468181760&transaction_fbid=1405234473924772 ## Alternative considered I considered using a labeled parameter, but - This would make it so that we have three patterns in the code base for the same thing, rather than just two. - I think it's generally good practice to use normal function dispatch Reviewed By: madgen Differential Revision: D79905615 fbshipit-source-id: 95e7171d1d288e183df7fbf5ac1d4c1aa87fdc5a
1 parent b7f4594 commit 608b318

File tree

9 files changed

+50
-24
lines changed

9 files changed

+50
-24
lines changed

hphp/hack/src/typing/env/typing_env.ml

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -942,18 +942,22 @@ module M = struct
942942
in
943943
suggest_member members mid
944944

945-
let get_member is_method env (class_ : Cls.t) mid =
945+
let get_method env (class_ : Cls.t) mid =
946946
(* The type of a member is stored separately in the heap. This means that
947947
* any user of the member also has a dependency on the class where the member
948948
* originated.
949949
*)
950-
let ce_opt =
951-
if is_method then
952-
Cls.get_method class_ mid [@alert "-dependencies"]
953-
else
954-
Cls.get_prop class_ mid [@alert "-dependencies"]
955-
in
956-
Deps.add_member_dep ~is_method ~is_static:false env class_ mid ce_opt;
950+
let ce_opt = (Cls.get_method class_ mid [@alert "-dependencies"]) in
951+
Deps.add_member_dep ~is_method:true ~is_static:false env class_ mid ce_opt;
952+
ce_opt
953+
954+
let get_prop env (class_ : Cls.t) mid =
955+
(* The type of a member is stored separately in the heap. This means that
956+
* any user of the member also has a dependency on the class where the member
957+
* originated.
958+
*)
959+
let ce_opt = (Cls.get_prop class_ mid [@alert "-dependencies"]) in
960+
Deps.add_member_dep ~is_method:false ~is_static:false env class_ mid ce_opt;
957961
ce_opt
958962

959963
let suggest_member is_method class_ mid =

hphp/hack/src/typing/env/typing_env.mli

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -49,8 +49,11 @@ module Expose_to_tast_env : sig
4949
(** Get class constants declaration from the appropriate backend and add dependency. *)
5050
val consts : env -> class_decl -> (string * class_const) list
5151

52-
(** Get class member declaration from the appropriate backend and add dependency. *)
53-
val get_member : bool -> env -> class_decl -> string -> class_elt option
52+
(** Get class method declaration from the appropriate backend and add dependency. *)
53+
val get_method : env -> class_decl -> string -> class_elt option
54+
55+
(** Get class property declaration from the appropriate backend and add dependency. *)
56+
val get_prop : env -> class_decl -> string -> class_elt option
5457

5558
(** Get static member declaration of a class from the appropriate backend and add dependency. *)
5659
val get_static_member :

hphp/hack/src/typing/folded_class.mli

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -126,7 +126,7 @@ val get_typeconst : t -> string -> typeconst_type option
126126
val get_prop : t -> string -> class_elt option
127127
[@@alert
128128
dependencies
129-
"Direct use of `get_prop` will not register a dependency. You probably want to use `Typing_env.get_member` instead"]
129+
"Direct use of `get_prop` will not register a dependency. You probably want to use `Typing_env.get_prop` instead"]
130130

131131
val get_sprop : t -> string -> class_elt option
132132
[@@alert
@@ -136,7 +136,7 @@ val get_sprop : t -> string -> class_elt option
136136
val get_method : t -> string -> class_elt option
137137
[@@alert
138138
dependencies
139-
"Direct use of `get_method` will not register a dependency. You probably want to use `Typing_env.get_member` instead"]
139+
"Direct use of `get_method` will not register a dependency. You probably want to use `Typing_env.get_method` instead"]
140140

141141
val get_smethod : t -> string -> class_elt option
142142
[@@alert

hphp/hack/src/typing/nastInitCheck.ml

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -68,8 +68,7 @@ let lookup_props env class_name props =
6868
else
6969
Typing_env.get_class env class_name
7070
|> Decl_entry.to_option
71-
|> Option.bind ~f:(fun cls ->
72-
Typing_env.get_member false env cls name)
71+
|> Option.bind ~f:(fun cls -> Typing_env.get_prop env cls name)
7372
|> Option.bind ~f:(fun ce ->
7473
Some (Lazy.force ce.Typing_defs.ce_type))
7574
in

hphp/hack/src/typing/tast_check/const_write_check.ml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ let check_static_const_prop tenv class_ (pos, id) =
2424

2525
(* Requires id to be a property *)
2626
let check_const_prop env tenv class_ (pos, id) cty =
27-
let cprop = Typing_env.get_member false tenv class_ id in
27+
let cprop = Typing_env.get_prop tenv class_ id in
2828
Option.iter cprop ~f:(fun ce ->
2929
if get_ce_const ce then
3030
if

hphp/hack/src/typing/typing.ml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5213,7 +5213,7 @@ end = struct
52135213
let* dsl_name = dsl_opt in
52145214
let* cls = Env.get_class env dsl_name |> Decl_entry.to_option in
52155215
let* { ce_type = (lazy fun_ty); _ } =
5216-
Env.get_member true env cls SN.ExpressionTrees.splice
5216+
Env.get_method env cls SN.ExpressionTrees.splice
52175217
in
52185218
(* Pull the type of third argument to `MyDsl::splice()`.
52195219
If we change the number of arguments this method takes, we might need to
@@ -8882,7 +8882,7 @@ end = struct
88828882
let expr = ((), pos, expr_) in
88838883
unbound_name env class_name expr
88848884
| Decl_entry.Found folded_class -> begin
8885-
match Env.get_member true env folded_class (snd method_name) with
8885+
match Env.get_method env folded_class (snd method_name) with
88868886
| None ->
88878887
let () =
88888888
let err = unbound_method class_name method_name folded_class in

hphp/hack/src/typing/typing_extends.ml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -827,7 +827,7 @@ let maybe_poison_ancestors
827827
| Decl_entry.NotYetAvailable ->
828828
(child_class, child_return_ty)
829829
| Decl_entry.Found c ->
830-
(match Env.get_member true env c member_name with
830+
(match Env.get_method env c member_name with
831831
| None -> (child_class, child_return_ty)
832832
| Some elt ->
833833
let (lazy fty) = elt.ce_type in

hphp/hack/src/typing/typing_exts.ml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -58,8 +58,8 @@ let lookup_magic_type (env : env) use_pos (class_ : locl_ty) (fname : string) :
5858
let ce_type =
5959
let lookup_def c =
6060
Option.first_some
61-
(Env.get_member true env c fname)
62-
(Env.get_member true env c "format_wild")
61+
(Env.get_method env c fname)
62+
(Env.get_method env c "format_wild")
6363
in
6464
Env.get_class env className |> Decl_entry.to_option >>= lookup_def
6565
>>= fun { ce_type = (lazy ty); ce_pos = (lazy pos); _ } ->

hphp/hack/src/typing/typing_object_get.ml

Lines changed: 24 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -222,7 +222,12 @@ let sound_dynamic_err_opt args env ((_, id_str) as id) read_context =
222222
| Decl_entry.Found self_class
223223
when Cls.get_support_dynamic_type self_class || not (Cls.final self_class)
224224
->
225-
(match Env.get_member args.is_method env self_class id_str with
225+
(match
226+
if args.is_method then
227+
Env.get_method env self_class id_str
228+
else
229+
Env.get_prop env self_class id_str
230+
with
226231
| Some { ce_visibility = Vprivate _; ce_type = (lazy ty); _ }
227232
when not args.is_method ->
228233
if read_context then
@@ -274,7 +279,12 @@ let widen_class_for_obj_get ~is_method ~nullsafe member_name env ty =
274279
| Decl_entry.NotYetAvailable ->
275280
default ()
276281
| Decl_entry.Found class_info ->
277-
(match Env.get_member is_method env class_info member_name with
282+
(match
283+
if is_method then
284+
Env.get_method env class_info member_name
285+
else
286+
Env.get_prop env class_info member_name
287+
with
278288
| Some { ce_origin; _ } ->
279289
(* If this member was inherited then we obtain the type from which
280290
* it is inherited as our wider type *)
@@ -534,7 +544,12 @@ and obj_get_concrete_class
534544
else
535545
(env, params)
536546
in
537-
let old_member_info = Env.get_member args.is_method env class_info id_str in
547+
let old_member_info =
548+
if args.is_method then
549+
Env.get_method env class_info id_str
550+
else
551+
Env.get_prop env class_info id_str
552+
in
538553
let self_id = Option.value (Env.get_self_id env) ~default:"" in
539554
let ancestor_tyargs =
540555
match Cls.get_ancestor class_info self_id with
@@ -565,7 +580,12 @@ and obj_get_concrete_class
565580
| Decl_entry.NotYetAvailable ->
566581
(old_member_info, false)
567582
| Decl_entry.Found self_class -> begin
568-
match Env.get_member args.is_method env self_class id_str with
583+
match
584+
if args.is_method then
585+
Env.get_method env self_class id_str
586+
else
587+
Env.get_prop env self_class id_str
588+
with
569589
| Some ({ ce_visibility = Vprivate _; _ } as ce) ->
570590
let ce =
571591
Decl_instantiate.(

0 commit comments

Comments
 (0)