Repository navigation
Parse proc call shorthand .() and ::() after a command with a do block - #9793
edipofederle wants to merge 1 commit into
Conversation
A paren-less command call carrying a do...end block parses as
block_call : command do_block. block_call could chain a named method
(.foo / ::foo) but had no production for the call shorthand .() / ::(),
which only existed under method_call (requiring a primary_value receiver).
So 'o.m "foo" do end.("buz")' raised a syntax error on the '('.
Add two block_call productions mirroring the method_call shorthand
(split call_op vs tCOLON2 to match new_call's overloads), matching
CRuby's 'block_call call_op2 paren_args'. Regenerated RubyParser and
RipperParser via jay; no new grammar conflicts.
Fixes the previously-excluded MRI test_parse.rb test_call_command.
Closes jruby#9270
|
@headius could you take a look when possible? Thanks |
| /*% %*/ | ||
| /*% ripper: method_add_block!(command_call!($1, $2, $3, $4), $5) %*/ | ||
| } | ||
| | block_call call_op paren_args { |
There was a problem hiding this comment.
Seems to correspond to this in CRuby:
https://github.com/ruby/ruby/blob/b2ec82d4ea84f5526b3759923159a039bc5c1d3b/parse.y#L5187-L5192
| /*% %*/ | ||
| /*% ripper: method_add_arg!(call!($1, $2, ID2VAL(idCall)), $3) %*/ | ||
| } | ||
| | block_call tCOLON2 paren_args { |
There was a problem hiding this comment.
I could not locate this production in parse.y. Maybe @enebo can have a look or we just go with it since it's working.
|
I think you should replace the first production line with: Notice the call_op2 vs call_op. I think with what you can lazy operator calling would still be broken. Delete the second production: That is already covered by call_op2 once you change the first production line. |
|
@edipofederle In addition to the above did you run ./tool/generate_parser to make those .java files? It shows a conflict and unless you made this before sferik's recent changes that should not happen. |
enebo
left a comment
There was a problem hiding this comment.
Make the changes I requested and it should be good to go. I find the lack of safe merging weird. There should be no conflict and yet I see sferik as your last commit to the RubyParser.java file. Perhaps there was a followup commit from that work?
| /*% %*/ | ||
| /*% ripper: method_add_block!(command_call!($1, $2, $3, $4), $5) %*/ | ||
| } | ||
| | block_call call_op paren_args { |
| | block_call call_op paren_args { | ||
| /*%%%*/ | ||
| $$ = p.new_call($1, $2, LexingCommon.CALL, $3, null, @3.start()); | ||
| p.safe_navigation_end($<Node>$, @3.end, @3.end, true); // o.m foo do end.(): the parentheses are the message |
There was a problem hiding this comment.
Remove this comment. At a minimum it is a confusing example since the block is on 'foo' and not 'o.m'.
| /*% %*/ | ||
| /*% ripper: method_add_arg!(call!($1, $2, ID2VAL(idCall)), $3) %*/ | ||
| } | ||
| | block_call tCOLON2 paren_args { |
There was a problem hiding this comment.
Delete this whole production. Using call_op2 fixes removes the need for this.
Fixes #9270: the proc-call shorthand
.()/::()now parses when it follows a paren-less command call that carries ado...endblock.Problem
o.m "foo", "bar" do endparses asblock_call : command do_block.block_callcould chain a named method (.foo/::foo) but had no production for the call shorthand.()/::(). That shorthand lived only undermethod_call, which requires aprimary_valuereceiver, and ablock_callis not one, so the(after
./::had no matching rule.Fix
Add two productions to
block_callinRubyParser.y, mirroring the existingmethod_callshorthand (JRuby splitscall_opvstCOLON2becausenew_callhas distinct overloads). This matches CRuby'sblock_call call_op2 paren_argsinparse.y.RubyParser.javaandRipperParser.javawere regenerated withjay; no new grammar conflicts were introduced.Testing
test/mri/ruby/test_parse.rbtest_call_command(the CRuby regression test for this exact case) now passes; its entry intest/mri/excludes/TestParse.rbis removed.test_parse.rb: failures drop by exactly one (the fixed test); no new failures or errors.test/mri/ripper/test_ripper.rbresults are unchanged (RipperParser regeneration introduced no regression)."foobarbuz"; named-method chaining after a do-block and.()/::()/&.()on ordinary receivers continue to work.