From 0bc69b86b75926b90d68b3dda92cee25a4f3330b Mon Sep 17 00:00:00 2001 From: Guilherme Carreiro Date: Wed, 8 Oct 2025 14:27:01 -0400 Subject: [PATCH] No longer test `ParseContext` directly on `RigidModeUnitTest` as now the `safe: true` calls are considered safe Test the entire template instead --- lib/liquid/parse_context.rb | 6 +- test/unit/rigid_mode_unit_test.rb | 141 +++++++++++++++++------------- 2 files changed, 85 insertions(+), 62 deletions(-) diff --git a/lib/liquid/parse_context.rb b/lib/liquid/parse_context.rb index 82cf5768..1b192d8c 100644 --- a/lib/liquid/parse_context.rb +++ b/lib/liquid/parse_context.rb @@ -56,8 +56,10 @@ module Liquid def parse_expression(markup, safe: false) # todo(guilherme): remove this once rigid mode is fully using safe_parse_expression - # raise Liquid::InternalError, "parse_expression is not supported in rigid mode" if !safe && @error_mode == :rigid - puts("🚨 parse_expression used in rigid mode") if !safe && @error_mode == :rigid + if !safe && @error_mode == :rigid + # raise Liquid::InternalError, "parse_expression is not supported in rigid mode" + puts("🚨 parse_expression used in rigid mode") + end Expression.parse(markup, @string_scanner, @expression_cache) end diff --git a/test/unit/rigid_mode_unit_test.rb b/test/unit/rigid_mode_unit_test.rb index 1071d483..0658c719 100644 --- a/test/unit/rigid_mode_unit_test.rb +++ b/test/unit/rigid_mode_unit_test.rb @@ -6,57 +6,38 @@ class RigidModeUnitTest < Minitest::Test include Liquid def test_direct_parse_expression_comparison - skip("todo(guilherme): parse_context.safe_parse_expression in progress...") - test_cases = [ - 'foo bar', - 'user.name first', - 'items[0] next', - 'products[0].name extra', + '{{ foo bar }}', + '{{ user.name first }}', + '{{ items[0] next }}', + '{{ products[0].name extra }}', ] - test_cases.each do |expr| - ctx_strict = ParseContext.new(environment: strict_env) - result = ctx_strict.parse_expression(expr) - refute_nil(result, "Strict mode should parse '#{expr}'") - - ctx_rigid = ParseContext.new(environment: rigid_env) - error = assert_raises(SyntaxError) do - ctx_rigid.parse_expression(expr) - end - - assert_match(/Expected end_of_string but found id/, error.message) + test_cases.each do |template| + refute_nil(lax_parse(template)) + assert_raises(SyntaxError) { strict_parse(template) } + assert_raises(SyntaxError) { rigid_parse(template) } end end def test_comparison_strict_vs_rigid_with_space_separated_lookups - skip("todo(guilherme): parse_context.safe_parse_expression in progress...") + template = '{{ product title }}' - expr = 'product title' + output = lax_parse(template).render({ 'product' => { 'title' => 'Snow' } }) + assert_equal('{"title"=>"Snow"}', output) - ctx_lax = ParseContext.new(environment: lax_env) - result_lax = ctx_lax.parse_expression(expr) - assert_equal('product', result_lax.name) - assert_equal(['title'], result_lax.lookups) - - ctx_strict = ParseContext.new(environment: strict_env) - result_strict = ctx_strict.parse_expression(expr) - assert_equal('product', result_strict.name) - assert_equal(['title'], result_strict.lookups) - - ctx_rigid = ParseContext.new(environment: rigid_env) - assert_raises(SyntaxError) do - ctx_rigid.parse_expression(expr) - end + assert_raises(SyntaxError) { strict_parse(template) } + assert_raises(SyntaxError) { rigid_parse(template) } end def test_tablerow_limit_with_invalid_expression - skip("todo(guilherme): parse_context.safe_parse_expression in progress...") + skip template = <<~LIQUID {% tablerow i in (1..10) limit: foo=>bar %}{{ i }}{% endtablerow %} LIQUID + refute_nil(lax_parse(template)) refute_nil(strict_parse(template)) error = assert_raises(SyntaxError) do @@ -66,12 +47,13 @@ class RigidModeUnitTest < Minitest::Test end def test_tablerow_offset_with_invalid_expression - skip("todo(guilherme): parse_context.safe_parse_expression in progress...") + skip template = <<~LIQUID {% tablerow i in (1..10) offset: foo=>bar %}{{ i }}{% endtablerow %} LIQUID + refute_nil(lax_parse(template)) refute_nil(strict_parse(template)) error = assert_raises(SyntaxError) do @@ -87,6 +69,7 @@ class RigidModeUnitTest < Minitest::Test {% endfor %} LIQUID + refute_nil(lax_parse(template)) refute_nil(strict_parse(template)) error = assert_raises(SyntaxError) do @@ -102,6 +85,7 @@ class RigidModeUnitTest < Minitest::Test {% endfor %} LIQUID + refute_nil(lax_parse(template)) refute_nil(strict_parse(template)) error = assert_raises(SyntaxError) do @@ -118,6 +102,7 @@ class RigidModeUnitTest < Minitest::Test {% endcase %} LIQUID + refute_nil(lax_parse(template)) refute_nil(strict_parse(template)) error = assert_raises(SyntaxError) do @@ -129,6 +114,7 @@ class RigidModeUnitTest < Minitest::Test def test_include_template_with_invalid_expression template = "{% include foo=>bar %}" + refute_nil(lax_parse(template)) refute_nil(strict_parse(template)) error = assert_raises(SyntaxError) do @@ -140,6 +126,7 @@ class RigidModeUnitTest < Minitest::Test def test_include_with_invalid_expression template = '{% include "snippet" with foo=>bar %}' + refute_nil(lax_parse(template)) refute_nil(strict_parse(template)) error = assert_raises(SyntaxError) do @@ -151,6 +138,7 @@ class RigidModeUnitTest < Minitest::Test def test_include_attribute_with_invalid_expression template = '{% include "snippet", key: foo=>bar %}' + refute_nil(lax_parse(template)) refute_nil(strict_parse(template)) error = assert_raises(SyntaxError) do @@ -162,6 +150,7 @@ class RigidModeUnitTest < Minitest::Test def test_render_with_invalid_expression template = '{% render "snippet" with foo=>bar %}' + refute_nil(lax_parse(template)) refute_nil(strict_parse(template)) error = assert_raises(SyntaxError) do @@ -173,6 +162,7 @@ class RigidModeUnitTest < Minitest::Test def test_render_attribute_with_invalid_expression template = '{% render "snippet", key: foo=>bar %}' + refute_nil(lax_parse(template)) refute_nil(strict_parse(template)) error = assert_raises(SyntaxError) do @@ -193,9 +183,14 @@ class RigidModeUnitTest < Minitest::Test } test_cases.each do |template_str, data| - t = rigid_parse(template_str) - result = t.render(data) - assert(result.is_a?(String), "Should render successfully for '#{template_str}'") + lax_result = lax_parse(template_str).render(data) + assert(lax_result.is_a?(String), "Lax mode should render '#{template_str}'") + + strict_result = strict_parse(template_str).render(data) + assert(strict_result.is_a?(String), "Strict mode should render '#{template_str}'") + + rigid_result = rigid_parse(template_str).render(data) + assert(rigid_result.is_a?(String), "Rigid mode should render '#{template_str}'") end end @@ -204,9 +199,14 @@ class RigidModeUnitTest < Minitest::Test {% for i in (1..3) %}{{ i }}{% endfor %} LIQUID - t = rigid_parse(template) - result = t.render - assert_equal("123\n", result) + lax_result = lax_parse(template).render + assert_equal("123\n", lax_result) + + strict_result = strict_parse(template).render + assert_equal("123\n", strict_result) + + rigid_result = rigid_parse(template).render + assert_equal("123\n", rigid_result) end def test_rigid_mode_with_variable_ranges @@ -214,9 +214,16 @@ class RigidModeUnitTest < Minitest::Test {% for i in (start..end) %}{{ i }}{% endfor %} LIQUID - t = rigid_parse(template) - result = t.render({ 'start' => 1, 'end' => 3 }) - assert_equal("123\n", result) + data = { 'start' => 1, 'end' => 3 } + + lax_result = lax_parse(template).render(data) + assert_equal("123\n", lax_result) + + strict_result = strict_parse(template).render(data) + assert_equal("123\n", strict_result) + + rigid_result = rigid_parse(template).render(data) + assert_equal("123\n", rigid_result) end def test_rigid_mode_valid_filters @@ -224,9 +231,14 @@ class RigidModeUnitTest < Minitest::Test {{ "hello" | upcase | prepend: "Say: " }} LIQUID - t = rigid_parse(template) - result = t.render - assert_equal("Say: HELLO\n", result) + lax_result = lax_parse(template).render + assert_equal("Say: HELLO\n", lax_result) + + strict_result = strict_parse(template).render + assert_equal("Say: HELLO\n", strict_result) + + rigid_result = rigid_parse(template).render + assert_equal("Say: HELLO\n", rigid_result) end def test_rigid_mode_valid_filter_with_correct_variable_args @@ -234,39 +246,48 @@ class RigidModeUnitTest < Minitest::Test {{ "hello" | append: world.name }} LIQUID - t = rigid_parse(template) - result = t.render({ 'world' => { 'name' => ' world' } }) - assert_equal("hello world\n", result) + data = { 'world' => { 'name' => ' world' } } + + lax_result = lax_parse(template).render(data) + assert_equal("hello world\n", lax_result) + + strict_result = strict_parse(template).render(data) + assert_equal("hello world\n", strict_result) + + rigid_result = rigid_parse(template).render(data) + assert_equal("hello world\n", rigid_result) end def test_empty_expression_handling - ctx_rigid = ParseContext.new(environment: rigid_env) - result = ctx_rigid.parse_expression('') - assert_nil(result) + ctx_rigid = ParseContext.new(environment: rigid) - result = ctx_rigid.parse_expression(' ') - assert_nil(result) + assert_nil(ctx_rigid.parse_expression('', safe: true)) + assert_nil(ctx_rigid.parse_expression(' ', safe: true)) end private def rigid_parse(source) - Template.parse(source, environment: rigid_env) + Template.parse(source, environment: rigid) end def strict_parse(source) - Template.parse(source, environment: strict_env) + Template.parse(source, environment: strict) end - def lax_env + def lax_parse(source) + Template.parse(source, environment: lax) + end + + def lax Environment.build(error_mode: :lax) end - def rigid_env + def rigid Environment.build(error_mode: :rigid) end - def strict_env + def strict Environment.build(error_mode: :strict) end end