From bed4271ce2d706e50e210b717f7960f5cf743eeb Mon Sep 17 00:00:00 2001 From: Benoit Daloze Date: Tue, 20 Jan 2026 21:20:06 +0100 Subject: [PATCH 1/2] Check using Prism nodes if a command call has any arguments in Ripper translator * We don't know what `on_*` events might return so we cannot assume it's an Array. * See https://github.com/ruby/prism/issues/3838#issuecomment-3774702396 --- lib/prism/translation/ripper.rb | 18 +++++++++++++----- test/prism/ruby/ripper_test.rb | 12 ++++++++++++ 2 files changed, 25 insertions(+), 5 deletions(-) diff --git a/lib/prism/translation/ripper.rb b/lib/prism/translation/ripper.rb index afe30bb5db..41dd699438 100644 --- a/lib/prism/translation/ripper.rb +++ b/lib/prism/translation/ripper.rb @@ -1112,7 +1112,7 @@ def visit_call_node(node) else arguments, block = visit_call_node_arguments(node.arguments, node.block, trailing_comma?(node.arguments&.location || node.location, node.closing_loc || node.location)) call = - if node.opening_loc.nil? && arguments&.any? + if node.opening_loc.nil? && get_arguments_and_block(node.arguments, node.block).first.any? bounds(node.location) on_command(message, arguments) elsif !node.opening_loc.nil? @@ -1179,17 +1179,25 @@ def visit_call_node(node) end end - # Visit the arguments and block of a call node and return the arguments - # and block as they should be used. - private def visit_call_node_arguments(arguments_node, block_node, trailing_comma) + # Extract the arguments and block Ripper-style, which means if the block + # is like `&b` then it's moved to arguments. + private def get_arguments_and_block(arguments_node, block_node) arguments = arguments_node&.arguments || [] block = block_node if block.is_a?(BlockArgumentNode) - arguments << block + arguments += [block] block = nil end + [arguments, block] + end + + # Visit the arguments and block of a call node and return the arguments + # and block as they should be used. + private def visit_call_node_arguments(arguments_node, block_node, trailing_comma) + arguments, block = get_arguments_and_block(arguments_node, block_node) + [ if arguments.length == 1 && arguments.first.is_a?(ForwardingArgumentsNode) visit(arguments.first) diff --git a/test/prism/ruby/ripper_test.rb b/test/prism/ruby/ripper_test.rb index 174c90cbca..8848c6a8c0 100644 --- a/test/prism/ruby/ripper_test.rb +++ b/test/prism/ruby/ripper_test.rb @@ -110,6 +110,14 @@ class PrismEvents < Translation::Ripper include Events end + class ObjectEvents < Translation::Ripper + Prism::Translation::Ripper::PARSER_EVENTS.each do |event| + define_method(:"on_#{event}") do |*args| + Object.new + end + end + end + def test_events source = "1 rescue 2" ripper = RipperEvents.new(source) @@ -152,6 +160,10 @@ def test_internals def assert_ripper_sexp_raw(source) assert_equal Ripper.sexp_raw(source), Prism::Translation::Ripper.sexp_raw(source) + + # Similar to test/ripper/assert_parse_files.rb in CRuby + object_events = ObjectEvents.new(source) + assert_nothing_raised { object_events.parse } end def assert_ripper_lex(source) From 52c4fa785eeebf25b77f2ea5dad0109435c96b7c Mon Sep 17 00:00:00 2001 From: Earlopain <14981592+Earlopain@users.noreply.github.com> Date: Wed, 21 Jan 2026 13:19:42 +0100 Subject: [PATCH 2/2] Also handle `BasicObject` as a return value We should touch these as little as possible and just pass them along --- lib/prism/translation/ripper.rb | 51 +++++++++++++++++---------------- test/prism/ruby/ripper_test.rb | 18 +++++++----- 2 files changed, 37 insertions(+), 32 deletions(-) diff --git a/lib/prism/translation/ripper.rb b/lib/prism/translation/ripper.rb index 41dd699438..735217d2e0 100644 --- a/lib/prism/translation/ripper.rb +++ b/lib/prism/translation/ripper.rb @@ -832,7 +832,7 @@ def visit_array_pattern_node(node) # foo(bar) # ^^^ def visit_arguments_node(node) - arguments, _ = visit_call_node_arguments(node, nil, false) + arguments, _, _ = visit_call_node_arguments(node, nil, false) arguments end @@ -1042,16 +1042,16 @@ def visit_call_node(node) case node.name when :[] receiver = visit(node.receiver) - arguments, block = visit_call_node_arguments(node.arguments, node.block, trailing_comma?(node.arguments&.location || node.location, node.closing_loc)) + arguments, block, has_ripper_block = visit_call_node_arguments(node.arguments, node.block, trailing_comma?(node.arguments&.location || node.location, node.closing_loc)) bounds(node.location) call = on_aref(receiver, arguments) - if block.nil? - call - else + if has_ripper_block bounds(node.location) on_method_add_block(call, block) + else + call end when :[]= receiver = visit(node.receiver) @@ -1110,7 +1110,7 @@ def visit_call_node(node) if node.variable_call? on_vcall(message) else - arguments, block = visit_call_node_arguments(node.arguments, node.block, trailing_comma?(node.arguments&.location || node.location, node.closing_loc || node.location)) + arguments, block, has_ripper_block = visit_call_node_arguments(node.arguments, node.block, trailing_comma?(node.arguments&.location || node.location, node.closing_loc || node.location)) call = if node.opening_loc.nil? && get_arguments_and_block(node.arguments, node.block).first.any? bounds(node.location) @@ -1123,11 +1123,11 @@ def visit_call_node(node) on_method_add_arg(on_fcall(message), on_args_new) end - if block.nil? - call - else + if has_ripper_block bounds(node.block.location) on_method_add_block(call, block) + else + call end end end @@ -1151,7 +1151,7 @@ def visit_call_node(node) bounds(node.location) on_assign(on_field(receiver, call_operator, message), value) else - arguments, block = visit_call_node_arguments(node.arguments, node.block, trailing_comma?(node.arguments&.location || node.location, node.closing_loc || node.location)) + arguments, block, has_ripper_block = visit_call_node_arguments(node.arguments, node.block, trailing_comma?(node.arguments&.location || node.location, node.closing_loc || node.location)) call = if node.opening_loc.nil? bounds(node.location) @@ -1169,11 +1169,11 @@ def visit_call_node(node) on_method_add_arg(on_call(receiver, call_operator, message), arguments) end - if block.nil? - call - else + if has_ripper_block bounds(node.block.location) on_method_add_block(call, block) + else + call end end end @@ -1211,7 +1211,8 @@ def visit_call_node(node) on_args_add_block(args, false) end end, - visit(block) + visit(block), + block != nil, ] end @@ -1648,10 +1649,10 @@ def visit_def_node(node) end bounds(node.location) - if receiver.nil? - on_def(name, parameters, bodystmt) - else + if receiver on_defs(receiver, operator, name, parameters, bodystmt) + else + on_def(name, parameters, bodystmt) end end @@ -2049,7 +2050,7 @@ def visit_in_node(node) # ^^^^^^^^^^^^^^^ def visit_index_operator_write_node(node) receiver = visit(node.receiver) - arguments, _ = visit_call_node_arguments(node.arguments, node.block, trailing_comma?(node.arguments&.location || node.location, node.closing_loc)) + arguments, _, _ = visit_call_node_arguments(node.arguments, node.block, trailing_comma?(node.arguments&.location || node.location, node.closing_loc)) bounds(node.location) target = on_aref_field(receiver, arguments) @@ -2066,7 +2067,7 @@ def visit_index_operator_write_node(node) # ^^^^^^^^^^^^^^^^ def visit_index_and_write_node(node) receiver = visit(node.receiver) - arguments, _ = visit_call_node_arguments(node.arguments, node.block, trailing_comma?(node.arguments&.location || node.location, node.closing_loc)) + arguments, _, _ = visit_call_node_arguments(node.arguments, node.block, trailing_comma?(node.arguments&.location || node.location, node.closing_loc)) bounds(node.location) target = on_aref_field(receiver, arguments) @@ -2083,7 +2084,7 @@ def visit_index_and_write_node(node) # ^^^^^^^^^^^^^^^^ def visit_index_or_write_node(node) receiver = visit(node.receiver) - arguments, _ = visit_call_node_arguments(node.arguments, node.block, trailing_comma?(node.arguments&.location || node.location, node.closing_loc)) + arguments, _, _ = visit_call_node_arguments(node.arguments, node.block, trailing_comma?(node.arguments&.location || node.location, node.closing_loc)) bounds(node.location) target = on_aref_field(receiver, arguments) @@ -2100,7 +2101,7 @@ def visit_index_or_write_node(node) # ^^^^^^^^ def visit_index_target_node(node) receiver = visit(node.receiver) - arguments, _ = visit_call_node_arguments(node.arguments, node.block, trailing_comma?(node.arguments&.location || node.location, node.closing_loc)) + arguments, _, _ = visit_call_node_arguments(node.arguments, node.block, trailing_comma?(node.arguments&.location || node.location, node.closing_loc)) bounds(node.location) on_aref_field(receiver, arguments) @@ -3130,7 +3131,7 @@ def visit_string_node(node) # super(foo) # ^^^^^^^^^^ def visit_super_node(node) - arguments, block = visit_call_node_arguments(node.arguments, node.block, trailing_comma?(node.arguments&.location || node.location, node.rparen_loc || node.location)) + arguments, block, has_ripper_block = visit_call_node_arguments(node.arguments, node.block, trailing_comma?(node.arguments&.location || node.location, node.rparen_loc || node.location)) if !node.lparen_loc.nil? bounds(node.lparen_loc) @@ -3140,11 +3141,11 @@ def visit_super_node(node) bounds(node.location) call = on_super(arguments) - if block.nil? - call - else + if has_ripper_block bounds(node.block.location) on_method_add_block(call, block) + else + call end end diff --git a/test/prism/ruby/ripper_test.rb b/test/prism/ruby/ripper_test.rb index 8848c6a8c0..6e9dcee4c9 100644 --- a/test/prism/ruby/ripper_test.rb +++ b/test/prism/ruby/ripper_test.rb @@ -111,10 +111,18 @@ class PrismEvents < Translation::Ripper end class ObjectEvents < Translation::Ripper + OBJECT = BasicObject.new Prism::Translation::Ripper::PARSER_EVENTS.each do |event| - define_method(:"on_#{event}") do |*args| - Object.new - end + define_method(:"on_#{event}") { |*args| OBJECT } + end + end + + Fixture.each_for_current_ruby(except: incorrect) do |fixture| + define_method("#{fixture.test_name}_events") do + source = fixture.read + # Similar to test/ripper/assert_parse_files.rb in CRuby + object_events = ObjectEvents.new(source) + assert_nothing_raised { object_events.parse } end end @@ -160,10 +168,6 @@ def test_internals def assert_ripper_sexp_raw(source) assert_equal Ripper.sexp_raw(source), Prism::Translation::Ripper.sexp_raw(source) - - # Similar to test/ripper/assert_parse_files.rb in CRuby - object_events = ObjectEvents.new(source) - assert_nothing_raised { object_events.parse } end def assert_ripper_lex(source)