Skip to content

Commit f9363cb

Browse files
mheibermeta-codesync[bot]
mheiber
authored and
meta-codesync[bot]
committed
noop refactor: extract needs_concrete logic
Summary: ## What Extract the logic for needs-concrete checks to a separate file. No change in behavior ## Why Maintainability: - It was hard to tell if `--config needs_concrete=true` was affecting the behavior of other checks, due to needs_concrete concerns being mixed in with a big pattern match. - Stop typing.ml bloat ## Context https://fburl.com/hack_needs_concrete Reviewed By: madgen Differential Revision: D90016485 fbshipit-source-id: c137336a159bd4138dca93a891df16848b41932b
1 parent 35048e8 commit f9363cb

4 files changed

Lines changed: 163 additions & 91 deletions

File tree

‎hphp/hack/src/typing/dune‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -231,6 +231,7 @@
231231
typing_local_ops
232232
typing_named_params
233233
typing_native
234+
typing_needs_concrete
234235
typing_param
235236
typing_per_cont_ops
236237
typing_regex

‎hphp/hack/src/typing/typing.ml‎

Lines changed: 8 additions & 91 deletions
Original file line numberDiff line numberDiff line change
@@ -1125,31 +1125,7 @@ let is_hack_collection env ty =
11251125

11261126
let check_class_get
11271127
env p def_pos cid mid ce (_, _cid_pos, e) function_pointer is_method =
1128-
let needs_concrete_is_enabled =
1129-
TypecheckerOptions.needs_concrete env.genv.tcopt
1130-
in
1131-
let callee_is_needs_concrete_method =
1132-
is_method && get_ce_readonly_prop_or_needs_concrete ce
1133-
in
1134-
let check_needs_concrete_call (via : [ `Static | `Self | `Parent ]) : unit =
1135-
(* `self` and `parent` forward the referent of `static` so are just as dangerous *)
1136-
if
1137-
needs_concrete_is_enabled
1138-
&& callee_is_needs_concrete_method
1139-
&& not (Env.static_points_to_concrete_class env)
1140-
then
1141-
Typing_warning_utils.add
1142-
env
1143-
( p,
1144-
Typing_warning.Call_needs_concrete,
1145-
{
1146-
Typing_warning.Call_needs_concrete.call_pos = p;
1147-
class_name = cid;
1148-
meth_name = mid;
1149-
decl_pos = def_pos;
1150-
via = (via :> [ `Id | `Static | `Self | `Parent ]);
1151-
} )
1152-
in
1128+
Typing_needs_concrete.check_class_get env p def_pos cid mid ce e is_method;
11531129
match e with
11541130
| CIself when get_ce_abstract ce -> begin
11551131
match Env.get_self_id env with
@@ -1235,33 +1211,6 @@ let check_class_get
12351211
primary
12361212
@@ Primary.Static_synthetic_method
12371213
{ class_name; meth_name = mid; pos = p; decl_pos = def_pos })
1238-
| CI _ when needs_concrete_is_enabled && callee_is_needs_concrete_method ->
1239-
Env.get_class env cid
1240-
|> Decl_entry.to_option
1241-
|> Option.iter ~f:(fun class_ ->
1242-
let is_concrete =
1243-
let is_non_abstract = not (Cls.abstract class_) in
1244-
let is_final_non_consistent_construct =
1245-
match snd @@ Typing_env.get_construct env class_ with
1246-
| FinalClass -> true
1247-
| Inconsistent
1248-
| ConsistentConstruct ->
1249-
false
1250-
in
1251-
is_non_abstract || is_final_non_consistent_construct
1252-
in
1253-
if not is_concrete then
1254-
Typing_warning_utils.add
1255-
env
1256-
( p,
1257-
Typing_warning.Call_needs_concrete,
1258-
{
1259-
Typing_warning.Call_needs_concrete.call_pos = p;
1260-
class_name = cid;
1261-
meth_name = mid;
1262-
decl_pos = def_pos;
1263-
via = `Id;
1264-
} ))
12651214
| CI (_, class_name) ->
12661215
(match Env.get_class env class_name with
12671216
| Decl_entry.NotYetAvailable
@@ -1330,32 +1279,11 @@ let check_class_get
13301279
})
13311280
end
13321281
end)
1333-
| CIself -> check_needs_concrete_call `Self
1334-
| CIparent -> check_needs_concrete_call `Parent
1335-
| CIstatic ->
1336-
let () = check_needs_concrete_call `Static in
1337-
if
1338-
needs_concrete_is_enabled
1339-
&& get_ce_abstract ce
1340-
&& not (Env.static_points_to_concrete_class env)
1341-
then
1342-
(* We check for abstract access via `static`
1343-
* as part of the "needs concrete" feature, because
1344-
* checking for calls to `abstract` functions for
1345-
* `self`/`parent`/classname, etc. is already covered by other type
1346-
* errors such as Primary.Self_abstract_call, Primary.Parent_abstract_call, etc.
1347-
*)
1348-
Typing_warning_utils.add
1349-
env
1350-
( p,
1351-
Typing_warning.Abstract_access_via_static,
1352-
{
1353-
Typing_warning.Abstract_access_via_static.access_pos = p;
1354-
class_name = cid;
1355-
member_name = mid;
1356-
decl_pos = def_pos;
1357-
} )
1358-
| CIexpr _ -> ()
1282+
| CIself
1283+
| CIparent
1284+
| CIstatic
1285+
| CIexpr _ ->
1286+
()
13591287

13601288
module Fun_id : sig
13611289
(** Synthesize the type of a function identifier. If no type arguments are
@@ -11933,24 +11861,13 @@ end = struct
1193311861
let (env, tal, te, classes) =
1193411862
class_id_for_new ~exact ~is_attribute ~is_catch p env cid explicit_targs
1193511863
in
11864+
Typing_needs_concrete.check_instantiation env p cid;
1193611865
begin
1193711866
match cid with
1193811867
| CIstatic
1193911868
when TypecheckerOptions.needs_concrete env.genv.tcopt
1194011869
&& not (Env.static_points_to_concrete_class env) ->
11941-
Env.get_self_class env
11942-
|> Decl_entry.to_option
11943-
|> Option.iter ~f:(fun class_ ->
11944-
Typing_warning_utils.add
11945-
env
11946-
( p,
11947-
Typing_warning.Uninstantiable_class_via_static,
11948-
{
11949-
Typing_warning.Uninstantiable_class_via_static.usage_pos =
11950-
p;
11951-
class_name = Cls.name class_;
11952-
decl_pos = Cls.pos class_;
11953-
} ))
11870+
()
1195411871
| _ ->
1195511872
List.iter classes ~f:(function
1195611873
| `Dynamic -> ()

