bump version to 5.6.1

Bump version patch

.github/workflows/liquid.yml

@@ -13,7 +13,8 @@ jobs:
         entry:
           - { ruby: 3.0, allowed-failure: false } # minimum supported
           - { ruby: 3.2, allowed-failure: false }
-          - { ruby: 3.3, allowed-failure: false } # latest
+          - { ruby: 3.3, allowed-failure: false }
+          - { ruby: "3.4.0-rc1", allowed-failure: false } # latest
           - { ruby: ruby-head, allowed-failure: false }
     name: Test Ruby ${{ matrix.entry.ruby }}
     steps:

.ruby-version

@@ -1 +1 @@
-3.3.4
+3.3.6

History.md

@@ -1,5 +1,15 @@
 # Liquid Change Log
 
+## 5.6.1 2025-01-07
+
+Add `omit_blank_nodes` parse option to skip blank nodes in the AST (#1870) [Michael Go]
+
+## 5.6.0 (unreleased)
+
+### Fixes
+
+* Fix Tokenizer to handle null source value (#1873) [Bahar Pourazar]
+
 ## 5.5.0 2024-03-21
 
 Please reference the GitHub release for more information.

Rakefile

@@ -81,21 +81,10 @@ namespace :benchmark do
   end
 
   desc "Run unit benchmarks"
-  namespace :unit do
-    desc "Run all unit benchmarks"
-    task :all do
-      Dir["./performance/unit/*_benchmark.rb"].each do |file|
-        puts "🧪 Running #{file}"
-        ruby file
-      end
-    end
-
-    %w[lexer loom].each do |benchmark|
-      desc "Run the #{benchmark} benchmark"
-      task benchmark.to_sym do
-        puts "🧪 Running #{benchmark}"
-        ruby "./performance/unit/#{benchmark}_benchmark.rb"
-      end
+  task :unit do
+    Dir["./performance/unit/*_benchmark.rb"].each do |file|
+      puts "🧪 Running #{file}"
+      ruby file
     end
   end
 end

lib/liquid.rb

@@ -86,4 +86,3 @@ require 'liquid/partial_cache'
 require 'liquid/usage'
 require 'liquid/registers'
 require 'liquid/template_factory'
-require 'liquid/loom'

lib/liquid/block_body.rb

@@ -31,11 +31,10 @@ module Liquid
       end
     end
 
-    # TODO: Freeze the nodelist after optimization
-    # def freeze
-    #   @nodelist.freeze
-    #   super
-    # end
+    def freeze
+      @nodelist.freeze
+      super
+    end
 
     private def parse_for_liquid_tag(tokenizer, parse_context)
       while (token = tokenizer.shift)
@@ -59,6 +58,9 @@ module Liquid
             return yield tag_name, markup
           end
           new_tag = tag.parse(tag_name, markup, tokenizer, parse_context)
+
+          next if parse_context.omit_blank_nodes && blank_node?(new_tag)
+
           @blank &&= new_tag.blank?
           @nodelist << new_tag
         end
@@ -154,13 +156,10 @@ module Liquid
             return yield tag_name, markup
           end
           new_tag = tag.parse(tag_name, markup, tokenizer, parse_context)
+
+          next if parse_context.omit_blank_nodes && blank_node?(new_tag)
+
           @blank &&= new_tag.blank?
-
-          if parse_context.eager_optimize
-            next if new_tag.nodelist&.all? { !_1.is_a?(String) && _1.nodelist.empty? } # this is an empty block
-            next if new_tag.is_a?(If) && new_tag.blocks.empty? # this is an empty If block
-          end
-
           @nodelist << new_tag
         when token.start_with?(VARSTART)
           whitespace_handler(token, parse_context)
@@ -276,5 +275,18 @@ module Liquid
     def raise_missing_variable_terminator(token, parse_context)
       BlockBody.raise_missing_variable_terminator(token, parse_context)
     end
+
+    def blank_node?(node)
+      case node
+      when Comment
+        true
+      when BlockBody
+        true if node.nodelist.empty?
+      when Tag
+        node.nodelist.all? { |n| blank_node?(n) }
+      else
+        false
+      end
+    end
   end
 end

lib/liquid/loom.rb

@@ -1,123 +0,0 @@
-# frozen_string_literal: true
-
-module Liquid
-  class Loom
-    MERGABLE_IF_OPERATORS = ["==", ">", "<", "!="].freeze
-    EQUAL_OP = "==".freeze
-
-    class << self
-      def optimize(template)
-        new(template).optimize
-      end
-    end
-
-    def initialize template
-      @root = template.root
-    end
-
-    def optimize
-      merge_if_blocks
-    end
-
-    def merge_if_blocks
-      nodelist_list = [@root.nodelist]
-
-      while nodelist_list.any?
-        next_nodelist_list = []
-
-        nodelist_list.each do |nodelist|
-          i = 0
-          while i < nodelist.length
-            node = nodelist[i]
-            chain_if_blocks(nodelist, node, i) if node.is_a?(If)
-            i += 1
-          end
-        end
-
-        nodelist_list = next_nodelist_list
-      end
-    end
-
-    private
-
-    def mergable_if_blocks?(target_if, next_if)
-      target_left = target_if.blocks.first.left
-      target_right = target_if.blocks.first.right
-      next_left = next_if.blocks.first.left
-      next_right = next_if.blocks.first.right
-
-      used_variables = Hash.new { |h, k| h[k] = 0 }
-
-      [
-        target_if.blocks.first.left,
-        target_if.blocks.first.right,
-        next_if.blocks.first.left,
-        next_if.blocks.first.right
-      ].each do |var|
-        if var.is_a?(VariableLookup)
-          used_variables[var.name] += 1
-        end
-      end
-
-      return if used_variables.keys.count > 1
-
-      most_used_variable_name = used_variables.keys[0]
-
-      # TODO: I probably can't do this
-      # It might be possible to get different result between a > b and b < a
-      # Move most commonly used variable to the left side
-      if (target_left.is_a?(VariableLookup) && target_left.name != most_used_variable_name) || (target_right.is_a?(VariableLookup) && target_right.name == most_used_variable_name)
-        target_left, target_right = target_right, target_left
-      end
-
-      if (next_left.is_a?(VariableLookup) && next_left.name != most_used_variable_name) || (next_right.is_a?(VariableLookup) && next_right.name == most_used_variable_name)
-        next_left, next_right = next_right, next_left
-      end
-
-      return false unless target_left.is_a?(VariableLookup) && next_left.is_a?(VariableLookup)
-      return false if target_left.name != next_left.name
-
-      return false if target_right.nil? || next_right.nil?
-
-
-      # we need to be conversative here and only can merge ==, >, <, and != operators
-      target_operator = target_if.blocks.first.operator
-      next_operator = next_if.blocks.first.operator
-
-      return false unless MERGABLE_IF_OPERATORS.include?(target_operator) && MERGABLE_IF_OPERATORS.include?(next_operator)
-
-      return false if target_operator == next_operator && target_right == next_right
-
-      return false if target_right.is_a?(VariableLookup) || next_right.is_a?(VariableLookup)
-
-      true
-    end
-
-    def chain_if_blocks(nodelist, first_if_node, first_if_index)
-      used_variables = Set.new
-
-      # only check the top level Condition (ignore children conditions for now)
-      first_if_node.blocks.each do |condition|
-        used_variables << condition.left
-        used_variables << condition.right if condition.right
-      end
-
-      if_blocks = []
-
-      nodelist[first_if_index + 1..-1].each do |node|
-        break unless node.is_a?(If)
-
-        # check if the variables used in the current block are used in the previous block
-        break unless mergable_if_blocks?(first_if_node, node)
-
-        if_blocks << node
-      end
-
-      nodelist.delete_if { |node| if_blocks.include?(node) }
-
-      if_blocks.each do |if_block|
-        first_if_node.blocks << if_block.blocks.first
-      end
-    end
-  end
-end

lib/liquid/parse_context.rb

@@ -3,7 +3,7 @@
 module Liquid
   class ParseContext
     attr_accessor :locale, :line_number, :trim_whitespace, :depth
-    attr_reader :partial, :warnings, :error_mode, :environment, :eager_optimize
+    attr_reader :partial, :warnings, :error_mode, :environment, :omit_blank_nodes
 
     def initialize(options = Const::EMPTY_HASH)
       @environment = options.fetch(:environment, Environment.default)
@@ -11,7 +11,10 @@ module Liquid
 
       @locale   = @template_options[:locale] ||= I18n.new
       @warnings = []
-      @eager_optimize = options.fetch(:eager_optimize, ENV["OPTIMIZE"] == "true")
+
+      # remove blank nodes such as
+      # comment tags, empty if tags, etc from the AST
+      @omit_blank_nodes = options.fetch(:omit_blank_nodes, false)
 
       self.depth   = 0
       self.partial = false

lib/liquid/tags/if.rb

@@ -23,7 +23,6 @@ module Liquid
     def initialize(tag_name, markup, options)
       super
       @blocks = []
-      @has_else_block = false
       push_block('if', markup)
     end
 
@@ -34,44 +33,17 @@ module Liquid
     def parse(tokens)
       while parse_body(@blocks.last.attachment, tokens)
       end
-
-      if parse_context.eager_optimize && definitive_false_statement?
-        @blocks.clear
-      else
-        @blocks.reverse_each do |block|
-          block.attachment.remove_blank_strings if blank?
-          block.attachment.freeze
-        end
+      @blocks.reverse_each do |block|
+        block.attachment.remove_blank_strings if blank?
+        block.attachment.freeze
       end
     end
 
-    def definitive_false_statement?
-      # check if any blocks have variable lookups
-      @blocks.each do |condition|
-        return false if condition.left.is_a?(VariableLookup) || condition.right&.is_a?(VariableLookup)
-
-        child_condition = condition.child_condition
-
-        while child_condition
-          return false if child_condition&.left.is_a?(VariableLookup) || child_condition&.right&.is_a?(VariableLookup)
-          child_condition = child_condition.child_condition
-        end
-      end
-
-      # check if all blocks are false
-      @blocks.each do |condition|
-        return false if condition.evaluate
-      end
-
-      true
-    end
-
     ELSE_TAG_NAMES = ['elsif', 'else'].freeze
     private_constant :ELSE_TAG_NAMES
 
     def unknown_tag(tag, markup, tokens)
       if ELSE_TAG_NAMES.include?(tag)
-        @has_else_block = true
         push_block(tag, markup)
       else
         super

lib/liquid/template.rb

@@ -82,11 +82,7 @@ module Liquid
       # See Liquid::Profiler for more information
       def parse(source, options = {})
         environment = options[:environment] || Environment.default
-        template = new(environment: environment).parse(source, options)
-
-        Loom.optimize(template) if options[:eager_optimize]
-
-        template
+        new(environment: environment).parse(source, options)
       end
     end
 

lib/liquid/tokenizer.rb

@@ -5,7 +5,7 @@ module Liquid
     attr_reader :line_number, :for_liquid_tag
 
     def initialize(source, line_numbers = false, line_number: nil, for_liquid_tag: false)
-      @source         = source
+      @source         = source.to_s.to_str
       @line_number    = line_number || (line_numbers ? 1 : nil)
       @for_liquid_tag = for_liquid_tag
       @offset         = 0

lib/liquid/version.rb

@@ -2,5 +2,5 @@
 # frozen_string_literal: true
 
 module Liquid
-  VERSION = "5.6.0.rc2"
+  VERSION = "5.6.1"
 end

performance/tests/tribble/404.liquid

@@ -1,4 +1,3 @@
-  <div id="page" class="innerpage clearfix">
 
     <div id="text-page">
       <div class="entry">

performance/unit/loom_benchmark.rb

@@ -1,56 +0,0 @@
-# frozen_string_literal: true
-
-require "benchmark/ips"
-require 'liquid'
-
-RubyVM::YJIT.enable
-
-TEMPLATE = <<~LIQUID
-{% if false %}
-  {% for i in (1..1000000) %}
-    {{ "Hello world!" }}
-  {% endfor %}
-{% endif %}
-
-{% assign result = 1 %}
-{% if foo == 1 %}{% assign result = 1 %}{% endif %}{% if foo == 2 %}{% assign result = 2 %}{% endif %}{% if foo == 3 %}{% assign result = 3 %}{% endif %}
-Result: {{ result }}
-LIQUID
-
-baseline_template = Liquid::Template.parse(TEMPLATE, eager_optimize: false)
-optimized_template = Liquid::Template.parse(TEMPLATE, eager_optimize: true)
-
-[nil, 1, 2, 3].each do |foo|
-  baseline_output = baseline_template.render('foo' => foo)
-  optimized_output = optimized_template.render('foo' => foo)
-
-  if baseline_output != optimized_output
-    puts "WARNING! Baseline and optimized templates render differently for foo=#{foo}"
-    puts "Baseline: #{baseline_output}"
-    puts "Optimized: #{optimized_output}"
-
-    raise
-  end
-end
-
-def render(template, foo)
-  template.render('foo' => foo)
-end
-
-Benchmark.ips do |x|
-  x.config(time: 20, warmup: 3)
-
-  x.report("baseline") do
-    [nil, 1, 2, 3].each do |foo|
-      render(baseline_template, foo)
-    end
-  end
-
-  x.report("optimized") do
-    [nil, 1, 2, 3].each do |foo|
-      render(optimized_template, foo)
-    end
-  end
-
-  x.compare!
-end

test/unit/block_unit_test.rb

@@ -53,8 +53,103 @@ class BlockUnitTest < Minitest::Test
     assert_equal(3, template.root.nodelist.size)
   end
 
+  def test_remove_empty_for_blocks_with_optimization_option
+    source = <<~LIQUID.chomp
+      {% for i in (1..1000000) %}
+      {% endfor %}
+    LIQUID
+
+    assert_root_nodelist_size(source, 0, omit_blank_nodes: true)
+
+    source = <<~LIQUID.chomp
+      {% for i in (1..1000000) %}
+      {% else %}
+      {% endfor %}
+    LIQUID
+
+    assert_root_nodelist_size(source, 0, omit_blank_nodes: true)
+
+    source = <<~LIQUID.chomp
+      {% for i in list %}
+        i
+      {% endfor %}
+    LIQUID
+
+    assert_root_nodelist_size(source, 1, omit_blank_nodes: true)
+
+    source = <<~LIQUID.chomp
+      {% for i in list %}
+      {% else %}
+        1
+      {% endfor %}
+    LIQUID
+
+    assert_root_nodelist_size(source, 1, omit_blank_nodes: true)
+  end
+
+  def test_remove_comment_nodes_with_optimization_option
+    source = <<~LIQUID.chomp
+      {% comment %}
+        {% if true %}
+        {% endif %}
+      {% endcomment %}
+    LIQUID
+
+    assert_root_nodelist_size(source, 0, omit_blank_nodes: true)
+
+    source = <<~LIQUID.chomp
+      {% liquid
+        comment
+          if true
+          endif
+        endcomment
+      %}
+    LIQUID
+
+    assert_root_nodelist_size(source, 0, omit_blank_nodes: true)
+  end
+
+  def test_remove_if_nodes_with_optimization_option
+    source = <<~LIQUID.chomp
+      {% if true %}
+      {% endif %}
+    LIQUID
+
+    assert_root_nodelist_size(source, 0, omit_blank_nodes: true)
+
+    source = <<~LIQUID.chomp
+      {% unless true %}
+      {% endunless %}
+    LIQUID
+
+    assert_root_nodelist_size(source, 0, omit_blank_nodes: true)
+
+    source = <<~LIQUID.chomp
+      {% if false %}
+      {% else %}
+      {% endif %}
+    LIQUID
+
+    assert_root_nodelist_size(source, 0, omit_blank_nodes: true)
+
+    source = <<~LIQUID.chomp
+      {% if false %}
+      {% else %}
+        Hello!
+      {% endif %}
+    LIQUID
+
+    assert_root_nodelist_size(source, 1, omit_blank_nodes: true)
+  end
+
   private
 
+  def assert_root_nodelist_size(source, expected_size, parse_options = {})
+    template = Liquid::Template.parse(source, parse_options)
+
+    assert_equal(expected_size, template.root.nodelist.size)
+  end
+
   def block_types(nodelist)
     nodelist.collect(&:class)
   end

test/unit/eager_optimize_test.rb

@@ -1,186 +0,0 @@
-# frozen_string_literal: true
-
-require 'test_helper'
-
-class EagerOptimizeTest < Minitest::Test
-  include Liquid
-
-  def test_remove_empty_blocks
-    source = <<~LIQUID.gsub(/\n/, '')
-      {% for i in (1..1000000) %}
-      {% endfor %}
-    LIQUID
-
-    template = Liquid::Template.parse(source, eager_optimize: true)
-    assert_equal(0, total_node_count(template))
-  end
-
-  def test_remove_false_if_block
-    source = <<~LIQUID.gsub(/\n/, '')
-      {% if false %}
-        {% if true %}
-          {% if true %}
-            {% if true %}
-              {{ "Hello world!" }}
-            {% endif %}
-          {% endif %}
-        {% endif %}
-      {% endif %}
-    LIQUID
-
-    template = Liquid::Template.parse(source, eager_optimize: true)
-    assert_equal(0, total_node_count(template))
-
-    source = <<~LIQUID.gsub(/\n/, '')
-      {% if false %}
-        {% for i in (1..1000000) %}
-          {{ "Hello world!" }}
-        {% endfor %}
-      {% endif %}
-    LIQUID
-
-    template = Liquid::Template.parse(source, eager_optimize: true)
-    assert_equal(0, total_node_count(template))
-  end
-
-  def test_remove_multiple_false_if_block
-    source = <<~LIQUID.gsub(/\n/, '')
-      {% if false %}
-        {% if true %}
-          {% if true %}
-            {% if true %}
-              {{ "Hello world!" }}
-            {% endif %}
-          {% endif %}
-        {% endif %}
-      {% endif %}
-    LIQUID
-
-    template = Liquid::Template.parse(source, eager_optimize: true)
-    assert_equal(0, total_node_count(template))
-  end
-
-  def test_merge_if_blocks
-    # for now, work with consecutive if blocks without any String nodes in between
-    source = <<~LIQUID.gsub(/\n/, '')
-      {% if foo == 1 %}
-        foo: {{ foo }}
-      {% endif %}
-      {% if foo == 2 %}
-        foo: {{ foo }}
-      {% endif %}
-      {% if foo == 3 %}
-        foo: {{ foo }}
-      {% endif %}
-    LIQUID
-
-    assert_optimization([Liquid::If], source, { "foo" => nil })
-    assert_optimization([Liquid::If], source, { "foo" => 1 })
-    assert_optimization([Liquid::If], source, { "foo" => 2 })
-    assert_optimization([Liquid::If], source, { "foo" => 5 })
-
-    source = <<~LIQUID.gsub(/\n/, '')
-      {% assign bar = "application" %}
-      {% if foo == 1 %}
-        foo: {{ foo }}
-      {% endif %}
-      {% if foo == 2 and bar contains "app" %}
-        foo: {{ foo }}
-      {% endif %}
-      {% if 3 == foo and bar == "application" %}
-        foo: {{ foo }}
-      {% endif %}
-    LIQUID
-
-    assert_optimization([Liquid::Assign, Liquid::If], source)
-  end
-
-  def test_does_not_merge_if_blocks
-    assert_optimization([Liquid::If, Liquid::If], <<~LIQUID.gsub(/\n/, ''))
-      {% if foo == 1 %}
-        foo: {{ foo }}
-      {% endif %}
-      {% if k == 1 %}
-        foo: {{ foo }}
-      {% endif %}
-    LIQUID
-
-    assert_optimization([Liquid::If, Liquid::If], <<~LIQUID.gsub(/\n/, ''))
-      {% if foo == 1 %}
-        foo: {{ foo }}
-      {% endif %}
-      {% if foo == 1 %}
-        foo: {{ foo }}
-      {% endif %}
-    LIQUID
-
-    assert_optimization([Liquid::If, Liquid::If], <<~LIQUID.gsub(/\n/, ''))
-      {% if foo == 1 %}
-        foo: {{ foo }}
-      {% endif %}
-      {% if a == foo %}
-        foo: {{ foo }}
-      {% endif %}
-    LIQUID
-
-    assert_optimization([Liquid::If, Liquid::If], <<~LIQUID.gsub(/\n/, ''))
-      {% if foo %}
-        foo: {{ foo }}
-      {% endif %}
-      {% if foo %}
-        foo: {{ foo }}
-      {% endif %}
-    LIQUID
-
-    assert_optimization([Liquid::If, Liquid::If], <<~LIQUID.gsub(/\n/, ''))
-      {% if foo == 1 %}
-        foo: {{ foo }}
-      {% endif %}
-      {% if foo >= 1 %}
-        foo: {{ foo }}
-      {% endif %}
-    LIQUID
-
-    assert_optimization([Liquid::If, Liquid::If], <<~LIQUID.gsub(/\n/, ''))
-      {% if foo == 1 %}
-        foo: {{ foo }}
-      {% endif %}
-      {% if 1  %}
-        foo: {{ foo }}
-      {% endif %}
-    LIQUID
-  end
-
-  private
-
-  def assert_optimization(expected, source, context = { "foo" => 1 })
-    template = Template.parse(source, eager_optimize: true)
-    assert_equal(expected, template.root.nodelist.map(&:class),)
-
-    baseline_template = Template.parse(source, eager_optimize: false)
-
-    assert_equal(
-      baseline_template.render(context),
-      template.render(context),
-    )
-  end
-
-  def total_node_count(template)
-    root = template.root
-    children = root.nodelist
-    count = 0
-
-    while children.any?
-      next_children = []
-
-      children.each do |node|
-        count += 1 unless node.is_a?(Liquid::BlockBody)
-        next_children.concat(node.nodelist) if node.respond_to?(:nodelist) && node.nodelist
-      end
-
-      children = next_children
-    end
-
-    count
-  end
-end

test/unit/template_unit_test.rb

@@ -7,13 +7,13 @@ class TemplateUnitTest < Minitest::Test
 
   def test_sets_default_localization_in_document
     t = Template.new
-    t.parse('{%comment%}{%endcomment%}')
+    t.parse('{%raw%}{%endraw%}')
     assert_instance_of(I18n, t.root.nodelist[0].options[:locale])
   end
 
   def test_sets_default_localization_in_context_with_quick_initialization
     t = Template.new
-    t.parse('{%comment%}{%endcomment%}', locale: I18n.new(fixture("en_locale.yml")))
+    t.parse('{%raw%}{%endraw%}', locale: I18n.new(fixture("en_locale.yml")))
 
     locale = t.root.nodelist[0].options[:locale]
     assert_instance_of(I18n, locale)

test/unit/tokenizer_unit_test.rb

@@ -30,6 +30,10 @@ class TokenizerTest < Minitest::Test
     assert_equal([1, 1, 3], tokenize_line_numbers(" {{\n funk \n}} "))
   end
 
+  def test_tokenize_with_nil_source_returns_empty_array
+    assert_equal([], tokenize(nil))
+  end
+
   private
 
   def new_tokenizer(source, parse_context: Liquid::ParseContext.new, start_line_number: nil)