Skip to content

Commit b7e247c

Browse files
Earlopainkddnewton
Earlopain
authored and
kddnewton
committed
[Feature #19107] Allow trailing comma in method signature
1 parent 16674e5 commit b7e247c

File tree

8 files changed

+276
-15
lines changed

8 files changed

+276
-15
lines changed

‎snapshots/4.1/trailing_comma_after_method_arguments.txt‎

Lines changed: 207 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,207 @@
1+
@ ProgramNode (location: (1,0)-(15,5))
2+
├── flags: ∅
3+
├── locals: []
4+
└── statements:
5+
@ StatementsNode (location: (1,0)-(15,5))
6+
├── flags: ∅
7+
└── body: (length: 6)
8+
├── @ DefNode (location: (1,0)-(1,19))
9+
│ ├── flags: newline
10+
│ ├── name: :foo
11+
│ ├── name_loc: (1,4)-(1,7) = "foo"
12+
│ ├── receiver: ∅
13+
│ ├── parameters:
14+
│ │ @ ParametersNode (location: (1,8)-(1,13))
15+
│ │ ├── flags: ∅
16+
│ │ ├── requireds: (length: 3)
17+
│ │ │ ├── @ RequiredParameterNode (location: (1,8)-(1,9))
18+
│ │ │ │ ├── flags: ∅
19+
│ │ │ │ └── name: :a
20+
│ │ │ ├── @ RequiredParameterNode (location: (1,10)-(1,11))
21+
│ │ │ │ ├── flags: ∅
22+
│ │ │ │ └── name: :b
23+
│ │ │ └── @ RequiredParameterNode (location: (1,12)-(1,13))
24+
│ │ │ ├── flags: ∅
25+
│ │ │ └── name: :c
26+
│ │ ├── optionals: (length: 0)
27+
│ │ ├── rest: ∅
28+
│ │ ├── posts: (length: 0)
29+
│ │ ├── keywords: (length: 0)
30+
│ │ ├── keyword_rest: ∅
31+
│ │ └── block: ∅
32+
│ ├── body: ∅
33+
│ ├── locals: [:a, :b, :c]
34+
│ ├── def_keyword_loc: (1,0)-(1,3) = "def"
35+
│ ├── operator_loc: ∅
36+
│ ├── lparen_loc: (1,7)-(1,8) = "("
37+
│ ├── rparen_loc: (1,14)-(1,15) = ")"
38+
│ ├── equal_loc: ∅
39+
│ └── end_keyword_loc: (1,16)-(1,19) = "end"
40+
├── @ DefNode (location: (3,0)-(3,20))
41+
│ ├── flags: newline
42+
│ ├── name: :foo
43+
│ ├── name_loc: (3,4)-(3,7) = "foo"
44+
│ ├── receiver: ∅
45+
│ ├── parameters:
46+
│ │ @ ParametersNode (location: (3,8)-(3,14))
47+
│ │ ├── flags: ∅
48+
│ │ ├── requireds: (length: 2)
49+
│ │ │ ├── @ RequiredParameterNode (location: (3,8)-(3,9))
50+
│ │ │ │ ├── flags: ∅
51+
│ │ │ │ └── name: :a
52+
│ │ │ └── @ RequiredParameterNode (location: (3,10)-(3,11))
53+
│ │ │ ├── flags: ∅
54+
│ │ │ └── name: :b
55+
│ │ ├── optionals: (length: 0)
56+
│ │ ├── rest:
57+
│ │ │ @ RestParameterNode (location: (3,12)-(3,14))
58+
│ │ │ ├── flags: ∅
59+
│ │ │ ├── name: :c
60+
│ │ │ ├── name_loc: (3,13)-(3,14) = "c"
61+
│ │ │ └── operator_loc: (3,12)-(3,13) = "*"
62+
│ │ ├── posts: (length: 0)
63+
│ │ ├── keywords: (length: 0)
64+
│ │ ├── keyword_rest: ∅
65+
│ │ └── block: ∅
66+
│ ├── body: ∅
67+
│ ├── locals: [:a, :b, :c]
68+
│ ├── def_keyword_loc: (3,0)-(3,3) = "def"
69+
│ ├── operator_loc: ∅
70+
│ ├── lparen_loc: (3,7)-(3,8) = "("
71+
│ ├── rparen_loc: (3,15)-(3,16) = ")"
72+
│ ├── equal_loc: ∅
73+
│ └── end_keyword_loc: (3,17)-(3,20) = "end"
74+
├── @ DefNode (location: (5,0)-(5,19))
75+
│ ├── flags: newline
76+
│ ├── name: :foo
77+
│ ├── name_loc: (5,4)-(5,7) = "foo"
78+
│ ├── receiver: ∅
79+
│ ├── parameters:
80+
│ │ @ ParametersNode (location: (5,8)-(5,13))
81+
│ │ ├── flags: ∅
82+
│ │ ├── requireds: (length: 2)
83+
│ │ │ ├── @ RequiredParameterNode (location: (5,8)-(5,9))
84+
│ │ │ │ ├── flags: ∅
85+
│ │ │ │ └── name: :a
86+
│ │ │ └── @ RequiredParameterNode (location: (5,10)-(5,11))
87+
│ │ │ ├── flags: ∅
88+
│ │ │ └── name: :b
89+
│ │ ├── optionals: (length: 0)
90+
│ │ ├── rest:
91+
│ │ │ @ RestParameterNode (location: (5,12)-(5,13))
92+
│ │ │ ├── flags: ∅
93+
│ │ │ ├── name: ∅
94+
│ │ │ ├── name_loc: ∅
95+
│ │ │ └── operator_loc: (5,12)-(5,13) = "*"
96+
│ │ ├── posts: (length: 0)
97+
│ │ ├── keywords: (length: 0)
98+
│ │ ├── keyword_rest: ∅
99+
│ │ └── block: ∅
100+
│ ├── body: ∅
101+
│ ├── locals: [:a, :b]
102+
│ ├── def_keyword_loc: (5,0)-(5,3) = "def"
103+
│ ├── operator_loc: ∅
104+
│ ├── lparen_loc: (5,7)-(5,8) = "("
105+
│ ├── rparen_loc: (5,14)-(5,15) = ")"
106+
│ ├── equal_loc: ∅
107+
│ └── end_keyword_loc: (5,16)-(5,19) = "end"
108+
├── @ DefNode (location: (7,0)-(7,21))
109+
│ ├── flags: newline
110+
│ ├── name: :foo
111+
│ ├── name_loc: (7,4)-(7,7) = "foo"
112+
│ ├── receiver: ∅
113+
│ ├── parameters:
114+
│ │ @ ParametersNode (location: (7,8)-(7,15))
115+
│ │ ├── flags: ∅
116+
│ │ ├── requireds: (length: 2)
117+
│ │ │ ├── @ RequiredParameterNode (location: (7,8)-(7,9))
118+
│ │ │ │ ├── flags: ∅
119+
│ │ │ │ └── name: :a
120+
│ │ │ └── @ RequiredParameterNode (location: (7,10)-(7,11))
121+
│ │ │ ├── flags: ∅
122+
│ │ │ └── name: :b
123+
│ │ ├── optionals: (length: 0)
124+
│ │ ├── rest: ∅
125+
│ │ ├── posts: (length: 0)
126+
│ │ ├── keywords: (length: 0)
127+
│ │ ├── keyword_rest:
128+
│ │ │ @ KeywordRestParameterNode (location: (7,12)-(7,15))
129+
│ │ │ ├── flags: ∅
130+
│ │ │ ├── name: :c
131+
│ │ │ ├── name_loc: (7,14)-(7,15) = "c"
132+
│ │ │ └── operator_loc: (7,12)-(7,14) = "**"
133+
│ │ └── block: ∅
134+
│ ├── body: ∅
135+
│ ├── locals: [:a, :b, :c]
136+
│ ├── def_keyword_loc: (7,0)-(7,3) = "def"
137+
│ ├── operator_loc: ∅
138+
│ ├── lparen_loc: (7,7)-(7,8) = "("
139+
│ ├── rparen_loc: (7,16)-(7,17) = ")"
140+
│ ├── equal_loc: ∅
141+
│ └── end_keyword_loc: (7,18)-(7,21) = "end"
142+
├── @ DefNode (location: (9,0)-(9,20))
143+
│ ├── flags: newline
144+
│ ├── name: :foo
145+
│ ├── name_loc: (9,4)-(9,7) = "foo"
146+
│ ├── receiver: ∅
147+
│ ├── parameters:
148+
│ │ @ ParametersNode (location: (9,8)-(9,14))
149+
│ │ ├── flags: ∅
150+
│ │ ├── requireds: (length: 2)
151+
│ │ │ ├── @ RequiredParameterNode (location: (9,8)-(9,9))
152+
│ │ │ │ ├── flags: ∅
153+
│ │ │ │ └── name: :a
154+
│ │ │ └── @ RequiredParameterNode (location: (9,10)-(9,11))
155+
│ │ │ ├── flags: ∅
156+
│ │ │ └── name: :b
157+
│ │ ├── optionals: (length: 0)
158+
│ │ ├── rest: ∅
159+
│ │ ├── posts: (length: 0)
160+
│ │ ├── keywords: (length: 0)
161+
│ │ ├── keyword_rest:
162+
│ │ │ @ KeywordRestParameterNode (location: (9,12)-(9,14))
163+
│ │ │ ├── flags: ∅
164+
│ │ │ ├── name: ∅
165+
│ │ │ ├── name_loc: ∅
166+
│ │ │ └── operator_loc: (9,12)-(9,14) = "**"
167+
│ │ └── block: ∅
168+
│ ├── body: ∅
169+
│ ├── locals: [:a, :b]
170+
│ ├── def_keyword_loc: (9,0)-(9,3) = "def"
171+
│ ├── operator_loc: ∅
172+
│ ├── lparen_loc: (9,7)-(9,8) = "("
173+
│ ├── rparen_loc: (9,15)-(9,16) = ")"
174+
│ ├── equal_loc: ∅
175+
│ └── end_keyword_loc: (9,17)-(9,20) = "end"
176+
└── @ DefNode (location: (11,0)-(15,5))
177+
├── flags: newline
178+
├── name: :foo
179+
├── name_loc: (11,4)-(11,7) = "foo"
180+
├── receiver: ∅
181+
├── parameters:
182+
│ @ ParametersNode (location: (12,2)-(14,3))
183+
│ ├── flags: ∅
184+
│ ├── requireds: (length: 3)
185+
│ │ ├── @ RequiredParameterNode (location: (12,2)-(12,3))
186+
│ │ │ ├── flags: ∅
187+
│ │ │ └── name: :a
188+
│ │ ├── @ RequiredParameterNode (location: (13,2)-(13,3))
189+
│ │ │ ├── flags: ∅
190+
│ │ │ └── name: :b
191+
│ │ └── @ RequiredParameterNode (location: (14,2)-(14,3))
192+
│ │ ├── flags: ∅
193+
│ │ └── name: :c
194+
│ ├── optionals: (length: 0)
195+
│ ├── rest: ∅
196+
│ ├── posts: (length: 0)
197+
│ ├── keywords: (length: 0)
198+
│ ├── keyword_rest: ∅
199+
│ └── block: ∅
200+
├── body: ∅
201+
├── locals: [:a, :b, :c]
202+
├── def_keyword_loc: (11,0)-(11,3) = "def"
203+
├── operator_loc: ∅
204+
├── lparen_loc: (11,7)-(11,8) = "("
205+
├── rparen_loc: (15,0)-(15,1) = ")"
206+
├── equal_loc: ∅
207+
└── end_keyword_loc: (15,2)-(15,5) = "end"

‎src/prism.c‎

Lines changed: 41 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -13907,6 +13907,43 @@ update_parameter_state(pm_parser_t *parser, pm_token_t *token, pm_parameters_ord
1390713907
return true;
1390813908
}
1390913909

13910+
static inline void
13911+
parse_parameters_handle_trailing_comma(
13912+
pm_parser_t *parser,
13913+
pm_parameters_node_t *params,
13914+
pm_parameters_order_t order,
13915+
bool in_block,
13916+
bool allows_trailing_comma
13917+
) {
13918+
if (!allows_trailing_comma) {
13919+
pm_parser_err_previous(parser, PM_ERR_PARAMETER_WILD_LOOSE_COMMA);
13920+
return;
13921+
}
13922+
13923+
if (in_block) {
13924+
if (order >= PM_PARAMETERS_ORDER_NAMED) {
13925+
// foo do |bar,|; end
13926+
pm_node_t *param = UP(pm_implicit_rest_node_create(parser, &parser->previous));
13927+
13928+
if (params->rest == NULL) {
13929+
pm_parameters_node_rest_set(params, param);
13930+
} else {
13931+
pm_parser_err_node(parser, UP(param), PM_ERR_PARAMETER_SPLAT_MULTI);
13932+
pm_parameters_node_posts_append(params, UP(param));
13933+
}
13934+
} else {
13935+
// foo do |*bar,|; end
13936+
pm_parser_err_previous(parser, PM_ERR_PARAMETER_WILD_LOOSE_COMMA);
13937+
}
13938+
} else {
13939+
// https://bugs.ruby-lang.org/issues/19107
13940+
// Allow `def foo(bar,); end`, `def foo(*bar,); end`, etc. but not `def foo(...,); end`
13941+
if (parser->version < PM_OPTIONS_VERSION_CRUBY_4_1 || order == PM_PARAMETERS_ORDER_NOTHING_AFTER) {
13942+
pm_parser_err_previous(parser, PM_ERR_PARAMETER_WILD_LOOSE_COMMA);
13943+
}
13944+
}
13945+
}
13946+
1391013947
/**
1391113948
* Parse a list of parameters on a method definition.
1391213949
*/
@@ -14255,20 +14292,7 @@ parse_parameters(
1425514292
}
1425614293
default:
1425714294
if (parser->previous.type == PM_TOKEN_COMMA) {
14258-
if (allows_trailing_comma && order >= PM_PARAMETERS_ORDER_NAMED) {
14259-
// If we get here, then we have a trailing comma in a
14260-
// block parameter list.
14261-
pm_node_t *param = UP(pm_implicit_rest_node_create(parser, &parser->previous));
14262-
14263-
if (params->rest == NULL) {
14264-
pm_parameters_node_rest_set(params, param);
14265-
} else {
14266-
pm_parser_err_node(parser, UP(param), PM_ERR_PARAMETER_SPLAT_MULTI);
14267-
pm_parameters_node_posts_append(params, UP(param));
14268-
}
14269-
} else {
14270-
pm_parser_err_previous(parser, PM_ERR_PARAMETER_WILD_LOOSE_COMMA);
14271-
}
14295+
parse_parameters_handle_trailing_comma(parser, params, order, in_block, allows_trailing_comma);
1427214296
}
1427314297

1427414298
parsing = false;
@@ -18865,7 +18889,9 @@ parse_expression_prefix(pm_parser_t *parser, pm_binding_power_t binding_power, b
1886518889
if (match1(parser, PM_TOKEN_PARENTHESIS_RIGHT)) {
1886618890
params = NULL;
1886718891
} else {
18868-
params = parse_parameters(parser, PM_BINDING_POWER_DEFINED, true, false, true, true, false, (uint16_t) (depth + 1));
18892+
// https://bugs.ruby-lang.org/issues/19107
18893+
bool allow_trailing_comma = parser->version >= PM_OPTIONS_VERSION_CRUBY_4_1;
18894+
params = parse_parameters(parser, PM_BINDING_POWER_DEFINED, true, allow_trailing_comma, true, true, false, (uint16_t) (depth + 1));
1886918895
}
1887018896

1887118897
lex_state_set(parser, PM_LEX_STATE_BEG);

test/prism/errors/do_not_allow_trailing_commas_in_method_parameters.txt renamed to test/prism/errors/3.3-4.0/do_not_allow_trailing_commas_in_method_parameters.txt

File renamed without changes.

‎test/prism/errors/4.1/do_not_allow_trailing_commas_after_terminating_arguments.txt‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
def foo(a,b,...,);end
2+
^ unexpected `,` in parameters
3+
4+
def foo(a,b,&block,);end
5+
^ unexpected `,` in parameters
6+

‎test/prism/fixtures/4.1/trailing_comma_after_method_arguments.txt‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
def foo(a,b,c,);end
2+
3+
def foo(a,b,*c,);end
4+
5+
def foo(a,b,*,);end
6+
7+
def foo(a,b,**c,);end
8+
9+
def foo(a,b,**,);end
10+
11+
def foo(
12+
a,
13+
b,
14+
c,
15+
);end

‎test/prism/fixtures_test.rb‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,8 @@ class FixturesTest < TestCase
2828
except << "command_method_call_2.txt"
2929
# https://bugs.ruby-lang.org/issues/21669
3030
except << "4.1/void_value.txt"
31+
# https://bugs.ruby-lang.org/issues/19107
32+
except << "4.1/trailing_comma_after_method_arguments.txt"
3133

3234
Fixture.each_for_current_ruby(except: except) do |fixture|
3335
define_method(fixture.test_name) { assert_valid_syntax(fixture.read) }

‎test/prism/locals_test.rb‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,9 @@ class LocalsTest < TestCase
3131

3232
# https://bugs.ruby-lang.org/issues/21669
3333
"4.1/void_value.txt",
34+
35+
# https://bugs.ruby-lang.org/issues/19107
36+
"4.1/trailing_comma_after_method_arguments.txt",
3437
]
3538

3639
Fixture.each_for_current_ruby(except: except) do |fixture|

‎test/prism/ruby/ripper_test.rb‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,8 @@ class RipperTest < TestCase
3939

4040
# https://bugs.ruby-lang.org/issues/21669
4141
incorrect << "4.1/void_value.txt"
42+
# https://bugs.ruby-lang.org/issues/19107
43+
incorrect << "4.1/trailing_comma_after_method_arguments.txt"
4244

4345
# Skip these tests that we haven't implemented yet.
4446
omitted_sexp_raw = [

0 commit comments

Comments
 (0)