[flow][match] Parse and error on invalid object pattern shorthand

Summary:
Parse and give a good Flow error explaining the alternatives for this invalid syntax.
In the future I will also add 2 quickfixes (one for each alternative).

Changelog: [internal]

Reviewed By: panagosg7

Differential Revision: D70499683

fbshipit-source-id: 111a005422d77b2058d4490328064f2ab0f542ce

src/analysis/env_builder/name_def.ml

@@ -382,10 +382,15 @@ end = struct
 
   and object_properties ~visit_binding ~visit_expression ~visit_intermediate acc properties =
     Base.List.fold properties ~init:[] ~f:(fun used_props prop ->
-        let (_, { ObjectPattern.Property.key; pattern; shorthand = _; comments = _ }) = prop in
-        let (acc, prop_name) = object_property acc key in
-        visit_pattern ~visit_binding ~visit_expression ~visit_intermediate acc pattern;
-        prop_name :: used_props
+        match prop with
+        | ( _,
+            ObjectPattern.Property.Valid
+              { ObjectPattern.Property.key; pattern; shorthand = _; comments = _ }
+          ) ->
+          let (acc, prop_name) = object_property acc key in
+          visit_pattern ~visit_binding ~visit_expression ~visit_intermediate acc pattern;
+          prop_name :: used_props
+        | (_, ObjectPattern.Property.InvalidShorthand _) -> used_props
     )
 end
 

src/analysis/env_builder/name_resolver.ml

@@ -3272,10 +3272,11 @@ module Make (Context : C) (FlowAPIUtils : F with type cx = Context.t) :
               this#add_single_refinement key ~refining_locs:(L.LSet.singleton loc) refi
             | None -> ());
             let bindings =
-              Base.List.fold properties ~init:bindings ~f:(fun bindings prop ->
-                  let (loc, { ObjectPattern.Property.key; pattern; shorthand = _; comments = _ }) =
-                    prop
-                  in
+              Base.List.fold properties ~init:bindings ~f:(fun bindings -> function
+                | ( loc,
+                    ObjectPattern.Property.Valid
+                      { ObjectPattern.Property.key; pattern; shorthand = _; comments = _ }
+                  ) ->
                   let (property, propname) =
                     match key with
                     | ObjectPattern.Property.Identifier ((_, { Ast.Identifier.name; _ }) as id) ->
@@ -3307,6 +3308,7 @@ module Make (Context : C) (FlowAPIUtils : F with type cx = Context.t) :
                     | None -> ())
                   | None -> ());
                   recurse member pattern bindings
+                | (_, ObjectPattern.Property.InvalidShorthand _) -> bindings
               )
             in
             bindings_of_rest bindings rest

src/analysis/scope_builder.ml

@@ -301,6 +301,12 @@ module Make (L : Loc_sig.S) (Api : Scope_api_sig.S with module L = L) :
 
       method! match_object_pattern_property_key key = key
 
