Skip to content

Commit a264781

Browse files
Andrew Kennedymeta-codesync[bot]
authored andcommitted
Expand splat tuple in signature help
Summary: User experience with methods and functions making use of type-splat parameters is degraded compared to the equivalent ordinary functions. In particular, "signature help", and also various features of auto-complete (e.g. enum constants) are blocked by the presence of a splat parameter e.g. in the following code ``` class Splat<Targs as (mixed...)> {} final class TwoInts extends Splat<(int, int)> {} class MyOtherClass { public static function genericSplatMethod<Targs as (mixed...)>( Splat<Targs> $splat, ... Targs $args, ): void {} } function testsplat(): void { MyOtherClass::genericSplatMethod(new TwoInts(), 1, 2); } ``` Now suppose you are typing the call to `genericSplatMethod` in the IDE and you've typed the first argument. This is the current behaviour: {F1984875663} Better would be to see the splat expanded, to multiple arguments: {F1984875638} To make this happen, we add a helper function `expand_splat_param_in_function_type` to `Tast_env` that does this expansion of `...(int, int) $args` to `int $args[0], int $args[1]`. The unexpanded splat also blocks various completions such as enums. Solution is to call the helper function in appropriate places in `autocompleteService.ml`. Reviewed By: francesco-zappa-nardelli Differential Revision: D91140789 fbshipit-source-id: 144873217c882837d6cdf9aec4028f96f75903d4
1 parent 806dcb9 commit a264781

13 files changed

Lines changed: 247 additions & 15 deletions

hphp/hack/src/client_and_server/autocompleteService.ml

Lines changed: 20 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,7 @@ let expand_and_strip_dynamic env ty =
8888
let ty = Tast_env.strip_dynamic env ty in
8989
Tast_env.expand_type env ty
9090

91+
(* Strip both ~ and supportdyn off a type *)
9192
let expand_and_strip_supportdyn env ty =
9293
let (env, ty) = expand_and_strip_dynamic env ty in
9394
let (_, _, ty) =
@@ -96,6 +97,16 @@ let expand_and_strip_supportdyn env ty =
9697
in
9798
ty
9899

100+
(* Strip ~ and supportdyn off a type that is expected to be a function type.
101+
* Also expand splatted tuples to multiple parameters
102+
*)
103+
let expand_fun env ty =
104+
let ty = expand_and_strip_supportdyn env ty in
105+
let ty = Tast_env.expand_splat_param_in_function_type env ty in
106+
match get_node ty with
107+
| Tfun ft -> Some ft
108+
| _ -> None
109+
99110
let expand_and_strip_dynamic env ty =
100111
let (_, ty) = expand_and_strip_dynamic env ty in
101112
ty
@@ -334,9 +345,8 @@ let insert_text_for_ty
334345
Tast_env.localize_no_subst env ~ignore_errors:true decl_ty
335346
in
336347
(* Functions that support dynamic will be wrapped by supportdyn<_> *)
337-
let ty = expand_and_strip_supportdyn env ty in
338-
match Typing_defs.get_node ty with
339-
| Tfun ft -> insert_text_for_fun_call env autocomplete_context label ft
348+
match expand_fun env ty with
349+
| Some ft -> insert_text_for_fun_call env autocomplete_context label ft
340350
| _ -> InsertLiterally label
341351

342352
let autocomplete_shape_key autocomplete_context fields id =
@@ -1142,10 +1152,8 @@ let autocomplete_enum_class_label_call env f args =
11421152
String.equal Naming_special_names.Classes.cEnumClassLabel name
11431153
in
11441154
let (fty, _, _) = f in
1145-
(* Functions that support dynamic will be wrapped by supportdyn<_> *)
1146-
let fty = expand_and_strip_supportdyn env fty in
1147-
match get_node fty with
1148-
| Tfun { ft_params; _ } ->
1155+
match expand_fun env fty with
1156+
| Some { ft_params; _ } ->
11491157
let ty_args = zip_truncate args ft_params in
11501158
List.iter
11511159
~f:(fun (arg, arg_ty) ->
@@ -2040,14 +2048,13 @@ let visitor
20402048
| _ -> ())
20412049
| _ -> ()
20422050
end
2043-
| (_, _, Aast.(Call { func = (recv_ty, _, _); args; _ })) ->
2044-
(* Functions that support dynamic will be wrapped by supportdyn<_> *)
2045-
let recv_ty = expand_and_strip_supportdyn env recv_ty in
2046-
(match deref recv_ty with
2047-
| (_r, Tfun ft) ->
2051+
| (_, _, Aast.(Call { func = (recv_ty, _, _); args; _ })) -> begin
2052+
match expand_fun env recv_ty with
2053+
| Some ft ->
20482054
autocomplete_shape_literal_in_call env ft args;
20492055
autocomplete_enum_value_in_call env ft args
2050-
| _ -> ())
2056+
| _ -> ()
2057+
end
20512058
| (_, p, Aast.EnumClassLabel (opt_cname, n)) when is_auto_complete n ->
20522059
autocomplete_enum_class_label env opt_cname (p, n) None
20532060
| (_, _, Aast.Efun { Aast.ef_fun = f; _ })

