WIP3

.github/workflows/liquid.yml

@@ -56,11 +56,7 @@ jobs:
           bundler-cache: true
           bundler: latest
       - name: Run liquid-spec for all adapters
-        run: |
-          for adapter in spec/*.rb; do
-            echo "=== Running $adapter ==="
-            bundle exec liquid-spec run "$adapter" --no-max-failures
-          done
+        run: bin/liquid-spec-all-adapters
 
   memory_profile:
     runs-on: ubuntu-latest

History.md

@@ -3,21 +3,22 @@
 ## 6.0.0
 
 ### Features
-* (TODO) Add support for boolean expressions everywhere
+* Add support for boolean expressions everywhere
   * As variable output `{{ a or b }}`
   * As filter argument `{{ collection | where: 'prop', a or b }}`
   * As tag argument `{% render 'snip', enabled: a or b %}`
   * As conditional tag argument `{% if cond %}` (extending previous behaviour)
-* (TODO) Add support for subexpression prioritization and associativity
+* Add support for subexpression prioritization and associativity
   * In ascending order of priority:
     * Logical: `and`, `or` (right to left)
     * Equality: `==`, `!=`, `<>` (left to right)
     * Comparison: `>`, `>=`, `<`, `<=`, `contains` (left to right)
+    * Groupings: `( expr )`
   - For example, this is now supported
     * `{{ a > b == c < d or e == f }}` which is equivalent to
     * `{{ ((a > b) == (c < d)) or (e == f) }}`
-- (TODO) Add support for parenthesized expressions
-  * e.g. `(a or b) and c`
+- Add support for parenthesized expressions
+  * e.g. `(a or b) == c`
 
 ### Architectural changes
 * `parse_expression` and `safe_parse_expression` have been removed from `Tag` and `ParseContext`
@@ -32,10 +33,17 @@
   * `:lax` and `lax_parse` is no longer supported
   * `:strict` and `strict_parse` is no longer supported
   * `strict2_parse` is renamed to `parse_markup`
-* The `warnings` system has been removed.
-* `Parser#expression` is renamed to `Parser#expression_string`
-* `safe_parse_expression` methods are replaced by `Parser#expression`
-* `parse_expression` methods are replaced by `Parser#unsafe_parse_expression`
+* Expressions
+  * The `warnings` system has been removed.
+  * `Parser#expression` is renamed to `Parser#expression_string`
+  * `safe_parse_expression` methods are replaced by `Parser#expression`
+  * `parse_expression` methods are replaced by `Parser#unsafe_parse_expression`
+* `Condition`
+  * `new(expr)` no longer accepts an `op` or `right`. Logic moved to BinaryExpression.
+  * `Condition#or` and `Condition#and` were replaced by `BinaryExpression`.
+  * `Condition#child_relation` replaced by `BinaryExpression`.
+  * `Condition.operations` was removed.
+  * `Condtion::MethodLiteral` was moved to the `Liquid` namespace
 
 ### Migrating from `^5.11.0`
 - In custom tags that include `ParserSwitching`, rename `strict2_parse` to `parse_markup`

bin/liquid-spec-all-adapters

@@ -0,0 +1,5 @@
+#!/usr/bin/env bash
+for adapter in spec/*.rb; do
+  echo "=== Running $adapter ==="
+  bundle exec liquid-spec run "$adapter" --no-max-failures
+done

lib/liquid.rb

@@ -62,6 +62,8 @@ module Liquid
 require 'liquid/tags'
 require "liquid/environment"
 require 'liquid/lexer'
+require 'liquid/method_literal'
+require 'liquid/binary_expression'
 require 'liquid/parser'
 require 'liquid/i18n'
 require 'liquid/drop'

lib/liquid/binary_expression.rb

@@ -0,0 +1,94 @@
+# frozen_string_literal: true
+
+module Liquid
+  class BinaryExpression
+    attr_reader :operator
+    attr_accessor :left_node, :right_node
+
+    def initialize(left, operator, right)
+      @left_node = left
+      @operator = operator
+      @right_node = right
+    end
+
+    def evaluate(context)
+      left = value(left_node, context)
+
+      # logical relation short circuiting
+      if operator == 'and'
+        return left && value(right_node, context)
+      elsif operator == 'or'
+        return left || value(right_node, context)
+      end
+
+      right = value(right_node, context)
+
+      case operator
+      when '>'
+        left > right if can_compare?(left, right)
+      when '>='
+        left >= right if can_compare?(left, right)
+      when '<'
+        left < right if can_compare?(left, right)
+      when '<='
+        left <= right if can_compare?(left, right)
+      when '=='
+        equal_variables(left, right)
+      when '!=', '<>'
+        !equal_variables(left, right)
+      when 'contains'
+        contains(left, right)
+      else
+        raise(Liquid::ArgumentError, "Unknown operator #{operator}")
+      end
+    rescue ::ArgumentError => e
+      raise Liquid::ArgumentError, e.message
+    end
+
+    def to_s
+      "(#{left_node.inspect} #{operator} #{right_node.inspect})"
+    end
+
+    private
+
+    def value(expr, context)
+      Utils.to_liquid_value(context.evaluate(expr))
+    end
+
+    def can_compare?(left, right)
+      left.respond_to?(operator) && right.respond_to?(operator) && !left.is_a?(Hash) && !right.is_a?(Hash)
+    end
+
+    def contains(left, right)
+      if left && right && left.respond_to?(:include?)
+        right = right.to_s if left.is_a?(String)
+        left.include?(right)
+      else
+        false
+      end
+    rescue Encoding::CompatibilityError
+      # "✅".b.include?("✅") raises Encoding::CompatibilityError despite being materially equal
+      left.b.include?(right.b)
+    end
+
+    def apply_method_literal(node, other)
+      node.apply(other)
+    end
+
+    def equal_variables(left, right)
+      return apply_method_literal(left, right) if left.is_a?(MethodLiteral)
+      return apply_method_literal(right, left) if right.is_a?(MethodLiteral)
+
+      left == right
+    end
+
+    class ParseTreeVisitor < Liquid::ParseTreeVisitor
+      def children
+        [
+          @node.left_node,
+          @node.right_node,
+        ]
+      end
+    end
+  end
+end

lib/liquid/condition.rb

@@ -5,93 +5,19 @@ module Liquid
   #
   # Example:
   #
-  #   c = Condition.new(1, '==', 1)
+  #   c = Condition.new(expr)
   #   c.evaluate #=> true
   #
   class Condition # :nodoc:
-    @@operators = {
-      '==' => ->(cond, left, right) {  cond.send(:equal_variables, left, right) },
-      '!=' => ->(cond, left, right) { !cond.send(:equal_variables, left, right) },
-      '<>' => ->(cond, left, right) { !cond.send(:equal_variables, left, right) },
-      '<' => :<,
-      '>' => :>,
-      '>=' => :>=,
-      '<=' => :<=,
-      'contains' => lambda do |_cond, left, right|
-        if left && right && left.respond_to?(:include?)
-          right = right.to_s if left.is_a?(String)
-          left.include?(right)
-        else
-          false
-        end
-      rescue Encoding::CompatibilityError
-        # "✅".b.include?("✅") raises Encoding::CompatibilityError despite being materially equal
-        left.b.include?(right.b)
-      end,
-    }
+    attr_reader :attachment
+    attr_accessor :left
 
-    class MethodLiteral
-      attr_reader :method_name, :to_s
-
-      def initialize(method_name, to_s)
-        @method_name = method_name
-        @to_s = to_s
-      end
-    end
-
-    @@method_literals = {
-      'blank' => MethodLiteral.new(:blank?, '').freeze,
-      'empty' => MethodLiteral.new(:empty?, '').freeze,
-    }
-
-    def self.operators
-      @@operators
-    end
-
-    def self.parse_expression(parser)
-      markup = parser.expression_string
-      @@method_literals[markup] || parser.unsafe_parse_expression(markup)
-    end
-
-    attr_reader :attachment, :child_condition
-    attr_accessor :left, :operator, :right
-
-    def initialize(left = nil, operator = nil, right = nil)
-      @left     = left
-      @operator = operator
-      @right    = right
-
-      @child_relation  = nil
-      @child_condition = nil
+    def initialize(left = nil)
+      @left = left
     end
 
     def evaluate(context = deprecated_default_context)
-      condition = self
-      result = nil
-      loop do
-        result = interpret_condition(condition.left, condition.right, condition.operator, context)
-
-        case condition.child_relation
-        when :or
-          break if Liquid::Utils.to_liquid_value(result)
-        when :and
-          break unless Liquid::Utils.to_liquid_value(result)
-        else
-          break
-        end
-        condition = condition.child_condition
-      end
-      result
-    end
-
-    def or(condition)
-      @child_relation  = :or
-      @child_condition = condition
-    end
-
-    def and(condition)
-      @child_relation  = :and
-      @child_condition = condition
+      context.evaluate(left)
     end
 
     def attach(attachment)
@@ -112,91 +38,6 @@ def inspect
 
     private
 
-    def equal_variables(left, right)
-      if left.is_a?(MethodLiteral)
-        return call_method_literal(left, right)
-      end
-
-      if right.is_a?(MethodLiteral)
-        return call_method_literal(right, left)
-      end
-
-      left == right
-    end
-
-    def call_method_literal(literal, value)
-      method_name = literal.method_name
-
-      # If the object responds to the method (e.g., ActiveSupport is loaded), use it
-      if value.respond_to?(method_name)
-        value.send(method_name)
-      else
-        # Emulate ActiveSupport's blank?/empty? to make Liquid invariant
-        # to whether ActiveSupport is loaded or not
-        case method_name
-        when :blank?
-          liquid_blank?(value)
-        when :empty?
-          liquid_empty?(value)
-        else
-          false
-        end
-      end
-    end
-
-    # Implement blank? semantics matching ActiveSupport
-    # blank? returns true for nil, false, empty strings, whitespace-only strings,
-    # empty arrays, and empty hashes
-    def liquid_blank?(value)
-      case value
-      when NilClass, FalseClass
-        true
-      when TrueClass, Numeric
-        false
-      when String
-        # Blank if empty or whitespace only (matches ActiveSupport)
-        value.empty? || value.match?(/\A\s*\z/)
-      when Array, Hash
-        value.empty?
-      else
-        # Fall back to empty? if available, otherwise false
-        value.respond_to?(:empty?) ? value.empty? : false
-      end
-    end
-
-    # Implement empty? semantics
-    # Note: nil is NOT empty. empty? checks if a collection has zero elements.
-    def liquid_empty?(value)
-      case value
-      when String, Array, Hash
-        value.empty?
-      else
-        value.respond_to?(:empty?) ? value.empty? : false
-      end
-    end
-
-    def interpret_condition(left, right, op, context)
-      # If the operator is empty this means that the decision statement is just
-      # a single variable. We can just poll this variable from the context and
-      # return this as the result.
-      return context.evaluate(left) if op.nil?
-
-      left  = Liquid::Utils.to_liquid_value(context.evaluate(left))
-      right = Liquid::Utils.to_liquid_value(context.evaluate(right))
-
-      operation = self.class.operators[op] || raise(Liquid::ArgumentError, "Unknown operator #{op}")
-
-      if operation.respond_to?(:call)
-        operation.call(self, left, right)
-      elsif left.respond_to?(operation) && right.respond_to?(operation) && !left.is_a?(Hash) && !right.is_a?(Hash)
-        begin
-          left.send(operation, right)
-        rescue ::ArgumentError => e
-          raise Liquid::ArgumentError, e.message
-        end
-      end
-    end
-
     def deprecated_default_context
       warn("DEPRECATION WARNING: Condition#evaluate without a context argument is deprecated " \
         "and will be removed from Liquid 6.0.0.")
@@ -207,8 +48,6 @@ class ParseTreeVisitor < Liquid::ParseTreeVisitor
       def children
         [
           @node.left,
-          @node.right,
-          @node.child_condition,
           @node.attachment
         ].compact
       end

lib/liquid/expression.rb

@@ -9,8 +9,8 @@ class Expression
       '' => nil,
       'true' => true,
       'false' => false,
-      'blank' => '',
-      'empty' => '',
+      'blank' => MethodLiteral::BLANK,
+      'empty' => MethodLiteral::EMPTY,
     }.freeze
 
     DOT = ".".ord