Replace if tag relation parsing by Parser#expression

- Remove Condition#child_relation
- Remove Condition#and
- Remove Condition#or
- Simplify Condition#evaluate

Author: charlespwd

lib/liquid/condition.rb

@@ -9,43 +9,15 @@ module Liquid
   #   c.evaluate #=> true
   #
   class Condition # :nodoc:
-    attr_reader :attachment, :child_condition
+    attr_reader :attachment
     attr_accessor :left, :operator, :right
 
     def initialize(left = nil)
       @left = left
-
-      @child_relation  = nil
-      @child_condition = nil
     end
 
     def evaluate(context = deprecated_default_context)
-      condition = self
-      result = nil
-      loop do
-        result = context.evaluate(condition.left)
-
-        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)
@@ -76,7 +48,6 @@ class ParseTreeVisitor < Liquid::ParseTreeVisitor
       def children
         [
           @node.left,
-          @node.child_condition,
           @node.attachment
         ].compact
       end

lib/liquid/tags/if.rb

@@ -75,26 +75,11 @@ def push_block(tag, markup)
 
     def parse_markup(markup)
       p = @parse_context.new_parser(markup)
-      condition = parse_binary_comparisons(p)
+      condition = Condition.new(p.expression)
       p.consume(:end_of_string)
       condition
     end
 
-    def parse_binary_comparisons(p)
-      condition = parse_comparison(p)
-      first_condition = condition
-      while (op = p.id?('and') || p.id?('or'))
-        child_condition = parse_comparison(p)
-        condition.send(op, child_condition)
-        condition = child_condition
-      end
-      first_condition
-    end
-
-    def parse_comparison(p)
-      Condition.new(p.expression)
-    end
-
     class ParseTreeVisitor < Liquid::ParseTreeVisitor
       def children
         @node.blocks

test/unit/condition_unit_test.rb

@@ -105,31 +105,37 @@ def test_contains_with_string_left_operand_coerces_right_operand_to_string
   end
 
   def test_or_condition
-    false_expr = Parser.new('1 == 2').expression
-    true_expr = Parser.new('1 == 1').expression
+    false_expr = '1 == 2'
+    true_expr = '1 == 1'
 
-    condition = Condition.new(false_expr)
+    condition = Condition.new(expression(false_expr))
     assert_equal(false, condition.evaluate(Context.new))
 
-    condition.or(Condition.new(false_expr))
+    condition = Condition.new(expression("#{false_expr} or #{false_expr}"))
     assert_equal(false, condition.evaluate(Context.new))
 
-    condition.or(Condition.new(true_expr))
+    condition = Condition.new(expression("#{false_expr} or #{true_expr}"))
+    assert_equal(true, condition.evaluate(Context.new))
+
+    condition = Condition.new(expression("#{true_expr} or #{false_expr}"))
     assert_equal(true, condition.evaluate(Context.new))
   end
 
   def test_and_condition
-    false_expr = Parser.new('1 == 2').expression
-    true_expr = Parser.new('1 == 1').expression
+    false_expr = '1 == 2'
+    true_expr = '1 == 1'
 
-    condition = Condition.new(true_expr)
+    condition = Condition.new(expression(true_expr))
     assert_equal(true, condition.evaluate(Context.new))
 
-    condition.and(Condition.new(true_expr))
-    assert_equal(true, condition.evaluate(Context.new))
+    condition = Condition.new(expression("#{true_expr} and #{false_expr}"))
+    assert_equal(false, condition.evaluate(Context.new))
 
-    condition.and(Condition.new(false_expr))
+    condition = Condition.new(expression("#{false_expr} and #{true_expr}"))
     assert_equal(false, condition.evaluate(Context.new))
+
+    condition = Condition.new(expression("#{true_expr} and #{true_expr}"))
+    assert_equal(true, condition.evaluate(Context.new))
   end
 
   def test_left_or_right_may_contain_operators
@@ -206,4 +212,8 @@ def assert_evaluates_argument_error(left, op, right)
       Condition.new(expr).evaluate(@context)
     end
   end
+
+  def expression(markup)
+    Parser.new(markup).expression
+  end
 end # ConditionTest