[parser] Rename and consolidate RestProperty to RestElement

Summary:
Previously, the rest node for object patterns was called `RestProperty`. Now, ESTree specifies it as `RestElement` (https://github.com/estree/estree/blob/master/es2015.md#restelement, https://github.com/estree/estree/blob/master/es2018.md#patterns). Babel Parser and Acorn parse it as `RestElement`.

Because array patterns also use `RestElement`, we not only rename, but also consolidate into a single node `Pattern.RestElement`, and deduplicate some code.

Reviewed By: mroch

Differential Revision: D20721855

fbshipit-source-id: 730b13ccd64a774ec3a76d770fde389fe8439e80

packages/flow-parser/test/custom_ast_types.js

@@ -201,11 +201,7 @@ def("MetaProperty")
 def("ObjectPattern")
   .bases("Pattern")
   .build("properties")
-  .field("properties", [or(def("RestProperty"), def("Property"))]);
-def("RestProperty")
-  .bases("Node")
-  .build("argument")
-  .field("argument", def("Expression"));
+  .field("properties", [or(def("RestElement"), def("Property"))]);
 
 // https://github.com/benjamn/ast-types/issues/183
 def("Super")

packages/flow-parser/test/esprima_test_runner.js

@@ -121,7 +121,7 @@ function handleSpecialObjectCompare(esprima, flow, env) {
         var prop = esprima.properties[i];
         switch (prop.type) {
           case 'SpreadProperty':
-            prop.type = 'RestProperty';
+            prop.type = 'RestElement';
             break;
         }
       }

src/parser/declaration_parser.ml

@@ -71,25 +71,26 @@ module Declaration (Parse : Parser_common.PARSER) (Type : Type_parser.TYPE) : DE
           check_env)
     and _object check_env o = List.fold_left object_property check_env o.Pattern.Object.properties
     and object_property check_env =
-      Pattern.Object.(
-        function
-        | Property (_, property) ->
-          Property.(
-            let check_env =
-              match property.key with
-              | Identifier id -> identifier_no_dupe_check check_env id
-              | _ -> check_env
-            in
-            pattern check_env property.pattern)
-        | RestProperty (_, { RestProperty.argument; comments = _ }) -> pattern check_env argument)
+      let open Pattern.Object in
+      function
+      | Property (_, property) ->
+        Property.(
+          let check_env =
+            match property.key with
+            | Identifier id -> identifier_no_dupe_check check_env id
+            | _ -> check_env
+          in
+          pattern check_env property.pattern)
+      | RestElement (_, { Pattern.RestElement.argument; comments = _ }) ->
+        pattern check_env argument
     and _array check_env arr = List.fold_left array_element check_env arr.Pattern.Array.elements
     and array_element check_env =
-      Pattern.Array.(
-        function
-        | None -> check_env
-        | Some (Element (_, { Element.argument; default = _ })) -> pattern check_env argument
-        | Some (RestElement (_, { RestElement.argument; comments = _ })) ->
-          pattern check_env argument)
+      let open Pattern.Array in
+      function
+      | None -> check_env
+      | Some (Element (_, { Element.argument; default = _ })) -> pattern check_env argument
+      | Some (RestElement (_, { Pattern.RestElement.argument; comments = _ })) ->
+        pattern check_env argument
     and identifier_pattern check_env { Pattern.Identifier.name = id; _ } = identifier check_env id
     and identifier (env, param_names) ((loc, { Identifier.name; comments = _ }) as id) =
       if SSet.mem name param_names then error_at env (loc, Parse_error.StrictParamDupe);

src/parser/estree_translator.ml

@@ -1080,14 +1080,15 @@ with type t = Impl.t = struct
         let params = List.rev (rest :: rev_params) in
         array params
       | (_, { params; rest = None }) -> array_of_list function_param params
+    and rest_element loc { Pattern.RestElement.argument; comments } =
+      node ?comments "RestElement" loc [("argument", pattern argument)]
     and array_pattern_element =