hphp/hack/src/client_and_server/serverSignatureHelp.ml

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -168,6 +168,12 @@ let go_quarantined
168168
| Some info ->
169169
let ty = ServerInferType.get_type info in
170170
let tast_env = ServerInferType.get_env info in
171+
(* Expand splat parameters e.g.
172+
* function(int $a, ...(string, bool) $b):void
173+
* will be shown as
174+
* function(int $a, string $b[0], bool $b[1]):void
175+
*)
176+
let ty = Tast_env.expand_splat_param_in_function_type tast_env ty in
171177
let siginfo_label =
172178
Tast_env.print_ty_with_identity tast_env ty occurrence def_opt
173179
in

hphp/hack/src/typing/tast_env.ml

Lines changed: 94 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,101 @@ let print_error_ty = Typing_print.error
4444
let print_hint env hint =
4545
print_decl_ty env @@ Decl_hint.hint (get_decl_env env) hint
4646

47+
let make_param_from_tuple ~is_optional ~splat ix pos name ty =
48+
Typing_defs.
49+
{
50+
fp_pos = pos;
51+
fp_name =
52+
(match name with
53+
| None -> None
54+
| Some n -> Some (Printf.sprintf "%s[%d]" n ix));
55+
fp_type = ty;
56+
fp_flags =
57+
make_fp_flags
58+
~mode:FPnormal
59+
~accept_disposable:false
60+
~is_optional
61+
~readonly:false
62+
~ignore_readonly_error:false
63+
~splat
64+
~named:false;
65+
fp_def_value = None;
66+
}
67+
68+
(* Given a function type whose last parameter $p is a type splat of a tuple,
69+
* expand the tuple into parameters named $p[0], $p[1] etc.
70+
*
71+
* For example
72+
* (function(int $x, ...(string, optional bool) $p):void)
73+
* expands to
74+
* (function(int $x, string $p[0], optional bool $p[1]):void)
75+
*)
76+
let expand_splat_param_in_function_type env ty =
77+
let (_, ty) = Typing_env.expand_type env ty in
78+
match Typing_defs.deref ty with
79+
| (r, Typing_defs.Tfun ft) ->
80+
let (ft_params, ft_flags) =
81+
match List.last ft.ft_params with
82+
| Some fp when Typing_defs.get_fp_splat fp -> begin
83+
let (_, ty) = Typing_env.expand_type env fp.fp_type in
84+
match Typing_defs.get_node ty with
85+
| Typing_defs.Ttuple { t_required; t_optional; t_extra } ->
86+
let ft_params =
87+
List.drop_last_exn ft.ft_params
88+
@ List.mapi t_required ~f:(fun ix ty ->
89+
make_param_from_tuple
90+
~is_optional:false
91+
~splat:false
92+
ix
93+
fp.fp_pos
94+
fp.fp_name
95+
ty)
96+
@ List.mapi t_optional ~f:(fun ix ty ->
97+
make_param_from_tuple
98+
~is_optional:true
99+
~splat:false
100+
(ix + List.length t_required)
101+
fp.fp_pos
102+
fp.fp_name
103+
ty)
104+
in
105+
let (extra_params, ft_flags) =
106+
match t_extra with
107+
(* Closed tuples are represented by a variadic type of nothing *)
108+
| Tvariadic ty when Typing_utils.is_nothing env ty ->
109+
([], ft.ft_flags)
110+
(* Convert final splat or variadic tuple element into splat or variadic parameter *)
111+
| Tvariadic ty
112+
| Tsplat ty ->
113+
let splat =
114+
match t_extra with
115+
| Tsplat _ -> true
116+
| Tvariadic _ -> false
117+
in
118+
( [
119+
make_param_from_tuple
120+
~is_optional:false
121+
~splat
122+
(List.length t_optional + List.length t_required)
123+
fp.fp_pos
124+
fp.fp_name
125+
ty;
126+
],
127+
if splat then
128+
ft.ft_flags
129+
else
130+
Typing_defs_flags.Fun.set_variadic true ft.ft_flags )
131+
in
132+
(ft_params @ extra_params, ft_flags)
133+
| _ -> (ft.ft_params, ft.ft_flags)
134+
end
135+
| _ -> (ft.ft_params, ft.ft_flags)
136+
in
137+
Typing_defs.mk (r, Typing_defs.Tfun { ft with ft_params; ft_flags })
138+
| _ -> ty
139+
47140
let print_ty_with_identity env ty sym_occurrence sym_definition =
141+
let ty = expand_splat_param_in_function_type env ty in
48142
Typing_print.full_with_identity
49143
~hide_internals:true
50144
env