+      method! match_object_pattern_property prop =
+        let open Ast.MatchPattern.ObjectPattern.Property in
+        match prop with
+        | (_, Valid _) -> super#match_object_pattern_property prop
+        | (_, InvalidShorthand _) -> prop
+
       (* don't rename the `Foo` in `enum E { Foo }` *)
       method! enum_member_identifier id = id
 

src/parser/estree_translator.ml

@@ -732,17 +732,31 @@ with type t = Impl.t = struct
           | ObjectPattern.Property.NumberLiteral lit -> number_literal lit
           | ObjectPattern.Property.Identifier id -> identifier id
         in
-        let property (loc, { ObjectPattern.Property.key; pattern; shorthand; comments }) =
-          node
-            ?comments
-            "MatchObjectPatternProperty"
-            loc
-            [
-              ("key", property_key key);
-              ("pattern", match_pattern pattern);
-              ("shorthand", bool shorthand);
-            ]
+        let property = function
+          | ( loc,
+              ObjectPattern.Property.Valid
+                { ObjectPattern.Property.key; pattern; shorthand; comments }
+            ) ->
+            node
+              ?comments
+              "MatchObjectPatternProperty"
+              loc
+              [
+                ("key", property_key key);
+                ("pattern", match_pattern pattern);
+                ("shorthand", bool shorthand);
+              ]
+          | (loc, ObjectPattern.Property.InvalidShorthand id) ->
+            node
+              "MatchObjectPatternProperty"
+              loc
+              [
+                ("key", identifier id);
+                ("pattern", match_identifier_pattern id);
+                ("shorthand", bool true);
+              ]
         in
+
         node
           ?comments:(format_internal_comments comments)
           "MatchObjectPattern"

src/parser/flow_ast.ml

@@ -2013,14 +2013,19 @@ and MatchPattern : sig
         | NumberLiteral of ('M * 'M NumberLiteral.t)
         | Identifier of ('M, 'T) Identifier.t
 
-      and ('M, 'T) t = 'M * ('M, 'T) t'
-
-      and ('M, 'T) t' = {
+      and ('M, 'T) property = {
         key: ('M, 'T) key;
         pattern: ('M, 'T) MatchPattern.t;
         shorthand: bool;
         comments: ('M, unit) Syntax.t option;
       }
+
+      and ('M, 'T) t = 'M * ('M, 'T) t'
+
+      and ('M, 'T) t' =
+        | Valid of ('M, 'T) property
+        (* Invalid code parsed so we can error with quick-fix. *)
+        | InvalidShorthand of ('M, 'M) Identifier.t
       [@@deriving show]
     end
 

src/parser/flow_ast_mapper.ml

@@ -2676,14 +2676,21 @@ class ['loc] mapper =
     method match_object_pattern_property
         (prop : ('loc, 'loc) Ast.MatchPattern.ObjectPattern.Property.t) =
       let open Ast.MatchPattern.ObjectPattern.Property in
-      let (loc, { key; pattern; shorthand; comments }) = prop in
-      let key' = this#match_object_pattern_property_key key in
-      let pattern' = this#match_pattern pattern in
-      let comments' = this#syntax_opt comments in
-      if key == key' && pattern == pattern' && comments == comments' then
-        prop
-      else
-        (loc, { key = key'; pattern = pattern'; shorthand; comments = comments' })
+      match prop with
+      | (loc, Valid { key; pattern; shorthand; comments }) ->
+        let key' = this#match_object_pattern_property_key key in
+        let pattern' = this#match_pattern pattern in
+        let comments' = this#syntax_opt comments in
+        if key == key' && pattern == pattern' && comments == comments' then
+          prop
+        else
+          (loc, Valid { key = key'; pattern = pattern'; shorthand; comments = comments' })
+      | (loc, InvalidShorthand id) ->
+        let id' = this#identifier id in
+        if id == id' then
+          prop
+        else
+          (loc, InvalidShorthand id')
 
     method match_object_pattern_property_key
         (key : ('loc, 'loc) Ast.MatchPattern.ObjectPattern.Property.key) =

src/parser/flow_ast_utils.ml

@@ -86,7 +86,11 @@ let rec pattern_has_binding =
 
 let rec match_pattern_has_binding =
   let open MatchPattern in
-  let property (_, { ObjectPattern.Property.pattern = p; _ }) = match_pattern_has_binding p in
+  let property = function
+    | (_, ObjectPattern.Property.Valid { ObjectPattern.Property.pattern = p; _ }) ->
+      match_pattern_has_binding p
+    | (_, ObjectPattern.Property.InvalidShorthand _) -> false
+  in
   let rest_has_binding = function
     | Some (_, { RestPattern.argument = Some _; comments = _ }) -> true
     | _ -> false

src/parser/match_pattern_parser.ml

@@ -276,19 +276,30 @@ module Match_pattern (Parse : PARSER) : Parser_common.MATCH_PATTERN = struct
             let pattern = (loc, BindingPattern binding) in
             let trailing = Eat.trailing_comments env in
             let comments = Flow_ast_utils.mk_comments_opt ~leading ~trailing () in
-            { ObjectPattern.Property.key; pattern; shorthand = true; comments }
+            ObjectPattern.Property.Valid
+              { ObjectPattern.Property.key; pattern; shorthand = true; comments }
           in
           match Peek.token env with
           | T_CONST -> shorthand_prop (binding_pattern env ~kind:Ast.Variable.Const)
           | T_LET -> shorthand_prop (binding_pattern env ~kind:Ast.Variable.Let)
           | T_VAR -> shorthand_prop (binding_pattern env ~kind:Ast.Variable.Var)
+          | _
+            when Peek.is_identifier env
+                 &&
+                 match Peek.ith_token ~i:1 env with
+                 | T_COMMA
+                 | T_RCURLY ->
+                   true
+                 | _ -> false ->
+            ObjectPattern.Property.InvalidShorthand (identifier_name env)
           | _ ->
             let key = property_key env in
             Expect.token env T_COLON;
             let pattern = match_pattern env in
             let trailing = Eat.trailing_comments env in
             let comments = Flow_ast_utils.mk_comments_opt ~leading ~trailing () in
-            { ObjectPattern.Property.key; pattern; shorthand = false; comments }
+            ObjectPattern.Property.Valid
+              { ObjectPattern.Property.key; pattern; shorthand = false; comments }
       )
     in
     let rec properties env acc =

src/parser/test/flow/match/pattern-object.js

@@ -8,4 +8,6 @@ const e = match (x) {
   {const x, ...let y}: y,
   {const x, ...var z}: y,
   {const x, ...}: 1,
+  {x}: 1,
+  {x, foo: 1}: 1,
 };

src/parser/test/flow/match/pattern-object.tree.json

@@ -1,17 +1,17 @@
 {
   "type":"Program",
-  "loc":{"source":null,"start":{"line":1,"column":0},"end":{"line":11,"column":2}},
-  "range":[0,234],
+  "loc":{"source":null,"start":{"line":1,"column":0},"end":{"line":13,"column":2}},
+  "range":[0,262],
   "body":[
     {
       "type":"VariableDeclaration",
-      "loc":{"source":null,"start":{"line":1,"column":0},"end":{"line":11,"column":2}},
-      "range":[0,234],
+      "loc":{"source":null,"start":{"line":1,"column":0},"end":{"line":13,"column":2}},
+      "range":[0,262],
       "declarations":[
         {
           "type":"VariableDeclarator",
-          "loc":{"source":null,"start":{"line":1,"column":6},"end":{"line":11,"column":1}},
-          "range":[6,233],
+          "loc":{"source":null,"start":{"line":1,"column":6},"end":{"line":13,"column":1}},
+          "range":[6,261],
           "id":{
             "type":"Identifier",
             "loc":{"source":null,"start":{"line":1,"column":6},"end":{"line":1,"column":7}},
@@ -22,8 +22,8 @@
           },
           "init":{
             "type":"MatchExpression",
-            "loc":{"source":null,"start":{"line":1,"column":10},"end":{"line":11,"column":1}},
-            "range":[10,233],
+            "loc":{"source":null,"start":{"line":1,"column":10},"end":{"line":13,"column":1}},
+            "range":[10,261],
             "argument":{
               "type":"Identifier",
               "loc":{"source":null,"start":{"line":1,"column":17},"end":{"line":1,"column":18}},
@@ -611,6 +611,128 @@
                   "raw":"1"
                 },
                 "guard":null
+              },
+              {
+                "type":"MatchExpressionCase",
+                "loc":{"source":null,"start":{"line":11,"column":2},"end":{"line":11,"column":9}},
+                "range":[234,241],
+                "pattern":{
+                  "type":"MatchObjectPattern",
+                  "loc":{"source":null,"start":{"line":11,"column":2},"end":{"line":11,"column":5}},
+                  "range":[234,237],
+                  "properties":[
+                    {
+                      "type":"MatchObjectPatternProperty",
+                      "loc":{"source":null,"start":{"line":11,"column":3},"end":{"line":11,"column":4}},
+                      "range":[235,236],
+                      "key":{
+                        "type":"Identifier",
+                        "loc":{"source":null,"start":{"line":11,"column":3},"end":{"line":11,"column":4}},
+                        "range":[235,236],
+                        "name":"x",
+                        "typeAnnotation":null,
+                        "optional":false
+                      },
+                      "pattern":{
+                        "type":"MatchIdentifierPattern",
+                        "loc":{"source":null,"start":{"line":11,"column":3},"end":{"line":11,"column":4}},
+                        "range":[235,236],
+                        "id":{
+                          "type":"Identifier",
+                          "loc":{"source":null,"start":{"line":11,"column":3},"end":{"line":11,"column":4}},
+                          "range":[235,236],
+                          "name":"x",
+                          "typeAnnotation":null,
+                          "optional":false
+                        }
+                      },
+                      "shorthand":true
+                    }
+                  ],
+                  "rest":null
+                },
+                "body":{
+                  "type":"Literal",
+                  "loc":{"source":null,"start":{"line":11,"column":7},"end":{"line":11,"column":8}},
+                  "range":[239,240],
+                  "value":1,
+                  "raw":"1"
+                },
+                "guard":null
+              },
+              {
+                "type":"MatchExpressionCase",
+                "loc":{"source":null,"start":{"line":12,"column":2},"end":{"line":12,"column":17}},
+                "range":[244,259],
+                "pattern":{
+                  "type":"MatchObjectPattern",
+                  "loc":{"source":null,"start":{"line":12,"column":2},"end":{"line":12,"column":13}},
+                  "range":[244,255],
+                  "properties":[
+                    {
+                      "type":"MatchObjectPatternProperty",
+                      "loc":{"source":null,"start":{"line":12,"column":3},"end":{"line":12,"column":4}},
+                      "range":[245,246],
+                      "key":{
+                        "type":"Identifier",
+                        "loc":{"source":null,"start":{"line":12,"column":3},"end":{"line":12,"column":4}},
+                        "range":[245,246],
+                        "name":"x",
+                        "typeAnnotation":null,
+                        "optional":false
+                      },
+                      "pattern":{
+                        "type":"MatchIdentifierPattern",
+                        "loc":{"source":null,"start":{"line":12,"column":3},"end":{"line":12,"column":4}},
+                        "range":[245,246],
+                        "id":{
+                          "type":"Identifier",
+                          "loc":{"source":null,"start":{"line":12,"column":3},"end":{"line":12,"column":4}},
+                          "range":[245,246],
+                          "name":"x",
+                          "typeAnnotation":null,
+                          "optional":false
+                        }
+                      },
+                      "shorthand":true
+                    },
+                    {
+                      "type":"MatchObjectPatternProperty",
+                      "loc":{"source":null,"start":{"line":12,"column":6},"end":{"line":12,"column":12}},
+                      "range":[248,254],
+                      "key":{
+                        "type":"Identifier",
+                        "loc":{"source":null,"start":{"line":12,"column":6},"end":{"line":12,"column":9}},
+                        "range":[248,251],
+                        "name":"foo",
+                        "typeAnnotation":null,
+                        "optional":false
+                      },
+                      "pattern":{
+                        "type":"MatchLiteralPattern",
+                        "loc":{"source":null,"start":{"line":12,"column":11},"end":{"line":12,"column":12}},
+                        "range":[253,254],
+                        "literal":{
+                          "type":"Literal",
+                          "loc":{"source":null,"start":{"line":12,"column":11},"end":{"line":12,"column":12}},
+                          "range":[253,254],
+                          "value":1,
+                          "raw":"1"
+                        }
+                      },
+                      "shorthand":false
+                    }
+                  ],
+                  "rest":null
+                },
+                "body":{
+                  "type":"Literal",
+                  "loc":{"source":null,"start":{"line":12,"column":15},"end":{"line":12,"column":16}},
+                  "range":[257,258],
+                  "value":1,
+                  "raw":"1"
+                },
+                "guard":null
               }
             ]
           }

src/parser_utils/flow_polymorphic_ast_mapper.ml

@@ -2196,11 +2196,15 @@ class virtual ['M, 'T, 'N, 'U] mapper =
     method match_object_pattern_property (prop : ('M, 'T) Ast.MatchPattern.ObjectPattern.Property.t)
         : ('N, 'U) Ast.MatchPattern.ObjectPattern.Property.t =
       let open Ast.MatchPattern.ObjectPattern.Property in
-      let (loc, { key; pattern; shorthand; comments }) = prop in
-      let key' = this#match_object_pattern_property_key key in
-      let pattern' = this#match_pattern pattern in
-      let comments' = this#syntax_opt comments in
-      (this#on_loc_annot loc, { key = key'; pattern = pattern'; shorthand; comments = comments' })
+      match prop with
+      | (loc, Valid { key; pattern; shorthand; comments }) ->
+        let key' = this#match_object_pattern_property_key key in
+        let pattern' = this#match_pattern pattern in
+        let comments' = this#syntax_opt comments in
+        ( this#on_loc_annot loc,
+          Valid { key = key'; pattern = pattern'; shorthand; comments = comments' }
+        )
+      | (loc, InvalidShorthand id) -> (this#on_loc_annot loc, InvalidShorthand (this#identifier id))
 
     method match_object_pattern_property_key
         (key : ('M, 'T) Ast.MatchPattern.ObjectPattern.Property.key)