-      Pattern.Array.(
-        function
-        | Element (loc, { Element.argument; default = Some default }) ->
-          node "AssignmentPattern" loc [("left", pattern argument); ("right", expression default)]
-        | Element (_loc, { Element.argument; default = None }) -> pattern argument
-        | RestElement (loc, { RestElement.argument; comments }) ->
-          node ?comments "RestElement" loc [("argument", pattern argument)])
+      let open Pattern.Array in
+      function
+      | Element (loc, { Element.argument; default = Some default }) ->
+        node "AssignmentPattern" loc [("left", pattern argument); ("right", expression default)]
+      | Element (_loc, { Element.argument; default = None }) -> pattern argument
+      | RestElement (loc, el) -> rest_element loc el
     and object_property =
       let open Expression.Object in
       function
@@ -1126,37 +1127,36 @@ with type t = Impl.t = struct
       | SpreadProperty (loc, { SpreadProperty.argument; comments }) ->
         node ?comments "SpreadProperty" loc [("argument", expression argument)]
     and object_pattern_property =
-      Pattern.Object.(
-        function
-        | Property (loc, { Property.key; pattern = patt; default; shorthand }) ->
-          let (key, computed, comments) =
-            match key with
-            | Property.Literal lit -> (literal lit, false, None)
-            | Property.Identifier id -> (identifier id, false, None)
-            | Property.Computed (_, { ComputedKey.expression = expr; comments }) ->
-              (expression expr, true, comments)
-          in
-          let value =
-            match default with
-            | Some default ->
-              let loc = Loc.btwn (fst patt) (fst default) in
-              node "AssignmentPattern" loc [("left", pattern patt); ("right", expression default)]
-            | None -> pattern patt
-          in
-          node
-            ?comments
-            "Property"
-            loc
-            [
-              ("key", key);
-              ("value", value);
-              ("kind", string "init");
-              ("method", bool false);
-              ("shorthand", bool shorthand);
-              ("computed", bool computed);
-            ]
-        | RestProperty (loc, { RestProperty.argument; comments }) ->
-          node ?comments "RestProperty" loc [("argument", pattern argument)])
+      let open Pattern.Object in
+      function
+      | Property (loc, { Property.key; pattern = patt; default; shorthand }) ->
+        let (key, computed, comments) =
+          match key with
+          | Property.Literal lit -> (literal lit, false, None)
+          | Property.Identifier id -> (identifier id, false, None)
+          | Property.Computed (_, { ComputedKey.expression = expr; comments }) ->
+            (expression expr, true, comments)
+        in
+        let value =
+          match default with
+          | Some default ->
+            let loc = Loc.btwn (fst patt) (fst default) in
+            node "AssignmentPattern" loc [("left", pattern patt); ("right", expression default)]
+          | None -> pattern patt
+        in
+        node
+          ?comments
+          "Property"
+          loc
+          [
+            ("key", key);
+            ("value", value);
+            ("kind", string "init");
+            ("method", bool false);
+            ("shorthand", bool shorthand);
+            ("computed", bool computed);
+          ]
+      | RestElement (loc, el) -> rest_element loc el
     and expression_or_spread =
       let open Expression in
       function

src/parser/flow_ast.ml

@@ -1471,6 +1471,16 @@ end =
   JSX
 
 and Pattern : sig
+  module RestElement : sig
+    type ('M, 'T) t = 'M * ('M, 'T) t'
+
+    and ('M, 'T) t' = {
+      argument: ('M, 'T) Pattern.t;
+      comments: ('M, unit) Syntax.t option;
+    }
+    [@@deriving show]
+  end
+
   module Object : sig
     module Property : sig
       type ('M, 'T) key =
@@ -1489,19 +1499,9 @@ and Pattern : sig
       [@@deriving show]
     end
 
-    module RestProperty : sig
-      type ('M, 'T) t = 'M * ('M, 'T) t'
-
-      and ('M, 'T) t' = {
-        argument: ('M, 'T) Pattern.t;
-        comments: ('M, unit) Syntax.t option;
-      }
-      [@@deriving show]
-    end
-
     type ('M, 'T) property =
       | Property of ('M, 'T) Property.t
