* Move expression handling from variable.rb to expression.rb

* Update test suite to validate parity
* Remove parentheses handling
* Split boolean into comparison and logical expressions

.github/workflows/liquid.yml

@@ -14,8 +14,21 @@ jobs:
           - { ruby: 3.0, allowed-failure: false } # minimum supported
           - { ruby: 3.2, allowed-failure: false }
           - { ruby: 3.3, allowed-failure: false }
+          - { ruby: 3.3, allowed-failure: false }
           - { ruby: 3.4, allowed-failure: false } # latest
+          - {
+              ruby: 3.4,
+              allowed-failure: false,
+              rubyopt: "--enable-frozen-string-literal",
+            }
+          - { ruby: 3.4, allowed-failure: false, rubyopt: "--yjit" }
           - { ruby: ruby-head, allowed-failure: false }
+          - {
+              ruby: ruby-head,
+              allowed-failure: false,
+              rubyopt: "--enable-frozen-string-literal",
+            }
+          - { ruby: ruby-head, allowed-failure: false, rubyopt: "--yjit" }
     name: Test Ruby ${{ matrix.entry.ruby }}
     steps:
       - uses: actions/checkout@v3
@@ -26,6 +39,8 @@ jobs:
           bundler: latest
       - run: bundle exec rake
         continue-on-error: ${{ matrix.entry.allowed-failure }}
+        env:
+          RUBYOPT: ${{ matrix.entry.rubyopt }}
 
   memory_profile:
     runs-on: ubuntu-latest

History.md

@@ -2,6 +2,10 @@
 
 ## 5.8.1 (unreleased)
 
+## 5.8.1
+
+* Fix `{% doc %}` tag to be visitable [Guilherme Carreiro]
+
 ## 5.8.0
 
 * Introduce the new `{% doc %}` tag [Guilherme Carreiro]

lib/liquid.rb

@@ -80,6 +80,8 @@ require 'liquid/variable_lookup'
 require 'liquid/range_lookup'
 require 'liquid/resource_limits'
 require 'liquid/expression'
+require 'liquid/expression/comparison_expression'
+require 'liquid/expression/logical_expression'
 require 'liquid/template'
 require 'liquid/condition'
 require 'liquid/utils'

lib/liquid/expression.rb

@@ -51,6 +51,9 @@ module Liquid
       end
 
       def inner_parse(markup, ss, cache)
