Merge pull request #88 from reliantsolutions/fix-codegen

Fix codegen

Author: crimson-knight

.github/workflows/ci.yml

@@ -21,10 +21,10 @@ jobs:
         uses: actions/checkout@v3
       - name: Install Crystal
         uses: crystal-lang/install-crystal@v1
+      - name: Check formatting
+        run: crystal tool format --check
       - name: Install shards
         run: shards update
       - name: Run tests
         run: crystal spec --order=random
-      - name: Check formatting
-        run: crystal tool format --check
         if: matrix.os == 'ubuntu-latest'

.gitignore

@@ -7,6 +7,8 @@
 *~
 \#*\#
 
+/spec/integration/codegen-test-*
+
 # Libraries don't need dependency lock
 # Dependencies will be locked in application that uses them
 /shard.lock

spec/integration/integration_spec.cr

@@ -1,6 +1,17 @@
 require "yaml"
 require "../spec_helper"
 
+# [golden-liquid](https://github.com/jg-rp/golden-liquid) is a test suite for
+# liquid template, tests are found in spec/integration/golden_liquid.yaml, a
+# list of tests that are expected to fail can be found at
+# spec/integration/golden_liquid.pending.
+#
+# All golden liquid tests are tagged with `golden`. Tests are run in two modes,
+# using the render visitor directly (tagged with `render`) and using the
+# codegen visitor (tagged with `codegen`), besides a numeric tag for each test.
+#
+# For code gen tests Crystal code is written in files like
+# `spec/integration/codegen-test-XXX.cr` where XXX is the test number.
 class GoldenTest
   include YAML::Serializable
 
@@ -25,6 +36,40 @@ class GoldenTest
     ctx
   end
 
+  private def context_to_code(context : Liquid::Context) : String
+    String.build do |str|
+      str << "Liquid::Context{"
+      context.each do |key, value|
+        key.inspect(str)
+        str << " => " << any_to_code(value)
+        str << ", "
+      end
+      str << "}"
+    end
+  end
+
+  private def any_to_code(any : Liquid::Any) : String
+    raw = any.raw
+    String.build do |str|
+      str << "Liquid::Any.new("
+
+      if raw.is_a?(Array)
+        str << "["
+        raw.each { |item| str << any_to_code(item) << ", " }
+        str << "] of Liquid::Any"
+      elsif raw.is_a?(Hash)
+        str << "{"
+        raw.each do |key, value|
+          str << key.inspect << "=>" << any_to_code(value) << ", "
+        end
+        str << "} of String => Liquid::Any"
+      else
+        raw.inspect(str)
+      end
+      str << ")"
+    end
+  end
+
   def test!
     if @error
       expect_raises(LiquidException) do
@@ -34,6 +79,55 @@ class GoldenTest
       Parser.parse(@template).render(context).should eq(@want)
     end
   end
+
+  def codegen_test!(test_group, test_number)
+    test_path = Path[__DIR__, "codegen-test-#{test_number}.cr"]
+    test = File.open(test_path, "w")
+    test.puts("# #{test_group.name}.#{@name}\n\n")
+    generate_codegen_test_source(test)
+    output = `crystal run #{Process.quote(test.path)} --error-trace`
+    $?.exit_code.should eq(0)
+    output.should eq(@want) unless @error
+  end
+
+  private def generate_codegen_test_source(io) : Nil
+    error_mode = @strict || @error ? Context::ErrorMode::Strict : Context::ErrorMode::Lax
+
+    io.puts(<<-CRYSTAL)
+    require "../../src/liquid"
+
+    TEMPLATE =<<-LIQUID
+    #{Liquid::CodeGenVisitor.escape(@template)}
+    LIQUID
+
+    WANT =<<-TEXT
+    #{Liquid::CodeGenVisitor.escape(@want)}
+    TEXT
+
+    #  CONTEXT
+    expects_error = #{@error}
+    context = #{context_to_code(context)}
+    context.error_mode = :#{error_mode}
+
+    #  CODEGEN OUTPUT
+    CRYSTAL
+
+    tpl = Liquid::Template.parse(@template)
+    visitor = CodeGenVisitor.new(io)
+    tpl.root.accept(visitor)
+    io.puts(<<-CRYSTAL)
+    begin
+      Liquid::Template.new(root).render(context, STDOUT)
+    rescue ex : Liquid::InvalidExpression
+      raise ex unless expects_error
+    end
+    CRYSTAL
+
+  rescue ex : Liquid::LiquidException
+    io << "abort(" << ex.message.inspect << ") unless expects_error\n"
+  ensure
+    io.close
+  end
 end
 
 class GoldenTestGroup
