From 5782a9e1d7326d5911eb4e320a54a085a621642b Mon Sep 17 00:00:00 2001 From: Albert Chu Date: Tue, 4 Mar 2025 12:59:45 -0700 Subject: [PATCH 01/22] TDD: Unit tests for new liquid syntax --- test/unit/boolean_unit_test.rb | 115 +++++++++++++++++++++ test/unit/infix_operators_unit_test.rb | 138 +++++++++++++++++++++++++ 2 files changed, 253 insertions(+) create mode 100644 test/unit/boolean_unit_test.rb create mode 100644 test/unit/infix_operators_unit_test.rb diff --git a/test/unit/boolean_unit_test.rb b/test/unit/boolean_unit_test.rb new file mode 100644 index 000000000..10f6017c6 --- /dev/null +++ b/test/unit/boolean_unit_test.rb @@ -0,0 +1,115 @@ +# frozen_string_literal: true + +require 'test_helper' + +class BooleanUnitTest < Minitest::Test + include Liquid + + def test_simple_boolean_comparison + template = Liquid::Template.parse("{{ 1 > 0 }}") + assert_equal(true, template.render) + + template = Liquid::Template.parse("{{ 1 < 0 }}") + assert_equal(false, template.render) + end + + def test_boolean_assignment_shorthand + template = Liquid::Template.parse("{% assign lazy_load = media_position > 1 %}{{ lazy_load }}") + assert_equal(false, template.render("media_position" => 1)) + assert_equal(true, template.render("media_position" => 2)) + end + + def test_boolean_and_operator + template = Liquid::Template.parse("{{ true and true }}") + assert_equal(true, template.render) + + template = Liquid::Template.parse("{{ true and false }}") + assert_equal(false, template.render) + end + + def test_boolean_or_operator + template = Liquid::Template.parse("{{ true or false }}") + assert_equal(true, template.render) + + template = Liquid::Template.parse("{{ false or false }}") + assert_equal(false, template.render) + end + + def test_boolean_operator_aliases + template = Liquid::Template.parse("{{ true && true }}") + assert_equal(true, template.render) + + template = Liquid::Template.parse("{{ true && false }}") + assert_equal(false, template.render) + + template = Liquid::Template.parse("{{ false || true }}") + assert_equal(true, template.render) + + template = Liquid::Template.parse("{{ false || false }}") + assert_equal(false, template.render) + end + + def test_operator_precedence_with_parentheses + template = Liquid::Template.parse("{{ false and (false or true) }}") + assert_equal(false, template.render) + + template = Liquid::Template.parse("{{ false && (false || true) }}") + assert_equal(false, template.render) + end + + def test_operator_precedence_without_parentheses + template = Liquid::Template.parse("{{ false and false or true }}") + assert_equal(true, template.render) + + template = Liquid::Template.parse("{{ false && false || true }}") + assert_equal(true, template.render) + end + + def test_complex_boolean_expressions + template = Liquid::Template.parse("{{ true and true and true }}") + assert_equal(true, template.render) + + template = Liquid::Template.parse("{{ true and false and true }}") + assert_equal(false, template.render) + + template = Liquid::Template.parse("{{ false or false or true }}") + assert_equal(true, template.render) + + template = Liquid::Template.parse("{{ true && true && true }}") + assert_equal(true, template.render) + + template = Liquid::Template.parse("{{ true && false && true }}") + assert_equal(false, template.render) + + template = Liquid::Template.parse("{{ false || false || true }}") + assert_equal(true, template.render) + end + + def test_boolean_with_variables + template = Liquid::Template.parse("{{ a and b }}") + assert_equal(true, template.render("a" => true, "b" => true)) + assert_equal(false, template.render("a" => true, "b" => false)) + + template = Liquid::Template.parse("{{ a or b }}") + assert_equal(true, template.render("a" => false, "b" => true)) + assert_equal(false, template.render("a" => false, "b" => false)) + + template = Liquid::Template.parse("{{ a && b }}") + assert_equal(true, template.render("a" => true, "b" => true)) + assert_equal(false, template.render("a" => true, "b" => false)) + + template = Liquid::Template.parse("{{ a || b }}") + assert_equal(true, template.render("a" => false, "b" => true)) + assert_equal(false, template.render("a" => false, "b" => false)) + end + + def test_mixed_boolean_expressions + template = Liquid::Template.parse("{{ a > b and c < d }}") + assert_equal(true, template.render("a" => 5, "b" => 3, "c" => 2, "d" => 4)) + assert_equal(false, template.render("a" => 5, "b" => 3, "c" => 5, "d" => 4)) + + template = Liquid::Template.parse("{{ a > b && c < d }}") + assert_equal(true, template.render("a" => 5, "b" => 3, "c" => 2, "d" => 4)) + assert_equal(false, template.render("a" => 5, "b" => 3, "c" => 5, "d" => 4)) + end +end diff --git a/test/unit/infix_operators_unit_test.rb b/test/unit/infix_operators_unit_test.rb new file mode 100644 index 000000000..a90ba2aed --- /dev/null +++ b/test/unit/infix_operators_unit_test.rb @@ -0,0 +1,138 @@ +# frozen_string_literal: true + +require 'test_helper' + +class InfixOperatorsUnitTest < Minitest::Test + include Liquid + + def test_addition_operator + # Filter syntax + filter_template = Liquid::Template.parse("{{ num | plus: 3 }}") + # Infix syntax + infix_template = Liquid::Template.parse("{{ num + 3 }}") + + assert_equal(filter_template.render("num" => 5), infix_template.render("num" => 5)) + assert_equal("8", infix_template.render("num" => 5)) + end + + def test_subtraction_operator + # Filter syntax + filter_template = Liquid::Template.parse("{{ num | minus: 3 }}") + # Infix syntax + infix_template = Liquid::Template.parse("{{ num - 3 }}") + + assert_equal(filter_template.render("num" => 5), infix_template.render("num" => 5)) + assert_equal("2", infix_template.render("num" => 5)) + end + + def test_multiplication_operator + # Filter syntax + filter_template = Liquid::Template.parse("{{ num | times: 3 }}") + # Infix syntax + infix_template = Liquid::Template.parse("{{ num * 3 }}") + + assert_equal(filter_template.render("num" => 5), infix_template.render("num" => 5)) + assert_equal("15", infix_template.render("num" => 5)) + end + + def test_division_operator + # Filter syntax + filter_template = Liquid::Template.parse("{{ num | divided_by: 2 }}") + # Infix syntax + infix_template = Liquid::Template.parse("{{ num / 2 }}") + + assert_equal(filter_template.render("num" => 10), infix_template.render("num" => 10)) + assert_equal("5", infix_template.render("num" => 10)) + end + + def test_comparison_operators + # Greater than + assert_equal("true", Liquid::Template.parse("{{ 5 > 3 }}").render) + assert_equal("false", Liquid::Template.parse("{{ 3 > 5 }}").render) + + # Greater than or equal + assert_equal("true", Liquid::Template.parse("{{ 5 >= 5 }}").render) + assert_equal("false", Liquid::Template.parse("{{ 3 >= 5 }}").render) + + # Equal to + assert_equal("true", Liquid::Template.parse("{{ 5 == 5 }}").render) + assert_equal("false", Liquid::Template.parse("{{ 3 == 5 }}").render) + + # Less than or equal + assert_equal("true", Liquid::Template.parse("{{ 5 <= 5 }}").render) + assert_equal("false", Liquid::Template.parse("{{ 6 <= 5 }}").render) + + # Less than + assert_equal("true", Liquid::Template.parse("{{ 3 < 5 }}").render) + assert_equal("false", Liquid::Template.parse("{{ 5 < 3 }}").render) + end + + def test_logical_operators + # AND operator + assert_equal("true", Liquid::Template.parse("{{ true && true }}").render) + assert_equal("false", Liquid::Template.parse("{{ true && false }}").render) + + # OR operator + assert_equal("true", Liquid::Template.parse("{{ true || false }}").render) + assert_equal("false", Liquid::Template.parse("{{ false || false }}").render) + end + + def test_xor_operator + assert_equal("true", Liquid::Template.parse("{{ true ^ false }}").render) + assert_equal("false", Liquid::Template.parse("{{ true ^ true }}").render) + assert_equal("false", Liquid::Template.parse("{{ false ^ false }}").render) + end + + def test_operator_precedence + # (10 - 2) * 3 = 24 + assert_equal("24", Liquid::Template.parse("{{ (10 - 2) * 3 }}").render) + # 10 - (2 * 3) = 4 + assert_equal("4", Liquid::Template.parse("{{ 10 - (2 * 3) }}").render) + # Without parentheses, multiplication has higher precedence + # 10 - 2 * 3 = 10 - 6 = 4 + assert_equal("4", Liquid::Template.parse("{{ 10 - 2 * 3 }}").render) + end + + def test_complex_expressions + # Multiple operations + assert_equal("9", Liquid::Template.parse("{{ 3 + 2 * 3 }}").render) + assert_equal("15", Liquid::Template.parse("{{ (3 + 2) * 3 }}").render) + + # Mixed arithmetic and comparison + assert_equal("true", Liquid::Template.parse("{{ 3 + 2 > 4 }}").render) + assert_equal("false", Liquid::Template.parse("{{ 3 + 2 < 4 }}").render) + + # Mixed arithmetic and logical + assert_equal("true", Liquid::Template.parse("{{ 3 + 2 > 4 && 10 / 2 == 5 }}").render) + end + + def test_combined_operations + # In the proposed example + infix_template = Liquid::Template.parse("{% assign media_count = media_count - variant_images.size + 1 %}") + filter_template = Liquid::Template.parse("{% assign media_count = media_count | minus: variant_images.size | plus: 1 %}") + + # Check that both templates have the same effect + context1 = Context.new("media_count" => 10, "variant_images" => [1, 2, 3]) + context2 = Context.new("media_count" => 10, "variant_images" => [1, 2, 3]) + + infix_template.render(context1) + filter_template.render(context2) + + assert_equal(context1["media_count"], context2["media_count"]) + assert_equal(8, context1["media_count"]) + end + + def test_with_variables + template = Liquid::Template.parse("{{ a + b * c }}") + assert_equal("11", template.render("a" => 5, "b" => 2, "c" => 3)) + + template = Liquid::Template.parse("{{ (a + b) * c }}") + assert_equal("21", template.render("a" => 5, "b" => 2, "c" => 3)) + end + + def test_chained_comparisons + template = Liquid::Template.parse("{{ a < b && b < c }}") + assert_equal("true", template.render("a" => 1, "b" => 5, "c" => 10)) + assert_equal("false", template.render("a" => 1, "b" => 15, "c" => 10)) + end +end From b036feb20a66f5b281b4656cabce081f9b675cde Mon Sep 17 00:00:00 2001 From: Albert Chu Date: Tue, 4 Mar 2025 16:33:27 -0700 Subject: [PATCH 02/22] Removed infix operators from this PR --- test/unit/boolean_unit_test.rb | 41 -------- test/unit/infix_operators_unit_test.rb | 138 ------------------------- 2 files changed, 179 deletions(-) delete mode 100644 test/unit/infix_operators_unit_test.rb diff --git a/test/unit/boolean_unit_test.rb b/test/unit/boolean_unit_test.rb index 10f6017c6..564e0d704 100644 --- a/test/unit/boolean_unit_test.rb +++ b/test/unit/boolean_unit_test.rb @@ -35,34 +35,14 @@ def test_boolean_or_operator assert_equal(false, template.render) end - def test_boolean_operator_aliases - template = Liquid::Template.parse("{{ true && true }}") - assert_equal(true, template.render) - - template = Liquid::Template.parse("{{ true && false }}") - assert_equal(false, template.render) - - template = Liquid::Template.parse("{{ false || true }}") - assert_equal(true, template.render) - - template = Liquid::Template.parse("{{ false || false }}") - assert_equal(false, template.render) - end - def test_operator_precedence_with_parentheses template = Liquid::Template.parse("{{ false and (false or true) }}") assert_equal(false, template.render) - - template = Liquid::Template.parse("{{ false && (false || true) }}") - assert_equal(false, template.render) end def test_operator_precedence_without_parentheses template = Liquid::Template.parse("{{ false and false or true }}") assert_equal(true, template.render) - - template = Liquid::Template.parse("{{ false && false || true }}") - assert_equal(true, template.render) end def test_complex_boolean_expressions @@ -74,15 +54,6 @@ def test_complex_boolean_expressions template = Liquid::Template.parse("{{ false or false or true }}") assert_equal(true, template.render) - - template = Liquid::Template.parse("{{ true && true && true }}") - assert_equal(true, template.render) - - template = Liquid::Template.parse("{{ true && false && true }}") - assert_equal(false, template.render) - - template = Liquid::Template.parse("{{ false || false || true }}") - assert_equal(true, template.render) end def test_boolean_with_variables @@ -93,23 +64,11 @@ def test_boolean_with_variables template = Liquid::Template.parse("{{ a or b }}") assert_equal(true, template.render("a" => false, "b" => true)) assert_equal(false, template.render("a" => false, "b" => false)) - - template = Liquid::Template.parse("{{ a && b }}") - assert_equal(true, template.render("a" => true, "b" => true)) - assert_equal(false, template.render("a" => true, "b" => false)) - - template = Liquid::Template.parse("{{ a || b }}") - assert_equal(true, template.render("a" => false, "b" => true)) - assert_equal(false, template.render("a" => false, "b" => false)) end def test_mixed_boolean_expressions template = Liquid::Template.parse("{{ a > b and c < d }}") assert_equal(true, template.render("a" => 5, "b" => 3, "c" => 2, "d" => 4)) assert_equal(false, template.render("a" => 5, "b" => 3, "c" => 5, "d" => 4)) - - template = Liquid::Template.parse("{{ a > b && c < d }}") - assert_equal(true, template.render("a" => 5, "b" => 3, "c" => 2, "d" => 4)) - assert_equal(false, template.render("a" => 5, "b" => 3, "c" => 5, "d" => 4)) end end diff --git a/test/unit/infix_operators_unit_test.rb b/test/unit/infix_operators_unit_test.rb deleted file mode 100644 index a90ba2aed..000000000 --- a/test/unit/infix_operators_unit_test.rb +++ /dev/null @@ -1,138 +0,0 @@ -# frozen_string_literal: true - -require 'test_helper' - -class InfixOperatorsUnitTest < Minitest::Test - include Liquid - - def test_addition_operator - # Filter syntax - filter_template = Liquid::Template.parse("{{ num | plus: 3 }}") - # Infix syntax - infix_template = Liquid::Template.parse("{{ num + 3 }}") - - assert_equal(filter_template.render("num" => 5), infix_template.render("num" => 5)) - assert_equal("8", infix_template.render("num" => 5)) - end - - def test_subtraction_operator - # Filter syntax - filter_template = Liquid::Template.parse("{{ num | minus: 3 }}") - # Infix syntax - infix_template = Liquid::Template.parse("{{ num - 3 }}") - - assert_equal(filter_template.render("num" => 5), infix_template.render("num" => 5)) - assert_equal("2", infix_template.render("num" => 5)) - end - - def test_multiplication_operator - # Filter syntax - filter_template = Liquid::Template.parse("{{ num | times: 3 }}") - # Infix syntax - infix_template = Liquid::Template.parse("{{ num * 3 }}") - - assert_equal(filter_template.render("num" => 5), infix_template.render("num" => 5)) - assert_equal("15", infix_template.render("num" => 5)) - end - - def test_division_operator - # Filter syntax - filter_template = Liquid::Template.parse("{{ num | divided_by: 2 }}") - # Infix syntax - infix_template = Liquid::Template.parse("{{ num / 2 }}") - - assert_equal(filter_template.render("num" => 10), infix_template.render("num" => 10)) - assert_equal("5", infix_template.render("num" => 10)) - end - - def test_comparison_operators - # Greater than - assert_equal("true", Liquid::Template.parse("{{ 5 > 3 }}").render) - assert_equal("false", Liquid::Template.parse("{{ 3 > 5 }}").render) - - # Greater than or equal - assert_equal("true", Liquid::Template.parse("{{ 5 >= 5 }}").render) - assert_equal("false", Liquid::Template.parse("{{ 3 >= 5 }}").render) - - # Equal to - assert_equal("true", Liquid::Template.parse("{{ 5 == 5 }}").render) - assert_equal("false", Liquid::Template.parse("{{ 3 == 5 }}").render) - - # Less than or equal - assert_equal("true", Liquid::Template.parse("{{ 5 <= 5 }}").render) - assert_equal("false", Liquid::Template.parse("{{ 6 <= 5 }}").render) - - # Less than - assert_equal("true", Liquid::Template.parse("{{ 3 < 5 }}").render) - assert_equal("false", Liquid::Template.parse("{{ 5 < 3 }}").render) - end - - def test_logical_operators - # AND operator - assert_equal("true", Liquid::Template.parse("{{ true && true }}").render) - assert_equal("false", Liquid::Template.parse("{{ true && false }}").render) - - # OR operator - assert_equal("true", Liquid::Template.parse("{{ true || false }}").render) - assert_equal("false", Liquid::Template.parse("{{ false || false }}").render) - end - - def test_xor_operator - assert_equal("true", Liquid::Template.parse("{{ true ^ false }}").render) - assert_equal("false", Liquid::Template.parse("{{ true ^ true }}").render) - assert_equal("false", Liquid::Template.parse("{{ false ^ false }}").render) - end - - def test_operator_precedence - # (10 - 2) * 3 = 24 - assert_equal("24", Liquid::Template.parse("{{ (10 - 2) * 3 }}").render) - # 10 - (2 * 3) = 4 - assert_equal("4", Liquid::Template.parse("{{ 10 - (2 * 3) }}").render) - # Without parentheses, multiplication has higher precedence - # 10 - 2 * 3 = 10 - 6 = 4 - assert_equal("4", Liquid::Template.parse("{{ 10 - 2 * 3 }}").render) - end - - def test_complex_expressions - # Multiple operations - assert_equal("9", Liquid::Template.parse("{{ 3 + 2 * 3 }}").render) - assert_equal("15", Liquid::Template.parse("{{ (3 + 2) * 3 }}").render) - - # Mixed arithmetic and comparison - assert_equal("true", Liquid::Template.parse("{{ 3 + 2 > 4 }}").render) - assert_equal("false", Liquid::Template.parse("{{ 3 + 2 < 4 }}").render) - - # Mixed arithmetic and logical - assert_equal("true", Liquid::Template.parse("{{ 3 + 2 > 4 && 10 / 2 == 5 }}").render) - end - - def test_combined_operations - # In the proposed example - infix_template = Liquid::Template.parse("{% assign media_count = media_count - variant_images.size + 1 %}") - filter_template = Liquid::Template.parse("{% assign media_count = media_count | minus: variant_images.size | plus: 1 %}") - - # Check that both templates have the same effect - context1 = Context.new("media_count" => 10, "variant_images" => [1, 2, 3]) - context2 = Context.new("media_count" => 10, "variant_images" => [1, 2, 3]) - - infix_template.render(context1) - filter_template.render(context2) - - assert_equal(context1["media_count"], context2["media_count"]) - assert_equal(8, context1["media_count"]) - end - - def test_with_variables - template = Liquid::Template.parse("{{ a + b * c }}") - assert_equal("11", template.render("a" => 5, "b" => 2, "c" => 3)) - - template = Liquid::Template.parse("{{ (a + b) * c }}") - assert_equal("21", template.render("a" => 5, "b" => 2, "c" => 3)) - end - - def test_chained_comparisons - template = Liquid::Template.parse("{{ a < b && b < c }}") - assert_equal("true", template.render("a" => 1, "b" => 5, "c" => 10)) - assert_equal("false", template.render("a" => 1, "b" => 15, "c" => 10)) - end -end From f32c0fb4fb246501b7b0becb9eed99ef7f976bca Mon Sep 17 00:00:00 2001 From: Albert Chu Date: Tue, 4 Mar 2025 16:45:26 -0700 Subject: [PATCH 03/22] TDD: Improved unit tests in boolean_unit_test.rb Added tests for existing usage cases to avoid breaking important logic when introducing changes in subsequent commits. --- test/unit/boolean_unit_test.rb | 227 ++++++++++++++++++++++++++++++--- 1 file changed, 208 insertions(+), 19 deletions(-) diff --git a/test/unit/boolean_unit_test.rb b/test/unit/boolean_unit_test.rb index 564e0d704..df5012ee0 100644 --- a/test/unit/boolean_unit_test.rb +++ b/test/unit/boolean_unit_test.rb @@ -7,68 +7,257 @@ class BooleanUnitTest < Minitest::Test def test_simple_boolean_comparison template = Liquid::Template.parse("{{ 1 > 0 }}") - assert_equal(true, template.render) + assert_equal("true", template.render) template = Liquid::Template.parse("{{ 1 < 0 }}") - assert_equal(false, template.render) + assert_equal("false", template.render) end def test_boolean_assignment_shorthand template = Liquid::Template.parse("{% assign lazy_load = media_position > 1 %}{{ lazy_load }}") - assert_equal(false, template.render("media_position" => 1)) - assert_equal(true, template.render("media_position" => 2)) + assert_equal("false", template.render("media_position" => 1)) + assert_equal("true", template.render("media_position" => 2)) end def test_boolean_and_operator template = Liquid::Template.parse("{{ true and true }}") - assert_equal(true, template.render) + assert_equal("true", template.render) template = Liquid::Template.parse("{{ true and false }}") - assert_equal(false, template.render) + assert_equal("false", template.render) end def test_boolean_or_operator template = Liquid::Template.parse("{{ true or false }}") - assert_equal(true, template.render) + assert_equal("true", template.render) template = Liquid::Template.parse("{{ false or false }}") - assert_equal(false, template.render) + assert_equal("false", template.render) end def test_operator_precedence_with_parentheses template = Liquid::Template.parse("{{ false and (false or true) }}") - assert_equal(false, template.render) + assert_equal("false", template.render) end def test_operator_precedence_without_parentheses template = Liquid::Template.parse("{{ false and false or true }}") - assert_equal(true, template.render) + assert_equal("true", template.render) end def test_complex_boolean_expressions template = Liquid::Template.parse("{{ true and true and true }}") - assert_equal(true, template.render) + assert_equal("true", template.render) template = Liquid::Template.parse("{{ true and false and true }}") - assert_equal(false, template.render) + assert_equal("false", template.render) template = Liquid::Template.parse("{{ false or false or true }}") - assert_equal(true, template.render) + assert_equal("true", template.render) end def test_boolean_with_variables template = Liquid::Template.parse("{{ a and b }}") - assert_equal(true, template.render("a" => true, "b" => true)) - assert_equal(false, template.render("a" => true, "b" => false)) + assert_equal("true", template.render("a" => true, "b" => true)) + assert_equal("false", template.render("a" => true, "b" => false)) template = Liquid::Template.parse("{{ a or b }}") - assert_equal(true, template.render("a" => false, "b" => true)) - assert_equal(false, template.render("a" => false, "b" => false)) + assert_equal("true", template.render("a" => false, "b" => true)) + assert_equal("false", template.render("a" => false, "b" => false)) end def test_mixed_boolean_expressions template = Liquid::Template.parse("{{ a > b and c < d }}") - assert_equal(true, template.render("a" => 5, "b" => 3, "c" => 2, "d" => 4)) - assert_equal(false, template.render("a" => 5, "b" => 3, "c" => 5, "d" => 4)) + assert_equal("true", template.render("a" => 5, "b" => 3, "c" => 2, "d" => 4)) + assert_equal("false", template.render("a" => 5, "b" => 3, "c" => 5, "d" => 4)) + end + + def test_negation_operator + template = Liquid::Template.parse("{{ !true }}") + assert_equal("false", template.render) + + template = Liquid::Template.parse("{{ !false }}") + assert_equal("true", template.render) + + template = Liquid::Template.parse("{{ !(true and true) }}") + assert_equal("false", template.render) + end + + def test_equality_operators + template = Liquid::Template.parse("{{ 1 == 1 }}") + assert_equal("true", template.render) + + template = Liquid::Template.parse("{{ 1 != 2 }}") + assert_equal("true", template.render) + + template = Liquid::Template.parse("{{ 'hello' == 'hello' }}") + assert_equal("true", template.render) + end + + def test_nil_in_boolean_context + template = Liquid::Template.parse("{{ nil and true }}") + assert_equal("false", template.render) + + template = Liquid::Template.parse("{{ nil or true }}") + assert_equal("true", template.render) + + template = Liquid::Template.parse("{{ !nil }}") + assert_equal("true", template.render) + end + + def test_truthy_falsy_values + template = Liquid::Template.parse("{% if empty_string %}true{% else %}false{% endif %}") + assert_equal(false, template.render("empty_string" => "")) + + template = Liquid::Template.parse("{% if zero %}true{% else %}false{% endif %}") + assert_equal(false, template.render("zero" => 0)) + + template = Liquid::Template.parse("{% if text %}true{% else %}false{% endif %}") + assert_equal(true, template.render("text" => "hello")) + end + + def test_complex_precedence + template = Liquid::Template.parse("{{ !a and b or c }}") + assert_equal("true", template.render("a" => false, "b" => true, "c" => false)) + assert_equal("true", template.render("a" => true, "b" => false, "c" => true)) + assert_equal("false", template.render("a" => true, "b" => false, "c" => false)) + end + + def test_string_comparison_with_blank + # Non-empty string against blank + template = Liquid::Template.parse("{{ text != blank }}") + assert_equal(true, template.render("text" => "hello")) + + template = Liquid::Template.parse("{{ text == blank }}") + assert_equal(false, template.render("text" => "hello")) + + # Empty string against blank + template = Liquid::Template.parse("{{ empty_text != blank }}") + assert_equal(false, template.render("empty_text" => "")) + + template = Liquid::Template.parse("{{ empty_text == blank }}") + assert_equal(true, template.render("empty_text" => "")) + end + + def test_nil_comparison_with_blank + template = Liquid::Template.parse("{{ nil_value != blank }}") + assert_equal(false, template.render("nil_value" => nil)) + + template = Liquid::Template.parse("{{ nil_value == blank }}") + assert_equal(true, template.render("nil_value" => nil)) + + # Undefined variable is treated as nil + template = Liquid::Template.parse("{{ undefined != blank }}") + assert_equal(false, template.render) + + template = Liquid::Template.parse("{{ undefined == blank }}") + assert_equal(true, template.render) + end + + def test_empty_collections_with_blank + template = Liquid::Template.parse("{{ empty_array == blank }}") + assert_equal(true, template.render("empty_array" => [])) + + template = Liquid::Template.parse("{{ empty_array != blank }}") + assert_equal(false, template.render("empty_array" => [])) + + template = Liquid::Template.parse("{{ empty_hash == blank }}") + assert_equal(true, template.render("empty_hash" => {})) + + template = Liquid::Template.parse("{{ empty_hash != blank }}") + assert_equal(false, template.render("empty_hash" => {})) + + # Non-empty collections + template = Liquid::Template.parse("{{ array == blank }}") + assert_equal(false, template.render("array" => [1, 2, 3])) + + template = Liquid::Template.parse("{{ hash == blank }}") + assert_equal(false, template.render("hash" => { "key" => "value" })) + end + + def test_blank_in_conditional_statements + template = Liquid::Template.parse("{% if text != blank %}not blank{% else %}is blank{% endif %}") + assert_equal("not blank", template.render("text" => "hello")) + assert_equal("is blank", template.render("text" => "")) + + template = Liquid::Template.parse("{% if nil_value != blank %}not blank{% else %}is blank{% endif %}") + assert_equal("is blank", template.render("nil_value" => nil)) + + template = Liquid::Template.parse("{% if array != blank %}not blank{% else %}is blank{% endif %}") + assert_equal("not blank", template.render("array" => [1, 2, 3])) + assert_equal("is blank", template.render("array" => [])) + end + + def test_blank_with_other_operators + template = Liquid::Template.parse("{{ text != blank and number > 0 }}") + assert_equal("true", template.render("text" => "hello", "number" => 5)) + assert_equal("false", template.render("text" => "", "number" => 5)) + assert_equal("false", template.render("text" => "hello", "number" => 0)) + + template = Liquid::Template.parse("{{ text != blank or number > 0 }}") + assert_equal("true", template.render("text" => "hello", "number" => 0)) + assert_equal("true", template.render("text" => "", "number" => 5)) + assert_equal("false", template.render("text" => "", "number" => 0)) + end + + def test_basic_if_else_conditions + template = Liquid::Template.parse("{% if true %}success{% else %}failure{% endif %}") + assert_equal("success", template.render) + + template = Liquid::Template.parse("{% if false %}failure{% else %}success{% endif %}") + assert_equal("success", template.render) + end + + def test_if_with_comparisons + template = Liquid::Template.parse("{% if 10 > 5 %}greater{% else %}not greater{% endif %}") + assert_equal("greater", template.render) + + template = Liquid::Template.parse("{% if 5 == 5 %}equal{% else %}not equal{% endif %}") + assert_equal("equal", template.render) + + template = Liquid::Template.parse("{% if 3 < 2 %}smaller{% else %}not smaller{% endif %}") + assert_equal("not smaller", template.render) + end + + def test_if_with_variables + template = Liquid::Template.parse("{% if value %}has value{% else %}no value{% endif %}") + assert_equal("has value", template.render("value" => true)) + assert_equal("no value", template.render("value" => false)) + assert_equal("no value", template.render("value" => nil)) + assert_equal("has value", template.render("value" => "text")) + assert_equal("no value", template.render("value" => "")) + end + + def test_if_with_variable_comparisons + template = Liquid::Template.parse("{% if count > 5 %}high{% else %}low{% endif %}") + assert_equal("high", template.render("count" => 10)) + assert_equal("low", template.render("count" => 3)) + end + + def test_nested_if_conditions + template = Liquid::Template.parse("{% if a %}{% if b %}both{% else %}a only{% endif %}{% else %}none{% endif %}") + assert_equal("both", template.render("a" => true, "b" => true)) + assert_equal("a only", template.render("a" => true, "b" => false)) + assert_equal("none", template.render("a" => false, "b" => true)) + end + + def test_elsif_conditions + template = Liquid::Template.parse("{% if a %}a{% elsif b %}b{% else %}c{% endif %}") + assert_equal("a", template.render("a" => true, "b" => true)) + assert_equal("b", template.render("a" => false, "b" => true)) + assert_equal("c", template.render("a" => false, "b" => false)) + end + + def test_unless_conditions + template = Liquid::Template.parse("{% unless a %}not a{% else %}a{% endunless %}") + assert_equal("a", template.render("a" => true)) + assert_equal("not a", template.render("a" => false)) + end + + def test_if_with_comparison_and_logical_operator + template = Liquid::Template.parse("{% if a > 5 and b < 10 %}valid{% else %}invalid{% endif %}") + assert_equal("valid", template.render("a" => 7, "b" => 8)) + assert_equal("invalid", template.render("a" => 3, "b" => 8)) + assert_equal("invalid", template.render("a" => 7, "b" => 12)) end end From 6d4cffa000b575323cf7bbfdc3248d8d18e0db0a Mon Sep 17 00:00:00 2001 From: Albert Chu Date: Tue, 4 Mar 2025 17:08:23 -0700 Subject: [PATCH 04/22] Support for simple boolean comparisons and boolean assignments --- lib/liquid.rb | 1 + lib/liquid/boolean_expression.rb | 21 +++++++++++++++++++++ lib/liquid/variable.rb | 27 ++++++++++++++++++++------- 3 files changed, 42 insertions(+), 7 deletions(-) create mode 100644 lib/liquid/boolean_expression.rb diff --git a/lib/liquid.rb b/lib/liquid.rb index 4d0a71a64..445dfbf77 100644 --- a/lib/liquid.rb +++ b/lib/liquid.rb @@ -89,3 +89,4 @@ module Liquid require 'liquid/usage' require 'liquid/registers' require 'liquid/template_factory' +require 'liquid/boolean_expression' diff --git a/lib/liquid/boolean_expression.rb b/lib/liquid/boolean_expression.rb new file mode 100644 index 000000000..bccffea05 --- /dev/null +++ b/lib/liquid/boolean_expression.rb @@ -0,0 +1,21 @@ +# frozen_string_literal: true + +module Liquid + class BooleanExpression + def self.parse(markup, ss = StringScanner.new(""), cache = nil) + # Split the markup by comparison operators + if (match = markup.match(/\A\s*(.+?)\s*(==|!=|<>|<=|>=|<|>|contains)\s*(.+)\s*\z/)) + left = Expression.parse(match[1], ss, cache) + operator = match[2] + right = Expression.parse(match[3], ss, cache) + + # Create a condition object to evaluate the expression + condition = Condition.new(left, operator, right) + return condition + end + + # If no comparison operator is found, just parse as regular expression + Expression.parse(markup, ss, cache) + end + end +end diff --git a/lib/liquid/variable.rb b/lib/liquid/variable.rb index 209570654..fffa7fa94 100644 --- a/lib/liquid/variable.rb +++ b/lib/liquid/variable.rb @@ -17,6 +17,7 @@ class Variable FilterArgsRegex = /(?:#{FilterArgumentSeparator}|#{ArgumentSeparator})\s*((?:\w+\s*\:\s*)?#{QuotedFragment})/o JustTagAttributes = /\A#{TagAttributes}\z/o MarkupWithQuotedFragment = /(#{QuotedFragment})(.*)/om + ComparisonOperator = /==|!=|<>|<=|>=|<|>|contains/o attr_accessor :filters, :name, :line_number attr_reader :parse_context @@ -47,7 +48,14 @@ def lax_parse(markup) name_markup = Regexp.last_match(1) filter_markup = Regexp.last_match(2) - @name = parse_context.parse_expression(name_markup) + + # Check if name_markup contains a comparison operator + @name = if /\s*(#{ComparisonOperator})\s*/.match?(name_markup) + BooleanExpression.parse(name_markup) + else + parse_context.parse_expression(name_markup) + end + if filter_markup =~ FilterMarkupRegex filters = Regexp.last_match(1).scan(FilterParser) filters.each do |f| @@ -65,13 +73,18 @@ def strict_parse(markup) return if p.look(:end_of_string) - @name = parse_context.parse_expression(p.expression) - while p.consume?(:pipe) - filtername = p.consume(:id) - filterargs = p.consume?(:colon) ? parse_filterargs(p) : Const::EMPTY_ARRAY - @filters << parse_filter_expressions(filtername, filterargs) + # Check if markup contains a comparison operator + if /\s*(#{ComparisonOperator})\s*/.match?(markup) + @name = BooleanExpression.parse(markup) + else + @name = parse_context.parse_expression(p.expression) + while p.consume?(:pipe) + filtername = p.consume(:id) + filterargs = p.consume?(:colon) ? parse_filterargs(p) : Const::EMPTY_ARRAY + @filters << parse_filter_expressions(filtername, filterargs) + end + p.consume(:end_of_string) end - p.consume(:end_of_string) end def parse_filterargs(p) From 14b0d64b709749d4d3218261768e2bf6709d0712 Mon Sep 17 00:00:00 2001 From: Albert Chu Date: Tue, 4 Mar 2025 17:19:27 -0700 Subject: [PATCH 05/22] Rough support for parenthesis. Also better respect for and/or order precedence. --- lib/liquid/boolean_expression.rb | 112 ++++++++++++++++++++++++++++++- lib/liquid/variable.rb | 9 +-- 2 files changed, 115 insertions(+), 6 deletions(-) diff --git a/lib/liquid/boolean_expression.rb b/lib/liquid/boolean_expression.rb index bccffea05..4dae4fcce 100644 --- a/lib/liquid/boolean_expression.rb +++ b/lib/liquid/boolean_expression.rb @@ -3,7 +3,100 @@ module Liquid class BooleanExpression def self.parse(markup, ss = StringScanner.new(""), cache = nil) - # Split the markup by comparison operators + markup = markup.strip + + # Handle parenthesized expressions first + if markup.start_with?('(') && balance_parentheses(markup) + # Find the matching closing parenthesis + nesting = 0 + close_index = nil + + markup.chars.each_with_index do |char, i| + if char == '(' + nesting += 1 + elsif char == ')' + nesting -= 1 + if nesting == 0 + close_index = i + break + end + end + end + + if close_index && close_index < markup.length - 1 + # We have something like "(expr) rest" + paren_expr = markup[1...close_index] + rest = markup[close_index + 1..-1].strip + + # Check if rest starts with "and" or "or" (fixed the matching) + if rest =~ /\A(and|or)\s+/i + # Get the operator (and/or) + operator = ::Regexp.last_match(1).downcase + # Get the remaining part after the operator + remaining = rest[operator.length..-1].strip + + left_condition = parse(paren_expr, ss, cache) + right_condition = parse(remaining, ss, cache) + + condition = Condition.new(left_condition, nil, nil) + if operator == 'and' + condition.and(Condition.new(right_condition, nil, nil)) + else # operator == 'or' + condition.or(Condition.new(right_condition, nil, nil)) + end + + return condition + end + elsif close_index == markup.length - 1 + # Just a parenthesized expression "(expr)" + return parse(markup[1...close_index], ss, cache) + end + end + + # Check if we have something like "expr and (expr)" + if (match = markup.match(/\A\s*(.+?)\s+(and|or)\s+\((.+)\)\s*\z/i)) + left_expr = match[1] + operator = match[2].downcase + right_expr = match[3] + + left_condition = parse(left_expr, ss, cache) + right_condition = parse(right_expr, ss, cache) + + condition = Condition.new(left_condition, nil, nil) + if operator == 'and' + condition.and(Condition.new(right_condition, nil, nil)) + else # operator == 'or' + condition.or(Condition.new(right_condition, nil, nil)) + end + + return condition + end + + # First, try to handle OR operator (lower precedence) + if (match = markup.match(/\A\s*(.+?)\s+or\s+(.+)\s*\z/i)) + left = parse(match[1], ss, cache) + right = parse(match[2], ss, cache) + + # Create a condition for OR operation + condition = Condition.new(left, nil, nil) + condition.or(Condition.new(right, nil, nil)) + + return condition + end + + # Then try to handle AND operator (higher precedence) + if (match = markup.match(/\A\s*(.+?)\s+and\s+(.+)\s*\z/i)) + left = parse(match[1], ss, cache) + right = parse(match[2], ss, cache) + + # Create a condition for AND operation + condition = Condition.new(left, nil, nil) + condition.and(Condition.new(right, nil, nil)) + + return condition + end + + # Then try to parse as a comparison expression if (match = markup.match(/\A\s*(.+?)\s*(==|!=|<>|<=|>=|<|>|contains)\s*(.+)\s*\z/)) left = Expression.parse(match[1], ss, cache) operator = match[2] @@ -14,8 +107,23 @@ def self.parse(markup, ss = StringScanner.new(""), cache = nil) return condition end - # If no comparison operator is found, just parse as regular expression + # If no operator is found, just parse as regular expression Expression.parse(markup, ss, cache) end + + private + + def self.balance_parentheses(markup) + nesting = 0 + markup.each_char do |char| + if char == '(' + nesting += 1 + elsif char == ')' + nesting -= 1 + return false if nesting < 0 # Unbalanced + end + end + nesting == 0 # Should end with balanced parentheses + end end end diff --git a/lib/liquid/variable.rb b/lib/liquid/variable.rb index fffa7fa94..93a0c7720 100644 --- a/lib/liquid/variable.rb +++ b/lib/liquid/variable.rb @@ -18,6 +18,7 @@ class Variable JustTagAttributes = /\A#{TagAttributes}\z/o MarkupWithQuotedFragment = /(#{QuotedFragment})(.*)/om ComparisonOperator = /==|!=|<>|<=|>=|<|>|contains/o + LogicalOperator = /\s+(and|or)\s+/i attr_accessor :filters, :name, :line_number attr_reader :parse_context @@ -49,8 +50,8 @@ def lax_parse(markup) name_markup = Regexp.last_match(1) filter_markup = Regexp.last_match(2) - # Check if name_markup contains a comparison operator - @name = if /\s*(#{ComparisonOperator})\s*/.match?(name_markup) + # Check if name_markup contains a comparison operator or logical operator + @name = if name_markup =~ LogicalOperator || name_markup =~ ComparisonOperator BooleanExpression.parse(name_markup) else parse_context.parse_expression(name_markup) @@ -73,8 +74,8 @@ def strict_parse(markup) return if p.look(:end_of_string) - # Check if markup contains a comparison operator - if /\s*(#{ComparisonOperator})\s*/.match?(markup) + # Check if markup contains a comparison operator or logical operator + if markup =~ LogicalOperator || markup =~ ComparisonOperator @name = BooleanExpression.parse(markup) else @name = parse_context.parse_expression(p.expression) From 03feea967b475e6e169c4f5d060a595694b65f4e Mon Sep 17 00:00:00 2001 From: Albert Chu Date: Tue, 4 Mar 2025 17:52:16 -0700 Subject: [PATCH 06/22] Added a lot more boolean unit tests --- test/unit/boolean_unit_test.rb | 177 ++++++++++++++++++++++++--------- 1 file changed, 128 insertions(+), 49 deletions(-) diff --git a/test/unit/boolean_unit_test.rb b/test/unit/boolean_unit_test.rb index df5012ee0..7e9a2ef92 100644 --- a/test/unit/boolean_unit_test.rb +++ b/test/unit/boolean_unit_test.rb @@ -72,17 +72,6 @@ def test_mixed_boolean_expressions assert_equal("false", template.render("a" => 5, "b" => 3, "c" => 5, "d" => 4)) end - def test_negation_operator - template = Liquid::Template.parse("{{ !true }}") - assert_equal("false", template.render) - - template = Liquid::Template.parse("{{ !false }}") - assert_equal("true", template.render) - - template = Liquid::Template.parse("{{ !(true and true) }}") - assert_equal("false", template.render) - end - def test_equality_operators template = Liquid::Template.parse("{{ 1 == 1 }}") assert_equal("true", template.render) @@ -94,85 +83,67 @@ def test_equality_operators assert_equal("true", template.render) end - def test_nil_in_boolean_context - template = Liquid::Template.parse("{{ nil and true }}") - assert_equal("false", template.render) - - template = Liquid::Template.parse("{{ nil or true }}") - assert_equal("true", template.render) - - template = Liquid::Template.parse("{{ !nil }}") - assert_equal("true", template.render) - end - def test_truthy_falsy_values - template = Liquid::Template.parse("{% if empty_string %}true{% else %}false{% endif %}") - assert_equal(false, template.render("empty_string" => "")) - - template = Liquid::Template.parse("{% if zero %}true{% else %}false{% endif %}") - assert_equal(false, template.render("zero" => 0)) + template = Liquid::Template.parse("{% if empty_string %}truthy{% else %}falsey{% endif %}") + assert_equal("falsey", template.render("empty_string" => "")) - template = Liquid::Template.parse("{% if text %}true{% else %}false{% endif %}") - assert_equal(true, template.render("text" => "hello")) - end + template = Liquid::Template.parse("{% if zero %}truthy{% else %}falsey{% endif %}") + assert_equal("falsey", template.render("zero" => 0)) - def test_complex_precedence - template = Liquid::Template.parse("{{ !a and b or c }}") - assert_equal("true", template.render("a" => false, "b" => true, "c" => false)) - assert_equal("true", template.render("a" => true, "b" => false, "c" => true)) - assert_equal("false", template.render("a" => true, "b" => false, "c" => false)) + template = Liquid::Template.parse("{% if text %}truthy{% else %}falsey{% endif %}") + assert_equal("true", template.render("text" => "hello")) end def test_string_comparison_with_blank # Non-empty string against blank template = Liquid::Template.parse("{{ text != blank }}") - assert_equal(true, template.render("text" => "hello")) + assert_equal("true", template.render("text" => "hello")) template = Liquid::Template.parse("{{ text == blank }}") - assert_equal(false, template.render("text" => "hello")) + assert_equal("false", template.render("text" => "hello")) # Empty string against blank template = Liquid::Template.parse("{{ empty_text != blank }}") - assert_equal(false, template.render("empty_text" => "")) + assert_equal("false", template.render("empty_text" => "")) template = Liquid::Template.parse("{{ empty_text == blank }}") - assert_equal(true, template.render("empty_text" => "")) + assert_equal("true", template.render("empty_text" => "")) end def test_nil_comparison_with_blank template = Liquid::Template.parse("{{ nil_value != blank }}") - assert_equal(false, template.render("nil_value" => nil)) + assert_equal("false", template.render("nil_value" => nil)) template = Liquid::Template.parse("{{ nil_value == blank }}") - assert_equal(true, template.render("nil_value" => nil)) + assert_equal("true", template.render("nil_value" => nil)) # Undefined variable is treated as nil template = Liquid::Template.parse("{{ undefined != blank }}") - assert_equal(false, template.render) + assert_equal("false", template.render) template = Liquid::Template.parse("{{ undefined == blank }}") - assert_equal(true, template.render) + assert_equal("true", template.render) end def test_empty_collections_with_blank template = Liquid::Template.parse("{{ empty_array == blank }}") - assert_equal(true, template.render("empty_array" => [])) + assert_equal("true", template.render("empty_array" => [])) template = Liquid::Template.parse("{{ empty_array != blank }}") - assert_equal(false, template.render("empty_array" => [])) + assert_equal("false", template.render("empty_array" => [])) template = Liquid::Template.parse("{{ empty_hash == blank }}") - assert_equal(true, template.render("empty_hash" => {})) + assert_equal("true", template.render("empty_hash" => {})) template = Liquid::Template.parse("{{ empty_hash != blank }}") - assert_equal(false, template.render("empty_hash" => {})) + assert_equal("false", template.render("empty_hash" => {})) # Non-empty collections template = Liquid::Template.parse("{{ array == blank }}") - assert_equal(false, template.render("array" => [1, 2, 3])) + assert_equal("false", template.render("array" => [1, 2, 3])) template = Liquid::Template.parse("{{ hash == blank }}") - assert_equal(false, template.render("hash" => { "key" => "value" })) + assert_equal("false", template.render("hash" => { "key" => "value" })) end def test_blank_in_conditional_statements @@ -260,4 +231,112 @@ def test_if_with_comparison_and_logical_operator assert_equal("invalid", template.render("a" => 3, "b" => 8)) assert_equal("invalid", template.render("a" => 7, "b" => 12)) end + + # Basic nil rendering tests + def test_nil_renders_as_empty_string + template = Liquid::Template.parse("{{ nil }}") + assert_equal("", template.render) + end + + def test_nil_in_assigned_variable_renders_as_empty_string + template = Liquid::Template.parse("{% assign x = nil %}{{ x }}") + assert_equal("", template.render) + end + + # Nil comparison tests + def test_nil_equals_nil + template = Liquid::Template.parse("{{ nil == nil }}") + assert_equal("true", template.render) + end + + def test_nil_not_equals_nil + template = Liquid::Template.parse("{{ nil != nil }}") + assert_equal("false", template.render) + end + + def test_nil_not_equals_empty_string + template = Liquid::Template.parse("{{ nil == '' }}") + assert_equal("false", template.render) + + template = Liquid::Template.parse("{{ nil != '' }}") + assert_equal("true", template.render) + end + + # Variable tests with nil values + def test_variable_with_nil_value_in_comparisons + template = Liquid::Template.parse("{% assign x = nil %}{{ x == nil }}") + assert_equal("true", template.render) + + template = Liquid::Template.parse("{% assign x = nil %}{{ x != nil }}") + assert_equal("false", template.render) + end + + def test_variable_with_nil_value_compared_to_empty_string + template = Liquid::Template.parse("{% assign x = nil %}{{ x == '' }}") + assert_equal("false", template.render) + + template = Liquid::Template.parse("{% assign x = nil %}{{ x != '' }}") + assert_equal("true", template.render) + end + + # Tests with undefined variables + def test_undefined_variable_in_comparisons + template = Liquid::Template.parse("{{ undefined_var == nil }}") + assert_equal("true", template.render) + + template = Liquid::Template.parse("{{ undefined_var != nil }}") + assert_equal("false", template.render) + end + + def test_undefined_variable_compared_to_empty_string + template = Liquid::Template.parse("{{ undefined_var == '' }}") + assert_equal("false", template.render) + + template = Liquid::Template.parse("{{ undefined_var != '' }}") + assert_equal("true", template.render) + end + + # Tests with boolean values + def test_boolean_variable_in_comparisons + template = Liquid::Template.parse("{% assign t = true %}{{ t == true }}") + assert_equal("true", template.render) + + template = Liquid::Template.parse("{% assign f = false %}{{ f == false }}") + assert_equal("true", template.render) + end + + def test_boolean_variable_compared_to_nil + template = Liquid::Template.parse("{% assign t = true %}{{ t == nil }}") + assert_equal("false", template.render) + + template = Liquid::Template.parse("{% assign f = false %}{{ f == nil }}") + assert_equal("false", template.render) + + template = Liquid::Template.parse("{% assign f = false %}{{ f != nil }}") + assert_equal("true", template.render) + end + + # Mixed comparison tests + def test_nil_and_undefined_variables_in_boolean_expressions + template = Liquid::Template.parse("{% assign x = nil %}{{ x == undefined_var }}") + assert_equal("true", template.render) + + template = Liquid::Template.parse("{% assign x = nil %}{{ x != undefined_var }}") + assert_equal("false", template.render) + end + + def test_nil_variable_in_and_expression + template = Liquid::Template.parse("{% assign x = nil %}{{ x and true }}") + assert_equal("false", template.render) + end + + def test_nil_literal_in_or_expression + template = Liquid::Template.parse("{% assign x = nil %}{{ nil or true }}") + assert_equal("true", template.render) + end + + def test_nil_variable_in_or_expression + template = Liquid::Template.parse("{% assign x = nil %}{{ x or false }}") + assert_equal("false", template.render) + end end From 9b967690aa228c1f99a506b837820837d3a64b3a Mon Sep 17 00:00:00 2001 From: Guilherme Carreiro Date: Wed, 5 Mar 2025 14:49:10 +0100 Subject: [PATCH 07/22] Introduce support to boolean operators in the lexer --- lib/liquid/lexer.rb | 6 +++ test/unit/lexer_unit_test.rb | 72 ++++++++++++++++++++++++++++++++++++ 2 files changed, 78 insertions(+) diff --git a/lib/liquid/lexer.rb b/lib/liquid/lexer.rb index f1740dbad..26b74232a 100644 --- a/lib/liquid/lexer.rb +++ b/lib/liquid/lexer.rb @@ -14,6 +14,8 @@ class Lexer COMPARISON_LESS_THAN = [:comparison, "<"].freeze COMPARISON_LESS_THAN_OR_EQUAL = [:comparison, "<="].freeze COMPARISON_NOT_EQUAL_ALT = [:comparison, "<>"].freeze + BOOLEAN_AND = [:boolean_operator, "and"].freeze + BOOLEAN_OR = [:boolean_operator, "or"].freeze DASH = [:dash, "-"].freeze DOT = [:dot, "."].freeze DOTDOT = [:dotdot, ".."].freeze @@ -151,6 +153,10 @@ def tokenize(ss) # Special case for "contains" output << if type == :id && t == "contains" && output.last&.first != :dot COMPARISON_CONTAINS + elsif type == :id && t == "and" && output.last&.first != :dot + BOOLEAN_AND + elsif type == :id && t == "or" && output.last&.first != :dot + BOOLEAN_OR else [type, t] end diff --git a/test/unit/lexer_unit_test.rb b/test/unit/lexer_unit_test.rb index 73eeb7398..703bc9763 100644 --- a/test/unit/lexer_unit_test.rb +++ b/test/unit/lexer_unit_test.rb @@ -141,6 +141,78 @@ def test_error_with_invalid_utf8 ) end + def test_boolean_and_operator + exp = [ + [:id, "true"], + [:boolean_operator, "and"], + [:id, "false"], + [:end_of_string], + ] + act = tokenize("true and false") + assert_equal(exp, act) + end + + def test_boolean_or_operator + exp = [ + [:id, "false"], + [:boolean_operator, "or"], + [:id, "true"], + [:end_of_string], + ] + act = tokenize("false or true") + assert_equal(exp, act) + end + + def test_boolean_operators_in_complex_expressions + exp = [ + [:id, "a"], + [:boolean_operator, "and"], + [:id, "b"], + [:boolean_operator, "or"], + [:id, "c"], + [:end_of_string], + ] + act = tokenize("a and b or c") + assert_equal(exp, act) + end + + def test_boolean_operators_with_comparisons + exp = [ + [:id, "a"], + [:comparison, ">"], + [:number, "5"], + [:boolean_operator, "and"], + [:id, "b"], + [:comparison, "<"], + [:number, "10"], + [:end_of_string], + ] + act = tokenize("a > 5 and b < 10") + assert_equal(exp, act) + end + + def test_boolean_operators_as_property_names + exp = [ + [:id, "obj"], + [:dot, "."], + [:id, "and"], + [:dot, "."], + [:id, "property"], + [:end_of_string], + ] + act = tokenize("obj.and.property") + assert_equal(exp, act) + + exp = [ + [:id, "obj"], + [:dot, "."], + [:id, "or"], + [:end_of_string], + ] + act = tokenize("obj.or") + assert_equal(exp, act) + end + private def tokenize(input) From ba0bbe3c3fab797357ae689878a2685a9bb5e631 Mon Sep 17 00:00:00 2001 From: Guilherme Carreiro Date: Wed, 5 Mar 2025 14:49:59 +0100 Subject: [PATCH 08/22] Update the parser to use the new tokens --- lib/liquid/parser.rb | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/lib/liquid/parser.rb b/lib/liquid/parser.rb index 645dfa3a1..00faefdce 100644 --- a/lib/liquid/parser.rb +++ b/lib/liquid/parser.rb @@ -48,7 +48,7 @@ def look(type, ahead = 0) def expression token = @tokens[@p] - case token[0] + expr = case token[0] when :id str = consume str << variable_lookups @@ -69,6 +69,21 @@ def expression else raise SyntaxError, "#{token} is not a valid expression" end + if look(:comparison) + operator = consume(:comparison) + left = expr + right = expression + + "#{left} #{operator} #{right}" + elsif look(:boolean_operator) + operator = consume(:boolean_operator) + left = expr + right = expression + + "#{left} #{operator} #{right}" + else + expr + end end def argument From 26ccec12abbbf68c72d4d9801fadfeb1e4786958 Mon Sep 17 00:00:00 2001 From: Guilherme Carreiro Date: Wed, 5 Mar 2025 14:52:28 +0100 Subject: [PATCH 09/22] * Move expression handling from variable.rb to expression.rb * Update test suite to validate parity * Remove parentheses handling * Split boolean into comparison and logical expressions --- lib/liquid.rb | 3 +- lib/liquid/boolean_expression.rb | 129 ------- lib/liquid/expression.rb | 3 + .../expression/comparison_expression.rb | 29 ++ lib/liquid/expression/logical_expression.rb | 60 +++ lib/liquid/variable.rb | 28 +- test/unit/boolean_unit_test.rb | 359 ++++-------------- 7 files changed, 185 insertions(+), 426 deletions(-) delete mode 100644 lib/liquid/boolean_expression.rb create mode 100644 lib/liquid/expression/comparison_expression.rb create mode 100644 lib/liquid/expression/logical_expression.rb diff --git a/lib/liquid.rb b/lib/liquid.rb index 445dfbf77..69946a7ef 100644 --- a/lib/liquid.rb +++ b/lib/liquid.rb @@ -80,6 +80,8 @@ module Liquid require 'liquid/range_lookup' require 'liquid/resource_limits' require 'liquid/expression' +require 'liquid/expression/comparison_expression' +require 'liquid/expression/logical_expression' require 'liquid/template' require 'liquid/condition' require 'liquid/utils' @@ -89,4 +91,3 @@ module Liquid require 'liquid/usage' require 'liquid/registers' require 'liquid/template_factory' -require 'liquid/boolean_expression' diff --git a/lib/liquid/boolean_expression.rb b/lib/liquid/boolean_expression.rb deleted file mode 100644 index 4dae4fcce..000000000 --- a/lib/liquid/boolean_expression.rb +++ /dev/null @@ -1,129 +0,0 @@ -# frozen_string_literal: true - -module Liquid - class BooleanExpression - def self.parse(markup, ss = StringScanner.new(""), cache = nil) - markup = markup.strip - - # Handle parenthesized expressions first - if markup.start_with?('(') && balance_parentheses(markup) - # Find the matching closing parenthesis - nesting = 0 - close_index = nil - - markup.chars.each_with_index do |char, i| - if char == '(' - nesting += 1 - elsif char == ')' - nesting -= 1 - if nesting == 0 - close_index = i - break - end - end - end - - if close_index && close_index < markup.length - 1 - # We have something like "(expr) rest" - paren_expr = markup[1...close_index] - rest = markup[close_index + 1..-1].strip - - # Check if rest starts with "and" or "or" (fixed the matching) - if rest =~ /\A(and|or)\s+/i - # Get the operator (and/or) - operator = ::Regexp.last_match(1).downcase - # Get the remaining part after the operator - remaining = rest[operator.length..-1].strip - - left_condition = parse(paren_expr, ss, cache) - right_condition = parse(remaining, ss, cache) - - condition = Condition.new(left_condition, nil, nil) - if operator == 'and' - condition.and(Condition.new(right_condition, nil, nil)) - else # operator == 'or' - condition.or(Condition.new(right_condition, nil, nil)) - end - - return condition - end - elsif close_index == markup.length - 1 - # Just a parenthesized expression "(expr)" - return parse(markup[1...close_index], ss, cache) - end - end - - # Check if we have something like "expr and (expr)" - if (match = markup.match(/\A\s*(.+?)\s+(and|or)\s+\((.+)\)\s*\z/i)) - left_expr = match[1] - operator = match[2].downcase - right_expr = match[3] - - left_condition = parse(left_expr, ss, cache) - right_condition = parse(right_expr, ss, cache) - - condition = Condition.new(left_condition, nil, nil) - if operator == 'and' - condition.and(Condition.new(right_condition, nil, nil)) - else # operator == 'or' - condition.or(Condition.new(right_condition, nil, nil)) - end - - return condition - end - - # First, try to handle OR operator (lower precedence) - if (match = markup.match(/\A\s*(.+?)\s+or\s+(.+)\s*\z/i)) - left = parse(match[1], ss, cache) - right = parse(match[2], ss, cache) - - # Create a condition for OR operation - condition = Condition.new(left, nil, nil) - condition.or(Condition.new(right, nil, nil)) - - return condition - end - - # Then try to handle AND operator (higher precedence) - if (match = markup.match(/\A\s*(.+?)\s+and\s+(.+)\s*\z/i)) - left = parse(match[1], ss, cache) - right = parse(match[2], ss, cache) - - # Create a condition for AND operation - condition = Condition.new(left, nil, nil) - condition.and(Condition.new(right, nil, nil)) - - return condition - end - - # Then try to parse as a comparison expression - if (match = markup.match(/\A\s*(.+?)\s*(==|!=|<>|<=|>=|<|>|contains)\s*(.+)\s*\z/)) - left = Expression.parse(match[1], ss, cache) - operator = match[2] - right = Expression.parse(match[3], ss, cache) - - # Create a condition object to evaluate the expression - condition = Condition.new(left, operator, right) - return condition - end - - # If no operator is found, just parse as regular expression - Expression.parse(markup, ss, cache) - end - - private - - def self.balance_parentheses(markup) - nesting = 0 - markup.each_char do |char| - if char == '(' - nesting += 1 - elsif char == ')' - nesting -= 1 - return false if nesting < 0 # Unbalanced - end - end - nesting == 0 # Should end with balanced parentheses - end - end -end diff --git a/lib/liquid/expression.rb b/lib/liquid/expression.rb index adf340f1f..f1293bd7f 100644 --- a/lib/liquid/expression.rb +++ b/lib/liquid/expression.rb @@ -51,6 +51,9 @@ def parse(markup, ss = StringScanner.new(""), cache = nil) end 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) + if (markup.start_with?("(") && markup.end_with?(")")) && markup =~ RANGES_REGEX return RangeLookup.parse( Regexp.last_match(1), diff --git a/lib/liquid/expression/comparison_expression.rb b/lib/liquid/expression/comparison_expression.rb new file mode 100644 index 000000000..cac9e85ef --- /dev/null +++ b/lib/liquid/expression/comparison_expression.rb @@ -0,0 +1,29 @@ +# frozen_string_literal: true + +module Liquid + class Expression + class ComparisonExpression + COMPARISON_REGEX = /\A\s*(.+?)\s*(==|!=|<>|<=|>=|<|>|contains)\s*(.+)\s*\z/ + + class << self + def comparison?(markup) + markup.match(COMPARISON_REGEX) + end + + def parse(markup, ss, cache) + match = comparison?(markup) + + if match + left = Expression.parse(match[1].strip, ss, cache) + operator = match[2].strip + right = Expression.parse(match[3].strip, ss, cache) + + return Condition.new(left, operator, right) + end + + Condition.new(parse(markup, ss, cache), nil, nil) + end + end + end + end +end diff --git a/lib/liquid/expression/logical_expression.rb b/lib/liquid/expression/logical_expression.rb new file mode 100644 index 000000000..3ac061f01 --- /dev/null +++ b/lib/liquid/expression/logical_expression.rb @@ -0,0 +1,60 @@ +# frozen_string_literal: true + +module Liquid + class Expression + class LogicalExpression + LOGICAL_REGEX = /\A\s*(.+?)\s+(and|or)\s+(.+)\s*\z/i + EXPRESSIONS_AND_OPERATORS = /(?:\b(?:\s?and\s?|\s?or\s?)\b|(?:\s*(?!\b(?:\s?and\s?|\s?or\s?)\b)(?:#{QuotedFragment}|\S+)\s*)+)/o + BOOLEAN_OPERATORS = ['and', 'or'].freeze + + class << self + def logical?(markup) + markup.match(LOGICAL_REGEX) + end + + def boolean_operator?(markup) + BOOLEAN_OPERATORS.include?(markup) + end + + 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), nil, nil) + end + + until expressions.empty? + operator = expressions.pop.to_s.strip + next unless boolean_operator?(operator) + + expr = expressions.pop.to_s.strip + + 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), nil, nil) + end + + if operator == 'and' + new_condition.and(condition) + else # operator == 'or' + new_condition.or(condition) + end + + condition = new_condition + end + + condition + end + end + end + end +end diff --git a/lib/liquid/variable.rb b/lib/liquid/variable.rb index 93a0c7720..209570654 100644 --- a/lib/liquid/variable.rb +++ b/lib/liquid/variable.rb @@ -17,8 +17,6 @@ class Variable FilterArgsRegex = /(?:#{FilterArgumentSeparator}|#{ArgumentSeparator})\s*((?:\w+\s*\:\s*)?#{QuotedFragment})/o JustTagAttributes = /\A#{TagAttributes}\z/o MarkupWithQuotedFragment = /(#{QuotedFragment})(.*)/om - ComparisonOperator = /==|!=|<>|<=|>=|<|>|contains/o - LogicalOperator = /\s+(and|or)\s+/i attr_accessor :filters, :name, :line_number attr_reader :parse_context @@ -49,14 +47,7 @@ def lax_parse(markup) name_markup = Regexp.last_match(1) filter_markup = Regexp.last_match(2) - - # Check if name_markup contains a comparison operator or logical operator - @name = if name_markup =~ LogicalOperator || name_markup =~ ComparisonOperator - BooleanExpression.parse(name_markup) - else - parse_context.parse_expression(name_markup) - end - + @name = parse_context.parse_expression(name_markup) if filter_markup =~ FilterMarkupRegex filters = Regexp.last_match(1).scan(FilterParser) filters.each do |f| @@ -74,18 +65,13 @@ def strict_parse(markup) return if p.look(:end_of_string) - # Check if markup contains a comparison operator or logical operator - if markup =~ LogicalOperator || markup =~ ComparisonOperator - @name = BooleanExpression.parse(markup) - else - @name = parse_context.parse_expression(p.expression) - while p.consume?(:pipe) - filtername = p.consume(:id) - filterargs = p.consume?(:colon) ? parse_filterargs(p) : Const::EMPTY_ARRAY - @filters << parse_filter_expressions(filtername, filterargs) - end - p.consume(:end_of_string) + @name = parse_context.parse_expression(p.expression) + while p.consume?(:pipe) + filtername = p.consume(:id) + filterargs = p.consume?(:colon) ? parse_filterargs(p) : Const::EMPTY_ARRAY + @filters << parse_filter_expressions(filtername, filterargs) end + p.consume(:end_of_string) end def parse_filterargs(p) diff --git a/test/unit/boolean_unit_test.rb b/test/unit/boolean_unit_test.rb index 7e9a2ef92..7e0822de2 100644 --- a/test/unit/boolean_unit_test.rb +++ b/test/unit/boolean_unit_test.rb @@ -6,337 +6,146 @@ class BooleanUnitTest < Minitest::Test include Liquid def test_simple_boolean_comparison - template = Liquid::Template.parse("{{ 1 > 0 }}") - assert_equal("true", template.render) - - template = Liquid::Template.parse("{{ 1 < 0 }}") - assert_equal("false", template.render) - end - - def test_boolean_assignment_shorthand - template = Liquid::Template.parse("{% assign lazy_load = media_position > 1 %}{{ lazy_load }}") - assert_equal("false", template.render("media_position" => 1)) - assert_equal("true", template.render("media_position" => 2)) + assert_parity("1 > 0", "true") + assert_parity("1 < 0", "false") end def test_boolean_and_operator - template = Liquid::Template.parse("{{ true and true }}") - assert_equal("true", template.render) - - template = Liquid::Template.parse("{{ true and false }}") - assert_equal("false", template.render) + assert_parity("true and true", "true") + assert_parity("true and false", "false") end def test_boolean_or_operator - template = Liquid::Template.parse("{{ true or false }}") - assert_equal("true", template.render) - - template = Liquid::Template.parse("{{ false or false }}") - assert_equal("false", template.render) - end - - def test_operator_precedence_with_parentheses - template = Liquid::Template.parse("{{ false and (false or true) }}") - assert_equal("false", template.render) + assert_parity("true or false", "true") + assert_parity("false or false", "false") end - def test_operator_precedence_without_parentheses - template = Liquid::Template.parse("{{ false and false or true }}") - assert_equal("true", template.render) + def test_operator_precedence + assert_parity("false and false or true", "false") end def test_complex_boolean_expressions - template = Liquid::Template.parse("{{ true and true and true }}") - assert_equal("true", template.render) - - template = Liquid::Template.parse("{{ true and false and true }}") - assert_equal("false", template.render) - - template = Liquid::Template.parse("{{ false or false or true }}") - assert_equal("true", template.render) + assert_parity("true and true and true", "true") + assert_parity("true and false and true", "false") + assert_parity("false or false or true", "true") end def test_boolean_with_variables - template = Liquid::Template.parse("{{ a and b }}") - assert_equal("true", template.render("a" => true, "b" => true)) - assert_equal("false", template.render("a" => true, "b" => false)) - - template = Liquid::Template.parse("{{ a or b }}") - assert_equal("true", template.render("a" => false, "b" => true)) - assert_equal("false", template.render("a" => false, "b" => false)) - end - - def test_mixed_boolean_expressions - template = Liquid::Template.parse("{{ a > b and c < d }}") - assert_equal("true", template.render("a" => 5, "b" => 3, "c" => 2, "d" => 4)) - assert_equal("false", template.render("a" => 5, "b" => 3, "c" => 5, "d" => 4)) - end - - def test_equality_operators - template = Liquid::Template.parse("{{ 1 == 1 }}") - assert_equal("true", template.render) - - template = Liquid::Template.parse("{{ 1 != 2 }}") - assert_equal("true", template.render) - - template = Liquid::Template.parse("{{ 'hello' == 'hello' }}") - assert_equal("true", template.render) - end - - def test_truthy_falsy_values - template = Liquid::Template.parse("{% if empty_string %}truthy{% else %}falsey{% endif %}") - assert_equal("falsey", template.render("empty_string" => "")) - - template = Liquid::Template.parse("{% if zero %}truthy{% else %}falsey{% endif %}") - assert_equal("falsey", template.render("zero" => 0)) - - template = Liquid::Template.parse("{% if text %}truthy{% else %}falsey{% endif %}") - assert_equal("true", template.render("text" => "hello")) + assert_parity("a and b", "true", { "a" => true, "b" => true }) + assert_parity("a and b", "false", { "a" => true, "b" => false }) + assert_parity("a or b", "true", { "a" => false, "b" => true }) + assert_parity("a or b", "false", { "a" => false, "b" => false }) end - def test_string_comparison_with_blank - # Non-empty string against blank - template = Liquid::Template.parse("{{ text != blank }}") - assert_equal("true", template.render("text" => "hello")) - - template = Liquid::Template.parse("{{ text == blank }}") - assert_equal("false", template.render("text" => "hello")) - - # Empty string against blank - template = Liquid::Template.parse("{{ empty_text != blank }}") - assert_equal("false", template.render("empty_text" => "")) - - template = Liquid::Template.parse("{{ empty_text == blank }}") - assert_equal("true", template.render("empty_text" => "")) + def test_nil_equals_nil + assert_parity("nil == nil", "true") end - def test_nil_comparison_with_blank - template = Liquid::Template.parse("{{ nil_value != blank }}") - assert_equal("false", template.render("nil_value" => nil)) - - template = Liquid::Template.parse("{{ nil_value == blank }}") - assert_equal("true", template.render("nil_value" => nil)) - - # Undefined variable is treated as nil - template = Liquid::Template.parse("{{ undefined != blank }}") - assert_equal("false", template.render) - - template = Liquid::Template.parse("{{ undefined == blank }}") - assert_equal("true", template.render) + def test_nil_not_equals_nil + assert_parity("nil != nil", "false") end - def test_empty_collections_with_blank - template = Liquid::Template.parse("{{ empty_array == blank }}") - assert_equal("true", template.render("empty_array" => [])) - - template = Liquid::Template.parse("{{ empty_array != blank }}") - assert_equal("false", template.render("empty_array" => [])) - - template = Liquid::Template.parse("{{ empty_hash == blank }}") - assert_equal("true", template.render("empty_hash" => {})) - - template = Liquid::Template.parse("{{ empty_hash != blank }}") - assert_equal("false", template.render("empty_hash" => {})) - - # Non-empty collections - template = Liquid::Template.parse("{{ array == blank }}") - assert_equal("false", template.render("array" => [1, 2, 3])) - - template = Liquid::Template.parse("{{ hash == blank }}") - assert_equal("false", template.render("hash" => { "key" => "value" })) + def test_nil_not_equals_empty_string + assert_parity("nil == ''", "false") + assert_parity("nil != ''", "true") end - def test_blank_in_conditional_statements - template = Liquid::Template.parse("{% if text != blank %}not blank{% else %}is blank{% endif %}") - assert_equal("not blank", template.render("text" => "hello")) - assert_equal("is blank", template.render("text" => "")) - - template = Liquid::Template.parse("{% if nil_value != blank %}not blank{% else %}is blank{% endif %}") - assert_equal("is blank", template.render("nil_value" => nil)) - - template = Liquid::Template.parse("{% if array != blank %}not blank{% else %}is blank{% endif %}") - assert_equal("not blank", template.render("array" => [1, 2, 3])) - assert_equal("is blank", template.render("array" => [])) + def test_undefined_variable_in_comparisons + assert_parity("undefined_var == nil", "true") + assert_parity("undefined_var != nil", "false") end - def test_blank_with_other_operators - template = Liquid::Template.parse("{{ text != blank and number > 0 }}") - assert_equal("true", template.render("text" => "hello", "number" => 5)) - assert_equal("false", template.render("text" => "", "number" => 5)) - assert_equal("false", template.render("text" => "hello", "number" => 0)) - - template = Liquid::Template.parse("{{ text != blank or number > 0 }}") - assert_equal("true", template.render("text" => "hello", "number" => 0)) - assert_equal("true", template.render("text" => "", "number" => 5)) - assert_equal("false", template.render("text" => "", "number" => 0)) + def test_undefined_variable_compared_to_empty_string + assert_parity("undefined_var == ''", "false") + assert_parity("undefined_var != ''", "true") end - def test_basic_if_else_conditions - template = Liquid::Template.parse("{% if true %}success{% else %}failure{% endif %}") - assert_equal("success", template.render) - - template = Liquid::Template.parse("{% if false %}failure{% else %}success{% endif %}") - assert_equal("success", template.render) + def test_boolean_variable_in_comparisons + assert_parity("t == true", "true", { "t" => true }) + assert_parity("f == false", "true", { "f" => false }) end - def test_if_with_comparisons - template = Liquid::Template.parse("{% if 10 > 5 %}greater{% else %}not greater{% endif %}") - assert_equal("greater", template.render) - - template = Liquid::Template.parse("{% if 5 == 5 %}equal{% else %}not equal{% endif %}") - assert_equal("equal", template.render) - - template = Liquid::Template.parse("{% if 3 < 2 %}smaller{% else %}not smaller{% endif %}") - assert_equal("not smaller", template.render) + def test_boolean_variable_compared_to_nil + assert_parity("t == nil", "false", { "t" => true }) + assert_parity("f == nil", "false", { "f" => false }) + assert_parity("f != nil", "true", { "f" => false }) end - def test_if_with_variables - template = Liquid::Template.parse("{% if value %}has value{% else %}no value{% endif %}") - assert_equal("has value", template.render("value" => true)) - assert_equal("no value", template.render("value" => false)) - assert_equal("no value", template.render("value" => nil)) - assert_equal("has value", template.render("value" => "text")) - assert_equal("no value", template.render("value" => "")) + def test_nil_and_undefined_variables_in_boolean_expressions + assert_parity("x == undefined_var", "true", { "x" => nil }) + assert_parity("x != undefined_var", "false", { "x" => nil }) end - def test_if_with_variable_comparisons - template = Liquid::Template.parse("{% if count > 5 %}high{% else %}low{% endif %}") - assert_equal("high", template.render("count" => 10)) - assert_equal("low", template.render("count" => 3)) + def test_nil_literal_in_or_expression + assert_parity("nil or true", "true") end - def test_nested_if_conditions - template = Liquid::Template.parse("{% if a %}{% if b %}both{% else %}a only{% endif %}{% else %}none{% endif %}") - assert_equal("both", template.render("a" => true, "b" => true)) - assert_equal("a only", template.render("a" => true, "b" => false)) - assert_equal("none", template.render("a" => false, "b" => true)) + def test_nil_variable_in_or_expression + assert_parity("x or false", "false", { "x" => nil }) end - def test_elsif_conditions - template = Liquid::Template.parse("{% if a %}a{% elsif b %}b{% else %}c{% endif %}") - assert_equal("a", template.render("a" => true, "b" => true)) - assert_equal("b", template.render("a" => false, "b" => true)) - assert_equal("c", template.render("a" => false, "b" => false)) + def test_mixed_boolean_expressions + assert_parity("a > b and c < d", "true", { "a" => 99, "b" => 0, "c" => 0, "d" => 99 }) + assert_parity("a > b and c < d", "false", { "a" => 99, "b" => 0, "c" => 99, "d" => 0 }) end - def test_unless_conditions - template = Liquid::Template.parse("{% unless a %}not a{% else %}a{% endunless %}") - assert_equal("a", template.render("a" => true)) - assert_equal("not a", template.render("a" => false)) + def test_boolean_assignment_shorthand + template = Liquid::Template.parse("{% assign lazy_load = media_position > 1 %}{{ lazy_load }}") + assert_equal("false", template.render("media_position" => 1)) + assert_equal("true", template.render("media_position" => 2)) end - def test_if_with_comparison_and_logical_operator - template = Liquid::Template.parse("{% if a > 5 and b < 10 %}valid{% else %}invalid{% endif %}") - assert_equal("valid", template.render("a" => 7, "b" => 8)) - assert_equal("invalid", template.render("a" => 3, "b" => 8)) - assert_equal("invalid", template.render("a" => 7, "b" => 12)) + def test_equality_operators + assert_parity_todo!("1 == 1", "true") + assert_parity_todo!("1 != 2", "true") + assert_parity_todo!("'hello' == 'hello'", "true") end - # Basic nil rendering tests def test_nil_renders_as_empty_string - template = Liquid::Template.parse("{{ nil }}") - assert_equal("", template.render) - end - - def test_nil_in_assigned_variable_renders_as_empty_string - template = Liquid::Template.parse("{% assign x = nil %}{{ x }}") - assert_equal("", template.render) - end - - # Nil comparison tests - def test_nil_equals_nil - template = Liquid::Template.parse("{{ nil == nil }}") - assert_equal("true", template.render) - end - - def test_nil_not_equals_nil - template = Liquid::Template.parse("{{ nil != nil }}") - assert_equal("false", template.render) - end - - def test_nil_not_equals_empty_string - template = Liquid::Template.parse("{{ nil == '' }}") - assert_equal("false", template.render) - - template = Liquid::Template.parse("{{ nil != '' }}") - assert_equal("true", template.render) - end - - # Variable tests with nil values - def test_variable_with_nil_value_in_comparisons - template = Liquid::Template.parse("{% assign x = nil %}{{ x == nil }}") - assert_equal("true", template.render) - - template = Liquid::Template.parse("{% assign x = nil %}{{ x != nil }}") - assert_equal("false", template.render) - end - - def test_variable_with_nil_value_compared_to_empty_string - template = Liquid::Template.parse("{% assign x = nil %}{{ x == '' }}") - assert_equal("false", template.render) - - template = Liquid::Template.parse("{% assign x = nil %}{{ x != '' }}") - assert_equal("true", template.render) - end - - # Tests with undefined variables - def test_undefined_variable_in_comparisons - template = Liquid::Template.parse("{{ undefined_var == nil }}") - assert_equal("true", template.render) - - template = Liquid::Template.parse("{{ undefined_var != nil }}") - assert_equal("false", template.render) + assert_parity_todo!("nil", "false") end - def test_undefined_variable_compared_to_empty_string - template = Liquid::Template.parse("{{ undefined_var == '' }}") - assert_equal("false", template.render) - - template = Liquid::Template.parse("{{ undefined_var != '' }}") - assert_equal("true", template.render) + 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") end - # Tests with boolean values - def test_boolean_variable_in_comparisons - template = Liquid::Template.parse("{% assign t = true %}{{ t == true }}") - assert_equal("true", template.render) - - template = Liquid::Template.parse("{% assign f = false %}{{ f == false }}") - assert_equal("true", template.render) + def test_if_with_variables + assert_parity_todo!("value", "true", { "value" => true }) + assert_parity_todo!("value", "false", { "value" => false }) + assert_parity_todo!("value", "false", { "value" => nil }) + assert_parity_todo!("value", "true", { "value" => "text" }) + assert_parity_todo!("value", "true", { "value" => "" }) end - def test_boolean_variable_compared_to_nil - template = Liquid::Template.parse("{% assign t = true %}{{ t == nil }}") - assert_equal("false", template.render) - - template = Liquid::Template.parse("{% assign f = false %}{{ f == nil }}") - assert_equal("false", template.render) - - template = Liquid::Template.parse("{% assign f = false %}{{ f != nil }}") - assert_equal("true", template.render) + def test_nil_variable_in_and_expression + assert_parity_todo!("x and true", "false", { "x" => nil }) end - # Mixed comparison tests - def test_nil_and_undefined_variables_in_boolean_expressions - template = Liquid::Template.parse("{% assign x = nil %}{{ x == undefined_var }}") - assert_equal("true", template.render) + private - template = Liquid::Template.parse("{% assign x = nil %}{{ x != undefined_var }}") - assert_equal("false", template.render) + def assert_parity_todo!(liquid_expression, expected_result, args = {}) + assert_parity_scenario(:condition, "{% if #{liquid_expression} %}true{% else %}false{% endif %}", expected_result, args) + test_name = caller_locations(1, 1)[0].label + puts "\e[33mTODO: parity for '#{test_name}'\e[0m" end - def test_nil_variable_in_and_expression - template = Liquid::Template.parse("{% assign x = nil %}{{ x and true }}") - assert_equal("false", template.render) + def assert_parity(liquid_expression, expected_result, args = {}) + assert_parity_scenario(:condition, "{% if #{liquid_expression} %}true{% else %}false{% endif %}", expected_result, args) + assert_parity_scenario(:expression, "{{ #{liquid_expression} }}", expected_result, args) end - def test_nil_literal_in_or_expression - template = Liquid::Template.parse("{% assign x = nil %}{{ nil or true }}") - assert_equal("true", template.render) - end + def assert_parity_scenario(kind, template, exp_output, args = {}) + act_output = Liquid::Template.parse(template).render(args) - def test_nil_variable_in_or_expression - template = Liquid::Template.parse("{% assign x = nil %}{{ x or false }}") - assert_equal("false", template.render) + assert_equal(exp_output, act_output, <<~ERROR_MESSAGE) + #{kind.to_s.capitalize} template failure: + --- + #{template} + --- + args: #{args.inspect} + ERROR_MESSAGE end end From b57f4fcbcbf3cf84c7e6b68f114b0a7a93165615 Mon Sep 17 00:00:00 2001 From: Albert Chu Date: Wed, 5 Mar 2025 15:33:17 -0700 Subject: [PATCH 10/22] Support usecase where a nil variable value is used in a logical expression --- lib/liquid/expression.rb | 10 ++++---- lib/liquid/expression/logical_expression.rb | 4 ++-- lib/liquid/variable_lookup.rb | 20 +++++++++++++--- test/unit/boolean_unit_test.rb | 26 ++++++++++++++++----- 4 files changed, 44 insertions(+), 16 deletions(-) diff --git a/lib/liquid/expression.rb b/lib/liquid/expression.rb index f1293bd7f..c36583c1b 100644 --- a/lib/liquid/expression.rb +++ b/lib/liquid/expression.rb @@ -28,7 +28,7 @@ class Expression 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 @@ def parse(markup, ss = StringScanner.new(""), cache = nil) 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 @@ def inner_parse(markup, ss, cache) 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 3ac061f01..47bd6f5ba 100644 --- a/lib/liquid/expression/logical_expression.rb +++ b/lib/liquid/expression/logical_expression.rb @@ -26,7 +26,7 @@ def parse(markup, ss, cache) 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 @@ def parse(markup, ss, cache) 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 340c0b66d..e4167f2b3 100644 --- a/lib/liquid/variable_lookup.rb +++ b/lib/liquid/variable_lookup.rb @@ -5,12 +5,14 @@ class VariableLookup 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 @@ def lookup_command?(lookup_index) 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 @@ def ==(other) 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 7e0822de2..492a2c424 100644 --- a/test/unit/boolean_unit_test.rb +++ b/test/unit/boolean_unit_test.rb @@ -106,10 +106,10 @@ def test_nil_renders_as_empty_string 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 @@ def test_if_with_variables 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 @@ def assert_parity_todo!(liquid_expression, expected_result, args = {}) 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) From b31f24bdf02af357876152a6d57f799ec187e07a Mon Sep 17 00:00:00 2001 From: Albert Chu Date: Wed, 5 Mar 2025 16:36:23 -0700 Subject: [PATCH 11/22] More boolean unit tests and enabled more existing parity cases --- test/unit/boolean_unit_test.rb | 27 +++++++++++++++++++-------- 1 file changed, 19 insertions(+), 8 deletions(-) diff --git a/test/unit/boolean_unit_test.rb b/test/unit/boolean_unit_test.rb index 492a2c424..84c29fea6 100644 --- a/test/unit/boolean_unit_test.rb +++ b/test/unit/boolean_unit_test.rb @@ -96,13 +96,17 @@ def test_boolean_assignment_shorthand end def test_equality_operators - assert_parity_todo!("1 == 1", "true") - assert_parity_todo!("1 != 2", "true") + assert_parity("1 == 1", "true") + assert_parity("1 != 2", "true") assert_parity_todo!("'hello' == 'hello'", "true") end def test_nil_renders_as_empty_string - assert_parity_todo!("nil", "false") + # No parity needed here. This is to ensure expressions rendered with {{ }} + # will still render as an empty string to preserve pre-existing behavior. + assert_expression("nil", "") + assert_expression("x", "", { "x" => nil }) + assert_parity_scenario(:expression, "hello {{ x }}", "hello ", { "x" => nil }) end def test_nil_comparison_with_blank @@ -113,11 +117,8 @@ def test_nil_comparison_with_blank end def test_if_with_variables - assert_parity_todo!("value", "true", { "value" => true }) - assert_parity_todo!("value", "false", { "value" => false }) - assert_parity_todo!("value", "false", { "value" => nil }) - assert_parity_todo!("value", "true", { "value" => "text" }) - assert_parity_todo!("value", "true", { "value" => "" }) + assert_parity("value", "true", { "value" => true }) + assert_parity("value", "false", { "value" => false }) end def test_nil_variable_in_and_expression @@ -130,6 +131,16 @@ def test_boolean_variable_in_and_expression assert_parity("x and true", "false", { "x" => false }) 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 }) + end + + def test_multi_variable_boolean_nil_or_expression + assert_parity("x or y", "true", { "x" => nil, "y" => true }) + assert_parity("y or x", "true", { "x" => true, "y" => nil }) + end + private def assert_parity_todo!(liquid_expression, expected_result, args = {}) From 9e6b628a68cf212c5d7c10f9d4170994671418ae Mon Sep 17 00:00:00 2001 From: Guilherme Carreiro Date: Thu, 6 Mar 2025 12:08:24 +0100 Subject: [PATCH 12/22] * Introduce support for literal comparisons (e.g., `{{ 'hello' == 'hello' }}`) * Evaluate expressions as truthy/falsy to unlock scenarios, such as `
` * Add additional scenarios to the expression test suite * Simplify `LogicalExpression` --- lib/liquid/expression.rb | 20 +++-- lib/liquid/expression/logical_expression.rb | 43 +++++----- lib/liquid/variable_lookup.rb | 20 +---- test/unit/boolean_unit_test.rb | 87 +++++++++++++++++++-- 4 files changed, 112 insertions(+), 58 deletions(-) diff --git a/lib/liquid/expression.rb b/lib/liquid/expression.rb index c36583c1b..29cfc09d4 100644 --- a/lib/liquid/expression.rb +++ b/lib/liquid/expression.rb @@ -26,31 +26,29 @@ class Expression 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 @@ def inner_parse(markup, ss, cache, logical_expression = false) 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 47bd6f5ba..9fcfa382e 100644 --- a/lib/liquid/expression/logical_expression.rb +++ b/lib/liquid/expression/logical_expression.rb @@ -19,34 +19,20 @@ def boolean_operator?(markup) 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 @@ def parse(markup, ss, cache) 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 e4167f2b3..340c0b66d 100644 --- a/lib/liquid/variable_lookup.rb +++ b/lib/liquid/variable_lookup.rb @@ -5,14 +5,12 @@ class VariableLookup 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 @@ def lookup_command?(lookup_index) 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 @@ def ==(other) 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 84c29fea6..383217e86 100644 --- a/test/unit/boolean_unit_test.rb +++ b/test/unit/boolean_unit_test.rb @@ -95,10 +95,58 @@ def test_boolean_assignment_shorthand 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 @@ def test_if_with_variables 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 From 263a73bd81bbab37a087720ce5f07791c48b1bba Mon Sep 17 00:00:00 2001 From: Guilherme Carreiro Date: Thu, 6 Mar 2025 12:20:18 +0100 Subject: [PATCH 13/22] Remove 'assert_parity_todo!' --- test/unit/boolean_unit_test.rb | 6 ------ 1 file changed, 6 deletions(-) diff --git a/test/unit/boolean_unit_test.rb b/test/unit/boolean_unit_test.rb index 383217e86..7bbd425a0 100644 --- a/test/unit/boolean_unit_test.rb +++ b/test/unit/boolean_unit_test.rb @@ -214,12 +214,6 @@ def test_multi_variable_boolean_nil_or_expression private - def assert_parity_todo!(liquid_expression, expected_result, args = {}) - assert_parity_scenario(:condition, "{% if #{liquid_expression} %}true{% else %}false{% endif %}", expected_result, args) - test_name = caller_locations(1, 1)[0].label - puts "\e[33mTODO: parity for '#{test_name}'\e[0m" - end - def assert_parity(liquid_expression, expected_result, args = {}) assert_condition(liquid_expression, expected_result, args) assert_expression(liquid_expression, expected_result, args) From 51a05c2781b5ef43d77c1b2f4012420340ee6ed9 Mon Sep 17 00:00:00 2001 From: Guilherme Carreiro Date: Fri, 7 Mar 2025 14:26:45 +0100 Subject: [PATCH 14/22] Blank parity --- test/unit/boolean_unit_test.rb | 170 +++++++++++++++++++++++++++++++++ 1 file changed, 170 insertions(+) diff --git a/test/unit/boolean_unit_test.rb b/test/unit/boolean_unit_test.rb index 7bbd425a0..7e85a31d8 100644 --- a/test/unit/boolean_unit_test.rb +++ b/test/unit/boolean_unit_test.rb @@ -212,6 +212,162 @@ def test_multi_variable_boolean_nil_or_expression assert_parity("y or x", "true", { "x" => true, "y" => nil }) end + def test_links_not_blank_with_drop_returns_true_for_all_cases + link = LinkDrop.new( + levels: 0, + links: [ + LinkDrop.new(levels: 1, links: [], title: "About", type: "page_link", url: "/pages/about"), + LinkDrop.new(levels: 1, links: [], title: "Contact", type: "page_link", url: "/pages/contact"), + ], + title: "Main Menu", + type: "menu", + url: nil, + ) + + template = <<~LIQUID + {%- if link.links != blank -%} + true + {%- else -%} + false + {%- endif -%} + LIQUID + + act_output = Liquid::Template.parse(template).render({ "link" => link }) + assert_equal("true", act_output) + + act_output = Liquid::Template.parse(template).render({ "link" => link.tap { |l| l.links = [] } }) + assert_equal("true", act_output) + + act_output = Liquid::Template.parse(template).render({ "link" => link.tap { |l| l.links = nil } }) + assert_equal("true", act_output) + + act_output = Liquid::Template.parse(template).render({ "link" => LinkDrop.new }) + assert_equal("true", act_output) + end + + def test_links_truthy_with_drop_returns_false_for_nil_and_empty_drop + link = LinkDrop.new( + levels: 0, + links: [ + LinkDrop.new(levels: 1, links: [], title: "About", type: "page_link", url: "/pages/about"), + LinkDrop.new(levels: 1, links: [], title: "Contact", type: "page_link", url: "/pages/contact"), + ], + title: "Main Menu", + type: "menu", + url: nil, + ) + + template = <<~LIQUID + {%- if link.links -%} + true + {%- else -%} + false + {%- endif -%} + LIQUID + + act_output = Liquid::Template.parse(template).render({ "link" => link }) + assert_equal("true", act_output) + + act_output = Liquid::Template.parse(template).render({ "link" => link.tap { |l| l.links = [] } }) + assert_equal("true", act_output) + + act_output = Liquid::Template.parse(template).render({ "link" => link.tap { |l| l.links = nil } }) + assert_equal("false", act_output) + + act_output = Liquid::Template.parse(template).render({ "link" => {} }) + assert_equal("false", act_output) + end + + def test_links_not_blank_with_hash_returns_true_for_all_cases + link = { + "levels" => 0, + "links" => [ + { + "levels" => 1, + "links" => [], + "title" => { "text" => "About" }, + "type" => "page_link", + "url" => "/pages/about", + }, + { + "levels" => 1, + "links" => [], + "title" => { "text" => "Contact" }, + "type" => "page_link", + "url" => "/pages/contact", + }, + ], + "title" => { "text" => "Main Menu" }, + "type" => "menu", + "url" => nil, + } + + template = <<~LIQUID + {%- if link.links != blank -%} + true + {%- else -%} + false + {%- endif -%} + LIQUID + + act_output = Liquid::Template.parse(template).render({ "link" => link }) + assert_equal("true", act_output) + + act_output = Liquid::Template.parse(template).render({ "link" => { **link, "links" => [] } }) + assert_equal("true", act_output) + + act_output = Liquid::Template.parse(template).render({ "link" => { **link, "links" => nil } }) + assert_equal("true", act_output) + + act_output = Liquid::Template.parse(template).render({ "link" => {} }) + assert_equal("true", act_output) + end + + def test_links_truthy_with_hash_returns_false_for_nil_and_empty_hash + link = { + "levels" => 0, + "links" => [ + { + "levels" => 1, + "links" => [], + "title" => { "text" => "About" }, + "type" => "page_link", + "url" => "/pages/about", + }, + { + "levels" => 1, + "links" => [], + "title" => { "text" => "Contact" }, + "type" => "page_link", + "url" => "/pages/contact", + }, + ], + "title" => { "text" => "Main Menu" }, + "type" => "menu", + "url" => nil, + } + + template = <<~LIQUID + {%- if link.links -%} + true + {%- else -%} + false + {%- endif -%} + LIQUID + + act_output = Liquid::Template.parse(template).render({ "link" => link }) + assert_equal("true", act_output) + + act_output = Liquid::Template.parse(template).render({ "link" => { **link, "links" => [] } }) + assert_equal("true", act_output) + + act_output = Liquid::Template.parse(template).render({ "link" => { **link, "links" => nil } }) + assert_equal("false", act_output) + + act_output = Liquid::Template.parse(template).render({ "link" => {} }) + assert_equal("false", act_output) + end + private def assert_parity(liquid_expression, expected_result, args = {}) @@ -238,4 +394,18 @@ def assert_parity_scenario(kind, template, exp_output, args = {}) args: #{args.inspect} ERROR_MESSAGE end + + class LinkDrop < Liquid::Drop + attr_accessor :levels, :links, :title, :type, :url + + def initialize(levels: nil, links: nil, title: nil, type: nil, url: nil) + super() + + @levels = levels + @links = links + @title = title + @type = type + @url = url + end + end end From b5c3d3fe8296014a2ccc801d2c3ca17298970299 Mon Sep 17 00:00:00 2001 From: Albert Chu Date: Mon, 10 Mar 2025 18:11:26 -0600 Subject: [PATCH 15/22] Introduced debugging gems --- Gemfile | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Gemfile b/Gemfile index a404c0d4b..c56416f60 100644 --- a/Gemfile +++ b/Gemfile @@ -25,6 +25,8 @@ group :development do end group :test do + gem 'ruby-lsp' + gem 'debug' gem 'rubocop', '~> 1.61.0' gem 'rubocop-shopify', '~> 2.12.0', require: false gem 'rubocop-performance', require: false From 08d36b09a2258afb1a675b0c1f994ee541d9fd33 Mon Sep 17 00:00:00 2001 From: Albert Chu Date: Mon, 10 Mar 2025 18:12:41 -0600 Subject: [PATCH 16/22] Fixed ComparisonExpression.parse with MethodLiterals --- lib/liquid/condition.rb | 12 ++++++++++-- lib/liquid/expression/comparison_expression.rb | 5 ++--- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/lib/liquid/condition.rb b/lib/liquid/condition.rb index e5c321dca..c482c8ecb 100644 --- a/lib/liquid/condition.rb +++ b/lib/liquid/condition.rb @@ -52,6 +52,10 @@ def self.parse_expression(parse_context, markup) @@method_literals[markup] || parse_context.parse_expression(markup) end + def self.parse(markup, ss, cache) + @@method_literals[markup] || Expression.parse(markup, ss, cache) + end + attr_reader :attachment, :child_condition attr_accessor :left, :operator, :right @@ -112,11 +116,15 @@ def inspect private def equal_variables(left, right) + if left.is_a?(MethodLiteral) && right.is_a?(MethodLiteral) + return left.to_s == right.to_s + end + if left.is_a?(MethodLiteral) if right.respond_to?(left.method_name) return right.send(left.method_name) else - return nil + return left.to_s == right end end @@ -124,7 +132,7 @@ def equal_variables(left, right) if left.respond_to?(right.method_name) return left.send(right.method_name) else - return nil + return right.to_s == left end end diff --git a/lib/liquid/expression/comparison_expression.rb b/lib/liquid/expression/comparison_expression.rb index cac9e85ef..189ea9529 100644 --- a/lib/liquid/expression/comparison_expression.rb +++ b/lib/liquid/expression/comparison_expression.rb @@ -14,10 +14,9 @@ def parse(markup, ss, cache) match = comparison?(markup) if match - left = Expression.parse(match[1].strip, ss, cache) + left = Condition.parse(match[1].strip, ss, cache) operator = match[2].strip - right = Expression.parse(match[3].strip, ss, cache) - + right = Condition.parse(match[3].strip, ss, cache) return Condition.new(left, operator, right) end From 430794dd5a391d29fff8568d24b4ff45468607c5 Mon Sep 17 00:00:00 2001 From: Albert Chu Date: Tue, 11 Mar 2025 17:00:57 -0600 Subject: [PATCH 17/22] Added test for operator reading bug --- test/unit/boolean_unit_test.rb | 37 ++++++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/test/unit/boolean_unit_test.rb b/test/unit/boolean_unit_test.rb index 7e85a31d8..f44413a4d 100644 --- a/test/unit/boolean_unit_test.rb +++ b/test/unit/boolean_unit_test.rb @@ -368,6 +368,43 @@ def test_links_truthy_with_hash_returns_false_for_nil_and_empty_hash assert_equal("false", act_output) end + # Note for Guilherme: + # This test actually does not pass in `main` branch right now however it does pass correctly in our changes. + # I am keeping it because it shows we fixed a bug with existing liquid. + # + # The functionality in main is that this liquid evaluates to false `{% if current_variant.id==variant.id %}selected{%- endif -%}` + # because of the missing whitespace around the boolean operator. + # + # Question for project channel: Do we need to respect the existing behaviour or can we keep this new (-improved, imo) behaviour? + def test_variant_selected_attribute_when_ids_match + # Define the Liquid template focusing on the selected attribute + template = <<~LIQUID + + LIQUID + + # Define the context for the template where the variant should be selected + context = { + "variant" => { + "id" => 420, + "title" => "Default Title", + }, + "current_variant" => { + "id" => 420, + }, + } + + # Expected output + expected_output = <<~HTML + + HTML + + # Render the template with the context + actual_output = Liquid::Template.parse(template).render(context) + + # Assert that the actual output matches the expected output + assert_equal(expected_output.delete("\n"), actual_output.delete("\n")) + end + private def assert_parity(liquid_expression, expected_result, args = {}) From 4b57b2bbd9ac00e950d298c058995c698becfc9e Mon Sep 17 00:00:00 2001 From: Albert Chu Date: Wed, 12 Mar 2025 14:08:45 -0600 Subject: [PATCH 18/22] Reintroduced broken conditional operators behaviour present in liquid main --- .../expression/comparison_expression.rb | 2 +- test/unit/boolean_unit_test.rb | 20 +++++++++---------- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/lib/liquid/expression/comparison_expression.rb b/lib/liquid/expression/comparison_expression.rb index 189ea9529..fb57fd202 100644 --- a/lib/liquid/expression/comparison_expression.rb +++ b/lib/liquid/expression/comparison_expression.rb @@ -3,7 +3,7 @@ module Liquid class Expression class ComparisonExpression - COMPARISON_REGEX = /\A\s*(.+?)\s*(==|!=|<>|<=|>=|<|>|contains)\s*(.+)\s*\z/ + COMPARISON_REGEX = /\A\s*(.+?)\s+(==|!=|<>|<=|>=|<|>|contains)\s+(.+)\s*\z/ class << self def comparison?(markup) diff --git a/test/unit/boolean_unit_test.rb b/test/unit/boolean_unit_test.rb index f44413a4d..20ea382eb 100644 --- a/test/unit/boolean_unit_test.rb +++ b/test/unit/boolean_unit_test.rb @@ -368,15 +368,9 @@ def test_links_truthy_with_hash_returns_false_for_nil_and_empty_hash assert_equal("false", act_output) end - # Note for Guilherme: - # This test actually does not pass in `main` branch right now however it does pass correctly in our changes. - # I am keeping it because it shows we fixed a bug with existing liquid. - # - # The functionality in main is that this liquid evaluates to false `{% if current_variant.id==variant.id %}selected{%- endif -%}` - # because of the missing whitespace around the boolean operator. - # - # Question for project channel: Do we need to respect the existing behaviour or can we keep this new (-improved, imo) behaviour? - def test_variant_selected_attribute_when_ids_match + # TESTING TECHNICALLY INCORRECT BEHAVIOUR OF LIQUID-RUBY + # If liquid-vm fails this test, we should change it. + def test_conditions_with_boolean_operators_without_whitespace_around_operator # Define the Liquid template focusing on the selected attribute template = <<~LIQUID @@ -394,8 +388,14 @@ def test_variant_selected_attribute_when_ids_match } # Expected output + # Note: Ideally we would like the whitespace around the boolean operator to be optional. + # So the more correct expected output would be: + # + # + # + # However, the existing behaviour in liquid-ruby is that the whitespace is required around the boolean operator. expected_output = <<~HTML - + HTML # Render the template with the context From 484f016e1b37b5eaf0a023fb192ded8c317d5bd9 Mon Sep 17 00:00:00 2001 From: Albert Chu Date: Wed, 12 Mar 2025 19:23:18 -0600 Subject: [PATCH 19/22] Added failing unit test that passes in main --- test/unit/boolean_unit_test.rb | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/test/unit/boolean_unit_test.rb b/test/unit/boolean_unit_test.rb index 20ea382eb..b63648b24 100644 --- a/test/unit/boolean_unit_test.rb +++ b/test/unit/boolean_unit_test.rb @@ -405,6 +405,35 @@ def test_conditions_with_boolean_operators_without_whitespace_around_operator assert_equal(expected_output.delete("\n"), actual_output.delete("\n")) end + # TESTING INCORRECT BEHAVIOUR OF LIQUID-RUBY + # If liquid-vm fails this test, we should change it. + def test_boolean_conditional_with_json_filter + # Define the Liquid template to test + template = <<~LIQUID + {{ template.name == 'index' | json }} + LIQUID + + # Define the context for the template where the template name is 'index' + context = { + "template" => { + "name" => "product", + }, + } + + # Expected output + # Note: I dont know what is the correct output here but this is the liquid-ruby 'main' output. + # + # It feels incorrect but I dont know whats better + expected_output = "product" + + # Render the template with the context + actual_parsed_template = Liquid::Template.parse(template) + actual_output = actual_parsed_template.render(context) + + # Assert that the actual output matches the expected output + assert_equal(expected_output, actual_output.strip) + end + private def assert_parity(liquid_expression, expected_result, args = {}) From e6e8221c78218a0971f020b27601edb96ffb8ba1 Mon Sep 17 00:00:00 2001 From: Albert Chu Date: Thu, 13 Mar 2025 10:53:24 -0600 Subject: [PATCH 20/22] Fixed lax parsing test case --- .../expression/comparison_expression.rb | 3 +++ test/unit/boolean_unit_test.rb | 24 +++++++++++-------- 2 files changed, 17 insertions(+), 10 deletions(-) diff --git a/lib/liquid/expression/comparison_expression.rb b/lib/liquid/expression/comparison_expression.rb index fb57fd202..5469c015d 100644 --- a/lib/liquid/expression/comparison_expression.rb +++ b/lib/liquid/expression/comparison_expression.rb @@ -3,6 +3,9 @@ module Liquid class Expression class ComparisonExpression + # We can improve the resiliency of lax parsing by not expecting whitespace + # surrounding the operator (ie \s+ => \s*). + # However this is not in parity with existing lax parsing behavior. COMPARISON_REGEX = /\A\s*(.+?)\s+(==|!=|<>|<=|>=|<|>|contains)\s+(.+)\s*\z/ class << self diff --git a/test/unit/boolean_unit_test.rb b/test/unit/boolean_unit_test.rb index b63648b24..10dae1134 100644 --- a/test/unit/boolean_unit_test.rb +++ b/test/unit/boolean_unit_test.rb @@ -4,7 +4,6 @@ class BooleanUnitTest < Minitest::Test include Liquid - def test_simple_boolean_comparison assert_parity("1 > 0", "true") assert_parity("1 < 0", "false") @@ -368,15 +367,11 @@ def test_links_truthy_with_hash_returns_false_for_nil_and_empty_hash assert_equal("false", act_output) end - # TESTING TECHNICALLY INCORRECT BEHAVIOUR OF LIQUID-RUBY - # If liquid-vm fails this test, we should change it. def test_conditions_with_boolean_operators_without_whitespace_around_operator - # Define the Liquid template focusing on the selected attribute template = <<~LIQUID LIQUID - # Define the context for the template where the variant should be selected context = { "variant" => { "id" => 420, @@ -394,15 +389,24 @@ def test_conditions_with_boolean_operators_without_whitespace_around_operator # # # However, the existing behaviour in liquid-ruby is that the whitespace is required around the boolean operator. - expected_output = <<~HTML + expected_lax_output = <<~HTML HTML - # Render the template with the context - actual_output = Liquid::Template.parse(template).render(context) + expected_strict_output = <<~HTML + + HTML - # Assert that the actual output matches the expected output - assert_equal(expected_output.delete("\n"), actual_output.delete("\n")) + # This bugged output only happens in lax mode. + prev_error_mode = Liquid::Environment.default.error_mode + Liquid::Environment.default.error_mode = :lax + actual_lax_output = Liquid::Template.parse(template).render(context) + Liquid::Environment.default.error_mode = prev_error_mode + + actual_strict_output = Liquid::Template.parse(template).render(context) + + assert_equal(expected_lax_output.delete("\n"), actual_lax_output.delete("\n")) + assert_equal(expected_strict_output.delete("\n"), actual_strict_output.delete("\n")) end # TESTING INCORRECT BEHAVIOUR OF LIQUID-RUBY From 6148604320142124ae6da97eff035bbd007c9433 Mon Sep 17 00:00:00 2001 From: Albert Chu Date: Thu, 13 Mar 2025 13:26:03 -0600 Subject: [PATCH 21/22] Added another failing unit test for behavior in main --- test/unit/boolean_unit_test.rb | 48 ++++++++++++++++++++++++++++++++++ 1 file changed, 48 insertions(+) diff --git a/test/unit/boolean_unit_test.rb b/test/unit/boolean_unit_test.rb index 10dae1134..b3fcf678a 100644 --- a/test/unit/boolean_unit_test.rb +++ b/test/unit/boolean_unit_test.rb @@ -438,8 +438,56 @@ def test_boolean_conditional_with_json_filter assert_equal(expected_output, actual_output.strip) end + # TESTING INCORRECT BEHAVIOUR OF LIQUID-RUBY + # If liquid-vm fails this test, we should change it. + def test_chained_conditional_with_object_contains + # Define the Liquid template to test + template = <<~LIQUID + {{ settings.prefilter_status and template contains 'collection' }} + LIQUID + + # Test with context containing 'collection' + context_with_collection = { + "template" => { + "name" => "collection", + }, + "settings" => { + "prefilter_status" => true, + }, + } + # NOTE: This is a bug that liquid-ruby `main` output returns the first value. + assert_with_lax_parsing(template, "true", context_with_collection) + + # Test with context not containing 'collection' + context_without_collection = { + "template" => { + "name" => "not-collection", + }, + "settings" => { + "prefilter_status" => true, + }, + } + # NOTE: This is a bug that liquid-ruby `main` output returns the first value. + assert_with_lax_parsing(template, "true", context_without_collection) + end + private + def assert_with_lax_parsing(template, expected_output, context = {}) + prev_error_mode = Liquid::Environment.default.error_mode + Liquid::Environment.default.error_mode = :lax + + begin + actual_output = Liquid::Template.parse(template).render(context) + rescue StandardError => e + actual_output = e.message + ensure + Liquid::Environment.default.error_mode = prev_error_mode + end + + assert_equal(expected_output.strip, actual_output.strip) + end + def assert_parity(liquid_expression, expected_result, args = {}) assert_condition(liquid_expression, expected_result, args) assert_expression(liquid_expression, expected_result, args) From 1ae2ff103e98868bf0eb240d9d0d0182b594eaf8 Mon Sep 17 00:00:00 2001 From: Albert Chu Date: Thu, 13 Mar 2025 16:41:17 -0600 Subject: [PATCH 22/22] Added more non-parity unit tests --- test/unit/boolean_unit_test.rb | 31 ++++++++++++++++++++++++------- 1 file changed, 24 insertions(+), 7 deletions(-) diff --git a/test/unit/boolean_unit_test.rb b/test/unit/boolean_unit_test.rb index b3fcf678a..dabf65094 100644 --- a/test/unit/boolean_unit_test.rb +++ b/test/unit/boolean_unit_test.rb @@ -398,14 +398,9 @@ def test_conditions_with_boolean_operators_without_whitespace_around_operator HTML # This bugged output only happens in lax mode. - prev_error_mode = Liquid::Environment.default.error_mode - Liquid::Environment.default.error_mode = :lax - actual_lax_output = Liquid::Template.parse(template).render(context) - Liquid::Environment.default.error_mode = prev_error_mode + assert_with_lax_parsing(template, expected_lax_output, context) - actual_strict_output = Liquid::Template.parse(template).render(context) - - assert_equal(expected_lax_output.delete("\n"), actual_lax_output.delete("\n")) + # Default test parsing mode (strict) works as properly expected assert_equal(expected_strict_output.delete("\n"), actual_strict_output.delete("\n")) end @@ -471,6 +466,28 @@ def test_chained_conditional_with_object_contains assert_with_lax_parsing(template, "true", context_without_collection) end + # TESTING INCORRECT BEHAVIOUR OF LIQUID-RUBY + # If liquid-vm fails this test, we should change it. + def test_assign_boolean_expression_to_variable + template = <<~LIQUID + {%- liquid + assign is_preview_mode = content_for_header contains "foo" or content_for_header contains "bar" + echo is_preview_mode + -%} + LIQUID + + context = { "content_for_header" => "Some content" } + + # Expected output + # This value should be "false" but it is the value of the variable from the failed expression. + assert_template_result("Some content", template, context) + + # This following validation should only be supported with our changes. It is the short-hand for the above template. + # The validation for it is the expected correct output. + template = Liquid::Template.parse("{% assign is_preview_mode = content_for_header contains 'foo' or content_for_header contains 'bar' %}{{ is_preview_mode }}") + assert_equal("false", template.render(context)) + end + private def assert_with_lax_parsing(template, expected_output, context = {})