Merge pull request #3867 from eregon/check-if-array-before-calling-any

Check using Prism nodes if a command call has any arguments in Ripper translator

Author: eregon

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,9 +1110,9 @@ 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? && 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?
@@ -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,27 +1169,35 @@ 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
       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)
@@ -1203,7 +1211,8 @@ def visit_call_node(node)
               on_args_add_block(args, false)
             end
           end,
-          visit(block)
+          visit(block),
+          block != nil,
         ]
       end
 
@@ -1640,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
 
@@ -2041,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)
@@ -2058,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)
@@ -2075,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)
@@ -2092,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)
@@ -3122,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)
@@ -3132,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
 

test/prism/ruby/ripper_test.rb

@@ -110,6 +110,22 @@ class PrismEvents < Translation::Ripper
       include Events
     end
 
+    class ObjectEvents < Translation::Ripper
+      OBJECT = BasicObject.new
+      Prism::Translation::Ripper::PARSER_EVENTS.each do |event|
+        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
+
     def test_events
       source = "1 rescue 2"
       ripper = RipperEvents.new(source)