Skip to content

Commit 20e36fa

Browse files
authored
Merge pull request #3652 from Earlopain/command-call-trailing-args
Reject argument command call taking a block with more trailing arguments
2 parents c62867b + 3a4e102 commit 20e36fa

10 files changed

Lines changed: 155 additions & 0 deletions

File tree

Lines changed: 96 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,96 @@
1+
@ ProgramNode (location: (1,0)-(3,17))
2+
├── flags: ∅
3+
├── locals: []
4+
└── statements:
5+
@ StatementsNode (location: (1,0)-(3,17))
6+
├── flags: ∅
7+
└── body: (length: 2)
8+
├── @ CallNode (location: (1,0)-(1,19))
9+
│ ├── flags: newline, ignore_visibility
10+
│ ├── receiver: ∅
11+
│ ├── call_operator_loc: ∅
12+
│ ├── name: :foo
13+
│ ├── message_loc: (1,0)-(1,3) = "foo"
14+
│ ├── opening_loc: (1,3)-(1,4) = "("
15+
│ ├── arguments:
16+
│ │ @ ArgumentsNode (location: (1,4)-(1,18))
17+
│ │ ├── flags: ∅
18+
│ │ └── arguments: (length: 1)
19+
│ │ └── @ CallNode (location: (1,4)-(1,18))
20+
│ │ ├── flags: ignore_visibility
21+
│ │ ├── receiver: ∅
22+
│ │ ├── call_operator_loc: ∅
23+
│ │ ├── name: :bar
24+
│ │ ├── message_loc: (1,4)-(1,7) = "bar"
25+
│ │ ├── opening_loc: ∅
26+
│ │ ├── arguments:
27+
│ │ │ @ ArgumentsNode (location: (1,8)-(1,11))
28+
│ │ │ ├── flags: ∅
29+
│ │ │ └── arguments: (length: 1)
30+
│ │ │ └── @ CallNode (location: (1,8)-(1,11))
31+
│ │ │ ├── flags: variable_call, ignore_visibility
32+
│ │ │ ├── receiver: ∅
33+
│ │ │ ├── call_operator_loc: ∅
34+
│ │ │ ├── name: :baz
35+
│ │ │ ├── message_loc: (1,8)-(1,11) = "baz"
36+
│ │ │ ├── opening_loc: ∅
37+
│ │ │ ├── arguments: ∅
38+
│ │ │ ├── closing_loc: ∅
39+
│ │ │ └── block: ∅
40+
│ │ ├── closing_loc: ∅
41+
│ │ └── block:
42+
│ │ @ BlockNode (location: (1,12)-(1,18))
43+
│ │ ├── flags: ∅
44+
│ │ ├── locals: []
45+
│ │ ├── parameters: ∅
46+
│ │ ├── body: ∅
47+
│ │ ├── opening_loc: (1,12)-(1,14) = "do"
48+
│ │ └── closing_loc: (1,15)-(1,18) = "end"
49+
│ ├── closing_loc: (1,18)-(1,19) = ")"
50+
│ └── block: ∅
51+
└── @ CallNode (location: (3,0)-(3,17))
52+
├── flags: newline, ignore_visibility
53+
├── receiver: ∅
54+
├── call_operator_loc: ∅
55+
├── name: :foo
56+
├── message_loc: (3,0)-(3,3) = "foo"
57+
├── opening_loc: (3,3)-(3,4) = "("
58+
├── arguments:
59+
│ @ ArgumentsNode (location: (3,4)-(3,16))
60+
│ ├── flags: ∅
61+
│ └── arguments: (length: 1)
62+
│ └── @ CallNode (location: (3,4)-(3,16))
63+
│ ├── flags: ignore_visibility
64+
│ ├── receiver: ∅
65+
│ ├── call_operator_loc: ∅
66+
│ ├── name: :bar
67+
│ ├── message_loc: (3,4)-(3,7) = "bar"
68+
│ ├── opening_loc: ∅
69+
│ ├── arguments:
70+
│ │ @ ArgumentsNode (location: (3,8)-(3,16))
71+
│ │ ├── flags: ∅
72+
│ │ └── arguments: (length: 2)
73+
│ │ ├── @ CallNode (location: (3,8)-(3,11))
74+
│ │ │ ├── flags: variable_call, ignore_visibility
75+
│ │ │ ├── receiver: ∅
76+
│ │ │ ├── call_operator_loc: ∅
77+
│ │ │ ├── name: :baz
78+
│ │ │ ├── message_loc: (3,8)-(3,11) = "baz"
79+
│ │ │ ├── opening_loc: ∅
80+
│ │ │ ├── arguments: ∅
81+
│ │ │ ├── closing_loc: ∅
82+
│ │ │ └── block: ∅
83+
│ │ └── @ CallNode (location: (3,13)-(3,16))
84+
│ │ ├── flags: variable_call, ignore_visibility
85+
│ │ ├── receiver: ∅
86+
│ │ ├── call_operator_loc: ∅
87+
│ │ ├── name: :bat
88+
│ │ ├── message_loc: (3,13)-(3,16) = "bat"
89+
│ │ ├── opening_loc: ∅
90+
│ │ ├── arguments: ∅
91+
│ │ ├── closing_loc: ∅
92+
│ │ └── block: ∅
93+
│ ├── closing_loc: ∅
94+
│ └── block: ∅
95+
├── closing_loc: (3,16)-(3,17) = ")"
96+
└── block: ∅

