From 1a01ec7a5ec8bc1df781bd5b13b4cabbb55eb109 Mon Sep 17 00:00:00 2001 From: Thomas Jiang Date: Fri, 2 Oct 2020 14:48:33 -0700 Subject: [PATCH] Reset local typing environment for ExprTrees Summary: Expression trees should not inherit the local typing environment. So the following should throw an error: ``` function test(): void { $x = 1; Code`$x + 1`; } ``` As the expression tree should not inherit the local type for `$x`. This diff resets the locals in the typing environment for expression trees. This diff does not account for splicing yet. Reviewed By: Wilfred Differential Revision: D23718691 fbshipit-source-id: 7589ea088278cabd56ea634520c39aa205f3469d --- hphp/hack/src/typing/typing.ml | 36 +++++++++---- .../expression_trees/bound_variable_lambda.php | 63 ++++++++++++++++++++++ .../expression_trees/bound_variable_lambda.php.exp | 1 + .../expression_trees/mismatched_types.php | 63 ++++++++++++++++++++++ .../expression_trees/mismatched_types.php.exp | 6 +++ .../expression_trees/unbound_variables.php | 63 ++++++++++++++++++++++ .../expression_trees/unbound_variables.php.exp | 2 + 7 files changed, 224 insertions(+), 10 deletions(-) create mode 100644 hphp/hack/test/typecheck/expression_trees/bound_variable_lambda.php create mode 100644 hphp/hack/test/typecheck/expression_trees/bound_variable_lambda.php.exp create mode 100644 hphp/hack/test/typecheck/expression_trees/mismatched_types.php create mode 100644 hphp/hack/test/typecheck/expression_trees/mismatched_types.php.exp create mode 100644 hphp/hack/test/typecheck/expression_trees/unbound_variables.php create mode 100644 hphp/hack/test/typecheck/expression_trees/unbound_variables.php.exp diff --git a/hphp/hack/src/typing/typing.ml b/hphp/hack/src/typing/typing.ml index 6d768c088b0..866aeff2874 100644 --- a/hphp/hack/src/typing/typing.ml +++ b/hphp/hack/src/typing/typing.ml @@ -2508,13 +2508,7 @@ and expr_ in let (env, ty) = Phase.localize_hint_with_self env hint in make_result env p (Aast.Cast (hint, te)) ty - | ExpressionTree (hint, e, e2) -> - let (env, te, ty) = expr env e in - (match e2 with - | Some e2 -> - let (env, te2, _ty2) = expr env e2 in - make_result env p (Aast.ExpressionTree (hint, te, Some te2)) ty - | None -> make_result env p (Aast.ExpressionTree (hint, te, None)) ty) + | ExpressionTree (hint, e, e2) -> expression_tree env p hint e e2 | Is (e, hint) -> Typing_kinding.Simple.check_well_kinded_hint env hint; let (env, te, _) = expr env e in @@ -2947,9 +2941,7 @@ and expr_ let s = enum ^ ":@" ^ atom in Errors.pu_typing p "identifier" s; expr_error env (Reason.Rwitness p) outer - | ET_Splice e -> - let (env, te, ty) = expr env e in - make_result env p (Aast.ET_Splice te) ty + | ET_Splice e -> et_splice env p e (* let ty = err_witness env cst_pos in *) and class_const ?(incl_tc = false) env p ((cpos, cid), mid) = @@ -3363,6 +3355,30 @@ and anon_make ?el ?ret_ty env lambda_pos f ft idl is_anon = (*****************************************************************************) (* End of anonymous functions. *) (*****************************************************************************) + +(*****************************************************************************) +(* Expression trees *) +(*****************************************************************************) +and expression_tree env p visitor_class e desugared_expr = + let (env, (te, ty)) = + Typing_lenv.stash_and_do env (Env.all_continuations env) (fun env -> + let env = Env.reinitialize_locals env in + let (env, te, ty) = expr env e in + (env, (te, ty))) + in + match desugared_expr with + | Some desugared_expr -> + let (env, te2, _ty2) = expr env desugared_expr in + make_result env p (Aast.ExpressionTree (visitor_class, te, Some te2)) ty + | None -> make_result env p (Aast.ExpressionTree (visitor_class, te, None)) ty + +and et_splice env p e = + let (env, te, ty) = expr env e in + make_result env p (Aast.ET_Splice te) ty + +(*****************************************************************************) +(* End expression trees *) +(*****************************************************************************) and requires_consistent_construct = function | CIstatic -> true | CIexpr _ -> true diff --git a/hphp/hack/test/typecheck/expression_trees/bound_variable_lambda.php b/hphp/hack/test/typecheck/expression_trees/bound_variable_lambda.php new file mode 100644 index 00000000000..23efee81d7a --- /dev/null +++ b/hphp/hack/test/typecheck/expression_trees/bound_variable_lambda.php @@ -0,0 +1,63 @@ +> + +// Placeholder definition so we don't get naming/typing errors. +class Code { + const type TAst = mixed; + // Simple literals. + public function intLiteral(int $_): this::TAst { + throw new Exception(); + } + public function boolLiteral(bool $_): this::TAst { + throw new Exception(); + } + public function stringLiteral(string $_): this::TAst { + throw new Exception(); + } + public function localVar(string $_): this::TAst { + throw new Exception(); + } + + // Operators + public function plus(this::TAst $_, this::TAst $_): this::TAst { + throw new Exception(); + } + public function call(string $_fnName, vec $_args): this::TAst { + throw new Exception(); + } + + // Statements. + public function assign(this::TAst $_, this::TAst $_): this::TAst { + throw new Exception(); + } + public function ifStatement( + this::TAst $_cond, + vec $_then_body, + vec $_else_body, + ): this::TAst { + throw new Exception(); + } + public function whileStatement( + this::TAst $_cond, + vec $_body, + ): this::TAst { + throw new Exception(); + } + public function returnStatement(?this::TAst $_): this::TAst { + throw new Exception(); + } + + public function lambdaLiteral( + vec $_args, + vec $_body, + ): this::TAst { + throw new Exception(); + } +} +function test(): void { + $_ = Code`() ==> { + $x = 1; + $x + 1; + }`; +} diff --git a/hphp/hack/test/typecheck/expression_trees/bound_variable_lambda.php.exp b/hphp/hack/test/typecheck/expression_trees/bound_variable_lambda.php.exp new file mode 100644 index 00000000000..4269126fceb --- /dev/null +++ b/hphp/hack/test/typecheck/expression_trees/bound_variable_lambda.php.exp @@ -0,0 +1 @@ +No errors diff --git a/hphp/hack/test/typecheck/expression_trees/mismatched_types.php b/hphp/hack/test/typecheck/expression_trees/mismatched_types.php new file mode 100644 index 00000000000..b6593f3845a --- /dev/null +++ b/hphp/hack/test/typecheck/expression_trees/mismatched_types.php @@ -0,0 +1,63 @@ +> + +// Placeholder definition so we don't get naming/typing errors. +class Code { + const type TAst = mixed; + // Simple literals. + public function intLiteral(int $_): this::TAst { + throw new Exception(); + } + public function boolLiteral(bool $_): this::TAst { + throw new Exception(); + } + public function stringLiteral(string $_): this::TAst { + throw new Exception(); + } + public function localVar(string $_): this::TAst { + throw new Exception(); + } + + // Operators + public function plus(this::TAst $_, this::TAst $_): this::TAst { + throw new Exception(); + } + public function call(string $_fnName, vec $_args): this::TAst { + throw new Exception(); + } + + // Statements. + public function assign(this::TAst $_, this::TAst $_): this::TAst { + throw new Exception(); + } + public function ifStatement( + this::TAst $_cond, + vec $_then_body, + vec $_else_body, + ): this::TAst { + throw new Exception(); + } + public function whileStatement( + this::TAst $_cond, + vec $_body, + ): this::TAst { + throw new Exception(); + } + public function returnStatement(?this::TAst $_): this::TAst { + throw new Exception(); + } + + public function lambdaLiteral( + vec $_args, + vec $_body, + ): this::TAst { + throw new Exception(); + } +} +function test(): void { + $_ = Code`() ==> { + $x = "hello"; + $x + 1; + }`; +} diff --git a/hphp/hack/test/typecheck/expression_trees/mismatched_types.php.exp b/hphp/hack/test/typecheck/expression_trees/mismatched_types.php.exp new file mode 100644 index 00000000000..b55424f4af1 --- /dev/null +++ b/hphp/hack/test/typecheck/expression_trees/mismatched_types.php.exp @@ -0,0 +1,6 @@ +File "mismatched_types.php", line 61, characters 5-10: +Typing error (Typing[4110]) + File "mismatched_types.php", line 61, characters 5-6: + Expected `num` because this is used in an arithmetic operation + File "mismatched_types.php", line 60, characters 10-16: + But got `string` diff --git a/hphp/hack/test/typecheck/expression_trees/unbound_variables.php b/hphp/hack/test/typecheck/expression_trees/unbound_variables.php new file mode 100644 index 00000000000..6d957637677 --- /dev/null +++ b/hphp/hack/test/typecheck/expression_trees/unbound_variables.php @@ -0,0 +1,63 @@ +> + +// Placeholder definition so we don't get naming/typing errors. +class Code { + const type TAst = mixed; + // Simple literals. + public function intLiteral(int $_): this::TAst { + throw new Exception(); + } + public function boolLiteral(bool $_): this::TAst { + throw new Exception(); + } + public function stringLiteral(string $_): this::TAst { + throw new Exception(); + } + public function localVar(string $_): this::TAst { + throw new Exception(); + } + + // Operators + public function plus(this::TAst $_, this::TAst $_): this::TAst { + throw new Exception(); + } + public function call(string $_fnName, vec $_args): this::TAst { + throw new Exception(); + } + + // Statements. + public function assign(this::TAst $_, this::TAst $_): this::TAst { + throw new Exception(); + } + public function ifStatement( + this::TAst $_cond, + vec $_then_body, + vec $_else_body, + ): this::TAst { + throw new Exception(); + } + public function whileStatement( + this::TAst $_cond, + vec $_body, + ): this::TAst { + throw new Exception(); + } + public function returnStatement(?this::TAst $_): this::TAst { + throw new Exception(); + } + + public function lambdaLiteral( + vec $_args, + vec $_body, + ): this::TAst { + throw new Exception(); + } +} +function test(): void { + $x = 1; + + // Expression Trees do not inherit local variables from the outer scope + $_ = Code`$x + 1`; +} diff --git a/hphp/hack/test/typecheck/expression_trees/unbound_variables.php.exp b/hphp/hack/test/typecheck/expression_trees/unbound_variables.php.exp new file mode 100644 index 00000000000..a6fc5d33146 --- /dev/null +++ b/hphp/hack/test/typecheck/expression_trees/unbound_variables.php.exp @@ -0,0 +1,2 @@ +File "unbound_variables.php", line 62, characters 13-14: +Variable `$x` is undefined, or not always defined. (Naming[2050]) -- 2.11.4.GIT