From 41d30aab432ea24caa0c122f8685f961b128982f Mon Sep 17 00:00:00 2001 From: Kevin Newton Date: Tue, 16 Jun 2026 15:40:30 -0400 Subject: [PATCH 1/6] Disallow trailing comma on jumps at EOF --- src/prism.c | 10 ++++++++++ test/prism/errors/return_trailing_comma.txt | 3 +++ test/prism/errors/return_trailing_comma_multiple.txt | 3 +++ 3 files changed, 16 insertions(+) create mode 100644 test/prism/errors/return_trailing_comma.txt create mode 100644 test/prism/errors/return_trailing_comma_multiple.txt diff --git a/src/prism.c b/src/prism.c index 1a2cc64596..a0c18dc0cd 100644 --- a/src/prism.c +++ b/src/prism.c @@ -19727,6 +19727,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 diff --git a/test/prism/errors/return_trailing_comma.txt b/test/prism/errors/return_trailing_comma.txt new file mode 100644 index 0000000000..eeb298b4e9 --- /dev/null +++ b/test/prism/errors/return_trailing_comma.txt @@ -0,0 +1,3 @@ +return a, + ^ unexpected end-of-input; expected an argument + diff --git a/test/prism/errors/return_trailing_comma_multiple.txt b/test/prism/errors/return_trailing_comma_multiple.txt new file mode 100644 index 0000000000..a92eb69781 --- /dev/null +++ b/test/prism/errors/return_trailing_comma_multiple.txt @@ -0,0 +1,3 @@ +return a, b, + ^ unexpected end-of-input; expected an argument + From ef928161a2596c1f2c6fea5fc0f348464ded61ff Mon Sep 17 00:00:00 2001 From: Kevin Newton Date: Wed, 17 Jun 2026 12:11:25 -0400 Subject: [PATCH 2/6] Disallow trailing comma on parenthesized yield --- src/prism.c | 24 ++++++++++++++++++---- test/prism/errors/yield_trailing_comma.txt | 3 +++ 2 files changed, 23 insertions(+), 4 deletions(-) create mode 100644 test/prism/errors/yield_trailing_comma.txt diff --git a/src/prism.c b/src/prism.c index a0c18dc0cd..e36edf5436 100644 --- a/src/prism.c +++ b/src/prism.c @@ -15133,9 +15133,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 +15163,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 +15192,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 +15207,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)) { diff --git a/test/prism/errors/yield_trailing_comma.txt b/test/prism/errors/yield_trailing_comma.txt new file mode 100644 index 0000000000..d1b1297f45 --- /dev/null +++ b/test/prism/errors/yield_trailing_comma.txt @@ -0,0 +1,3 @@ +def m; yield(a,); end + ^ unexpected ')'; expected an argument + From 741f18785fca787b5021be9c2d5c9cee81e65600 Mon Sep 17 00:00:00 2001 From: Kevin Newton Date: Wed, 17 Jun 2026 13:06:32 -0400 Subject: [PATCH 3/6] Fix up non-associativity in same binding power --- src/prism.c | 15 ++++++++++----- test/prism/errors/non_associative_equality.txt | 3 +++ test/prism/errors/non_associative_range.txt | 3 +++ test/prism/errors/rescue_pattern.txt | 1 + 4 files changed, 17 insertions(+), 5 deletions(-) create mode 100644 test/prism/errors/non_associative_equality.txt create mode 100644 test/prism/errors/non_associative_range.txt diff --git a/src/prism.c b/src/prism.c index e36edf5436..507cd11a4c 100644 --- a/src/prism.c +++ b/src/prism.c @@ -22218,9 +22218,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; } @@ -22238,10 +22243,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; } } diff --git a/test/prism/errors/non_associative_equality.txt b/test/prism/errors/non_associative_equality.txt new file mode 100644 index 0000000000..826f9a00aa --- /dev/null +++ b/test/prism/errors/non_associative_equality.txt @@ -0,0 +1,3 @@ +x = a == b != c + ^~ unexpected '!='; '==' is a non-associative operator + diff --git a/test/prism/errors/non_associative_range.txt b/test/prism/errors/non_associative_range.txt new file mode 100644 index 0000000000..e18985a62c --- /dev/null +++ b/test/prism/errors/non_associative_range.txt @@ -0,0 +1,3 @@ +x = 1...2..3 + ^~ unexpected ..; ... is a non-associative operator + diff --git a/test/prism/errors/rescue_pattern.txt b/test/prism/errors/rescue_pattern.txt index c85feb27bd..8546447e63 100644 --- a/test/prism/errors/rescue_pattern.txt +++ b/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 From cda91eef1d894b2ccae8bea9a69d203ee096139a Mon Sep 17 00:00:00 2001 From: Kevin Newton Date: Wed, 17 Jun 2026 15:28:56 -0400 Subject: [PATCH 4/6] Better handle trailing comma on pattern matching --- snapshots/pattern_trailing_comma.txt | 101 ++++++++++++++++++ src/prism.c | 13 ++- .../prism/fixtures/pattern_trailing_comma.txt | 8 ++ 3 files changed, 120 insertions(+), 2 deletions(-) create mode 100644 snapshots/pattern_trailing_comma.txt create mode 100644 test/prism/fixtures/pattern_trailing_comma.txt diff --git a/snapshots/pattern_trailing_comma.txt b/snapshots/pattern_trailing_comma.txt new file mode 100644 index 0000000000..18b458ac74 --- /dev/null +++ b/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) = "=>" diff --git a/src/prism.c b/src/prism.c index 507cd11a4c..b688704048 100644 --- a/src/prism.c +++ b/src/prism.c @@ -17396,8 +17396,17 @@ 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. They are only treated + // as a trailing comma when no rest has been parsed yet, so + // `x => a, *b,` still reports the dangling 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) || + (!trailing_rest && match2(parser, PM_TOKEN_NEWLINE, PM_TOKEN_EOF)) + ) { node = UP(pm_implicit_rest_node_create(parser, &parser->previous)); pm_node_list_append(parser->arena, &nodes, node); trailing_rest = true; diff --git a/test/prism/fixtures/pattern_trailing_comma.txt b/test/prism/fixtures/pattern_trailing_comma.txt new file mode 100644 index 0000000000..996664126e --- /dev/null +++ b/test/prism/fixtures/pattern_trailing_comma.txt @@ -0,0 +1,8 @@ +case x +in a, + 1 +in b, c, + 2 +end + +foo => bar, From 8c30c3c82a97bc95523fa0c57bd708790a07a340 Mon Sep 17 00:00:00 2001 From: Kevin Newton Date: Wed, 17 Jun 2026 15:46:30 -0400 Subject: [PATCH 5/6] Handle trailing comma after ... --- src/prism.c | 13 ++++++++++++- .../errors/forwarding_arguments_trailing_comma.txt | 3 +++ 2 files changed, 15 insertions(+), 1 deletion(-) create mode 100644 test/prism/errors/forwarding_arguments_trailing_comma.txt diff --git a/src/prism.c b/src/prism.c index b688704048..5254b53890 100644 --- a/src/prism.c +++ b/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; + } } } diff --git a/test/prism/errors/forwarding_arguments_trailing_comma.txt b/test/prism/errors/forwarding_arguments_trailing_comma.txt new file mode 100644 index 0000000000..5649494c96 --- /dev/null +++ b/test/prism/errors/forwarding_arguments_trailing_comma.txt @@ -0,0 +1,3 @@ +def foo(...); bar(...,); end + ^ invalid comma + From 8f5a439394141bbd5a9d95a441a7607912690493 Mon Sep 17 00:00:00 2001 From: Kevin Newton Date: Wed, 17 Jun 2026 16:42:57 -0400 Subject: [PATCH 6/6] Fix duplicative rest pattern in pattern matching --- src/prism.c | 14 ++++++++++---- .../errors/pattern_array_rest_trailing_comma.txt | 3 +++ test/prism/errors/pattern_match_implicit_rest.txt | 2 +- 3 files changed, 14 insertions(+), 5 deletions(-) create mode 100644 test/prism/errors/pattern_array_rest_trailing_comma.txt diff --git a/src/prism.c b/src/prism.c index 5254b53890..4a3f5d0e24 100644 --- a/src/prism.c +++ b/src/prism.c @@ -17411,13 +17411,19 @@ parse_pattern(pm_parser_t *parser, pm_constant_id_list_t *captures, uint8_t flag // 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. They are only treated - // as a trailing comma when no rest has been parsed yet, so - // `x => a, *b,` still reports the dangling comma. + // 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) || - (!trailing_rest && match2(parser, PM_TOKEN_NEWLINE, PM_TOKEN_EOF)) + 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; diff --git a/test/prism/errors/pattern_array_rest_trailing_comma.txt b/test/prism/errors/pattern_array_rest_trailing_comma.txt new file mode 100644 index 0000000000..7f767dc68d --- /dev/null +++ b/test/prism/errors/pattern_array_rest_trailing_comma.txt @@ -0,0 +1,3 @@ +foo => [bar, *baz,] + ^ unexpected rest pattern + diff --git a/test/prism/errors/pattern_match_implicit_rest.txt b/test/prism/errors/pattern_match_implicit_rest.txt index 8602c0add0..31a49bef54 100644 --- a/test/prism/errors/pattern_match_implicit_rest.txt +++ b/test/prism/errors/pattern_match_implicit_rest.txt @@ -1,3 +1,3 @@ a=>b, *, - ^ expected a pattern expression after `,` + ^ unexpected rest pattern