src/prism.c

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14443,6 +14443,17 @@ parse_arguments(pm_parser_t *parser, pm_arguments_t *arguments, bool accepts_for
1444314443
if (accepted_newline) {
1444414444
pm_parser_err_previous(parser, PM_ERR_INVALID_COMMA);
1444514445
}
14446+
14447+
// If this is a command call and an argument takes a block,
14448+
// there can be no further arguments. For example,
14449+
// `foo(bar 1 do end, 2)` should be rejected.
14450+
if (PM_NODE_TYPE_P(argument, PM_CALL_NODE)) {
14451+
pm_call_node_t *call = (pm_call_node_t *) argument;
14452+
if (call->opening_loc.start == NULL && call->arguments != NULL && call->block != NULL) {
14453+
pm_parser_err_previous(parser, PM_ERR_INVALID_COMMA);
14454+
break;
14455+
}
14456+
}
1444614457
} else {
1444714458
// If there is no comma at the end of the argument list then we're
1444814459
// done parsing arguments and can break out of this loop.
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,10 @@
11
[a b]
22
^ unexpected local variable or method; expected a `,` separator for the array elements
33

4+
5+
[
6+
a b do
7+
^ unexpected local variable or method; expected a `,` separator for the array elements
8+
end,
9+
]
10+
Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
foo(bar 1 do end, 2)
2+
^ invalid comma
3+
^ unexpected integer; expected a `)` to close the arguments
4+
^ unexpected integer, expecting end-of-input
5+
^ unexpected ')', expecting end-of-input
6+
^ unexpected ')', ignoring it
7+
8+
foo(bar 1 do end,)
9+
^ invalid comma
10+
11+
foo(1, bar 2 do end)
12+
^ unexpected integer; expected a `)` to close the arguments
13+
^ unexpected integer, expecting end-of-input
14+
^~ unexpected 'do', expecting end-of-input
15+
^~ unexpected 'do', ignoring it
16+
^~~ unexpected 'end', ignoring it
17+
^ unexpected ')', ignoring it
18+
19+
foo(1, bar 2)
20+
^ unexpected integer; expected a `)` to close the arguments
21+
^ unexpected integer, expecting end-of-input
22+
^ unexpected ')', expecting end-of-input
23+
^ unexpected ')', ignoring it
24+
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
foo(bar baz do end)
2+
3+
foo(bar baz, bat)

test/prism/fixtures_test.rb

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,8 @@ class FixturesTest < TestCase
2727
# Leaving these out until they are supported by parse.y.
2828
except << "leading_logical.txt"
2929
except << "endless_methods_command_call.txt"
30+
# https://bugs.ruby-lang.org/issues/21168#note-5
31+
except << "command_method_call_2.txt"
3032

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

test/prism/lex_test.rb

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,9 @@ class LexTest < TestCase
4848
# https://bugs.ruby-lang.org/issues/17398#note-12
4949
except << "endless_methods_command_call.txt"
5050

51+
# https://bugs.ruby-lang.org/issues/21168#note-5
52+
except << "command_method_call_2.txt"
53+
5154
Fixture.each(except: except) do |fixture|
5255
define_method(fixture.test_name) { assert_lex(fixture) }
5356
end

test/prism/ruby/parser_test.rb

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,9 @@ class ParserTest < TestCase
7070

7171
# Ruby >= 3.5 specific syntax
7272
"endless_methods_command_call.txt",
73+
74+
# https://bugs.ruby-lang.org/issues/21168#note-5
75+
"command_method_call_2.txt",
7376
]
7477

7578
# These files contain code that is being parsed incorrectly by the parser

test/prism/ruby/ripper_test.rb

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,9 @@ class RipperTest < TestCase
3333

3434
# https://bugs.ruby-lang.org/issues/17398#note-12
3535
"endless_methods_command_call.txt",
36+
37+
# https://bugs.ruby-lang.org/issues/21168#note-5
38+
"command_method_call_2.txt",
3639
]
3740

3841
# Skip these tests that we haven't implemented yet.

test/prism/ruby/ruby_parser_test.rb

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,9 @@ class RubyParserTest < TestCase
7979

8080
# Ruby >= 3.5 specific syntax
8181
"endless_methods_command_call.txt",
82+
83+
# https://bugs.ruby-lang.org/issues/21168#note-5
84+
"command_method_call_2.txt",
8285
]
8386

8487
Fixture.each(except: failures) do |fixture|

0 commit comments

Comments
 (0)