diff --git a/lib/liquid/expression.rb b/lib/liquid/expression.rb index c36583c1..29cfc09d 100644 --- a/lib/liquid/expression.rb +++ b/lib/liquid/expression.rb @@ -26,31 +26,29 @@ module Liquid RANGES_REGEX = /\A\(\s*(?>(\S+)\s*\.\.)\s*(\S+)\s*\)\z/ INTEGER_REGEX = /\A(-?\d+)\z/ FLOAT_REGEX = /\A(-?\d+)\.\d+\z/ + QUOTED_STRING = /\A#{QuotedString}\z/ class << self - def parse(markup, ss = StringScanner.new(""), cache = nil, logical_expression = false) + def parse(markup, ss = StringScanner.new(""), cache = nil) return unless markup markup = markup.strip # markup can be a frozen string - if (markup.start_with?('"') && markup.end_with?('"')) || - (markup.start_with?("'") && markup.end_with?("'")) - return markup[1..-2] - elsif LITERALS.key?(markup) - return LITERALS[markup] - end + return markup[1..-2] if QUOTED_STRING.match?(markup) + + return LITERALS[markup] if LITERALS.key?(markup) # Cache only exists during parsing if cache return cache[markup] if cache.key?(markup) - cache[markup] = inner_parse(markup, ss, cache, logical_expression).freeze + cache[markup] = inner_parse(markup, ss, cache).freeze else - inner_parse(markup, ss, nil, logical_expression).freeze + inner_parse(markup, ss, nil).freeze end end - def inner_parse(markup, ss, cache, logical_expression = false) + 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) @@ -66,7 +64,7 @@ module Liquid if (num = parse_number(markup, ss)) num else - VariableLookup.parse(markup, ss, cache, logical_expression) + VariableLookup.parse(markup, ss, cache) end end diff --git a/lib/liquid/expression/logical_expression.rb b/lib/liquid/expression/logical_expression.rb index 47bd6f5b..9fcfa382 100644 --- a/lib/liquid/expression/logical_expression.rb +++ b/lib/liquid/expression/logical_expression.rb @@ -19,34 +19,20 @@ module Liquid 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?(last_expr) - LogicalExpression.parse(last_expr, ss, cache) - else - Condition.new(Expression.parse(last_expr, ss, cache, true), nil, nil) - end + expression = expressions.pop + condition = parse_condition(expression, ss, cache) until expressions.empty? operator = expressions.pop.to_s.strip + next unless boolean_operator?(operator) - expr = expressions.pop.to_s.strip + expression = expressions.pop.to_s.strip + new_condition = parse_condition(expression, ss, cache) - new_condition = if ComparisonExpression.comparison?(expr) - ComparisonExpression.parse(expr, ss, cache) - elsif logical?(expr) - LogicalExpression.parse(expr, ss, cache) - else - Condition.new(Expression.parse(expr, ss, cache, true), nil, nil) - end - - if operator == 'and' - new_condition.and(condition) - else # operator == 'or' - new_condition.or(condition) + case operator + when 'and' then new_condition.and(condition) + when 'or' then new_condition.or(condition) end condition = new_condition @@ -54,6 +40,19 @@ module Liquid condition end + + private + + def parse_condition(expr, ss, cache) + return ComparisonExpression.parse(expr, ss, cache) if comparison?(expr) + return LogicalExpression.parse(expr, ss, cache) if logical?(expr) + + Condition.new(Expression.parse(expr, ss, cache), nil, nil) + end + + def comparison?(...) + ComparisonExpression.comparison?(...) + end end end end diff --git a/lib/liquid/variable_lookup.rb b/lib/liquid/variable_lookup.rb index e4167f2b..340c0b66 100644 --- a/lib/liquid/variable_lookup.rb +++ b/lib/liquid/variable_lookup.rb @@ -5,14 +5,12 @@ module Liquid COMMAND_METHODS = ['size', 'first', 'last'].freeze attr_reader :name, :lookups - attr_accessor :logical_expression - def self.parse(markup, string_scanner = StringScanner.new(""), cache = nil, logical_expression = false) - new(markup, string_scanner, cache, logical_expression) + def self.parse(markup, string_scanner = StringScanner.new(""), cache = nil) + new(markup, string_scanner, cache) end - def initialize(markup, string_scanner = StringScanner.new(""), cache = nil, logical_expression = false) - @logical_expression = logical_expression + def initialize(markup, string_scanner = StringScanner.new(""), cache = nil) lookups = markup.scan(VariableParser) name = lookups.shift @@ -47,17 +45,9 @@ module Liquid end def evaluate(context) - puts "variable_lookup #evaluate #{@name} #{logical_expression?}" name = context.evaluate(@name) object = context.find_variable(name) - # When evaluating a logical expression, this variable lookup is part of a chain of conditions - # If the variable lookup returns nil, we must use the falsey value of the variable lookup - # rather than nil which is reserved for the usecase of rendering nothing. - if logical_expression? && object.nil? - return false - end - @lookups.each_index do |i| key = context.evaluate(@lookups[i]) @@ -99,10 +89,6 @@ module Liquid self.class == other.class && state == other.state end - def logical_expression? - @logical_expression - end - protected def state diff --git a/test/unit/boolean_unit_test.rb b/test/unit/boolean_unit_test.rb index 84c29fea..383217e8 100644 --- a/test/unit/boolean_unit_test.rb +++ b/test/unit/boolean_unit_test.rb @@ -95,10 +95,58 @@ class BooleanUnitTest < Minitest::Test assert_equal("true", template.render("media_position" => 2)) end - def test_equality_operators - assert_parity("1 == 1", "true") - assert_parity("1 != 2", "true") - assert_parity_todo!("'hello' == 'hello'", "true") + def test_equality_operators_with_integer_literals + assert_expression("1", "1") + assert_expression("1 == 1", "true") + assert_expression("1 != 1", "false") + assert_expression("1 == 2", "false") + assert_expression("1 != 2", "true") + end + + def test_equality_operators_with_stirng_literals + assert_expression("'hello'", "hello") + assert_expression("'hello' == 'hello'", "true") + assert_expression("'hello' != 'hello'", "false") + assert_expression("'hello' == 'world'", "false") + assert_expression("'hello' != 'world'", "true") + end + + def test_equality_operators_with_float_literals + assert_expression("1.5", "1.5") + assert_expression("1.5 == 1.5", "true") + assert_expression("1.5 != 1.5", "false") + assert_expression("1.5 == 2.5", "false") + assert_expression("1.5 != 2.5", "true") + end + + def test_equality_operators_with_nil_literals + assert_expression("nil", "") + assert_expression("nil == nil", "true") + assert_expression("nil != nil", "false") + assert_expression("null == nil", "true") + assert_expression("null != nil", "false") + end + + def test_equality_operators_with_boolean_literals + assert_expression("true", "true") + assert_expression("false", "false") + assert_expression("true == true", "true") + assert_expression("true != true", "false") + assert_expression("false == false", "true") + assert_expression("false != false", "false") + assert_expression("true == false", "false") + assert_expression("true != false", "true") + end + + def test_equality_operators_with_empty_literals + assert_expression("empty", "") + assert_expression("empty == ''", "true") + assert_expression("empty == empty", "true") + assert_expression("empty != empty", "false") + assert_expression("blank == blank", "true") + assert_expression("blank != blank", "false") + assert_expression("empty == blank", "true") + assert_expression("empty != blank", "false") end def test_nil_renders_as_empty_string @@ -122,18 +170,41 @@ class BooleanUnitTest < Minitest::Test end def test_nil_variable_in_and_expression - assert_parity("x and true", "false", { "x" => nil }) - assert_parity("true and x", "false", { "x" => nil }) + assert_condition("x and true", "false", { "x" => nil }) + assert_condition("true and x", "false", { "x" => nil }) + + assert_expression("x and true", "", { "x" => nil }) + assert_expression("true and x", "", { "x" => nil }) end def test_boolean_variable_in_and_expression assert_parity("true and x", "false", { "x" => false }) assert_parity("x and true", "false", { "x" => false }) + + assert_parity("true and x", "true", { "x" => true }) + assert_parity("x and true", "true", { "x" => true }) + + assert_parity("true or x", "true", { "x" => false }) + assert_parity("x or true", "true", { "x" => false }) + + assert_parity("true or x", "true", { "x" => true }) + assert_parity("x or true", "true", { "x" => true }) end def test_multi_variable_boolean_nil_and_expression - assert_parity("x and y", "false", { "x" => nil, "y" => true }) - assert_parity("y and x", "false", { "x" => true, "y" => nil }) + assert_condition("x and y", "false", { "x" => nil, "y" => true }) + assert_condition("y and x", "false", { "x" => true, "y" => nil }) + + assert_expression("x and y", "", { "x" => nil, "y" => true }) + assert_expression("y and x", "", { "x" => true, "y" => nil }) + end + + def test_multi_truthy_variables_and_expressions + assert_condition("x or y", "true", { "x" => nil, "y" => "hello" }) + assert_condition("y or x", "true", { "x" => "hello", "y" => nil }) + + assert_expression("x or y", "hello", { "x" => nil, "y" => "hello" }) + assert_expression("y or x", "hello", { "x" => "hello", "y" => nil }) end def test_multi_variable_boolean_nil_or_expression