Trailing comma

snapshots/pattern_trailing_comma.txt

@@ -0,0 +1,101 @@
+@ ProgramNode (location: (1,0)-(8,11))
+├── flags: ∅
+├── locals: [:a, :b, :c, :bar]
+└── statements:
+    @ StatementsNode (location: (1,0)-(8,11))
+    ├── flags: ∅
+    └── body: (length: 2)
+        ├── @ CaseMatchNode (location: (1,0)-(6,3))
+        │   ├── flags: newline
+        │   ├── predicate:
+        │   │   @ CallNode (location: (1,5)-(1,6))
+        │   │   ├── flags: variable_call, ignore_visibility
+        │   │   ├── receiver: ∅
+        │   │   ├── call_operator_loc: ∅
+        │   │   ├── name: :x
+        │   │   ├── message_loc: (1,5)-(1,6) = "x"
+        │   │   ├── opening_loc: ∅
+        │   │   ├── arguments: ∅
+        │   │   ├── closing_loc: ∅
+        │   │   ├── equal_loc: ∅
+        │   │   └── block: ∅
+        │   ├── conditions: (length: 2)
+        │   │   ├── @ InNode (location: (2,0)-(3,3))
+        │   │   │   ├── flags: ∅
+        │   │   │   ├── pattern:
+        │   │   │   │   @ ArrayPatternNode (location: (2,3)-(3,3))
+        │   │   │   │   ├── flags: ∅
+        │   │   │   │   ├── constant: ∅
+        │   │   │   │   ├── requireds: (length: 2)
+        │   │   │   │   │   ├── @ LocalVariableTargetNode (location: (2,3)-(2,4))
+        │   │   │   │   │   │   ├── flags: ∅
+        │   │   │   │   │   │   ├── name: :a
+        │   │   │   │   │   │   └── depth: 0
+        │   │   │   │   │   └── @ IntegerNode (location: (3,2)-(3,3))
+        │   │   │   │   │       ├── flags: static_literal, decimal
+        │   │   │   │   │       └── value: 1
+        │   │   │   │   ├── rest: ∅
+        │   │   │   │   ├── posts: (length: 0)
+        │   │   │   │   ├── opening_loc: ∅
+        │   │   │   │   └── closing_loc: ∅
+        │   │   │   ├── statements: ∅
+        │   │   │   ├── in_loc: (2,0)-(2,2) = "in"
+        │   │   │   └── then_loc: ∅
+        │   │   └── @ InNode (location: (4,0)-(5,3))
+        │   │       ├── flags: ∅
+        │   │       ├── pattern:
+        │   │       │   @ ArrayPatternNode (location: (4,3)-(5,3))
+        │   │       │   ├── flags: ∅
+        │   │       │   ├── constant: ∅
+        │   │       │   ├── requireds: (length: 3)
+        │   │       │   │   ├── @ LocalVariableTargetNode (location: (4,3)-(4,4))
+        │   │       │   │   │   ├── flags: ∅
+        │   │       │   │   │   ├── name: :b
+        │   │       │   │   │   └── depth: 0
+        │   │       │   │   ├── @ LocalVariableTargetNode (location: (4,6)-(4,7))
+        │   │       │   │   │   ├── flags: ∅
+        │   │       │   │   │   ├── name: :c
+        │   │       │   │   │   └── depth: 0
+        │   │       │   │   └── @ IntegerNode (location: (5,2)-(5,3))
+        │   │       │   │       ├── flags: static_literal, decimal
+        │   │       │   │       └── value: 2
+        │   │       │   ├── rest: ∅
+        │   │       │   ├── posts: (length: 0)
+        │   │       │   ├── opening_loc: ∅
+        │   │       │   └── closing_loc: ∅
+        │   │       ├── statements: ∅
+        │   │       ├── in_loc: (4,0)-(4,2) = "in"
+        │   │       └── then_loc: ∅
+        │   ├── else_clause: ∅
+        │   ├── case_keyword_loc: (1,0)-(1,4) = "case"
+        │   └── end_keyword_loc: (6,0)-(6,3) = "end"
+        └── @ MatchRequiredNode (location: (8,0)-(8,11))
+            ├── flags: newline
+            ├── value:
+            │   @ CallNode (location: (8,0)-(8,3))
+            │   ├── flags: variable_call, ignore_visibility
+            │   ├── receiver: ∅
+            │   ├── call_operator_loc: ∅
+            │   ├── name: :foo
+            │   ├── message_loc: (8,0)-(8,3) = "foo"
+            │   ├── opening_loc: ∅
+            │   ├── arguments: ∅
+            │   ├── closing_loc: ∅
+            │   ├── equal_loc: ∅
+            │   └── block: ∅
+            ├── pattern:
+            │   @ ArrayPatternNode (location: (8,7)-(8,11))
+            │   ├── flags: ∅
+            │   ├── constant: ∅
+            │   ├── requireds: (length: 1)
+            │   │   └── @ LocalVariableTargetNode (location: (8,7)-(8,10))
+            │   │       ├── flags: ∅
+            │   │       ├── name: :bar
+            │   │       └── depth: 0
+            │   ├── rest:
+            │   │   @ ImplicitRestNode (location: (8,10)-(8,11))
+            │   │   └── flags: ∅
+            │   ├── posts: (length: 0)
+            │   ├── opening_loc: ∅
+            │   └── closing_loc: ∅
+            └── operator_loc: (8,4)-(8,6) = "=>"

