Simplify ripper tests in ripper translation

Author: kddnewton

lib/prism/translation/ripper.rb

@@ -2494,10 +2494,10 @@ def visit_string_node(node)
 
         parts.each do |part|
           if part.is_a?(StringNode)
-            if dedent_next
+            if dedent_next && !(content = part.content).chomp.empty?
               common_whitespace = [
                 common_whitespace || Float::INFINITY,
-                part.content[/\A\s*/].each_char.inject(0) do |part_whitespace, char|
+                content[/\A\s*/].each_char.inject(0) do |part_whitespace, char|
                   char == "\t" ? ((part_whitespace / 8 + 1) * 8) : (part_whitespace + 1)
                 end
               ].min
@@ -2509,7 +2509,7 @@ def visit_string_node(node)
           end
         end
 
-        common_whitespace
+        common_whitespace || 0
       end
 
       # Take the content of a string and return the index of the first character
@@ -2539,44 +2539,49 @@ def visit_string_node(node)
         if common_whitespace == 0
           bounds(parts.first.location)
 
-          previous_string = []
-          previous_result = base
+          string = []
+          result = base
 
           parts.each do |part|
             if part.is_a?(StringNode)
-              if previous_string.empty?
-                previous_string = [part]
+              if string.empty?
+                string = [part]
               else
-                previous_string << part
+                string << part
               end
             else
-              unless previous_string.empty?
-                bounds(previous_string[0].location)
-                previous_result = yield(previous_result, on_tstring_content(previous_string.map(&:content).join))
-                previous_string = []
+              unless string.empty?
+                bounds(string[0].location)
+                result = yield(result, on_tstring_content(string.map(&:content).join))
+                string = []
               end
 
-              previous_result = yield(previous_result, visit(part))
+              result = yield(result, visit(part))
             end
           end
 
-          unless previous_string.empty?
-            bounds(previous_string[0].location)
-            previous_result = yield(previous_result, on_tstring_content(previous_string.map(&:content).join))
+          unless string.empty?
+            bounds(string[0].location)
+            result = yield(result, on_tstring_content(string.map(&:content).join))
           end
 
-          previous_result
+          result
         else
           bounds(parts.first.location)
-          parts.inject(base) do |string_content, part|
+          parts.each.with_index(parts.length).inject(base) do |string_content, (part, index)|
             yield(
               string_content,
               if part.is_a?(StringNode)
-                content = part.content
-                trimmed_whitespace = heredoc_trimmed_whitespace(content, common_whitespace)
+                if index % 2 == 1
+                  content = part.content
+                  trimmed_whitespace = heredoc_trimmed_whitespace(content, common_whitespace)
 
-                bounds(part.content_loc.copy(start_offset: part.content_loc.start_offset + trimmed_whitespace))
-                on_tstring_content(part.content[trimmed_whitespace..])
+                  bounds(part.content_loc.copy(start_offset: part.content_loc.start_offset + trimmed_whitespace))
+                  on_tstring_content(content[trimmed_whitespace..])
+                else
+                  bounds(part.content_loc)
+                  on_tstring_content(part.content)
+                end
               else
                 visit(part)
               end

test/prism/ripper_test.rb

@@ -2,9 +2,6 @@
 
 require_relative "test_helper"
 
-File.delete("passing.txt") if File.exist?("passing.txt")
-File.delete("failing.txt") if File.exist?("failing.txt")
-
 module Prism
   class RipperTest < TestCase
     base = File.join(__dir__, "fixtures")
@@ -29,43 +26,36 @@ class RipperTest < TestCase
       "whitequark/lvar_injecting_match.txt"
     ]
 
