Engine: Ensure Engine produces valid Ruby in test suite (#1207)

Inspired by
marcoroth/reactionview#78 (comment)

This pull request updates our `assert_compiled_snapshot` assertion
helper to make sure the compiled snapshot is valid Ruby by passing the
engine output to Prism and checking for syntax errors.

Co-authored-by: Julián Pinzón Eslava <julian@buildkite.com>

Author: marcoroth

lib/herb/engine.rb

@@ -76,6 +76,8 @@ def initialize(input, properties = {})
       preamble = properties[:preamble] || "#{@bufvar} = #{bufval};"
       postamble = properties[:postamble] || "#{@bufvar}.to_s\n"
 
+      preamble = "#{preamble}; " unless preamble.empty? || preamble.end_with?(";", " ", "\n")
+
       @src << "# frozen_string_literal: true\n" if @freeze
 
       if properties[:ensure]
@@ -214,17 +216,13 @@ def add_expression(indicator, code)
 
     def add_expression_result(code)
       with_buffer {
-        @src << " << (" << code
-        @src << "\n" if heredoc?(code)
-        @src << comment_aware_newline(code) << ").to_s"
+        @src << " << (" << code << trailing_newline(code) << ").to_s"
       }
     end
 
     def add_expression_result_escaped(code)
       with_buffer {
-        @src << " << " << @escapefunc << "((" << code
-        @src << "\n" if heredoc?(code)
-        @src << comment_aware_newline(code) << "))"
+        @src << " << " << @escapefunc << "((" << code << trailing_newline(code) << "))"
       }
     end
 
@@ -238,18 +236,45 @@ def add_expression_block(indicator, code)
 
     def add_expression_block_result(code)
       with_buffer {
-        @src << " << " << code << comment_aware_newline(code)
+        @src << " << (" << code << trailing_newline(code)
       }
     end
 
     def add_expression_block_result_escaped(code)
       with_buffer {
-        @src << " << " << @escapefunc << "(" << code << comment_aware_newline(code) << ")"
+        @src << " << " << @escapefunc << "((" << code << trailing_newline(code)
       }
     end
 
-    def comment_aware_newline(code)
-      code.include?("#") ? "\n" : ""
+    def add_expression_block_end(code, escaped: false)
+      terminate_expression
+
+      trailing_newline = code.end_with?("\n")
+      code_stripped = code.chomp
+
+      @src.chomp! if @src.end_with?("\n") && code_stripped.start_with?(" ")
+
+      @src << " " << code_stripped
+      @src << (escaped ? "))" : ")")
+
+      @src << if code.include?("#") || trailing_newline
+                "\n"
+              else
+                ";"
+              end
+
+      @buffer_on_stack = false
+    end
+
+    def trailing_newline(code)
+      return "\n" if comment?(code)
+      return "\n" if heredoc?(code)
+
+      ""
+    end
+
+    def comment?(code)
+      code.include?("#")
     end
 
     def heredoc?(code)

lib/herb/engine/compiler.rb

@@ -22,7 +22,7 @@ def initialize(engine, options = {})
       def generate_output
         optimized_tokens = optimize_tokens(@tokens)
 
-        optimized_tokens.each do |type, value, context|
+        optimized_tokens.each do |type, value, context, escaped|
           case type
           when :text
             @engine.send(:add_text, value)
@@ -48,6 +48,8 @@ def generate_output
           when :expr_block_escaped
             indicator = @escape ? "=" : "=="
             @engine.send(:add_expression_block, indicator, value)
+          when :expr_block_end
+            @engine.send(:add_expression_block_end, value, escaped: escaped)
           end
         end
       end
@@ -281,7 +283,7 @@ def visit_erb_block_node(node)
                      end
 
           visit_all(node.body)
-          visit(node.end_node)
+          visit_erb_block_end_node(node.end_node, escaped: should_escape)
         else
           visit_erb_control_node(node) do
             visit_all(node.body)
@@ -290,6 +292,24 @@ def visit_erb_block_node(node)
         end
       end
 
+      def visit_erb_block_end_node(node, escaped: false)
+        has_left_trim = node.tag_opening.value.start_with?("<%-")
+
+        remove_trailing_whitespace_from_last_token! if has_left_trim
+
+        code = node.content.value.strip
+
+        if at_line_start?
+          lspace = extract_and_remove_lspace!
+          rspace = " \n"
+
+          @tokens << [:expr_block_end, "#{lspace}#{code}#{rspace}", current_context, escaped]
+          @trim_next_whitespace = true
+        else
+          @tokens << [:expr_block_end, code, current_context, escaped]
+        end
+      end
+
       def visit_erb_control_with_parts(node, *parts)
         visit_erb_control_node(node) do
           parts.each do |part|
@@ -394,7 +414,7 @@ def optimize_tokens(tokens)
         current_text = ""
         current_context = nil
 
-        compacted.each do |type, value, context|
+        compacted.each do |type, value, context, escaped|
           if type == :text
             current_text += value
             current_context ||= context
@@ -406,7 +426,7 @@ def optimize_tokens(tokens)
               current_context = nil
             end
 
-            optimized << [type, value, context]
+            optimized << [type, value, context, escaped]
           end
         end
 

sig/herb/engine.rbs

@@ -36,6 +36,7 @@ def assert_parsed_snapshot(source, **options)
 
   def assert_compiled_snapshot(source, options = {}, **kwargs)
     require_relative "../lib/herb/engine"
+    require "prism"
 
     enforce_erubi_equality = kwargs.delete(:enforce_erubi_equality) || false
     engine_options = options.merge(kwargs)
@@ -46,6 +47,18 @@ def assert_compiled_snapshot(source, options = {}, **kwargs)
     snapshot_key = { source: source, options: engine_options }.to_s
     assert_snapshot_matches(expected, snapshot_key)
 
+    prism_result = Prism.parse(engine.src)
+    syntax_errors = prism_result.errors.reject { |e| e.type == :invalid_yield }
+
+    assert syntax_errors.empty?, <<~MESSAGE
+      Compiled output is not valid Ruby:
+
+      #{syntax_errors.map { |e| "  - #{e.message} (line #{e.location.start_line})" }.join("\n")}
+
+      Compiled source:
+      #{engine.src}
+    MESSAGE
+
     if should_compare_with_erubi? || enforce_erubi_equality
       compare_with_erubi_compiled(source, engine.src, engine_options, enforce_equality: enforce_erubi_equality)
     end