-      | RestProperty of ('M, 'T) RestProperty.t
+      | RestElement of ('M, 'T) RestElement.t
 
     and ('M, 'T) t = {
       properties: ('M, 'T) property list;
@@ -1521,16 +1521,6 @@ and Pattern : sig
       [@@deriving show]
     end
 
-    module RestElement : sig
-      type ('M, 'T) t = 'M * ('M, 'T) t'
-
-      and ('M, 'T) t' = {
-        argument: ('M, 'T) Pattern.t;
-        comments: ('M, unit) Syntax.t option;
-      }
-      [@@deriving show]
-    end
-
     type ('M, 'T) element =
       | Element of ('M, 'T) Element.t
       | RestElement of ('M, 'T) RestElement.t

src/parser/flow_ast_utils.ml

@@ -19,7 +19,7 @@ let rec fold_bindings_of_pattern =
       Object.(
         function
         | Property (_, { Property.pattern = p; _ })
-        | RestProperty (_, { RestProperty.argument = p; comments = _ }) ->
+        | RestElement (_, { RestElement.argument = p; comments = _ }) ->
           fold_bindings_of_pattern f acc p)
     in
     let element f acc =

src/parser/parse_error.ml

@@ -103,7 +103,7 @@ type t =
   | AdjacentJSXElements
   | ParameterAfterRestParameter
   | ElementAfterRestElement
-  | PropertyAfterRestProperty
+  | PropertyAfterRestElement
   | DeclareAsync
   | DeclareClassElement
   | DeclareClassFieldInitializer
@@ -319,7 +319,7 @@ module PP = struct
       ^ "elements must be wrapped in an enclosing parent tag"
     | ParameterAfterRestParameter -> "Rest parameter must be final parameter of an argument list"
     | ElementAfterRestElement -> "Rest element must be final element of an array pattern"
-    | PropertyAfterRestProperty -> "Rest property must be final property of an object pattern"
+    | PropertyAfterRestElement -> "Rest property must be final property of an object pattern"
     | DeclareAsync ->
       "async is an implementation detail and isn't necessary for your declare function statement. It is sufficient for your declare function to just have a Promise return type."
     | DeclareClassElement -> "`declare` modifier can only appear on class fields."

src/parser/pattern_parser.ml

@@ -56,13 +56,13 @@ module Pattern (Parse : Parser_common.PARSER) (Type : Type_parser.TYPE) = struct
         properties env acc remaining
       | [SpreadProperty (loc, { SpreadProperty.argument; comments })] ->
         let acc =
-          Pattern.Object.(
-            RestProperty (loc, { RestProperty.argument = from_expr env argument; comments }))
+          Pattern.Object.RestElement
+            (loc, { Pattern.RestElement.argument = from_expr env argument; comments })
           :: acc
         in
         properties env acc []
       | SpreadProperty (loc, _) :: remaining ->
-        error_at env (loc, Parse_error.PropertyAfterRestProperty);
+        error_at env (loc, Parse_error.PropertyAfterRestElement);
         properties env acc remaining
     in
     fun env (loc, { Ast.Expression.Object.properties = props; comments = _ (* TODO *) }) ->
@@ -93,7 +93,8 @@ module Pattern (Parse : Parser_common.PARSER) (Type : Type_parser.TYPE) = struct
         let acc =
           match assignment_target env argument with
           | Some argument ->
-            Some Pattern.Array.(RestElement (loc, { RestElement.argument; comments })) :: acc
+            Some (Pattern.Array.RestElement (loc, { Pattern.RestElement.argument; comments }))
+            :: acc
           | None -> acc
         in
         elements env acc []
@@ -171,9 +172,9 @@ module Pattern (Parse : Parser_common.PARSER) (Type : Type_parser.TYPE) = struct
             pattern env restricted_error)
           env
       in
-      Pattern.Object.(
-        RestProperty
-          (loc, { RestProperty.argument; comments = Flow_ast_utils.mk_comments_opt ~leading () }))
+      Pattern.Object.RestElement
+        ( loc,
+          { Pattern.RestElement.argument; comments = Flow_ast_utils.mk_comments_opt ~leading () } )
     in
     let property_default env =
       match Peek.token env with