@@ -68,7 +162,6 @@ private def yaml_any_to_liquid_any(yaml : YAML::Any) : Liquid::Any
   end
 end
 
-# FIXME: One all tests pass we must remove this class
 class PendingGold
   @@pending : Array(String)?
 
@@ -84,15 +177,22 @@ end
 describe "Golden Liquid Tests" do
   i = 1
   skip_pending_tests = ENV["SKIP_PENDING"]?
+
   GoldenLiquid.from_yaml(File.read(File.join(__DIR__, "golden_liquid.yaml"))).test_groups.each do |test_group|
-    describe test_group.name do
+    describe test_group.name, tags: "golden" do
       test_group.tests.each do |test|
         if PendingGold.pending?(test_group.name, test.name)
           pending(test.name, line: i) unless skip_pending_tests
         else
-          it test.name, line: i do
+          it "#{test.name} [test-#{i} render]", tags: ["test-#{i}", "render"] do
             test.test!
           end
+
+          i += 1
+          dup_i = i
+          it "#{test.name} [test-#{i} codegen]", tags: ["test-#{i}", "codegen"] do
+            test.codegen_test!(test_group, dup_i)
+          end
         end
         i += 1
       end

src/liquid/blank.cr

@@ -17,5 +17,9 @@ module Liquid
     def ==(other : Any)
       self == other.raw
     end
+
+    def inspect(io : IO)
+      io << "Liquid::Blank.new"
+    end
   end
 end

src/liquid/blocks/block.cr

@@ -23,7 +23,7 @@ module Liquid::Block
       end
     end
 
-    protected def inspect(io : IO)
+    protected def inspect(io : IO, &)
       io << '<'
       io << '-' if lstrip?
       io << ' '

src/liquid/blocks/when.cr

@@ -2,7 +2,7 @@ require "./block"
 
 module Liquid::Block
   class When < InlineBlock
-    @when_expressions : Array(Expression)
+    getter when_expressions : Array(Expression)
 
     def initialize(content : String)
       @when_expressions = Array(Expression).new

src/liquid/codegen_visitor.cr

@@ -41,8 +41,21 @@ module Liquid
       @io << new_var << " = " << some << "\n"
     end
 
-    def escape(some : String)
-      some.gsub '"', "\\\""
+    def escape(text : String) : String
+      CodeGenVisitor.escape(text)
+    end
+
+    def self.escape(text : String) : String
+      text.gsub do |char|
+        case char
+        when '"'  then "\\\""
+        when '\n' then "\\n"
+        when '\r' then "\\r"
+        when '\t' then "\\t"
+        else
+          char
+        end
+      end
     end
 
     def visit(node : Node)
@@ -54,8 +67,7 @@ module Liquid
     end
 
     def visit(node : Assign)
-      to_io %(Liquid::Block::Assign.new("#{escape node.varname}",
-                    Liquid::Block::ExpressionNode.new("#{escape node.value.var}")))
+      to_io %(Liquid::Block::Assign.new("#{escape node.varname}", Liquid::Expression.new("#{escape node.value.expression}")))
     end
 
     def visit(node : Include)
@@ -70,9 +82,7 @@ module Liquid
     end
 
     def visit(node : Case)
-      def_to_io %(Liquid::Block::ExpressionNode.new(
-                   "#{escape node.expression.not_nil!.var}"))
-      def_to_io "Liquid::Block::Case.new(#{@last_var})"
+      def_to_io %(Liquid::Block::Case.new("#{escape node.expression.expression}"))
       push
       node.children.each &.accept self
       if arr = node.when
