Refactor AST of inout arguments

Summary:
Change the representation of arguments to Call nodes from `param_kind * expr` to `type Argument = Anormal of expr | Ainout of pos * expr`. This is isomorphic, so this diff shouldn't change any behaviour.  The motivation is to set of a subsequent diff that restricts the type of `Ainout` to those lvalues that can actually show up.

It was prevalent to match on `(_, Expr ...)` and not care if the argument was inout or not. Most of those have become explicit calls to `Aast_utils.arg_to_expr`, although sometimes the pattern matching structure had to be modified when the arg itself was nested inside of another pattern.

Reviewed By: andrewjkennedy

Differential Revision: D66544188

fbshipit-source-id: d17f4f71952f99cf4db43fcfa6fe65657f52134c

hphp/hack/src/annotated_ast/aast_defs.ml

@@ -881,12 +881,16 @@ and ('ex, 'en) call_expr = {
       (** function *)
   targs: 'ex targ list;
       (** explicit type annotations *)
-  args: ((Ast_defs.param_kind[@transform.opaque]) * ('ex, 'en) expr) list;
+  args: ('ex, 'en) argument list;
       (** positional args, plus their calling convention *)
   unpacked_arg: ('ex, 'en) expr option;
       (** unpacked arg *)
 }
 
+and ('ex, 'en) argument =
+  | Ainout of (pos[@transform.opaque]) * ('ex, 'en) expr
+  | Anormal of ('ex, 'en) expr
+
 and ('ex, 'en) user_attribute = {
   ua_name: sid;
   ua_params: ('ex, 'en) expr list;

hphp/hack/src/annotated_ast/aast_utils.ml

@@ -89,7 +89,7 @@ let get_return_from_fun e =
 let get_virtual_expr_from_et et =
   let get_body_helper e =
     match e with
-    | (_, _, Call { args = _ :: (Pnormal, (_, _, Shape fields)) :: _; _ }) ->
+    | (_, _, Call { args = _ :: Anormal (_, _, Shape fields) :: _; _ }) ->
       (match find_shape_field "type" fields with
       | Some (_, e) -> get_return_from_fun e
       | None -> None)
@@ -233,3 +233,18 @@ let get_param_default param =
   | Param_optional e -> e
 
 let get_expr_pos (_, p, _) = p
+
+let get_argument_pos a =
+  match a with
+  | Anormal e -> get_expr_pos e
+  | Ainout (_, e) -> get_expr_pos e
+
+let arg_to_expr arg =
+  match arg with
+  | Anormal e -> e
+  | Ainout (_, e) -> e
+
+let expr_to_arg pk e =
+  match pk with
+  | Ast_defs.Pnormal -> Anormal e
+  | Ast_defs.Pinout p -> Ainout (p, e)

hphp/hack/src/annotated_ast/aast_utils.mli

@@ -57,3 +57,13 @@ val get_param_default :
   ('a, 'b) Aast_defs.fun_param -> ('a, 'b) Aast_defs.expr option
 
 val get_expr_pos : ('a, 'b) Aast_defs.expr -> Pos.t
+
+(* Gets the position of the argument expression (not of the inout, if present) *)
+val get_argument_pos : ('a, 'b) Aast_defs.argument -> Pos.t
+
+(** Convert an argument to an expression, ignoring whether it's inout or not *)
+val arg_to_expr : ('a, 'b) Aast_defs.argument -> ('a, 'b) Aast_defs.expr
+
+(** Convert an an expression to an argument, using the supplied inout *)
+val expr_to_arg :
+  Ast_defs.param_kind -> ('a, 'b) Aast_defs.expr -> ('a, 'b) Aast_defs.argument

hphp/hack/src/client/ide_service/code_actions_services/refactors/flip_around_comma.ml

@@ -161,11 +161,11 @@ let visitor ~(cursor : Pos.t) =
             args
             |> List.map
                  ~f:
-                   Ast_defs.(
+                   Aast_defs.(
                      function
-                     | (Pinout inout_pos, expr) ->
+                     | Ainout (inout_pos, expr) ->
                        Pos.merge inout_pos (pos_of_expr expr)
-                     | (Pnormal, expr) -> pos_of_expr expr)
+                     | Anormal expr -> pos_of_expr expr)
             |> find_in_positions
           | Aast_defs.ValCollection (_, _, exprs)
           | Aast_defs.List exprs

hphp/hack/src/client/ide_service/code_actions_services/refactors/inline_method/inline_method_find_candidate.ml

@@ -110,8 +110,7 @@ let find_candidate ~(cursor : Pos.t) ~entry ctx : T.candidate option =
                   1 + Option.value count_opt ~default:0);
             if Pos.contains call_id_pos cursor then
               let call_arg_positions =
-                List.map param_kind_arg_pairs ~f:(fun (_, (_, arg_pos, _)) ->
-                    arg_pos)
+                List.map param_kind_arg_pairs ~f:Aast_utils.get_argument_pos
               in
               call_info :=
                 Some

hphp/hack/src/client_and_server/autocompleteService.ml

@@ -1121,9 +1121,9 @@ let autocomplete_enum_class_label_call env f args =
       ~f:(fun (arg, arg_ty) ->
         (* If the argument was wrapped in a hole, remove it *)
         let arg =
-          match arg with
-          | (_, (_, _, Aast.Hole (e, _, _, _))) -> e
-          | _ -> snd arg
+          match Aast_utils.arg_to_expr arg with
+          | (_, _, Aast.Hole (e, _, _, _)) -> e
+          | e -> e
         in
         match (arg, get_node (expand_and_strip_dynamic env arg_ty.fp_type)) with
         | ( (_, p, Aast.EnumClassLabel (None, n)),
@@ -1230,9 +1230,7 @@ let unwrap_holes ((_, _, e_) as e : Tast.expr) : Tast.expr =
    takes_shape(shape('x' => 123, '|'));
 *)
 let autocomplete_shape_literal_in_call
-    env
-    (ft : Typing_defs.locl_fun_type)
-    (args : (Ast_defs.param_kind * Tast.expr) list) : unit =
+    env (ft : Typing_defs.locl_fun_type) (args : Tast.argument list) : unit =
   let add_shape_key_result pos key =
     let ty = Tprim Aast_defs.Tstring in
     let reason = Typing_reason.witness pos in
@@ -1268,7 +1266,9 @@ let autocomplete_shape_literal_in_call
     | _ -> None
   in
 
-  let args = List.map args ~f:(fun (_, e) -> unwrap_holes e) in
+  let args =
+    List.map args ~f:(fun arg -> unwrap_holes (Aast_utils.arg_to_expr arg))
+  in
 
   List.iter
     ~f:(fun (arg, expected_ty) ->
@@ -1492,7 +1492,9 @@ let autocomplete_enum_case env (expr : Tast.expr) (cases : Tast.case list) =
    takes_enum(AUTO332); *)
 let autocomplete_enum_value_in_call env (ft : Typing_defs.locl_fun_type) args :
     unit =
-  let args = List.map args ~f:(fun (_, e) -> unwrap_holes e) in
+  let args =
+    List.map args ~f:(fun arg -> unwrap_holes (Aast_utils.arg_to_expr arg))
+  in
 
   List.iter
     ~f:(fun (arg, expected_ty) ->

hphp/hack/src/client_and_server/identifySymbolService.ml

@@ -142,12 +142,11 @@ let process_arg_names recv (args : Tast.expr list) : Result_set.t =
                 //^ Hover shows: Parameter: $name
 *)
 let process_callee_arg_names
-    enclosing_class
-    (recv : Tast.expr)
-    (args : (Ast_defs.param_kind * Tast.expr) list) : Result_set.t =
+    enclosing_class (recv : Tast.expr) (args : Tast.argument list) :
+    Result_set.t =
   let enclosing_class_name = Option.map ~f:snd enclosing_class in
   let recv = get_callee enclosing_class_name recv in
-  process_arg_names recv (List.map ~f:snd args)
+  process_arg_names recv (List.map ~f:Aast_utils.arg_to_expr args)
 
 (* Add parameter names for all arguments at an instantiation
    site. This enables us to show hover information on arguments.
@@ -466,12 +465,12 @@ let visitor =
               Call
                 {
                   func = (_, _, Class_const (_, (_, methName)));
-                  args = [(_, arg)];
+                  args = [arg];
                   _;
                 })
             when String.equal methName SN.ExpressionTrees.symbolType ->
             (* Treat MyVisitor::symbolType(foo<>) as just foo(). *)
-            self#on_expr env arg
+            self#on_argument env arg
           | _ -> self#zero
         else
           let expr_ =
@@ -498,7 +497,7 @@ let visitor =
       in
 
       let tala = self#on_list self#on_targ env tal in
-      let ela = self#on_list self#on_expr env (List.map ~f:snd el) in
+      let ela = self#on_list self#on_argument env el in
       let arg_names = process_callee_arg_names !class_name e el in
       let uea =
         Option.value_map

hphp/hack/src/client_and_server/serverInferType.ml

@@ -230,7 +230,7 @@ let base_visitor ~human_friendly ~under_dynamic line_char_pairs =
             match (args, ft_params) with
             | (arg :: args, fp :: ft_params) ->
               let { Typing_defs.fp_type; _ } = fp in
-              let (_, (arg_type, p, expr_)) = arg in
+              let (arg_type, p, expr_) = Aast_utils.arg_to_expr arg in
               let fp =
                 match (expr_, Typing_defs.get_node arg_type) with
                 | (Aast.EnumClassLabel (None, label), _)

hphp/hack/src/client_and_server/serverUtils.ml

@@ -271,7 +271,7 @@ let resugar_invariant_call env (cond : Tast.expr) (then_body : Tast.block) :
                   recv_pos,
                   Id (name_pos, SN.AutoimportedFunctions.invariant) );
               targs = [];
-              args = (Ast_defs.Pnormal, invariant_cond) :: args;
+              args = Aast_defs.Anormal invariant_cond :: args;
               unpacked_arg = None;
             }) )
   | _ -> None