‎hphp/hack/src/typing/typing_needs_concrete.ml‎

Lines changed: 127 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,127 @@
1+
(*
2+
* Copyright (c) Meta Platforms, Inc. and affiliates.
3+
*
4+
* This source code is licensed under the MIT license found in the
5+
* LICENSE file in the "hack" directory of this source tree.
6+
*
7+
*)
8+
open Hh_prelude
9+
10+
let check_class_get
11+
(env : Typing_env_types.env)
12+
(class_get_pos : Pos.t)
13+
(def_pos : Pos_or_decl.t)
14+
(cid : string)
15+
(mid : string)
16+
(ce : Typing_defs.class_elt)
17+
(e : ('ex, 'en) Aast_defs.class_id_)
18+
(is_method : bool) : unit =
19+
if TypecheckerOptions.needs_concrete env.genv.tcopt then
20+
let callee_is_needs_concrete_method : bool =
21+
is_method && Typing_defs.get_ce_readonly_prop_or_needs_concrete ce
22+
in
23+
let check_needs_concrete_call (via : [ `Static | `Self | `Parent ]) : unit =
24+
(* `self` and `parent` forward the referent of `static` so are just as dangerous *)
25+
if
26+
callee_is_needs_concrete_method
27+
&& not (Typing_env.static_points_to_concrete_class env)
28+
then
29+
Typing_warning_utils.add
30+
env
31+
( class_get_pos,
32+
Typing_warning.Call_needs_concrete,
33+
{
34+
Typing_warning.Call_needs_concrete.call_pos = class_get_pos;
35+
class_name = cid;
36+
meth_name = mid;
37+
decl_pos = def_pos;
38+
via = (via :> [ `Id | `Static | `Self | `Parent ]);
39+
} )
40+
in
41+
begin
42+
match e with
43+
| CI _ when callee_is_needs_concrete_method ->
44+
Typing_env.get_class env cid
45+
|> Decl_entry.to_option
46+
|> Option.iter ~f:(fun (class_ : Decl_provider.class_decl) ->
47+
let is_concrete : bool =
48+
let is_non_abstract : bool =
49+
not (Folded_class.abstract class_)
50+
in
51+
let is_final_non_consistent_construct : bool =
52+
match snd @@ Typing_env.get_construct env class_ with
53+
| Typing_defs.FinalClass -> true
54+
| Typing_defs.Inconsistent
55+
| Typing_defs.ConsistentConstruct ->
56+
false
57+
in
58+
is_non_abstract || is_final_non_consistent_construct
59+
in
60+
if not is_concrete then
61+
Typing_warning_utils.add
62+
env
63+
( class_get_pos,
64+
Typing_warning.Call_needs_concrete,
65+
{
66+
Typing_warning.Call_needs_concrete.call_pos =
67+
class_get_pos;
68+
class_name = cid;
69+
meth_name = mid;
70+
decl_pos = def_pos;
71+
via = `Id;
72+
} ))
73+
| CIself -> check_needs_concrete_call `Self
74+
| CIparent -> check_needs_concrete_call `Parent
75+
| CIstatic ->
76+
let () = check_needs_concrete_call `Static in
77+
if
78+
Typing_defs.get_ce_abstract ce
79+
&& not (Typing_env.static_points_to_concrete_class env)
80+
then
81+
(* We check for abstract access via `static`
82+
* as part of the "needs concrete" feature, because
83+
* checking for calls to `abstract` functions for
84+
* `self`/`parent`/classname, etc. is already covered by other type
85+
* errors such as Primary.Self_abstract_call, Primary.Parent_abstract_call, etc.
86+
*)
87+
Typing_warning_utils.add
88+
env
89+
( class_get_pos,
90+
Typing_warning.Abstract_access_via_static,
91+
{
92+
Typing_warning.Abstract_access_via_static.access_pos =
93+
class_get_pos;
94+
class_name = cid;
95+
member_name = mid;
96+
decl_pos = def_pos;
97+
} )
98+
| CI _ -> ()
99+
| CIexpr _ -> ()
100+
end
101+
102+
let check_instantiation
103+
(env : Typing_env_types.env)
104+
(instantiation_pos : Pos.t)
105+
(cid : ('ex, 'en) Aast_defs.class_id_) : unit =
106+
if TypecheckerOptions.needs_concrete env.genv.tcopt then
107+
match cid with
108+
| CIstatic when not (Typing_env.static_points_to_concrete_class env) ->
109+
Typing_env.get_self_class env
110+
|> Decl_entry.to_option
111+
|> Option.iter ~f:(fun (class_ : Decl_provider.class_decl) ->
112+
Typing_warning_utils.add
113+
env
114+
( instantiation_pos,
115+
Typing_warning.Uninstantiable_class_via_static,
116+
{
117+
Typing_warning.Uninstantiable_class_via_static.usage_pos =
118+
instantiation_pos;
119+
class_name = Folded_class.name class_;
120+
decl_pos = Folded_class.pos class_;
121+
} ))
122+
| CIstatic
123+
| CIself
124+
| CIparent
125+
| CI _
126+
| CIexpr _ ->
127+
()

‎hphp/hack/src/typing/typing_needs_concrete.mli‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
(*
2+
* Copyright (c) Meta Platforms, Inc. and affiliates.
3+
*
4+
* This source code is licensed under the MIT license found in the
5+
* LICENSE file in the "hack" directory of this source tree.
6+
*
7+
*)
8+
9+
(** Produce warnings for "needs concrete" violations.
10+
* https://fburl.com/hack_needs_concrete
11+
*)
12+
val check_class_get :
13+
Typing_env_types.env ->
14+
Pos.t ->
15+
Pos_or_decl.t ->
16+
string ->
17+
string ->
18+
Typing_defs.class_elt ->
19+
('ex, 'en) Aast_defs.class_id_ ->
20+
bool ->
21+
unit
22+
23+
(** Produce warnings for "needs concrete" violations.
24+
* https://fburl.com/hack_needs_concrete
25+
*)
26+
val check_instantiation :
27+
Typing_env_types.env -> Pos.t -> ('ex, 'en) Aast_defs.class_id_ -> unit

0 commit comments

Comments
 (0)