+        return LogicalExpression.parse(markup, ss, cache)    if LogicalExpression.logical?(markup)
+        return ComparisonExpression.parse(markup, ss, cache) if ComparisonExpression.comparison?(markup)
+
         if (markup.start_with?("(") && markup.end_with?(")")) && markup =~ RANGES_REGEX
           return RangeLookup.parse(
             Regexp.last_match(1),

lib/liquid/expression/comparison_expression.rb

@@ -0,0 +1,29 @@
+# frozen_string_literal: true
+
+module Liquid
+  class Expression
+    class ComparisonExpression
+      COMPARISON_REGEX = /\A\s*(.+?)\s*(==|!=|<>|<=|>=|<|>|contains)\s*(.+)\s*\z/
+
+      class << self
+        def comparison?(markup)
+          markup =~ COMPARISON_REGEX
+        end
+
+        def parse(markup, ss, cache)
+          match = markup.match(COMPARISON_REGEX)
+
+          if match
+            left = Expression.parse(match[1].strip, ss, cache)
+            operator = match[2].strip
+            right = Expression.parse(match[3].strip, ss, cache)
+
+            return Condition.new(left, operator, right)
+          end
+
+          Condition.new(parse(markup, ss, cache), nil, nil)
+        end
+      end
+    end
+  end
+end

lib/liquid/expression/logical_expression.rb

@@ -0,0 +1,56 @@
+# frozen_string_literal: true
+
+module Liquid
+  class Expression
+    class LogicalExpression
+      LOGICAL_REGEX = /\A\s*(.+?)\s+(and|or)\s+(.+)\s*\z/i
+      EXPRESSIONS_AND_OPERATORS = /(?:\b(?:\s?and\s?|\s?or\s?)\b|(?:\s*(?!\b(?:\s?and\s?|\s?or\s?)\b)(?:#{QuotedFragment}|\S+)\s*)+)/o
+      BOOLEAN_OPERATORS = ['and', 'or'].freeze
+
+      class << self
+        def logical?(markup)
+          markup =~ LOGICAL_REGEX
+        end
+
+        def parse(markup, ss, cache)
+          expressions = markup.scan(EXPRESSIONS_AND_OPERATORS)
+
+          last_expr = expressions.pop
+
+          condition = if ComparisonExpression.comparison?(last_expr)
+            ComparisonExpression.parse(last_expr, ss, cache)
+          elsif LOGICAL_REGEX.match?(last_expr)
+            LogicalExpression.parse(last_expr, ss, cache)
+          else
+            Condition.new(Expression.parse(last_expr, ss, cache), nil, nil)
+          end
+
+          until expressions.empty?
+            operator = expressions.pop.to_s.strip
+            next unless BOOLEAN_OPERATORS.include?(operator)
+
+            expr = expressions.pop.to_s.strip
+
+            new_condition = if ComparisonExpression.comparison?(expr)
+              ComparisonExpression.parse(expr, ss, cache)
+            elsif LOGICAL_REGEX.match?(expr)
+              LogicalExpression.parse(expr, ss, cache)
+            else
+              Condition.new(Expression.parse(expr, ss, cache), nil, nil)
+            end
+
+            if operator == 'and'
+              new_condition.and(condition)
+            else # operator == 'or'
+              new_condition.or(condition)
+            end
+
+            condition = new_condition
+          end
+
+          condition
+        end
+      end
+    end
+  end
+end

lib/liquid/lexer.rb

@@ -14,6 +14,8 @@ module Liquid
     COMPARISON_LESS_THAN = [:comparison, "<"].freeze
     COMPARISON_LESS_THAN_OR_EQUAL = [:comparison, "<="].freeze
     COMPARISON_NOT_EQUAL_ALT = [:comparison, "<>"].freeze
+    BOOLEAN_AND = [:boolean_operator, "and"].freeze
+    BOOLEAN_OR = [:boolean_operator, "or"].freeze
     DASH = [:dash, "-"].freeze
     DOT = [:dot, "."].freeze
     DOTDOT = [:dotdot, ".."].freeze
@@ -151,6 +153,10 @@ module Liquid
               # Special case for "contains"
               output << if type == :id && t == "contains" && output.last&.first != :dot
                 COMPARISON_CONTAINS
+              elsif type == :id && t == "and" && output.last&.first != :dot
+                BOOLEAN_AND
+              elsif type == :id && t == "or" && output.last&.first != :dot
+                BOOLEAN_OR
               else
                 [type, t]
               end

lib/liquid/parser.rb

@@ -48,7 +48,7 @@ module Liquid
 
     def expression
       token = @tokens[@p]
-      case token[0]
+      expr = case token[0]
       when :id
         str = consume
         str << variable_lookups
@@ -69,6 +69,21 @@ module Liquid
       else
         raise SyntaxError, "#{token} is not a valid expression"
       end
+      if look(:comparison)
+        operator = consume(:comparison)
+        left     = expr
+        right    = expression
+
+        "#{left} #{operator} #{right}"
+      elsif look(:boolean_operator)
+        operator = consume(:boolean_operator)
+        left     = expr
+        right    = expression
+
+        "#{left} #{operator} #{right}"
+      else
+        expr
+      end
     end
 
     def argument

lib/liquid/tags/doc.rb

@@ -55,6 +55,10 @@ module Liquid
       true
     end
 
+    def nodelist
+      []
+    end
+
     private
 
     def ensure_valid_markup(tag_name, markup, parse_context)

lib/liquid/version.rb

@@ -2,5 +2,5 @@
 # frozen_string_literal: true
 
 module Liquid
-  VERSION = "5.8.0"
+  VERSION = "5.8.1"
 end

test/unit/boolean_unit_test.rb

@@ -0,0 +1,151 @@
+# frozen_string_literal: true
+
+require 'test_helper'
+
+class BooleanUnitTest < Minitest::Test
+  include Liquid
+
+  def test_simple_boolean_comparison
+    assert_parity("1 > 0", "true")
+    assert_parity("1 < 0", "false")
+  end
+
+  def test_boolean_and_operator
+    assert_parity("true and true", "true")
+    assert_parity("true and false", "false")
+  end
+
+  def test_boolean_or_operator
+    assert_parity("true or false", "true")
+    assert_parity("false or false", "false")
+  end
+
+  def test_operator_precedence
+    assert_parity("false and false or true", "false")
+  end
+
+  def test_complex_boolean_expressions
+    assert_parity("true and true and true", "true")
+    assert_parity("true and false and true", "false")
+    assert_parity("false or false or true", "true")
+  end
+
+  def test_boolean_with_variables
+    assert_parity("a and b", "true", { "a" => true, "b" => true })
+    assert_parity("a and b", "false", { "a" => true, "b" => false })
+    assert_parity("a or b", "true", { "a" => false, "b" => true })
+    assert_parity("a or b", "false", { "a" => false, "b" => false })
+  end
+
+  def test_nil_equals_nil
+    assert_parity("nil == nil", "true")
+  end
+
+  def test_nil_not_equals_nil
+    assert_parity("nil != nil", "false")
+  end
+
+  def test_nil_not_equals_empty_string
+    assert_parity("nil == ''", "false")
+    assert_parity("nil != ''", "true")
+  end
+
+  def test_undefined_variable_in_comparisons
+    assert_parity("undefined_var == nil", "true")
+    assert_parity("undefined_var != nil", "false")
+  end
+
+  def test_undefined_variable_compared_to_empty_string
+    assert_parity("undefined_var == ''", "false")
+    assert_parity("undefined_var != ''", "true")
+  end
+
+  def test_boolean_variable_in_comparisons
+    assert_parity("t == true", "true", { "t" => true })
+    assert_parity("f == false", "true", { "f" => false })
+  end
+
+  def test_boolean_variable_compared_to_nil
+    assert_parity("t == nil", "false", { "t" => true })
+    assert_parity("f == nil", "false", { "f" => false })
+    assert_parity("f != nil", "true", { "f" => false })
+  end
+
+  def test_nil_and_undefined_variables_in_boolean_expressions
+    assert_parity("x == undefined_var", "true", { "x" => nil })
+    assert_parity("x != undefined_var", "false", { "x" => nil })
+  end
+
+  def test_nil_literal_in_or_expression
+    assert_parity("nil or true", "true")
+  end
+
+  def test_nil_variable_in_or_expression
+    assert_parity("x or false", "false", { "x" => nil })
+  end
+
+  def test_mixed_boolean_expressions
+    assert_parity("a > b and c < d", "true",  { "a" => 99, "b" => 0, "c" => 0,  "d" => 99 })
+    assert_parity("a > b and c < d", "false", { "a" => 99, "b" => 0, "c" => 99, "d" => 0  })
+  end
+
+  def test_boolean_assignment_shorthand
+    template = Liquid::Template.parse("{% assign lazy_load = media_position > 1 %}{{ lazy_load }}")
+    assert_equal("false", template.render("media_position" => 1))
+    assert_equal("true",  template.render("media_position" => 2))
+  end
+
+  def test_equality_operators
+    assert_parity_todo!("1 == 1", "true")
+    assert_parity_todo!("1 != 2", "true")
+    assert_parity_todo!("'hello' == 'hello'", "true")
+  end
+
+  def test_nil_renders_as_empty_string
+    assert_parity_todo!("nil", "false")
+  end
+
+  def test_nil_comparison_with_blank
+    assert_parity_todo!("nil_value == blank", "false")
+    assert_parity_todo!("nil_value != blank", "true")
+    assert_parity_todo!("undefined != blank", "true")
+    assert_parity_todo!("undefined == blank", "false")
+  end
+
+  def test_if_with_variables
+    assert_parity_todo!("value", "true",  { "value" => true })
+    assert_parity_todo!("value", "false", { "value" => false })
+    assert_parity_todo!("value", "false", { "value" => nil })
+    assert_parity_todo!("value", "true",  { "value" => "text" })
+    assert_parity_todo!("value", "true", { "value" => "" })
+  end
+
+  def test_nil_variable_in_and_expression
+    assert_parity_todo!("x and true", "false", { "x" => nil })
+  end
+
+  private
+
+  def assert_parity_todo!(liquid_expression, expected_result, args = {})
+    assert_parity_scenario(:condition, "{% if #{liquid_expression} %}true{% else %}false{% endif %}", expected_result, args)
+    test_name = caller_locations(1, 1)[0].label
+    puts "\e[33mTODO: parity for '#{test_name}'\e[0m"
+  end
+
+  def assert_parity(liquid_expression, expected_result, args = {})
+    assert_parity_scenario(:condition, "{% if #{liquid_expression} %}true{% else %}false{% endif %}", expected_result, args)
+    assert_parity_scenario(:expression, "{{ #{liquid_expression} }}", expected_result, args)
+  end
+
+  def assert_parity_scenario(kind, template, exp_output, args = {})
+    act_output = Liquid::Template.parse(template).render(args)
+
+    assert_equal(exp_output, act_output, <<~ERROR_MESSAGE)
+      #{kind.to_s.capitalize} template failure:
+      ---
+      #{template}
+      ---
+      args: #{args.inspect}
+    ERROR_MESSAGE
+  end
+end

test/unit/lexer_unit_test.rb

@@ -141,6 +141,78 @@ class LexerUnitTest < Minitest::Test
     )
   end
 
+  def test_boolean_and_operator
+    exp = [
+      [:id, "true"],
+      [:boolean_operator, "and"],
+      [:id, "false"],
+      [:end_of_string],
+    ]
+    act = tokenize("true and false")
+    assert_equal(exp, act)
+  end
+
+  def test_boolean_or_operator
+    exp = [
+      [:id, "false"],
+      [:boolean_operator, "or"],
+      [:id, "true"],
+      [:end_of_string],
+    ]
+    act = tokenize("false or true")
+    assert_equal(exp, act)
+  end
+
+  def test_boolean_operators_in_complex_expressions
+    exp = [
+      [:id, "a"],
+      [:boolean_operator, "and"],
+      [:id, "b"],
+      [:boolean_operator, "or"],
+      [:id, "c"],
+      [:end_of_string],
+    ]
+    act = tokenize("a and b or c")
+    assert_equal(exp, act)
+  end
+
+  def test_boolean_operators_with_comparisons
+    exp = [
+      [:id, "a"],
+      [:comparison, ">"],
+      [:number, "5"],
+      [:boolean_operator, "and"],
+      [:id, "b"],
+      [:comparison, "<"],
+      [:number, "10"],
+      [:end_of_string],
+    ]
+    act = tokenize("a > 5 and b < 10")
+    assert_equal(exp, act)
+  end
+
+  def test_boolean_operators_as_property_names
+    exp = [
+      [:id, "obj"],
+      [:dot, "."],
+      [:id, "and"],
+      [:dot, "."],
+      [:id, "property"],
+      [:end_of_string],
+    ]
+    act = tokenize("obj.and.property")
+    assert_equal(exp, act)
+
+    exp = [
+      [:id, "obj"],
+      [:dot, "."],
+      [:id, "or"],
+      [:end_of_string],
+    ]
+    act = tokenize("obj.or")
+    assert_equal(exp, act)
+  end
+
   private
 
   def tokenize(input)

test/unit/tags/doc_tag_unit_test.rb

@@ -157,4 +157,28 @@ class DocTagUnitTest < Minitest::Test
     assert_template_result('', "{% doc %}123{% enddoc\nxyz %}")
     assert_template_result('', "{% doc %}123{% enddoc\n   xyz  enddoc %}")
   end
+
+  def test_doc_tag_visitor
+    template_source = '{% doc %}{% enddoc %}'
+
+    assert_equal(
+      [Liquid::Doc],
+      visit(template_source),
+    )
+  end
+
+  private
+
+  def traversal(template)
+    ParseTreeVisitor
+      .for(Template.parse(template).root)
+      .add_callback_for(Liquid::Doc) do |tag|
+        tag_class = tag.class
+        tag_class
+      end
+  end
+
+  def visit(template)
+    traversal(template).visit.flatten.compact
+  end
 end