Skip to content

Commit

Permalink
[flow][match] Add better parse error for multiple match args
Browse files Browse the repository at this point in the history
Summary:
We don't allow multple match args (parsing as AssignmentExpression rather than Expression), to reserve the future possibilty for allowing multiple args (where they would mean the same as `match ([a, b]) {}`.

This is currently a parse error, but working on the Hermes parser version, I realized we can have a better parse error here. Rather than attempting in parse as a `match` call and then failing on the `{`, we can have a better parse error when a single argument is not supplied.

Changelog: [internal]

Reviewed By: SamChou19815

Differential Revision: D65851645

fbshipit-source-id: 5025e0db5c14cd9b46b7c7fc91e5a46b1c911a74
  • Loading branch information
gkz authored and facebook-github-bot committed Nov 14, 2024
1 parent 870e4cc commit efcd4dc
Show file tree
Hide file tree
Showing 9 changed files with 115 additions and 16 deletions.
24 changes: 16 additions & 8 deletions src/parser/expression_parser.ml
Original file line number Diff line number Diff line change
Expand Up @@ -1338,15 +1338,23 @@ module Expression
let start_loc = Peek.loc env in
(* Consume `match` as an identifier, in case it's a call expression. *)
let id = Parse.identifier env in
(* Allows trailing comma - ban? *)
(* Allows trailing comma. *)
let (args_loc, args) = arguments env in
(match args with
(* `match (<expr>) {` *)
| { Expression.ArgList.arguments = [Expression.Expression arg]; _ }
when (not (Peek.is_line_terminator env)) && Peek.token env = T_LCURLY ->
Cover_expr (match_expression ~start_loc ~leading ~arg env)
(* It's actually a call expression of the form `match(...)` *)
| _ ->
if (not (Peek.is_line_terminator env)) && Peek.token env = T_LCURLY then (
match args with
| { Ast.Expression.ArgList.arguments = [Ast.Expression.Expression arg]; _ } ->
Cover_expr (match_expression ~start_loc ~leading ~arg env)
| _ ->
error_at env (args_loc, Parse_error.MatchNonSingleArgument);
let error_arg =
( args_loc,
Ast.Expression.Sequence { Ast.Expression.Sequence.expressions = []; comments = None }
)
in
Cover_expr (match_expression ~start_loc ~leading ~arg:error_arg env)
) else
(* It's actually a call expression of the form `match(...)` *)
let callee = (fst id, Expression.Identifier id) in
let loc = Loc.btwn start_loc args_loc in
let comments = Flow_ast_utils.mk_comments_opt ~leading () in
Expand All @@ -1360,7 +1368,7 @@ module Expression
~in_optional_chain:false
env
start_loc
(Cover_expr (loc, call)))
(Cover_expr (loc, call))
| T_IDENTIFIER { raw = "abstract"; _ } when Peek.ith_token ~i:1 env = T_CLASS ->
Cover_expr (Parse.class_expression env)
| _ when Peek.is_identifier env ->
Expand Down
2 changes: 2 additions & 0 deletions src/parser/parse_error.ml
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,7 @@ type t =
| JSXAttributeValueEmptyExpression
| LiteralShorthandProperty
| MalformedUnicode
| MatchNonSingleArgument
| MethodInDestructuring
| MissingJSXClosingTag of string
| MissingTypeParam
Expand Down Expand Up @@ -393,6 +394,7 @@ module PP = struct
"JSX attributes must only be assigned a non-empty expression"
| LiteralShorthandProperty -> "Literals cannot be used as shorthand properties."
| MalformedUnicode -> "Malformed unicode"
| MatchNonSingleArgument -> "`match` only supports one argument"
| MethodInDestructuring -> "Object pattern can't contain methods"
| MissingJSXClosingTag name ->
Printf.sprintf "JSX element %s has no corresponding closing tag." name
Expand Down
24 changes: 16 additions & 8 deletions src/parser/statement_parser.ml
Original file line number Diff line number Diff line change
Expand Up @@ -584,15 +584,23 @@ module Statement
let start_loc = Peek.loc env in
(* Consume `match` as an identifier, in case it's a call expression. *)
let id = Parse.identifier env in
(* Allows trailing comma - ban? *)
(* Allows trailing comma. *)
let (args_loc, args) = Expression.arguments env in
match args with
(* `match (<expr>) {` *)
| { Ast.Expression.ArgList.arguments = [Ast.Expression.Expression arg]; _ }
when (not (Peek.is_line_terminator env)) && Peek.token env = T_LCURLY ->
match_statement ~start_loc ~leading ~arg env
(* It's actually a call expression statement of the form `match(...)` *)
| _ ->
(* `match (<exprs>) {` *)
if (not (Peek.is_line_terminator env)) && Peek.token env = T_LCURLY then (
match args with
| { Ast.Expression.ArgList.arguments = [Ast.Expression.Expression arg]; _ } ->
match_statement ~start_loc ~leading ~arg env
| _ ->
error_at env (args_loc, Parse_error.MatchNonSingleArgument);
let error_arg =
( args_loc,
Ast.Expression.Sequence { Ast.Expression.Sequence.expressions = []; comments = None }
)
in
match_statement ~start_loc ~leading ~arg:error_arg env
) else
(* It's actually a call expression statement of the form `match(...)` *)
let callee = (fst id, Ast.Expression.Identifier id) in
let expr_loc = Loc.btwn start_loc args_loc in
let comments = Flow_ast_utils.mk_comments_opt ~leading () in
Expand Down
1 change: 1 addition & 0 deletions src/parser/test/flow/match/expression-multiple-args.js
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
const e = match (a, b) {};
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{
"pattern_matching": true
}
47 changes: 47 additions & 0 deletions src/parser/test/flow/match/expression-multiple-args.tree.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
{
"errors":[
{
"loc":{"source":null,"start":{"line":1,"column":16},"end":{"line":1,"column":22}},
"message":"`match` only supports one argument"
}
],
"type":"Program",
"loc":{"source":null,"start":{"line":1,"column":0},"end":{"line":1,"column":26}},
"range":[0,26],
"body":[
{
"type":"VariableDeclaration",
"loc":{"source":null,"start":{"line":1,"column":0},"end":{"line":1,"column":26}},
"range":[0,26],
"declarations":[
{
"type":"VariableDeclarator",
"loc":{"source":null,"start":{"line":1,"column":6},"end":{"line":1,"column":25}},
"range":[6,25],
"id":{
"type":"Identifier",
"loc":{"source":null,"start":{"line":1,"column":6},"end":{"line":1,"column":7}},
"range":[6,7],
"name":"e",
"typeAnnotation":null,
"optional":false
},
"init":{
"type":"MatchExpression",
"loc":{"source":null,"start":{"line":1,"column":10},"end":{"line":1,"column":25}},
"range":[10,25],
"argument":{
"type":"SequenceExpression",
"loc":{"source":null,"start":{"line":1,"column":16},"end":{"line":1,"column":22}},
"range":[16,22],
"expressions":[]
},
"cases":[]
}
}
],
"kind":"const"
}
],
"comments":[]
}
1 change: 1 addition & 0 deletions src/parser/test/flow/match/statement-multiple-args.js
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
match (a, b) {}
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{
"pattern_matching": true
}
26 changes: 26 additions & 0 deletions src/parser/test/flow/match/statement-multiple-args.tree.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
{
"errors":[
{
"loc":{"source":null,"start":{"line":1,"column":6},"end":{"line":1,"column":12}},
"message":"`match` only supports one argument"
}
],
"type":"Program",
"loc":{"source":null,"start":{"line":1,"column":0},"end":{"line":1,"column":15}},
"range":[0,15],
"body":[
{
"type":"MatchStatement",
"loc":{"source":null,"start":{"line":1,"column":0},"end":{"line":1,"column":15}},
"range":[0,15],
"argument":{
"type":"SequenceExpression",
"loc":{"source":null,"start":{"line":1,"column":6},"end":{"line":1,"column":12}},
"range":[6,12],
"expressions":[]
},
"cases":[]
}
],
"comments":[]
}

0 comments on commit efcd4dc

Please sign in to comment.