From a2c50b2c1e9cc095188934d6b6cce93ba3eb0a2e Mon Sep 17 00:00:00 2001 From: George Zahariev Date: Tue, 4 Mar 2025 18:22:16 -0800 Subject: [PATCH] [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 | 13 +- src/analysis/env_builder/name_resolver.ml | 10 +- src/analysis/scope_builder.ml | 6 + src/parser/estree_translator.ml | 34 +++-- src/parser/flow_ast.ml | 11 +- src/parser/flow_ast_mapper.ml | 23 ++- src/parser/flow_ast_utils.ml | 6 +- src/parser/match_pattern_parser.ml | 15 +- src/parser/test/flow/match/pattern-object.js | 2 + .../test/flow/match/pattern-object.tree.json | 138 +++++++++++++++++- .../flow_polymorphic_ast_mapper.ml | 14 +- .../output/js_layout_generator.ml | 20 +-- .../get_def/get_def_process_location.ml | 22 +-- src/typing/debug_js.ml | 2 + src/typing/errors/error_message.ml | 10 ++ src/typing/errors/flow_intermediate_error.ml | 12 ++ .../errors/flow_intermediate_error_types.ml | 1 + src/typing/match_pattern.ml | 7 +- tests/match/match.exp | 22 ++- tests/match/pattern-errors.js | 11 ++ 20 files changed, 312 insertions(+), 67 deletions(-) diff --git a/src/analysis/env_builder/name_def.ml b/src/analysis/env_builder/name_def.ml index 37aa7c6d47d..2b99d78cbbe 100644 --- a/src/analysis/env_builder/name_def.ml +++ b/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 diff --git a/src/analysis/env_builder/name_resolver.ml b/src/analysis/env_builder/name_resolver.ml index 73209ada634..49781973737 100644 --- a/src/analysis/env_builder/name_resolver.ml +++ b/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 diff --git a/src/analysis/scope_builder.ml b/src/analysis/scope_builder.ml index e909ba473ad..020004f80d4 100644 --- a/src/analysis/scope_builder.ml +++ b/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 diff --git a/src/parser/estree_translator.ml b/src/parser/estree_translator.ml index f5d4047e20c..e81e6d096a4 100644 --- a/src/parser/estree_translator.ml +++ b/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" diff --git a/src/parser/flow_ast.ml b/src/parser/flow_ast.ml index 6f4555c53f7..990bc486a96 100644 --- a/src/parser/flow_ast.ml +++ b/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 diff --git a/src/parser/flow_ast_mapper.ml b/src/parser/flow_ast_mapper.ml index 40a9cecd907..3d0def692a6 100644 --- a/src/parser/flow_ast_mapper.ml +++ b/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) = diff --git a/src/parser/flow_ast_utils.ml b/src/parser/flow_ast_utils.ml index 25ed27c9773..c3773a43c27 100644 --- a/src/parser/flow_ast_utils.ml +++ b/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 diff --git a/src/parser/match_pattern_parser.ml b/src/parser/match_pattern_parser.ml index 6ba1f6621bd..4c279b42880 100644 --- a/src/parser/match_pattern_parser.ml +++ b/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 = diff --git a/src/parser/test/flow/match/pattern-object.js b/src/parser/test/flow/match/pattern-object.js index 6bcf6b8c2eb..99e094b6bc0 100644 --- a/src/parser/test/flow/match/pattern-object.js +++ b/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, }; diff --git a/src/parser/test/flow/match/pattern-object.tree.json b/src/parser/test/flow/match/pattern-object.tree.json index df118b9b9a7..5b44ca6dbfb 100644 --- a/src/parser/test/flow/match/pattern-object.tree.json +++ b/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 } ] } diff --git a/src/parser_utils/flow_polymorphic_ast_mapper.ml b/src/parser_utils/flow_polymorphic_ast_mapper.ml index 17e3771eaed..1c68516ff60 100644 --- a/src/parser_utils/flow_polymorphic_ast_mapper.ml +++ b/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) diff --git a/src/parser_utils/output/js_layout_generator.ml b/src/parser_utils/output/js_layout_generator.ml index 402d19d1184..d0555c21044 100644 --- a/src/parser_utils/output/js_layout_generator.ml +++ b/src/parser_utils/output/js_layout_generator.ml @@ -3425,15 +3425,17 @@ and match_object_pattern ~opts loc { Ast.MatchPattern.ObjectPattern.properties; in let props_rev = Base.List.rev_map - ~f:(fun (loc, { Property.key; pattern; shorthand; comments }) -> - layout_node_with_comments_opt - loc - comments - ( if shorthand then - match_pattern ~opts pattern - else - fuse [prop_key key; Atom ":"; pretty_space; match_pattern ~opts pattern] - )) + ~f:(function + | (loc, Property.Valid { Property.key; pattern; shorthand; comments }) -> + layout_node_with_comments_opt + loc + comments + ( if shorthand then + match_pattern ~opts pattern + else + fuse [prop_key key; Atom ":"; pretty_space; match_pattern ~opts pattern] + ) + | (_, Property.InvalidShorthand ident) -> identifier ident) properties in let props_rev = diff --git a/src/services/get_def/get_def_process_location.ml b/src/services/get_def/get_def_process_location.ml index 0a74103216e..15d29de90bb 100644 --- a/src/services/get_def/get_def_process_location.ml +++ b/src/services/get_def/get_def_process_location.ml @@ -745,16 +745,18 @@ class virtual ['T] searcher _cx ~is_local_use ~is_legit_require ~covers_target ~ location also covers the binding name. *) method! match_object_pattern_property prop = let open Ast.MatchPattern.ObjectPattern.Property in - let (loc, { key; pattern; shorthand; comments }) = prop in - let key = - if shorthand then - key - else - 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; pattern; shorthand; comments }) + match prop with + | (loc, Valid { key; pattern; shorthand; comments }) -> + let key = + if shorthand then + key + else + 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; pattern; shorthand; comments }) + | (_, InvalidShorthand _) -> prop end class typed_ast_searcher cx ~typed_ast:_ ~is_local_use ~is_legit_require ~covers_target ~purpose = diff --git a/src/typing/debug_js.ml b/src/typing/debug_js.ml index a22123427b7..89e9529fbc0 100644 --- a/src/typing/debug_js.ml +++ b/src/typing/debug_js.ml @@ -1890,6 +1890,8 @@ let dump_error_message = "EMatchInvalidPatternReference (%s) (%s)" (string_of_aloc loc) (dump_reason cx binding_reason) + | EMatchInvalidObjectShorthand { loc; name } -> + spf "EMatchInvalidObjectShorthand (%s) (%s)" (string_of_aloc loc) name | EUndocumentedFeature { loc } -> spf "EUndocumentedFeature (%s)" (string_of_aloc loc) | EDevOnlyRefinedLocInfo { refined_loc; refining_locs = _ } -> spf "EDevOnlyRefinedLocInfo {refined_loc=%s}" (string_of_aloc refined_loc) diff --git a/src/typing/errors/error_message.ml b/src/typing/errors/error_message.ml index 45389d10f4b..ae34e631c97 100644 --- a/src/typing/errors/error_message.ml +++ b/src/typing/errors/error_message.ml @@ -649,6 +649,10 @@ and 'loc t' = loc: 'loc; binding_reason: 'loc virtual_reason; } + | EMatchInvalidObjectShorthand of { + loc: 'loc; + name: string; + } | EUndocumentedFeature of { loc: 'loc } (* Dev only *) | EDevOnlyRefinedLocInfo of { @@ -1478,6 +1482,7 @@ let rec map_loc_of_error_message (f : 'a -> 'b) : 'a t' -> 'b t' = | EMatchInvalidAsPattern { loc } -> EMatchInvalidAsPattern { loc = f loc } | EMatchInvalidPatternReference { loc; binding_reason } -> EMatchInvalidPatternReference { loc = f loc; binding_reason = map_reason binding_reason } + | EMatchInvalidObjectShorthand { loc; name } -> EMatchInvalidObjectShorthand { loc = f loc; name } | EUndocumentedFeature { loc } -> EUndocumentedFeature { loc = f loc } | EDevOnlyInvalidatedRefinementInfo { read_loc; invalidation_info } -> EDevOnlyInvalidatedRefinementInfo @@ -1792,6 +1797,7 @@ let util_use_op_of_msg nope util = function | EMatchBindingInOrPattern _ | EMatchInvalidAsPattern _ | EMatchInvalidPatternReference _ + | EMatchInvalidObjectShorthand _ | EUndocumentedFeature _ -> nope @@ -2009,6 +2015,7 @@ let loc_of_msg : 'loc t' -> 'loc option = function | EMatchBindingInOrPattern { loc } -> Some loc | EMatchInvalidAsPattern { loc } -> Some loc | EMatchInvalidPatternReference { loc; _ } -> Some loc + | EMatchInvalidObjectShorthand { loc; _ } -> Some loc | EUndocumentedFeature { loc } -> Some loc | EDevOnlyRefinedLocInfo { refined_loc; refining_locs = _ } -> Some refined_loc | EDevOnlyInvalidatedRefinementInfo { read_loc; invalidation_info = _ } -> Some read_loc @@ -2989,6 +2996,8 @@ let friendly_message_of_msg = function | EMatchInvalidAsPattern { loc = _ } -> Normal MessageMatchInvalidAsPattern | EMatchInvalidPatternReference { loc = _; binding_reason } -> Normal (MessageMatchInvalidPatternReference { binding_reason }) + | EMatchInvalidObjectShorthand { loc = _; name } -> + Normal (MessageMatchInvalidObjectShorthand { name }) | EUndocumentedFeature { loc = _ } -> Normal MessageUndocumentedFeature let defered_in_speculation = function @@ -3344,4 +3353,5 @@ let error_code_of_message err : error_code option = | EMatchBindingInOrPattern _ -> Some MatchInvalidPattern | EMatchInvalidAsPattern _ -> Some MatchInvalidPattern | EMatchInvalidPatternReference _ -> Some MatchInvalidPattern + | EMatchInvalidObjectShorthand _ -> Some MatchInvalidPattern | EUndocumentedFeature _ -> Some UndocumentedFeature diff --git a/src/typing/errors/flow_intermediate_error.ml b/src/typing/errors/flow_intermediate_error.ml index c035f54e303..211560381a5 100644 --- a/src/typing/errors/flow_intermediate_error.ml +++ b/src/typing/errors/flow_intermediate_error.ml @@ -4109,6 +4109,18 @@ let to_printable_error : ref binding_reason; text " within the same match pattern it is defined."; ] + | MessageMatchInvalidObjectShorthand { name } -> + [ + text "Invalid object pattern property. Use "; + code (spf "{const %s}" name); + text " if you want to create a new variable with the value of property "; + code name; + text ", or use "; + code (spf "{%s: %s}" name name); + text " if you want to match property "; + code name; + text " against the value of the variable of the same name."; + ] in let rec convert_error_message { kind; loc; error_code; root; message; misplaced_source_file = _ } = diff --git a/src/typing/errors/flow_intermediate_error_types.ml b/src/typing/errors/flow_intermediate_error_types.ml index ad98ac1ddd1..141b58fe931 100644 --- a/src/typing/errors/flow_intermediate_error_types.ml +++ b/src/typing/errors/flow_intermediate_error_types.ml @@ -923,6 +923,7 @@ type 'loc message = | MessageMatchBindingInOrPattern | MessageMatchInvalidAsPattern | MessageMatchInvalidPatternReference of { binding_reason: 'loc virtual_reason } + | MessageMatchInvalidObjectShorthand of { name: string } type 'loc intermediate_error = { kind: Flow_errors_utils.error_kind; diff --git a/src/typing/match_pattern.ml b/src/typing/match_pattern.ml index deae20547e0..a26dd54c730 100644 --- a/src/typing/match_pattern.ml +++ b/src/typing/match_pattern.ml @@ -211,7 +211,7 @@ and object_properties cx ~on_identifier ~on_expression ~on_binding ~in_or_patter let open Ast.MatchPattern.ObjectPattern in let rec loop acc seen rev_props = function | [] -> List.rev rev_props - | (loc, { Property.key; pattern = p; shorthand; comments }) :: props -> + | (loc, Property.Valid { Property.key; pattern = p; shorthand; comments }) :: props -> let (acc, key, name) = object_property_key cx acc key in ( if SSet.mem name seen then let key_loc = @@ -231,7 +231,10 @@ and object_properties cx ~on_identifier ~on_expression ~on_binding ~in_or_patter Flow_js.add_output cx (Error_message.EMatchDuplicateObjectProperty { loc = key_loc; name }) ); let p = pattern_ cx ~on_identifier ~on_expression ~on_binding ~in_or_pattern acc p in - let prop = (loc, { Property.key; pattern = p; shorthand; comments }) in + let prop = (loc, Property.Valid { Property.key; pattern = p; shorthand; comments }) in + loop acc (SSet.add name seen) (prop :: rev_props) props + | ((loc, Property.InvalidShorthand (_, { Ast.Identifier.name; _ })) as prop) :: props -> + Flow_js.add_output cx (Error_message.EMatchInvalidObjectShorthand { loc; name }); loop acc (SSet.add name seen) (prop :: rev_props) props in loop acc SSet.empty [] props diff --git a/tests/match/match.exp b/tests/match/match.exp index bf6a9382c52..cc90430cfb5 100644 --- a/tests/match/match.exp +++ b/tests/match/match.exp @@ -783,6 +783,26 @@ References: ^ [1] +Error ------------------------------------------------------------------------------------------ pattern-errors.js:155:6 + +Invalid object pattern property. Use `{const foo}` if you want to create a new variable with the value of property +`foo`, or use `{foo: foo}` if you want to match property `foo` against the value of the variable of the same name. +[match-invalid-pattern] + + 155| {foo}: 0, + ^^^ + + +Error ------------------------------------------------------------------------------------------ pattern-errors.js:156:6 + +Invalid object pattern property. Use `{const foo}` if you want to create a new variable with the value of property +`foo`, or use `{foo: foo}` if you want to match property `foo` against the value of the variable of the same name. +[match-invalid-pattern] + + 156| {foo, bar: true}: 0, + ^^^ + + Error -------------------------------------------------------------------------------------------------- patterns.js:9:3 Cannot cast `out` to empty because number [1] is incompatible with empty [2]. [incompatible-cast] @@ -918,4 +938,4 @@ Cannot resolve name `a`. [cannot-resolve-name] -Found 75 errors +Found 77 errors diff --git a/tests/match/pattern-errors.js b/tests/match/pattern-errors.js index cecddfe7950..b36a3422a38 100644 --- a/tests/match/pattern-errors.js +++ b/tests/match/pattern-errors.js @@ -146,3 +146,14 @@ _: 0, }; } + +// Invalid object property shorthand +{ + declare const x: {foo: 1, bar: boolean}; + + const out = match (x) { + {foo}: 0, + {foo, bar: true}: 0, + _: 0, + }; +}