@@ -84,26 +94,42 @@ module Liquid
       pop
     end
 
-    def visit(node : For)
-      if node.begin && node.end
-        def_to_io %(Liquid::Block::For.new("#{node.loop_var}",
-                                          #{node.begin}, #{node.end}))
-      else
-        def_to_io "Liquid::Block::For.new(\"#{node.loop_var}\", \"#{node.loop_over}\")"
+    def visit(node : When)
+      expressions = node.when_expressions.map do |expression|
+        escape(expression.expression)
       end
+      def_to_io %(Liquid::Block::When.new("#{expressions.join(", ")}"))
+      push
+      node.children.each &.accept(self)
+      pop
+    end
+
+    def visit(node : For)
+      def_to_io %(Liquid::Block::For.new("#{node.loop_var}", #{node.loop_over.inspect}))
       push
       node.children.each &.accept(self)
       pop
     end
 
     def visit(node : ExpressionNode)
-      to_io %(Liquid::Block::ExpressionNode.new("#{escape node.var}"))
+      to_io %(Liquid::Block::ExpressionNode.new("#{escape node.expression}"))
     end
 
     def visit(node : If)
-      def_to_io %(Liquid::Block::ExpressionNode.new(
-                   "#{escape node.expression.not_nil!.var}"))
-      def_to_io "Liquid::Block::If.new(#{@last_var})"
+      def_to_io %(Liquid::Block::If.new("#{escape node.expression.expression}"))
+      push
+      node.children.each &.accept self
+      if arr = node.elsif
+        arr.each &.accept self
+      end
+      if e = node.else
+        e.accept self
+      end
+      pop
+    end
+
+    def visit(node : Unless)
+      def_to_io %(Liquid::Block::Unless.new("#{escape node.expression.expression}"))
       push
       node.children.each &.accept self
       if arr = node.elsif
@@ -116,7 +142,7 @@ module Liquid
     end
 
     def visit(node : ElsIf)
-      def_to_io "Liquid::Block::ElsIf.new( ExpressionNode.new(\"#{escape node.expression.var}\"))"
+      def_to_io %(Liquid::Block::ElsIf.new("#{escape node.expression.expression}"))
       push
       node.children.each &.accept self
       pop

src/liquid/context.cr

@@ -38,6 +38,7 @@ module Liquid
     delegate :strict?, to: @error_mode
     delegate :warn?, to: @error_mode
     delegate :lax?, to: @error_mode
+    delegate :each, to: @data
 
     @[Deprecated("Use `initialize(ErrorMode)` instead.")]
     def initialize(strict : Bool)
@@ -105,6 +106,17 @@ module Liquid
       @data[var] = value
     end
 
+    @[Deprecated("Use `set(String, ::Liquid::Any)`")]
+    def set(var : String, value : Hash(String, T)) : Any forall T
+      mapped_values = value.transform_values { |v| Any.new(v) }
+      set(var, mapped_values)
+    end
+
+    # Sets the value for *var* to the given *value*.
+    def set(var : String, value : Hash(String, Any)) : Any
+      set(var, Any.new(value))
+    end
+
     # Sets the value for *var* to an instance of `Liquid::Any` generated from *value*.
     def set(var : String, value : Any::Type) : Any
       set(var, Any.new(value))

src/liquid/for_loop.cr

@@ -10,7 +10,7 @@ module Liquid
     end
 
     @[Ignore]
-    def each
+    def each(&)
       collection = @collection
       if collection.is_a?(Array) || collection.is_a?(Range)
         collection.each do |val|

src/liquid/template.cr

@@ -49,12 +49,12 @@ module Liquid
 
       unless context
         context = "context"
-        io.puts <<-EOF
-context = Liquid::Context.new
-{% for var in @type.instance_vars %}
-    context.set {{var.id.stringify}}, @{{var.id}}
-{% end %}
-EOF
+        io.puts <<-CRYSTAL
+        context = Liquid::Context.new
+        {% for var in @type.instance_vars %}
+            context.set({{var.id.stringify}}, @{{var.id}})
+        {% end %}
+        CRYSTAL
       end
 
       root.accept visitor