src/prism.c

@@ -13995,7 +13995,18 @@ parse_arguments(pm_parser_t *parser, pm_arguments_t *arguments, bool accepts_for
 
         // If we hit the terminator, then that means we have a trailing comma so
         // we can accept that output as well.
-        if (match1(parser, terminator)) break;
+        if (match1(parser, terminator)) {
+            // A forwarding `...` argument must be the last argument and cannot
+            // be followed by a trailing comma, e.g. `foo(...,)`. A comma
+            // followed by another argument is already rejected at the top of
+            // this loop, so the only case left to reject here is the trailing
+            // one.
+            if (parsed_forwarding_arguments) {
+                pm_parser_err_previous(parser, PM_ERR_INVALID_COMMA);
+            }
+
+            break;
+        }
     }
 }
 
@@ -15133,9 +15144,15 @@ parse_block(pm_parser_t *parser, uint16_t depth) {
  * Parse a list of arguments and their surrounding parentheses if they are
  * present. It returns true if it found any pieces of arguments (parentheses,
  * arguments, or blocks).
+ *
+ * When `full_arguments` is true the caller is a method or `super` call, which
+ * use the full `opt_call_args` grammar: a block argument, argument forwarding,
+ * a trailing block, and a trailing comma are all permitted. When it is false
+ * the caller is `yield`, whose restricted `call_args` grammar permits none of
+ * these.
  */
 static bool
-parse_arguments_list(pm_parser_t *parser, pm_arguments_t *arguments, bool accepts_block, uint8_t flags, uint16_t depth) {
+parse_arguments_list(pm_parser_t *parser, pm_arguments_t *arguments, bool full_arguments, uint8_t flags, uint16_t depth) {
     /* Fast path: if the current token can't begin an expression and isn't
      * a parenthesis, block opener, or splat/block-pass operator, there are
      * no arguments to parse. */
@@ -15157,7 +15174,17 @@ parse_arguments_list(pm_parser_t *parser, pm_arguments_t *arguments, bool accept
             arguments->closing_loc = TOK2LOC(parser, &parser->previous);
         } else {
             pm_accepts_block_stack_push(parser, true);
-            parse_arguments(parser, arguments, accepts_block, PM_TOKEN_PARENTHESIS_RIGHT, (uint8_t) (flags & ~PM_PARSE_ACCEPTS_DO_BLOCK), (uint16_t) (depth + 1));
+            parse_arguments(parser, arguments, full_arguments, PM_TOKEN_PARENTHESIS_RIGHT, (uint8_t) (flags & ~PM_PARSE_ACCEPTS_DO_BLOCK), (uint16_t) (depth + 1));
+
+            // `yield` parses its arguments through the restricted `call_args`
+            // grammar, which (unlike the `opt_call_args` that method calls and
+            // `super` use) permits neither a block argument nor a trailing
+            // comma. `full_arguments` is false only for `yield`, so we use it
+            // to reject the trailing comma in `yield(a,)` that the arguments
+            // parser otherwise accepts before the closing parenthesis.
+            if (!full_arguments && parser->previous.type == PM_TOKEN_COMMA) {
+                PM_PARSER_ERR_TOKEN_FORMAT(parser, &parser->previous, PM_ERR_EXPECT_ARGUMENT, pm_token_str(parser->current.type));
+            }
 
             if (!accept1(parser, PM_TOKEN_PARENTHESIS_RIGHT)) {
                 PM_PARSER_ERR_TOKEN_FORMAT(parser, &parser->current, PM_ERR_ARGUMENT_TERM_PAREN, pm_token_str(parser->current.type));
@@ -15176,7 +15203,7 @@ parse_arguments_list(pm_parser_t *parser, pm_arguments_t *arguments, bool accept
         // If we get here, then the subsequent token cannot be used as an infix
         // operator. In this case we assume the subsequent token is part of an
         // argument to this method call.
-        parse_arguments(parser, arguments, accepts_block, PM_TOKEN_EOF, flags, (uint16_t) (depth + 1));
+        parse_arguments(parser, arguments, full_arguments, PM_TOKEN_EOF, flags, (uint16_t) (depth + 1));
 
         // If we have done with the arguments and still not consumed the comma,
         // then we have a trailing comma where we need to check whether it is
@@ -15191,7 +15218,7 @@ parse_arguments_list(pm_parser_t *parser, pm_arguments_t *arguments, bool accept
     // If we're at the end of the arguments, we can now check if there is a block
     // node that starts with a {. If there is, then we can parse it and add it to
     // the arguments.
-    if (accepts_block) {
+    if (full_arguments) {
         pm_block_node_t *block = NULL;
 
         if (accept1(parser, PM_TOKEN_BRACE_LEFT)) {
@@ -17380,8 +17407,23 @@ parse_pattern(pm_parser_t *parser, pm_constant_id_list_t *captures, uint8_t flag
 
         // Gather up all of the patterns into the list.
         while (accept1(parser, PM_TOKEN_COMMA)) {
-            // Break early here in case we have a trailing comma.
-            if (match7(parser, PM_TOKEN_KEYWORD_THEN, PM_TOKEN_BRACE_RIGHT, PM_TOKEN_BRACKET_RIGHT, PM_TOKEN_PARENTHESIS_RIGHT, PM_TOKEN_SEMICOLON, PM_TOKEN_KEYWORD_AND, PM_TOKEN_KEYWORD_OR)) {
+            // Break early here in case we have a trailing comma. The newline and
+            // EOF terminators cover a one-line match (`x => a,`) or a `case`/`in`
+            // clause (`in a,\n ...`); a newline is only lexed as a token here
+            // when `pattern_matching_newlines` is set, so this does not affect
+            // patterns nested in brackets or parentheses.
+            if (
+                match7(parser, PM_TOKEN_KEYWORD_THEN, PM_TOKEN_BRACE_RIGHT, PM_TOKEN_BRACKET_RIGHT, PM_TOKEN_PARENTHESIS_RIGHT, PM_TOKEN_SEMICOLON, PM_TOKEN_KEYWORD_AND, PM_TOKEN_KEYWORD_OR) ||
+                match2(parser, PM_TOKEN_NEWLINE, PM_TOKEN_EOF)
+            ) {
+                // A trailing comma forms an implicit rest pattern (`[a,]` is
+                // `[a, *]`). If a rest pattern has already been parsed, then
+                // this is a second rest, which is not allowed (e.g. `[a, *b,]`
+                // or `x => a, *b,`).
+                if (trailing_rest) {
+                    pm_parser_err_previous(parser, PM_ERR_PATTERN_REST);
+                }
+
                 node = UP(pm_implicit_rest_node_create(parser, &parser->previous));
                 pm_node_list_append(parser->arena, &nodes, node);
                 trailing_rest = true;
@@ -19727,6 +19769,16 @@ parse_expression_prefix(pm_parser_t *parser, pm_binding_power_t binding_power, u
                     if (!(flags & PM_PARSE_ACCEPTS_COMMAND_CALL) && arguments.arguments != NULL) {
                         PM_PARSER_ERR_TOKEN_FORMAT(parser, &next, PM_ERR_EXPECT_EOL_AFTER_STATEMENT, pm_token_str(next.type));
                     }
+
+                    // Reject a trailing comma, e.g. `return a,`. The arguments
+                    // parser silently accepts a trailing comma only when it is
+                    // immediately followed by the EOF terminator; in every other
+                    // case (e.g. `return a,;`) it reports the dangling comma
+                    // itself. We reject the accepted case here to stay in line
+                    // with the command call argument parsing above.
+                    if (parser->previous.type == PM_TOKEN_COMMA && match1(parser, PM_TOKEN_EOF)) {
+                        PM_PARSER_ERR_TOKEN_FORMAT(parser, &parser->previous, PM_ERR_EXPECT_ARGUMENT, pm_token_str(parser->current.type));
+                    }
                 }
 
                 // It's possible that we've parsed a block argument through our
@@ -22192,9 +22244,14 @@ parse_expression(pm_parser_t *parser, pm_binding_power_t binding_power, uint8_t
         // If the operator is nonassoc and we should not be able to parse the
         // upcoming infix operator, break.
         if (current_binding_powers.nonassoc) {
-            // If this is a non-assoc operator and we are about to parse the
-            // exact same operator, then we need to add an error.
-            if (match1(parser, current_token_type)) {
+            // If we are about to parse another non-associative operator at the
+            // same precedence as the one we just parsed, then we need to add an
+            // error. This covers chaining the same operator (`1 == 2 == 3`) as
+            // well as different operators that share a precedence, since they
+            // are equally non-associative with one another (`1 == 2 != 3`,
+            // `1...2..3`).
+            pm_binding_powers_t next_binding_powers = pm_binding_powers[parser->current.type];
+            if (next_binding_powers.nonassoc && next_binding_powers.left == current_binding_powers.left) {
                 PM_PARSER_ERR_TOKEN_FORMAT(parser, &parser->current, PM_ERR_NON_ASSOCIATIVE_OPERATOR, pm_token_str(parser->current.type), pm_token_str(current_token_type));
                 break;
             }
@@ -22212,10 +22269,10 @@ parse_expression(pm_parser_t *parser, pm_binding_power_t binding_power, uint8_t
                     break;
                 }
 
-                if (PM_BINDING_POWER_TERM <= pm_binding_powers[parser->current.type].left) {
+                if (PM_BINDING_POWER_TERM <= next_binding_powers.left) {
                     break;
                 }
-            } else if (current_binding_powers.left <= pm_binding_powers[parser->current.type].left) {
+            } else if (current_binding_powers.left <= next_binding_powers.left) {
                 break;
             }
         }

test/prism/errors/forwarding_arguments_trailing_comma.txt

@@ -0,0 +1,3 @@
+def foo(...); bar(...,); end
+                     ^ invalid comma
+

test/prism/errors/non_associative_equality.txt

@@ -0,0 +1,3 @@
+x = a == b != c
+           ^~ unexpected '!='; '==' is a non-associative operator
+

test/prism/errors/non_associative_range.txt

@@ -0,0 +1,3 @@
+x = 1...2..3
+         ^~ unexpected ..; ... is a non-associative operator
+

test/prism/errors/pattern_array_rest_trailing_comma.txt

@@ -0,0 +1,3 @@
+foo => [bar, *baz,]
+                 ^ unexpected rest pattern
+

test/prism/errors/pattern_match_implicit_rest.txt

@@ -1,3 +1,3 @@
 a=>b, *,
-        ^ expected a pattern expression after `,`
+       ^ unexpected rest pattern
 

test/prism/errors/rescue_pattern.txt

@@ -1,4 +1,5 @@
 a rescue b => c in d
+                ^~ unexpected 'in'; '=>' is a non-associative operator
                 ^~ unexpected 'in', expecting end-of-input
                 ^~ unexpected 'in', ignoring it
 

test/prism/errors/return_trailing_comma.txt

@@ -0,0 +1,3 @@
+return a,
+        ^ unexpected end-of-input; expected an argument
+

test/prism/errors/return_trailing_comma_multiple.txt

@@ -0,0 +1,3 @@
+return a, b,
+           ^ unexpected end-of-input; expected an argument
+

test/prism/errors/yield_trailing_comma.txt

@@ -0,0 +1,3 @@
+def m; yield(a,); end
+              ^ unexpected ')'; expected an argument
+

test/prism/fixtures/pattern_trailing_comma.txt

@@ -0,0 +1,8 @@
+case x
+in a,
+  1
+in b, c,
+  2
+end
+
+foo => bar,