diff --git a/lib/liquid/expression.rb b/lib/liquid/expression.rb index f1293bd7..c36583c1 100644 --- a/lib/liquid/expression.rb +++ b/lib/liquid/expression.rb @@ -28,7 +28,7 @@ module Liquid FLOAT_REGEX = /\A(-?\d+)\.\d+\z/ class << self - def parse(markup, ss = StringScanner.new(""), cache = nil) + def parse(markup, ss = StringScanner.new(""), cache = nil, logical_expression = false) return unless markup markup = markup.strip # markup can be a frozen string @@ -44,13 +44,13 @@ module Liquid if cache return cache[markup] if cache.key?(markup) - cache[markup] = inner_parse(markup, ss, cache).freeze + cache[markup] = inner_parse(markup, ss, cache, logical_expression).freeze else - inner_parse(markup, ss, nil).freeze + inner_parse(markup, ss, nil, logical_expression).freeze end end - def inner_parse(markup, ss, cache) + def inner_parse(markup, ss, cache, logical_expression = false) return LogicalExpression.parse(markup, ss, cache) if LogicalExpression.logical?(markup) return ComparisonExpression.parse(markup, ss, cache) if ComparisonExpression.comparison?(markup) @@ -66,7 +66,7 @@ module Liquid if (num = parse_number(markup, ss)) num else - VariableLookup.parse(markup, ss, cache) + VariableLookup.parse(markup, ss, cache, logical_expression) end end diff --git a/lib/liquid/expression/logical_expression.rb b/lib/liquid/expression/logical_expression.rb index 3ac061f0..47bd6f5b 100644 --- a/lib/liquid/expression/logical_expression.rb +++ b/lib/liquid/expression/logical_expression.rb @@ -26,7 +26,7 @@ module Liquid elsif logical?(last_expr) LogicalExpression.parse(last_expr, ss, cache) else - Condition.new(Expression.parse(last_expr, ss, cache), nil, nil) + Condition.new(Expression.parse(last_expr, ss, cache, true), nil, nil) end until expressions.empty? @@ -40,7 +40,7 @@ module Liquid elsif logical?(expr) LogicalExpression.parse(expr, ss, cache) else - Condition.new(Expression.parse(expr, ss, cache), nil, nil) + Condition.new(Expression.parse(expr, ss, cache, true), nil, nil) end if operator == 'and' diff --git a/lib/liquid/variable_lookup.rb b/lib/liquid/variable_lookup.rb index 340c0b66..e4167f2b 100644 --- a/lib/liquid/variable_lookup.rb +++ b/lib/liquid/variable_lookup.rb @@ -5,12 +5,14 @@ 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) - new(markup, string_scanner, cache) + def self.parse(markup, string_scanner = StringScanner.new(""), cache = nil, logical_expression = false) + new(markup, string_scanner, cache, logical_expression) end - def initialize(markup, string_scanner = StringScanner.new(""), cache = nil) + def initialize(markup, string_scanner = StringScanner.new(""), cache = nil, logical_expression = false) + @logical_expression = logical_expression lookups = markup.scan(VariableParser) name = lookups.shift @@ -45,9 +47,17 @@ 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]) @@ -89,6 +99,10 @@ 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 7e0822de..492a2c42 100644 --- a/test/unit/boolean_unit_test.rb +++ b/test/unit/boolean_unit_test.rb @@ -106,10 +106,10 @@ class BooleanUnitTest < Minitest::Test end def test_nil_comparison_with_blank - assert_parity_todo!("nil_value == blank", "false") - assert_parity_todo!("nil_value != blank", "true") - assert_parity_todo!("undefined != blank", "true") - assert_parity_todo!("undefined == blank", "false") + assert_parity("nil_value == blank", "false") + assert_parity("nil_value != blank", "true") + assert_parity("undefined != blank", "true") + assert_parity("undefined == blank", "false") end def test_if_with_variables @@ -121,7 +121,13 @@ class BooleanUnitTest < Minitest::Test end def test_nil_variable_in_and_expression - assert_parity_todo!("x and true", "false", { "x" => nil }) + assert_parity("x and true", "false", { "x" => nil }) + assert_parity("true and x", "false", { "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 }) end private @@ -133,10 +139,18 @@ class BooleanUnitTest < Minitest::Test end def assert_parity(liquid_expression, expected_result, args = {}) - assert_parity_scenario(:condition, "{% if #{liquid_expression} %}true{% else %}false{% endif %}", expected_result, args) + assert_condition(liquid_expression, expected_result, args) + assert_expression(liquid_expression, expected_result, args) + end + + def assert_expression(liquid_expression, expected_result, args = {}) assert_parity_scenario(:expression, "{{ #{liquid_expression} }}", expected_result, args) end + def assert_condition(liquid_condition, expected_result, args = {}) + assert_parity_scenario(:condition, "{% if #{liquid_condition} %}true{% else %}false{% endif %}", expected_result, args) + end + def assert_parity_scenario(kind, template, exp_output, args = {}) act_output = Liquid::Template.parse(template).render(args)