hphp/hack/src/elab/lift_await.rs

@@ -182,7 +182,7 @@ fn check_await_usage(expr: &Expr) -> AwaitUsage {
         }) => {
             let res = args
                 .iter()
-                .map(|(_, arg)| check_await_usage(arg))
+                .map(|arg| check_await_usage(arg.to_expr_ref()))
                 .fold(check_await_usage(func), combine_con);
             unpacked_arg
                 .iter()
@@ -554,8 +554,8 @@ impl LiftAwait {
                 unpacked_arg,
             }) => {
                 self.extract_await(func, con, seq, tmps);
-                for (_, arg) in args {
-                    self.extract_await(arg, con, seq, tmps)
+                for arg in args {
+                    self.extract_await(arg.to_expr_mut(), con, seq, tmps)
                 }
                 if let Some(unpack) = unpacked_arg {
                     self.extract_await(unpack, con, seq, tmps)

hphp/hack/src/elab/pass.rs

@@ -3,7 +3,7 @@
 // This source code is licensed under the MIT license found in the
 // LICENSE file in the "hack" directory of this source tree.
 //
-// @generated SignedSource<<dd90fc34a9b41bbb91c8ab6d91388dbd>>
+// @generated SignedSource<<d6821a100f7d9912f85752537ad20c9b>>
 //
 // To regenerate this file, run:
 //   hphp/hack/src/oxidized_regen.sh
@@ -536,6 +536,22 @@ pub trait Pass: PassClone {
         Continue(())
     }
     #[inline(always)]
+    fn on_ty_argument_top_down(
+        &mut self,
+        env: &Env,
+        elem: &mut Argument<Ex, En>,
+    ) -> ControlFlow<()> {
+        Continue(())
+    }
+    #[inline(always)]
+    fn on_ty_argument_bottom_up(
+        &mut self,
+        env: &Env,
+        elem: &mut Argument<Ex, En>,
+    ) -> ControlFlow<()> {
+        Continue(())
+    }
+    #[inline(always)]
     fn on_ty_user_attribute_top_down(
         &mut self,
         env: &Env,
@@ -2130,6 +2146,28 @@ impl Pass for Passes {
         Continue(())
     }
     #[inline(always)]
+    fn on_ty_argument_top_down(
+        &mut self,
+        env: &Env,
+        elem: &mut Argument<Ex, En>,
+    ) -> ControlFlow<()> {
+        for pass in &mut self.passes {
+            pass.on_ty_argument_top_down(env, elem)?;
+        }
+        Continue(())
+    }
+    #[inline(always)]
+    fn on_ty_argument_bottom_up(
+        &mut self,
+        env: &Env,
+        elem: &mut Argument<Ex, En>,
+    ) -> ControlFlow<()> {
+        for pass in &mut self.passes {
+            pass.on_ty_argument_bottom_up(env, elem)?;
+        }
+        Continue(())
+    }
+    #[inline(always)]
     fn on_ty_user_attribute_top_down(
         &mut self,
         env: &Env,

hphp/hack/src/elab/passes/elab_cross_package.rs

@@ -12,9 +12,9 @@ use nast::Expr_;
 use nast::Fun_;
 use nast::Id;
 use nast::Method_;
-use nast::ParamKind;
 use nast::Stmt;
 use nast::Stmt_;
+use oxidized::ast;
 use oxidized::ast::Pos;
 
 use crate::prelude::*;
@@ -67,29 +67,21 @@ fn assert_package_is_loaded(pkg: &Expr) -> Stmt {
             ),
             targs: vec![],
             args: vec![
-                (
-                    ParamKind::Pnormal,
-                    Expr(
-                        (),
-                        pos(),
-                        Expr_::Call(Box::new(CallExpr {
-                            func: Expr(
-                                (),
-                                pos(),
-                                Expr_::Id(Box::new(Id(
-                                    pos(),
-                                    pseudo_functions::PACKAGE_EXISTS.into(),
-                                ))),
-                            ),
-                            targs: vec![],
-                            args: vec![(ParamKind::Pnormal, { pkg.clone() })],
-                            unpacked_arg: None,
-                        })),
-                    ),
-                ),
-                (ParamKind::Pnormal, {
-                    Expr((), pos(), Expr_::String(msg.into()))
-                }),
+                ast::Argument::Anormal(Expr(
+                    (),
+                    pos(),
+                    Expr_::Call(Box::new(CallExpr {
+                        func: Expr(
+                            (),
+                            pos(),
+                            Expr_::Id(Box::new(Id(pos(), pseudo_functions::PACKAGE_EXISTS.into()))),
+                        ),
+                        targs: vec![],
+                        args: vec![ast::Argument::Anormal(pkg.clone())],
+                        unpacked_arg: None,
+                    })),
+                )),
+                ast::Argument::Anormal(Expr((), pos(), Expr_::String(msg.into()))),
             ],
             unpacked_arg: None,
         })),

hphp/hack/src/elab/passes/elab_expr_package.rs

@@ -8,7 +8,7 @@ use nast::CallExpr;
 use nast::Expr;
 use nast::Expr_;
 use nast::Id;
-use nast::ParamKind;
+use oxidized::ast;
 
 use crate::prelude::*;
 
@@ -27,10 +27,11 @@ impl Pass for ElabExprPackagePass {
                         Expr_::mk_id(Id(pos.clone(), pseudo_functions::PACKAGE_EXISTS.into())),
                     ),
                     targs: vec![],
-                    args: vec![(
-                        ParamKind::Pnormal,
-                        Expr::new((), pkg.pos().clone(), Expr_::mk_string(pkg.name().into())),
-                    )],
+                    args: vec![ast::Argument::Anormal(Expr::new(
+                        (),
+                        pkg.pos().clone(),
+                        Expr_::mk_string(pkg.name().into()),
+                    ))],
                     unpacked_arg: None,
                 }));
                 Break(())
