From 9d3e223c5c35300f08ab43fb8b888d0d416fb1d7 Mon Sep 17 00:00:00 2001 From: James Wu Date: Wed, 6 Dec 2017 10:48:26 -0800 Subject: [PATCH] Typing changes for reactive functions Summary: This adds a bunch of checks for reactive functions. Specifically: 1. Reactive functions may only call other reactive functions 2. Reactive functions may not set object properties(when we add mutability, we can allow this behavior for mutable objects) 3. Reactive functions cannot modify(append or set to) Vectors or Sets. 4. Reactive functions are subtypes of nonreactive functions, but not vice versa This isn't all the checks we need to make for reactive functions. Namely, at least these are missing: 1. Lambdas in reactive functions are treated automatically as non-reactive for now(until we can agree on a behavior here). Reviewed By: KendallHopkins Differential Revision: D6317851 fbshipit-source-id: 08e0c150cfd80bb2eca160dca4bbc088a7dfdf1e --- hphp/hack/src/typing/typing.ml | 37 ++++++++++++++++++++-- hphp/hack/src/typing/typing_env.ml | 5 +++ hphp/hack/src/typing/typing_env.mli | 2 ++ hphp/hack/src/typing/typing_env_types.ml | 2 ++ hphp/hack/src/typing/typing_env_types_sig.mli | 1 + hphp/hack/src/typing/typing_subtype.ml | 3 ++ hphp/hack/src/utils/errors/errors.ml | 25 +++++++++++++++ hphp/hack/src/utils/errors/errors_sig.ml | 4 +++ .../test/typecheck/reactive/reactive_calls1.php | 19 +++++++++++ .../typecheck/reactive/reactive_calls1.php.exp | 2 ++ .../test/typecheck/reactive/reactive_calls2.php | 14 ++++++++ .../typecheck/reactive/reactive_calls2.php.exp | 1 + .../test/typecheck/reactive/reactive_calls3.php | 11 +++++++ .../typecheck/reactive/reactive_calls3.php.exp | 2 ++ .../test/typecheck/reactive/reactive_calls4.php | 28 ++++++++++++++++ .../typecheck/reactive/reactive_calls4.php.exp | 6 ++++ .../typecheck/reactive/test_lambda_reactive.php | 16 ++++++++++ .../reactive/test_lambda_reactive.php.exp | 4 +-- hphp/hack/test/typecheck/reactive/test_obj_set.php | 10 ++++++ .../test/typecheck/reactive/test_obj_set.php.exp | 3 ++ .../hack/test/typecheck/reactive/test_obj_set2.php | 10 ++++++ .../test/typecheck/reactive/test_obj_set2.php.exp | 3 ++ .../hack/test/typecheck/reactive/test_obj_set3.php | 10 ++++++ .../test/typecheck/reactive/test_obj_set3.php.exp | 2 ++ 24 files changed, 215 insertions(+), 5 deletions(-) create mode 100644 hphp/hack/test/typecheck/reactive/reactive_calls1.php create mode 100644 hphp/hack/test/typecheck/reactive/reactive_calls1.php.exp create mode 100644 hphp/hack/test/typecheck/reactive/reactive_calls2.php create mode 100644 hphp/hack/test/typecheck/reactive/reactive_calls2.php.exp create mode 100644 hphp/hack/test/typecheck/reactive/reactive_calls3.php create mode 100644 hphp/hack/test/typecheck/reactive/reactive_calls3.php.exp create mode 100644 hphp/hack/test/typecheck/reactive/reactive_calls4.php create mode 100644 hphp/hack/test/typecheck/reactive/reactive_calls4.php.exp create mode 100644 hphp/hack/test/typecheck/reactive/test_lambda_reactive.php create mode 100644 hphp/hack/test/typecheck/reactive/test_obj_set.php create mode 100644 hphp/hack/test/typecheck/reactive/test_obj_set.php.exp create mode 100644 hphp/hack/test/typecheck/reactive/test_obj_set2.php create mode 100644 hphp/hack/test/typecheck/reactive/test_obj_set2.php.exp create mode 100644 hphp/hack/test/typecheck/reactive/test_obj_set3.php create mode 100644 hphp/hack/test/typecheck/reactive/test_obj_set3.php.exp diff --git a/hphp/hack/src/typing/typing.ml b/hphp/hack/src/typing/typing.ml index 5306f0c134e..2bec4560877 100644 --- a/hphp/hack/src/typing/typing.ml +++ b/hphp/hack/src/typing/typing.ml @@ -332,6 +332,8 @@ and fun_def tcopt f = let nb = TNBody.func_body tcopt f in let dep = Typing_deps.Dep.Fun (snd f.f_name) in let env = Env.empty tcopt (Pos.filename pos) (Some dep) in + let reactive = Attributes.mem SN.UserAttributes.uaReactive f.f_user_attributes in + let env = Env.set_env_reactive env reactive in NastCheck.fun_ env f nb; (* Fresh type environment is actually unnecessary, but I prefer to * have a guarantee that we are using a clean typing environment. *) @@ -1845,6 +1847,8 @@ and expr_ (* Improve hover type experience for parameters with known types *) List.iter2_exn f.f_params ft.ft_params (fun param ft_param -> Typing_hooks.dispatch_infer_ty_hook ft_param.fp_type param.param_pos env); + (* The lambda inherits its reactivity from the enclosing function *) + let ft = { ft with ft_reactive = Env.env_reactive env } in let inferred_ty = if is_explicit_ret then (Reason.Rwitness p, Tfun { ft with ft_ret = declared_ft.ft_ret }) @@ -2526,6 +2530,11 @@ and assign_ p ur env e1 ty2 = || x = SN.Collections.cImmVector || x = SN.Collections.cVec || x = SN.Collections.cConstVector -> + (* Vector assignment is illegal in a reactive context + but vec assignment is okay + *) + if x <> SN.Collections.cVec && Env.env_reactive env then + Errors.nonreactive_append p; let env, tel = List.map_env env el begin fun env e -> let env, te, _ = assign (fst e) env e elt_type in env, te @@ -2594,8 +2603,11 @@ and assign_ p ur env e1 ty2 = | [res] -> res | _ -> assign_simple p ur env e1 ty2 end + | _, Class_get _ | _, Obj_get _ -> + if Env.env_reactive env then + Errors.obj_set_reactive p; let lenv = env.Env.lenv in let no_fakes = LEnv.env_with_empty_fakes env in (* In this section, we check that the assignment is compatible with @@ -3669,6 +3681,13 @@ and array_append p env ty1 = | Terr -> env, (Reason.Rnone, Terr) + (* No reactive append on vector and set *) + | Tclass ((_, n), [ty]) + when (n = SN.Collections.cVector + || n = SN.Collections.cSet) && + Env.env_reactive env -> + Errors.nonreactive_append p; + env, ty | Tclass ((_, n), [ty]) when n = SN.Collections.cVector @@ -3682,10 +3701,14 @@ and array_append p env ty1 = without type parameters *) env, (Reason.Rnone, Tany) | Tclass ((_, n), [tkey; tvalue]) when n = SN.Collections.cMap -> - (* You can append a pair to a map *) - env, (Reason.Rmap_append p, Tclass ((p, SN.Collections.cPair), - [tkey; tvalue])) + if Env.env_reactive env then + Errors.nonreactive_append p; + (* You can append a pair to a map *) + env, (Reason.Rmap_append p, Tclass ((p, SN.Collections.cPair), + [tkey; tvalue])) | Tclass ((_, n), []) when n = SN.Collections.cMap -> + if Env.env_reactive env then + Errors.nonreactive_append p; (* Handle the case where "Map" was used as a typehint without type parameters *) env, (Reason.Rmap_append p, @@ -4447,6 +4470,12 @@ and call_ ~expected pos env fty el uel = let pos_def = Reason.to_pos r2 in let env, var_param = variadic_param env ft in + (* Reactive functions can only call reactive functions *) + (match Env.env_reactive env, ft.ft_reactive with + | true, false -> + Errors.nonreactive_function_call pos + | _ -> ()); + (* Force subtype with expected result *) let env = check_expected_ty "Call result" env ft.ft_ret expected in @@ -5646,6 +5675,8 @@ and method_def env m = let env = Env.env_with_locals env Typing_continuations.Map.empty Local_id.Map.empty in + let reactive = Attributes.mem SN.UserAttributes.uaReactive m.m_user_attributes in + let env = Env.set_env_reactive env reactive in let ety_env = { (Phase.env_with_self env) with from_class = Some CIstatic; } in let env, constraints = diff --git a/hphp/hack/src/typing/typing_env.ml b/hphp/hack/src/typing/typing_env.ml index 2bd9b1a4a7b..b3e44314a09 100644 --- a/hphp/hack/src/typing/typing_env.ml +++ b/hphp/hack/src/typing/typing_env.ml @@ -290,12 +290,17 @@ let empty tcopt file ~droot = { parent_id = ""; parent = Reason.none, Tany; fun_kind = Ast.FSync; + fun_reactive = false; anons = IMap.empty; file = file; }; global_tpenv = SMap.empty } +let set_env_reactive env reactive = + { env with genv = {env.genv with fun_reactive = reactive }} +let env_reactive env = + env.genv.fun_reactive let add_wclass env x = let dep = Dep.Class x in Option.iter env.decl_env.droot (fun root -> Typing_deps.add_idep root dep); diff --git a/hphp/hack/src/typing/typing_env.mli b/hphp/hack/src/typing/typing_env.mli index 2508803a759..b599caee4f8 100644 --- a/hphp/hack/src/typing/typing_env.mli +++ b/hphp/hack/src/typing/typing_env.mli @@ -115,6 +115,8 @@ val is_generic_parameter : env -> string -> bool val get_generic_parameters : env -> string list val add_fresh_generic_parameter : env -> string -> env * string val get_tpenv_size : env -> int +val set_env_reactive : env -> bool -> env +val env_reactive : env -> bool val freeze_local_env : env -> env val env_with_locals : env -> local_types -> local_history Local_id.Map.t -> env diff --git a/hphp/hack/src/typing/typing_env_types.ml b/hphp/hack/src/typing/typing_env_types.ml index 3bf32937de6..12adbe08eb1 100644 --- a/hphp/hack/src/typing/typing_env_types.ml +++ b/hphp/hack/src/typing/typing_env_types.ml @@ -83,6 +83,8 @@ and genv = { self : locl ty; static : bool; fun_kind : Ast.fun_kind; + (* Whether current function is reactive *) + fun_reactive : bool; anons : anon IMap.t; file : Relative_path.t; } diff --git a/hphp/hack/src/typing/typing_env_types_sig.mli b/hphp/hack/src/typing/typing_env_types_sig.mli index c047bed600d..0680f8ba0e1 100644 --- a/hphp/hack/src/typing/typing_env_types_sig.mli +++ b/hphp/hack/src/typing/typing_env_types_sig.mli @@ -80,6 +80,7 @@ and genv = { self : locl ty; static : bool; fun_kind : Ast.fun_kind; + fun_reactive : bool; anons : anon IMap.t; file : Relative_path.t; } diff --git a/hphp/hack/src/typing/typing_subtype.ml b/hphp/hack/src/typing/typing_subtype.ml index f012707bce5..07d8bbcb86c 100644 --- a/hphp/hack/src/typing/typing_subtype.ml +++ b/hphp/hack/src/typing/typing_subtype.ml @@ -386,6 +386,9 @@ and subtype_funs_generic ~check_return ~contravariant_arguments env r_sub ft_sub r_super ft_super = let p_sub = Reason.to_pos r_sub in let p_super = Reason.to_pos r_super in + (* Reactive functions are a subtype of non reactive ones, but not vice versa *) + if not ft_sub.ft_reactive && ft_super.ft_reactive then + Errors.fun_reactivity_mismatch p_super p_sub; if ft_sub.ft_is_coroutine <> ft_super.ft_is_coroutine then Errors.coroutinness_mismatch ft_super.ft_is_coroutine p_super p_sub; if (arity_min ft_sub.ft_arity) > (arity_min ft_super.ft_arity) diff --git a/hphp/hack/src/utils/errors/errors.ml b/hphp/hack/src/utils/errors/errors.ml index 0f1918ddc8e..08a6b1c9245 100644 --- a/hphp/hack/src/utils/errors/errors.ml +++ b/hphp/hack/src/utils/errors/errors.ml @@ -774,6 +774,10 @@ module Typing = struct let self_const_parent_not = 4197 (* DONT MODIFY!!!! *) let parent_const_self_not = 4198 (* DONT MODIFY!!!! *) let partially_valid_is_expression_hint = 4199 (* DONT MODIFY!!!! *) + let nonreactive_function_call = 4200 (* DONT MODIFY!!!! *) + let nonreactive_append = 4201 (* DONT MODIFY!!!! *) + let obj_set_reactive = 4202 (* DONT MODIFY!!!! *) + let fun_reactivity_mismatch = 4203 (* DONT MODIFY!!!! *) (* EXTEND HERE WITH NEW VALUES IF NEEDED *) end @@ -2210,6 +2214,12 @@ let fun_arity_mismatch pos1 pos2 = pos2, "Because of this definition"; ] +let fun_reactivity_mismatch pos1 pos2 = + add_list Typing.fun_reactivity_mismatch [ + pos1, "This function is reactive"; + pos2, "This function is not."; +] + let discarded_awaitable pos1 pos2 = add_list Typing.discarded_awaitable [ pos1, "This expression is of type Awaitable, but it's "^ @@ -2678,6 +2688,21 @@ let invalid_new_disposable pos = "'new' must not be used for disposable objects except in a 'using' statement" in add Typing.invalid_new_disposable pos msg +let nonreactive_function_call pos = + let msg = + "Reactive functions can only call other reactive functions" in + add Typing.nonreactive_function_call pos msg + +let nonreactive_append pos = + let msg = "Cannot append to a Hack Collection types in a reactive context" in + add Typing.nonreactive_append pos msg + +let obj_set_reactive pos = + let msg = ("This property is being mutated(used as an lvalue)" ^ + "\nYou cannot set non-mutable object properties in reactive functions") in + add Typing.obj_set_reactive pos msg + + (*****************************************************************************) (* Convert relative paths to absolute. *) (*****************************************************************************) diff --git a/hphp/hack/src/utils/errors/errors_sig.ml b/hphp/hack/src/utils/errors/errors_sig.ml index 8f5e436f61d..a8860585a98 100644 --- a/hphp/hack/src/utils/errors/errors_sig.ml +++ b/hphp/hack/src/utils/errors/errors_sig.ml @@ -401,6 +401,7 @@ module type S = sig val coroutine_call_outside_of_suspend : Pos.t -> unit val function_is_not_coroutine : Pos.t -> string -> unit val coroutinness_mismatch : bool -> Pos.t -> Pos.t -> unit + val fun_reactivity_mismatch : Pos.t -> Pos.t -> unit val this_as_lexical_variable : Pos.t -> unit val dollardollar_lvalue : Pos.t -> unit val duplicate_using_var : Pos.t -> unit @@ -414,6 +415,7 @@ module type S = sig val inout_params_special : Pos.t -> unit val inout_params_mix_byref : Pos.t -> Pos.t -> unit val inout_params_memoize : Pos.t -> Pos.t -> unit + val obj_set_reactive : Pos.t -> unit val global_in_reactive_context : Pos.t -> unit val inout_annotation_missing : Pos.t -> Pos.t -> unit val inout_annotation_unexpected : Pos.t -> Pos.t -> unit @@ -421,4 +423,6 @@ module type S = sig val inout_params_ret_by_ref : Pos.t -> Pos.t -> unit val xhp_required : Pos.t -> string -> (Pos.t * string) list -> unit val illegal_xhp_child : Pos.t -> (Pos.t * string) list -> unit + val nonreactive_function_call : Pos.t -> unit + val nonreactive_append : Pos.t -> unit end diff --git a/hphp/hack/test/typecheck/reactive/reactive_calls1.php b/hphp/hack/test/typecheck/reactive/reactive_calls1.php new file mode 100644 index 00000000000..7ab3e462a92 --- /dev/null +++ b/hphp/hack/test/typecheck/reactive/reactive_calls1.php @@ -0,0 +1,19 @@ +> +function foo(): void {} + +function baz(): void {} +function bar(): void { + // bar can call baz + baz(); + // bar can call foo + foo(); +} + +<<__Rx>> +function qux(): void { + // qux can call foo + foo(); + // qux cannot call baz + baz(); +} diff --git a/hphp/hack/test/typecheck/reactive/reactive_calls1.php.exp b/hphp/hack/test/typecheck/reactive/reactive_calls1.php.exp new file mode 100644 index 00000000000..b354c20cb97 --- /dev/null +++ b/hphp/hack/test/typecheck/reactive/reactive_calls1.php.exp @@ -0,0 +1,2 @@ +File "reactive_calls1.php", line 18, characters 3-7: +Reactive functions can only call other reactive functions (Typing[4200]) diff --git a/hphp/hack/test/typecheck/reactive/reactive_calls2.php b/hphp/hack/test/typecheck/reactive/reactive_calls2.php new file mode 100644 index 00000000000..890b198689e --- /dev/null +++ b/hphp/hack/test/typecheck/reactive/reactive_calls2.php @@ -0,0 +1,14 @@ +> +function returnsReactive(): Rx<(function(): void)> { + // UNSAFE +} + +<<__Rx>> +function foo(): void { + // returnsReactive is reactive + $x = returnsReactive(); + // thing that returnsReactive returns is also reactive, no errors + $x(); +} diff --git a/hphp/hack/test/typecheck/reactive/reactive_calls2.php.exp b/hphp/hack/test/typecheck/reactive/reactive_calls2.php.exp new file mode 100644 index 00000000000..4269126fceb --- /dev/null +++ b/hphp/hack/test/typecheck/reactive/reactive_calls2.php.exp @@ -0,0 +1 @@ +No errors diff --git a/hphp/hack/test/typecheck/reactive/reactive_calls3.php b/hphp/hack/test/typecheck/reactive/reactive_calls3.php new file mode 100644 index 00000000000..ea793d1d028 --- /dev/null +++ b/hphp/hack/test/typecheck/reactive/reactive_calls3.php @@ -0,0 +1,11 @@ +> +function append(bool $x): void { + if ($x) { + $y = Vector {}; + } else { + $y = Vector {7}; + } + $y[] = 5; +} diff --git a/hphp/hack/test/typecheck/reactive/reactive_calls3.php.exp b/hphp/hack/test/typecheck/reactive/reactive_calls3.php.exp new file mode 100644 index 00000000000..9ec51fb26cf --- /dev/null +++ b/hphp/hack/test/typecheck/reactive/reactive_calls3.php.exp @@ -0,0 +1,2 @@ +File "reactive_calls3.php", line 10, characters 3-6: +Cannot append to a Hack Collection types in a reactive context (Typing[4201]) diff --git a/hphp/hack/test/typecheck/reactive/reactive_calls4.php b/hphp/hack/test/typecheck/reactive/reactive_calls4.php new file mode 100644 index 00000000000..0ea2811e3c2 --- /dev/null +++ b/hphp/hack/test/typecheck/reactive/reactive_calls4.php @@ -0,0 +1,28 @@ +> +function returnsReactive(): Rx<(function(): void)> { + // UNSAFE +} + +<<__Rx>> +function returnsNormal(): (function(): void) { + // UNSAFE +} + +<<__Rx>> +function takesReactive(Rx<(function(): void)> $x): void {} + +<<__Rx>> +function takesNormal((function(): void) $x): void {} +<<__Rx>> +function foo(): void { + // should be fine + takesReactive(returnsReactive()); + // also okay + takesNormal(returnsReactive()); + takesNormal(returnsNormal()); + // not okay + takesReactive(returnsNormal()); + +} diff --git a/hphp/hack/test/typecheck/reactive/reactive_calls4.php.exp b/hphp/hack/test/typecheck/reactive/reactive_calls4.php.exp new file mode 100644 index 00000000000..719eba48f2f --- /dev/null +++ b/hphp/hack/test/typecheck/reactive/reactive_calls4.php.exp @@ -0,0 +1,6 @@ +File "reactive_calls4.php", line 26, characters 17-31: +Invalid argument (Typing[4203]) +File "reactive_calls4.php", line 14, characters 24-25: +This function is reactive +File "reactive_calls4.php", line 9, characters 27-44: +This function is not. diff --git a/hphp/hack/test/typecheck/reactive/test_lambda_reactive.php b/hphp/hack/test/typecheck/reactive/test_lambda_reactive.php new file mode 100644 index 00000000000..aba6ff92509 --- /dev/null +++ b/hphp/hack/test/typecheck/reactive/test_lambda_reactive.php @@ -0,0 +1,16 @@ +> +function test(): Rx<(function(): int)> { + $x = function() { + return 5; + }; + return $x; +} + +// should error +function test2(): Rx<(function(): int)> { + $x = function() { + return 5; + }; + return $x; +} diff --git a/hphp/hack/test/typecheck/reactive/test_lambda_reactive.php.exp b/hphp/hack/test/typecheck/reactive/test_lambda_reactive.php.exp index e3aff0abfb6..0d6e9053a8e 100644 --- a/hphp/hack/test/typecheck/reactive/test_lambda_reactive.php.exp +++ b/hphp/hack/test/typecheck/reactive/test_lambda_reactive.php.exp @@ -1,6 +1,6 @@ File "test_lambda_reactive.php", line 15, characters 10-11: -Invalid return type (Typing[4195]) -File "test_lambda_reactive.php", line 11, characters 20-21: +Invalid return type (Typing[4203]) +File "test_lambda_reactive.php", line 11, characters 19-20: This function is reactive File "test_lambda_reactive.php", line 12, characters 8-15: This function is not. diff --git a/hphp/hack/test/typecheck/reactive/test_obj_set.php b/hphp/hack/test/typecheck/reactive/test_obj_set.php new file mode 100644 index 00000000000..7d76dce5b06 --- /dev/null +++ b/hphp/hack/test/typecheck/reactive/test_obj_set.php @@ -0,0 +1,10 @@ +> +function test(Foo $x): void { + $x->value = 5; +} diff --git a/hphp/hack/test/typecheck/reactive/test_obj_set.php.exp b/hphp/hack/test/typecheck/reactive/test_obj_set.php.exp new file mode 100644 index 00000000000..a8f08145778 --- /dev/null +++ b/hphp/hack/test/typecheck/reactive/test_obj_set.php.exp @@ -0,0 +1,3 @@ +File "test_obj_set.php", line 9, characters 3-15: +This property is being mutated(used as an lvalue) +You cannot set non-mutable object properties in reactive functions (Typing[4202]) diff --git a/hphp/hack/test/typecheck/reactive/test_obj_set2.php b/hphp/hack/test/typecheck/reactive/test_obj_set2.php new file mode 100644 index 00000000000..d96aedd4eaf --- /dev/null +++ b/hphp/hack/test/typecheck/reactive/test_obj_set2.php @@ -0,0 +1,10 @@ +> +function test(Foo $x): void { + $x->value .= 5; +} diff --git a/hphp/hack/test/typecheck/reactive/test_obj_set2.php.exp b/hphp/hack/test/typecheck/reactive/test_obj_set2.php.exp new file mode 100644 index 00000000000..2009ebc72c7 --- /dev/null +++ b/hphp/hack/test/typecheck/reactive/test_obj_set2.php.exp @@ -0,0 +1,3 @@ +File "test_obj_set2.php", line 9, characters 3-16: +This property is being mutated(used as an lvalue) +You cannot set non-mutable object properties in reactive functions (Typing[4202]) diff --git a/hphp/hack/test/typecheck/reactive/test_obj_set3.php b/hphp/hack/test/typecheck/reactive/test_obj_set3.php new file mode 100644 index 00000000000..3d51300d846 --- /dev/null +++ b/hphp/hack/test/typecheck/reactive/test_obj_set3.php @@ -0,0 +1,10 @@ + $value) {} +} + +<<__Rx>> +function test(Foo $x): void { + $x->value[] = 5; +} diff --git a/hphp/hack/test/typecheck/reactive/test_obj_set3.php.exp b/hphp/hack/test/typecheck/reactive/test_obj_set3.php.exp new file mode 100644 index 00000000000..d684849bcc7 --- /dev/null +++ b/hphp/hack/test/typecheck/reactive/test_obj_set3.php.exp @@ -0,0 +1,2 @@ +File "test_obj_set3.php", line 9, characters 3-13: +Cannot append to a Hack Collection types in a reactive context (Typing[4201]) -- 2.11.4.GIT