Remove Condition.op, Condition.right

- Remove comparison expression logic

lib/liquid/condition.rb

@@ -5,51 +5,15 @@ 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,
-    }
-    @@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
+    attr_accessor :left
 
-    def initialize(left = nil, operator = nil, right = nil)
-      @left     = left
-      @operator = operator
-      @right    = right
+    def initialize(left = nil)
+      @left = left
 
       @child_relation  = nil
       @child_condition = nil
@@ -59,7 +23,7 @@ module Liquid
       condition = self
       result = nil
       loop do
-        result = interpret_condition(condition.left, condition.right, condition.operator, context)
+        result = context.evaluate(condition.left)
 
         case condition.child_relation
         when :or
@@ -101,92 +65,6 @@ module Liquid
     attr_reader :child_relation
 
     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.")
@@ -197,7 +75,6 @@ module Liquid
       def children
         [
           @node.left,
-          @node.right,
           @node.child_condition,
           @node.attachment
         ].compact

lib/liquid/tags/case.rb

@@ -99,8 +99,8 @@ module Liquid
       parser = @parse_context.new_parser(markup)
 
       loop do
-        expr = Condition.parse_expression(parser)
-        block = Condition.new(@left, '==', expr)
+        expr = BinaryExpression.new(@left, '==', Condition.parse_expression(parser))
+        block = Condition.new(expr)
         block.attach(body)
         @blocks << block
 

lib/liquid/tags/if.rb

@@ -73,10 +73,6 @@ module Liquid
       block.attach(new_body)
     end
 
-    def parse_expression(parser)
-      Condition.parse_expression(parser)
-    end
-
     def parse_markup(markup)
       p = @parse_context.new_parser(markup)
       condition = parse_binary_comparisons(p)

test/unit/condition_unit_test.rb

@@ -9,11 +9,6 @@ class ConditionUnitTest < Minitest::Test
     @context = Liquid::Context.new
   end
 
-  def test_basic_condition
-    assert_equal(false, Condition.new(1, '==', 2).evaluate(Context.new))
-    assert_equal(true,  Condition.new(1, '==', 1).evaluate(Context.new))
-  end
-
   def test_default_operators_evalute_true
     assert_evaluates_true(1, '==', 1)
     assert_evaluates_true(1, '!=', 2)
@@ -72,11 +67,11 @@ class ConditionUnitTest < Minitest::Test
   end
 
   def test_hash_compare_backwards_compatibility
-    assert_nil(Condition.new({}, '>', 2).evaluate(Context.new))
-    assert_nil(Condition.new(2, '>', {}).evaluate(Context.new))
-    assert_equal(false, Condition.new({}, '==', 2).evaluate(Context.new))
-    assert_equal(true, Condition.new({ 'a' => 1 }, '==', 'a' => 1).evaluate(Context.new))
-    assert_equal(true, Condition.new({ 'a' => 2 }, 'contains', 'a').evaluate(Context.new))
+    assert_evaluates_nil({}, '>', 2)
+    assert_evaluates_nil(2, '>', {})
+    assert_evaluates_false({}, '==', 2)
+    assert_evaluates_true({ 'a' => 1 }, '==', 'a' => 1)
+    assert_evaluates_true({ 'a' => 2 }, 'contains', 'a')
   end
 
   def test_contains_works_on_arrays
@@ -110,29 +105,30 @@ class ConditionUnitTest < Minitest::Test
   end
 
   def test_or_condition
-    condition = Condition.new(1, '==', 2)
+    false_expr = Parser.new('1 == 2').expression
+    true_expr = Parser.new('1 == 1').expression
+
+    condition = Condition.new(false_expr)
     assert_equal(false, condition.evaluate(Context.new))
 
-    condition.or(Condition.new(2, '==', 1))
-
+    condition.or(Condition.new(false_expr))
     assert_equal(false, condition.evaluate(Context.new))
 
-    condition.or(Condition.new(1, '==', 1))
-
+    condition.or(Condition.new(true_expr))
     assert_equal(true, condition.evaluate(Context.new))
   end
 
   def test_and_condition
-    condition = Condition.new(1, '==', 1)
+    false_expr = Parser.new('1 == 2').expression
+    true_expr = Parser.new('1 == 1').expression
 
+    condition = Condition.new(true_expr)
     assert_equal(true, condition.evaluate(Context.new))
 
-    condition.and(Condition.new(2, '==', 2))
-
+    condition.and(Condition.new(true_expr))
     assert_equal(true, condition.evaluate(Context.new))
 
-    condition.and(Condition.new(2, '==', 1))
-
+    condition.and(Condition.new(false_expr))
     assert_equal(false, condition.evaluate(Context.new))
   end
 
@@ -149,7 +145,8 @@ class ConditionUnitTest < Minitest::Test
     end
 
     _out, err = capture_io do
-      assert_equal(true, Condition.new(1, '==', 1).evaluate)
+      expr = Parser.new('1 == 1').expression
+      assert_equal(true, Condition.new(expr).evaluate)
     end
 
     expected = "DEPRECATION WARNING: Condition#evaluate without a context argument is deprecated " \
@@ -345,6 +342,14 @@ class ConditionUnitTest < Minitest::Test
 
   private
 
+  def assert_evaluates_nil(left, op, right)
+    expr = BinaryExpression.new(left, op, right)
+    assert_nil(
+      Condition.new(expr).evaluate(@context),
+      "Evaluated not nil: #{left.inspect} #{op} #{right.inspect}",
+    )
+  end
+
   def assert_evaluates_true(left, op, right)
     expr = BinaryExpression.new(left, op, right)
     assert(