diff --git a/lib/liquid/binary_expression.rb b/lib/liquid/binary_expression.rb index 8ac743f8..71d1c406 100644 --- a/lib/liquid/binary_expression.rb +++ b/lib/liquid/binary_expression.rb @@ -13,6 +13,14 @@ module Liquid 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 @@ -38,7 +46,7 @@ module Liquid end def to_s - "(#{left_node} #{operator} #{right_node})" + "(#{left_node.inspect} #{operator} #{right_node.inspect})" end private diff --git a/lib/liquid/parser.rb b/lib/liquid/parser.rb index 8d5151e7..f715e6ed 100644 --- a/lib/liquid/parser.rb +++ b/lib/liquid/parser.rb @@ -47,12 +47,30 @@ module Liquid tok[0] == type end - # expression := equality + # expression := logical + # logical := equality (("and" | "or") equality)* # equality := comparison (("==" | "!=" | "<>") comparison)* # comparison := primary ((">=" | ">" | "<" | "<=" | ... ) primary)* # primary := string | number | variable_lookup | range | boolean def expression - equality + logical + end + + # Logical relations in Liquid, unlike other languages, are right-to-left + # associative. This creates a right-leaning tree and is why the method + # looks a bit more complicated + # + # `a == b and b or c` is evaluated like (a and (b or c)) + def logical + expr = equality + while (operator = id?('and') || id?('or')) + if expr.is_a?(BinaryExpression) && (expr.operator == 'and' || expr.operator == 'or') + expr.right_node = BinaryExpression.new(expr.right_node, operator, equality) + else + expr = BinaryExpression.new(expr, operator, equality) + end + end + expr end def equality diff --git a/lib/liquid/tags/case.rb b/lib/liquid/tags/case.rb index fcc0809f..9288af3a 100644 --- a/lib/liquid/tags/case.rb +++ b/lib/liquid/tags/case.rb @@ -99,7 +99,7 @@ module Liquid parser = @parse_context.new_parser(markup) loop do - expr = BinaryExpression.new(@left, '==', Condition.parse_expression(parser)) + expr = BinaryExpression.new(@left, '==', parser.equality) block = Condition.new(expr) block.attach(body) @blocks << block diff --git a/test/unit/binary_expression_test.rb b/test/unit/binary_expression_test.rb index 19fe33d7..6b5bb236 100644 --- a/test/unit/binary_expression_test.rb +++ b/test/unit/binary_expression_test.rb @@ -2,6 +2,25 @@ require 'test_helper' +class ExecutionSpy + attr_reader :called + attr_accessor :value + + def initialize(value) + @called = false + @value = value + end + + def to_liquid_value + @called = true + @value + end + + def reset + @called = false + end +end + class BinaryExpressionTest < Minitest::Test include Liquid @@ -13,6 +32,32 @@ class BinaryExpressionTest < Minitest::Test assert_eval(true, BinaryExpression.new("abcd", "contains", "a")) end + def test_logical_expression_short_circuiting + spy = ExecutionSpy.new(true) + + # false or spy should try spy + assert_eval(true, BinaryExpression.new(false, 'or', spy)) + assert_equal(true, spy.called) + + spy.reset + + # true or spy should not call spy + assert_eval(true, BinaryExpression.new(true, 'or', spy)) + assert_equal(false, spy.called) + + spy.reset + + # true and spy should try spy + assert_eval(true, BinaryExpression.new(true, 'and', spy)) + assert_equal(true, spy.called) + + spy.reset + + # false and spy should not try spy + assert_eval(false, BinaryExpression.new(false, 'and', spy)) + assert_equal(false, spy.called) + end + def test_complex_evaluation # 1 > 2 == 2 > 3 assert_eval(true, BinaryExpression.new( diff --git a/test/unit/parser_unit_test.rb b/test/unit/parser_unit_test.rb index 1274fe92..003ea83b 100644 --- a/test/unit/parser_unit_test.rb +++ b/test/unit/parser_unit_test.rb @@ -77,6 +77,35 @@ class ParserUnitTest < Minitest::Test assert_equal(0..5, p.expression) end + def test_logical + p = new_parser("a and b") + expr = p.expression + assert(expr.is_a?(BinaryExpression)) + assert_equal('and', expr.operator) + assert_equal('a', expr.left_node.name) + assert_equal('b', expr.right_node.name) + + p = new_parser("a and b or c") + expr = p.expression + assert(expr.is_a?(BinaryExpression)) + assert_equal('and', expr.operator) + assert_equal('a', expr.left_node.name) + assert_equal('or', expr.right_node.operator) + assert_equal('b', expr.right_node.left_node.name) + assert_equal('c', expr.right_node.right_node.name) + + p = new_parser("a == b and c or d") + expr = p.expression + assert(expr.is_a?(BinaryExpression)) + assert_equal('and', expr.operator) + assert_equal('==', expr.left_node.operator) + assert_equal('a', expr.left_node.left_node.name) + assert_equal('b', expr.left_node.right_node.name) + assert_equal('or', expr.right_node.operator) + assert_equal('c', expr.right_node.left_node.name) + assert_equal('d', expr.right_node.right_node.name) + end + def test_equality p = new_parser("a == b") expr = p.expression