@@ -262,18 +263,17 @@ module Pattern (Parse : Parser_common.PARSER) (Type : Type_parser.TYPE) = struct
         List.rev acc
       | _ ->
         (match property env with
-        | Some ((Pattern.Object.Property (loc, _) | Pattern.Object.RestProperty (loc, _)) as prop)
-          ->
+        | Some ((Pattern.Object.Property (loc, _) | Pattern.Object.RestElement (loc, _)) as prop) ->
           let rest_trailing_comma =
             if seen_rest then (
-              error_at env (loc, Parse_error.PropertyAfterRestProperty);
+              error_at env (loc, Parse_error.PropertyAfterRestElement);
               None
             ) else
               rest_trailing_comma
           in
           let (seen_rest, rest_trailing_comma) =
             match prop with
-            | Pattern.Object.RestProperty _ ->
+            | Pattern.Object.RestElement _ ->
               ( true,
                 if Peek.token env = T_COMMA then
                   Some (Peek.loc env)
@@ -317,9 +317,12 @@ module Pattern (Parse : Parser_common.PARSER) (Type : Type_parser.TYPE) = struct
             env
         in
         let element =
-          Pattern.Array.(
-            RestElement
-              (loc, { RestElement.argument; comments = Flow_ast_utils.mk_comments_opt ~leading () }))
+          Pattern.Array.RestElement
+            ( loc,
+              {
+                Pattern.RestElement.argument;
+                comments = Flow_ast_utils.mk_comments_opt ~leading ();
+              } )
         in
         (* rest elements are always last, the closing ] should be next. but if not,
            error and keep going so we recover gracefully by parsing the rest of the

src/parser/statement_parser.ml

@@ -1044,30 +1044,29 @@ module Statement
 
   and extract_pattern_binding_names =
     let rec fold acc =
-      Pattern.(
-        function
-        | (_, Object { Object.properties; _ }) ->
-          List.fold_left
-            (fun acc prop ->
-              match prop with
-              | Object.Property (_, { Object.Property.pattern; _ })
-              | Object.RestProperty (_, { Object.RestProperty.argument = pattern; comments = _ }) ->
-                fold acc pattern)
-            acc
-            properties
-        | (_, Array { Array.elements; _ }) ->
-          List.fold_left
-            Array.(
-              fun acc elem ->
-                match elem with
-                | Some (Element (_, { Element.argument = pattern; default = _ }))
-                | Some (RestElement (_, { RestElement.argument = pattern; comments = _ })) ->
-                  fold acc pattern
-                | None -> acc)
-            acc
-            elements
-        | (_, Identifier { Pattern.Identifier.name; _ }) -> name :: acc
-        | (_, Expression _) -> failwith "Parser error: No such thing as an expression pattern!")
+      let open Pattern in
+      function
+      | (_, Object { Object.properties; _ }) ->
+        List.fold_left
+          (fun acc prop ->
+            match prop with
+            | Object.Property (_, { Object.Property.pattern; _ })
+            | Object.RestElement (_, { RestElement.argument = pattern; comments = _ }) ->
+              fold acc pattern)
+          acc
+          properties
+      | (_, Array { Array.elements; _ }) ->
+        List.fold_left
+          (fun acc elem ->
+            match elem with
+            | Some (Array.Element (_, { Array.Element.argument = pattern; default = _ }))
+            | Some (Array.RestElement (_, { RestElement.argument = pattern; comments = _ })) ->
+              fold acc pattern
+            | None -> acc)
+          acc
+          elements
+      | (_, Identifier { Pattern.Identifier.name; _ }) -> name :: acc
+      | (_, Expression _) -> failwith "Parser error: No such thing as an expression pattern!"
     in
     List.fold_left fold
 

src/parser/test/esprima/ES6/rest-property/assignment.diff

@@ -0,0 +1,15 @@
+{
+  "body": {
+    "0": {
+      "expression": {
+        "left": {
+          "properties" : {
+            "0": {
+              "type": "RestElement"
+            }
+          }
+        }
+      }
+    }
+  }
+}