hphp/hack/src/typing/tast_env.mli

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,12 @@ val expand_type : env -> Tast.ty -> env * Tast.ty
9292
recursively replacing them with the type they refer to. *)
9393
val fully_expand : env -> Tast.ty -> Tast.ty
9494

95+
(** Eliminate a type splat parameter in a function type that has been
96+
* instantiated to a tuple e.g. function(int $i, ...(string,bool) $a)
97+
* expands to function(int $i, string $a[0], bool $a[1])
98+
*)
99+
val expand_splat_param_in_function_type : env -> Tast.ty -> Tast.ty
100+
95101
(** Strip ~ from type *)
96102
val strip_dynamic : env -> Tast.ty -> Tast.ty
97103

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
<?hh
2+
3+
enum MyEnum: string {
4+
TYPE_A = "A value";
5+
TYPE_B = "B value";
6+
TYPE_C = "C value";
7+
}
8+
9+
class Splat<Targs as (mixed...)> {}
10+
final class Three extends Splat<(int, MyEnum, optional int)> {}
11+
function generic_splat_function<Targs as (mixed...)>(
12+
Splat<Targs> $splat,
13+
... Targs $args,
14+
): void {}
15+
16+
function demo(): void {
17+
generic_splat_function(new Three(), 2, AUTO332);
18+
}
Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
MyEnum::TYPE_C
2+
INSERT MyEnum::TYPE_C
3+
class constant
4+
SORT TEXT: none
5+
MyEnum::TYPE_B
6+
INSERT MyEnum::TYPE_B
7+
class constant
8+
SORT TEXT: none
9+
MyEnum::TYPE_A
10+
INSERT MyEnum::TYPE_A
11+
class constant
12+
SORT TEXT: none
Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
<?hh
2+
3+
enum MyEnum: string {
4+
TYPE_A = "A value";
5+
TYPE_B = "B value";
6+
TYPE_C = "C value";
7+
}
8+
9+
class Splat<Targs as (mixed...)> {}
10+
final class Three extends Splat<(int, MyEnum, optional int)> {}
11+
function generic_splat_function<Targs as (mixed...)>(
12+
Splat<Targs> $splat,
13+
... Targs $args,
14+
): void {}
15+
16+
function demo(): void {
17+
// We should only offer concrete values here, not e.g. MyEnum::getValues().
18+
generic_splat_function(new Three(), 2, MyEnum::AUTO332);
19+
}
Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
TYPE_C
2+
INSERT TYPE_C
3+
class constant
4+
SORT TEXT: none
5+
TYPE_B
6+
INSERT TYPE_B
7+
class constant
8+
SORT TEXT: none
9+
TYPE_A
10+
INSERT TYPE_A
11+
class constant
12+
SORT TEXT: none
Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
<?hh
2+
3+
type TS = shape('foo' => int, 'foobar' => float);
4+
5+
class Splat<Targs as (mixed...)> {}
6+
final class Three extends Splat<(int, TS, optional int)> {}
7+
function generic_splat_function<Targs as (mixed...)>(
8+
Splat<Targs> $splat,
9+
... Targs $args,
10+
): void {}
11+
12+
function demo(): void {
13+
generic_splat_function(new Three(), 2, shape('AUTO332'));
14+
}
Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
foo
2+
INSERT foo
3+
literal
4+
SORT TEXT: none
5+
foobar
6+
INSERT foobar
7+
literal
8+
SORT TEXT: none

0 commit comments

Comments
 (0)