-    heredocs = %w[
-      dos_endings.txt
-      heredocs_with_ignored_newlines.txt
-      seattlerb/heredoc__backslash_dos_format.txt
-      seattlerb/heredoc_backslash_nl.txt
-      seattlerb/heredoc_nested.txt
-      seattlerb/heredoc_squiggly.txt
-      seattlerb/heredoc_squiggly_blank_line_plus_interpolation.txt
-      seattlerb/heredoc_squiggly_blank_lines.txt
-      seattlerb/heredoc_squiggly_interp.txt
-      seattlerb/heredoc_squiggly_no_indent.txt
-      seattlerb/heredoc_squiggly_tabs.txt
-      seattlerb/heredoc_squiggly_tabs_extra.txt
-      seattlerb/heredoc_squiggly_visually_blank_lines.txt
-      spanning_heredoc.txt
-      tilde_heredocs.txt
-      unparser/corpus/semantic/dstr.txt
-      whitequark/dedenting_heredoc.txt
-      whitequark/dedenting_interpolating_heredoc_fake_line_continuation.txt
-      whitequark/dedenting_non_interpolating_heredoc_line_continuation.txt
-      whitequark/parser_bug_640.txt
-      whitequark/parser_drops_truncated_parts_of_squiggly_heredoc.txt
-      whitequark/parser_slash_slash_n_escaping_in_literals.txt
-      whitequark/slash_newline_in_heredocs.txt
-    ]
-
-    skips = incorrect | heredocs | %w[
-      seattlerb/block_call_dot_op2_brace_block.txt
-      seattlerb/block_command_operation_colon.txt
-      seattlerb/block_command_operation_dot.txt
-      whitequark/send_block_chain_cmd.txt
+    omitted = [
+      "dos_endings.txt",
+      "heredocs_with_ignored_newlines.txt",
+      "seattlerb/heredoc__backslash_dos_format.txt",
+      "seattlerb/heredoc_backslash_nl.txt",
+      "seattlerb/heredoc_nested.txt",
+      "seattlerb/heredoc_squiggly_blank_line_plus_interpolation.txt",
+      "seattlerb/heredoc_squiggly_no_indent.txt",
+      "spanning_heredoc.txt",
+      "tilde_heredocs.txt",
+      "unparser/corpus/semantic/dstr.txt",
+      "whitequark/dedenting_heredoc.txt",
+      "whitequark/parser_bug_640.txt",
+      "whitequark/parser_drops_truncated_parts_of_squiggly_heredoc.txt",
+      "whitequark/parser_slash_slash_n_escaping_in_literals.txt",
+      "whitequark/slash_newline_in_heredocs.txt",
+
+      "seattlerb/block_call_dot_op2_brace_block.txt",
+      "seattlerb/block_command_operation_colon.txt",
+      "seattlerb/block_command_operation_dot.txt",
+      "whitequark/send_block_chain_cmd.txt"
     ]
 
     relatives.each do |relative|
+      # Skip the tests that Ripper is reporting the wrong results for.
+      next if incorrect.include?(relative)
       filepath = File.join(__dir__, "fixtures", relative)
 
       define_method "test_ripper_#{relative}" do
+        omit("Not yet implemented") if omitted.include?(relative)
         source = File.read(filepath, binmode: true, external_encoding: Encoding::UTF_8)
 
         case relative
@@ -78,23 +68,14 @@ class RipperTest < TestCase
           source = "def __invalid_yield__\n#{source}\nend"
         end
 
-        assert_ripper(source, filepath, skips.include?(relative))
+        assert_ripper(source)
       end
     end
 
     private
 
-    def assert_ripper(source, filepath, allowed_failure)
-      expected = Ripper.sexp_raw(source)
-
-      begin
-        assert_equal expected, Prism::Translation::Ripper.sexp_raw(source)
-      rescue Exception, NoMethodError
-        File.open("failing.txt", "a") { |f| f.puts filepath }
-        raise unless allowed_failure
-      else
-        File.open("passing.txt", "a") { |f| f.puts filepath } if allowed_failure
-      end
+    def assert_ripper(source)
+      assert_equal Ripper.sexp_raw(source), Prism::Translation::Ripper.sexp_raw(source)
     end
   end
 end