From efcd4dcb70019d572dcd9467e1a30f547c66e438 Mon Sep 17 00:00:00 2001 From: George Zahariev Date: Wed, 13 Nov 2024 20:28:14 -0800 Subject: [PATCH] [flow][match] Add better parse error for multiple match args 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 --- src/parser/expression_parser.ml | 24 ++++++---- src/parser/parse_error.ml | 2 + src/parser/statement_parser.ml | 24 ++++++---- .../flow/match/expression-multiple-args.js | 1 + .../expression-multiple-args.options.json | 3 ++ .../match/expression-multiple-args.tree.json | 47 +++++++++++++++++++ .../flow/match/statement-multiple-args.js | 1 + .../statement-multiple-args.options.json | 3 ++ .../match/statement-multiple-args.tree.json | 26 ++++++++++ 9 files changed, 115 insertions(+), 16 deletions(-) create mode 100644 src/parser/test/flow/match/expression-multiple-args.js create mode 100644 src/parser/test/flow/match/expression-multiple-args.options.json create mode 100644 src/parser/test/flow/match/expression-multiple-args.tree.json create mode 100644 src/parser/test/flow/match/statement-multiple-args.js create mode 100644 src/parser/test/flow/match/statement-multiple-args.options.json create mode 100644 src/parser/test/flow/match/statement-multiple-args.tree.json diff --git a/src/parser/expression_parser.ml b/src/parser/expression_parser.ml index 624a47c1a4f..618a115d233 100644 --- a/src/parser/expression_parser.ml +++ b/src/parser/expression_parser.ml @@ -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 () {` *) - | { 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 @@ -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 -> diff --git a/src/parser/parse_error.ml b/src/parser/parse_error.ml index 50067d0f236..afacf1be6fd 100644 --- a/src/parser/parse_error.ml +++ b/src/parser/parse_error.ml @@ -105,6 +105,7 @@ type t = | JSXAttributeValueEmptyExpression | LiteralShorthandProperty | MalformedUnicode + | MatchNonSingleArgument | MethodInDestructuring | MissingJSXClosingTag of string | MissingTypeParam @@ -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 diff --git a/src/parser/statement_parser.ml b/src/parser/statement_parser.ml index b4e1923d4fc..f3e627cd56e 100644 --- a/src/parser/statement_parser.ml +++ b/src/parser/statement_parser.ml @@ -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 () {` *) - | { 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 () {` *) + 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 diff --git a/src/parser/test/flow/match/expression-multiple-args.js b/src/parser/test/flow/match/expression-multiple-args.js new file mode 100644 index 00000000000..f30e0b58ce0 --- /dev/null +++ b/src/parser/test/flow/match/expression-multiple-args.js @@ -0,0 +1 @@ +const e = match (a, b) {}; diff --git a/src/parser/test/flow/match/expression-multiple-args.options.json b/src/parser/test/flow/match/expression-multiple-args.options.json new file mode 100644 index 00000000000..a611993d016 --- /dev/null +++ b/src/parser/test/flow/match/expression-multiple-args.options.json @@ -0,0 +1,3 @@ +{ + "pattern_matching": true +} diff --git a/src/parser/test/flow/match/expression-multiple-args.tree.json b/src/parser/test/flow/match/expression-multiple-args.tree.json new file mode 100644 index 00000000000..95d1fd754f7 --- /dev/null +++ b/src/parser/test/flow/match/expression-multiple-args.tree.json @@ -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":[] +} diff --git a/src/parser/test/flow/match/statement-multiple-args.js b/src/parser/test/flow/match/statement-multiple-args.js new file mode 100644 index 00000000000..9c645b89337 --- /dev/null +++ b/src/parser/test/flow/match/statement-multiple-args.js @@ -0,0 +1 @@ +match (a, b) {} diff --git a/src/parser/test/flow/match/statement-multiple-args.options.json b/src/parser/test/flow/match/statement-multiple-args.options.json new file mode 100644 index 00000000000..a611993d016 --- /dev/null +++ b/src/parser/test/flow/match/statement-multiple-args.options.json @@ -0,0 +1,3 @@ +{ + "pattern_matching": true +} diff --git a/src/parser/test/flow/match/statement-multiple-args.tree.json b/src/parser/test/flow/match/statement-multiple-args.tree.json new file mode 100644 index 00000000000..ac9c8a5b0ee --- /dev/null +++ b/src/parser/test/flow/match/statement-multiple-args.tree.json @@ -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":[] +}