@@ -67,7 +68,8 @@ mod tests {
                 args,
                 ..
             }) if fn_name == pseudo_functions::PACKAGE_EXISTS => {
-                if let [(ParamKind::Pnormal, Expr(_, _, Expr_::String(fn_param_name)))] = &args[..]
+                if let [ast::Argument::Anormal(Expr(_, _, Expr_::String(fn_param_name)))] =
+                    &args[..]
                 {
                     fn_param_name == "foo"
                 } else {

hphp/hack/src/elab/transform.rs

@@ -3,7 +3,7 @@
 // This source code is licensed under the MIT license found in the
 // LICENSE file in the "hack" directory of this source tree.
 //
-// @generated SignedSource<<6f5e023411c62a93f680149e18b781cf>>
+// @generated SignedSource<<a522ff8ab90f77d0870b8a09ca421d89>>
 //
 // To regenerate this file, run:
 //   hphp/hack/src/oxidized_regen.sh
@@ -1209,6 +1209,27 @@ impl Transform for CallExpr {
         }
     }
 }
+impl Transform for Argument {
+    fn transform(&mut self, env: &Env, pass: &mut (impl Pass + Clone)) {
+        let mut in_pass = pass.clone();
+        if let Break(..) = pass.on_ty_argument_top_down(env, self) {
+            return;
+        }
+        stack_limit::maybe_grow(|| self.traverse(env, pass));
+        in_pass.on_ty_argument_bottom_up(env, self);
+    }
+    fn traverse(&mut self, env: &Env, pass: &mut (impl Pass + Clone)) {
+        match self {
+            Argument::Ainout(ref mut __binding_0, ref mut __binding_1) => {
+                {
+                    __binding_0.transform(env, &mut pass.clone())
+                }
+                { __binding_1.transform(env, &mut pass.clone()) }
+            }
+            Argument::Anormal(ref mut __binding_0) => __binding_0.transform(env, &mut pass.clone()),
+        }
+    }
+}
 impl Transform for UserAttribute {
     fn transform(&mut self, env: &Env, pass: &mut (impl Pass + Clone)) {
         let mut in_pass = pass.clone();