mirror of
https://github.com/Shopify/liquid.git
synced 2026-09-12 23:40:45 -07:00
Update infrastructure that handles parsing switching:
* Remove development helpers from parse context * Simplify strict_parse_with_error_mode_fallback and update documentation * Add unit tests for `Liquid::Expression` and `Liquid::ParseContext` * Update test helpers to work better with the `:rigid` mode
This commit is contained in:
committed by
Guilherme Carreiro
parent
4cd367d971
commit
738540a601
@@ -55,10 +55,15 @@ module Liquid
|
||||
end
|
||||
|
||||
def parse_expression(markup, safe: false)
|
||||
# todo(guilherme): remove this once rigid mode is fully using safe_parse_expression
|
||||
if !safe && @error_mode == :rigid
|
||||
# raise Liquid::InternalError, "parse_expression is not supported in rigid mode"
|
||||
puts("🚨 parse_expression used in rigid mode")
|
||||
# parse_expression is a widely used API. To maintain backward
|
||||
# compatibility while raising awareness about rigid parser standards,
|
||||
# the safe flag supports API users make a deliberate decision.
|
||||
#
|
||||
# In rigid mode, markup MUST come from a string returned by the parser
|
||||
# (e.g., parser.expression). We're not calling the parser here to
|
||||
# prevent redundant parser overhead.
|
||||
raise Liquid::InternalError, "unsafe parse_expression cannot be used in rigid mode"
|
||||
end
|
||||
|
||||
Expression.parse(markup, @string_scanner, @expression_cache)
|
||||
|
||||
@@ -6,16 +6,14 @@ module Liquid
|
||||
#
|
||||
# It's basically doing the same thing the {#parse_with_selected_parser},
|
||||
# except this will try the strict parser regardless of the error mode,
|
||||
# and fall back to the lax parser if the error mode is lax or warn.
|
||||
# and fall back to the lax parser if the error mode is lax or warn,
|
||||
# except when in rigid mode where it uses the rigid parser.
|
||||
#
|
||||
# @deprecated Use {#parse_with_selected_parser} instead.
|
||||
def strict_parse_with_error_mode_fallback(markup)
|
||||
case parse_context.error_mode
|
||||
when :rigid
|
||||
rigid_parse_with_error_context(markup)
|
||||
else
|
||||
strict_parse_with_error_context(markup)
|
||||
end
|
||||
return rigid_parse_with_error_context(markup) if rigid_mode?
|
||||
|
||||
strict_parse_with_error_context(markup)
|
||||
rescue SyntaxError => e
|
||||
case parse_context.error_mode
|
||||
when :rigid
|
||||
|
||||
@@ -71,7 +71,6 @@ class ExpressionTest < Minitest::Test
|
||||
|
||||
def test_expression_cache
|
||||
skip("Liquid-C does not support Expression caching") if defined?(Liquid::C) && Liquid::C.enabled
|
||||
skip("Rigid mode does not use Expression caching") if Liquid::Environment.default.error_mode == :rigid
|
||||
|
||||
cache = {}
|
||||
template = <<~LIQUID
|
||||
@@ -93,7 +92,6 @@ class ExpressionTest < Minitest::Test
|
||||
|
||||
def test_expression_cache_with_true_boolean
|
||||
skip("Liquid-C does not support Expression caching") if defined?(Liquid::C) && Liquid::C.enabled
|
||||
skip("Rigid mode does not use Expression caching") if Liquid::Environment.default.error_mode == :rigid
|
||||
|
||||
template = <<~LIQUID
|
||||
{% assign x = 1 %}
|
||||
@@ -118,7 +116,6 @@ class ExpressionTest < Minitest::Test
|
||||
|
||||
def test_expression_cache_with_lru_redux
|
||||
skip("Liquid-C does not support Expression caching") if defined?(Liquid::C) && Liquid::C.enabled
|
||||
skip("Rigid mode does not use Expression caching") if Liquid::Environment.default.error_mode == :rigid
|
||||
|
||||
cache = LruRedux::Cache.new(10)
|
||||
template = <<~LIQUID
|
||||
@@ -140,7 +137,6 @@ class ExpressionTest < Minitest::Test
|
||||
|
||||
def test_disable_expression_cache
|
||||
skip("Liquid-C does not support Expression caching") if defined?(Liquid::C) && Liquid::C.enabled
|
||||
skip("Rigid mode does not use Expression caching") if Liquid::Environment.default.error_mode == :rigid
|
||||
|
||||
template = <<~LIQUID
|
||||
{% assign x = 1 %}
|
||||
@@ -156,6 +152,35 @@ class ExpressionTest < Minitest::Test
|
||||
assert(parse_context.instance_variable_get(:@expression_cache).nil?)
|
||||
end
|
||||
|
||||
def test_safe_parse_with_variable_lookup
|
||||
parse_context = Liquid::ParseContext.new
|
||||
parser = parse_context.new_parser('product.title')
|
||||
result = Liquid::Expression.safe_parse(parser)
|
||||
|
||||
assert_instance_of(Liquid::VariableLookup, result)
|
||||
assert_equal('product', result.name)
|
||||
assert_equal(['title'], result.lookups)
|
||||
end
|
||||
|
||||
def test_safe_parse_with_number
|
||||
parse_context = Liquid::ParseContext.new
|
||||
parser = parse_context.new_parser('42')
|
||||
result = Liquid::Expression.safe_parse(parser)
|
||||
|
||||
assert_equal(42, result)
|
||||
end
|
||||
|
||||
def test_safe_parse_raises_syntax_error_for_invalid_expression
|
||||
parse_context = Liquid::ParseContext.new
|
||||
parser = parse_context.new_parser('')
|
||||
|
||||
error = assert_raises(Liquid::SyntaxError) do
|
||||
Liquid::Expression.safe_parse(parser)
|
||||
end
|
||||
|
||||
assert_match(/is not a valid expression/, error.message)
|
||||
end
|
||||
|
||||
private
|
||||
|
||||
def assert_expression_result(expect, markup, **assigns)
|
||||
|
||||
+5
-3
@@ -82,10 +82,12 @@ module Minitest
|
||||
Environment.dangerously_override(environment, &blk)
|
||||
end
|
||||
|
||||
def with_error_mode(mode)
|
||||
def with_error_mode(*modes)
|
||||
old_mode = Liquid::Environment.default.error_mode
|
||||
Liquid::Environment.default.error_mode = mode
|
||||
yield
|
||||
modes.each do |mode|
|
||||
Liquid::Environment.default.error_mode = mode
|
||||
yield
|
||||
end
|
||||
ensure
|
||||
Liquid::Environment.default.error_mode = old_mode
|
||||
end
|
||||
|
||||
@@ -0,0 +1,123 @@
|
||||
# frozen_string_literal: true
|
||||
|
||||
require 'test_helper'
|
||||
|
||||
class ParseContextUnitTest < Minitest::Test
|
||||
include Liquid
|
||||
|
||||
def test_safe_parse_expression_with_variable_lookup
|
||||
parser_strict = strict_parse_context.new_parser('product.title')
|
||||
result_strict = strict_parse_context.safe_parse_expression(parser_strict)
|
||||
|
||||
parser_rigid = rigid_parse_context.new_parser('product.title')
|
||||
result_rigid = rigid_parse_context.safe_parse_expression(parser_rigid)
|
||||
|
||||
assert_instance_of(VariableLookup, result_strict)
|
||||
assert_equal('product', result_strict.name)
|
||||
assert_equal(['title'], result_strict.lookups)
|
||||
|
||||
assert_instance_of(VariableLookup, result_rigid)
|
||||
assert_equal('product', result_rigid.name)
|
||||
assert_equal(['title'], result_rigid.lookups)
|
||||
end
|
||||
|
||||
def test_safe_parse_expression_raises_syntax_error_for_invalid_expression
|
||||
parser_strict = strict_parse_context.new_parser('')
|
||||
parser_rigid = rigid_parse_context.new_parser('')
|
||||
|
||||
error_strict = assert_raises(Liquid::SyntaxError) do
|
||||
strict_parse_context.safe_parse_expression(parser_strict)
|
||||
end
|
||||
assert_match(/is not a valid expression/, error_strict.message)
|
||||
|
||||
error_rigid = assert_raises(Liquid::SyntaxError) do
|
||||
rigid_parse_context.safe_parse_expression(parser_rigid)
|
||||
end
|
||||
|
||||
assert_match(/is not a valid expression/, error_rigid.message)
|
||||
end
|
||||
|
||||
def test_parse_expression_with_variable_lookup
|
||||
result_strict = strict_parse_context.parse_expression('product.title')
|
||||
|
||||
assert_instance_of(VariableLookup, result_strict)
|
||||
assert_equal('product', result_strict.name)
|
||||
assert_equal(['title'], result_strict.lookups)
|
||||
|
||||
error = assert_raises(Liquid::InternalError) do
|
||||
rigid_parse_context.parse_expression('product.title')
|
||||
end
|
||||
|
||||
assert_match(/unsafe parse_expression cannot be used in rigid mode/, error.message)
|
||||
end
|
||||
|
||||
def test_parse_expression_with_safe_true
|
||||
result_strict = strict_parse_context.parse_expression('product.title', safe: true)
|
||||
|
||||
assert_instance_of(VariableLookup, result_strict)
|
||||
assert_equal('product', result_strict.name)
|
||||
assert_equal(['title'], result_strict.lookups)
|
||||
|
||||
result_rigid = rigid_parse_context.parse_expression('product.title', safe: true)
|
||||
|
||||
assert_instance_of(VariableLookup, result_rigid)
|
||||
assert_equal('product', result_rigid.name)
|
||||
assert_equal(['title'], result_rigid.lookups)
|
||||
end
|
||||
|
||||
def test_parse_expression_with_empty_string
|
||||
result_strict = strict_parse_context.parse_expression('')
|
||||
assert_nil(result_strict)
|
||||
|
||||
error = assert_raises(Liquid::InternalError) do
|
||||
rigid_parse_context.parse_expression('')
|
||||
end
|
||||
|
||||
assert_match(/unsafe parse_expression cannot be used in rigid mode/, error.message)
|
||||
end
|
||||
|
||||
def test_parse_expression_with_empty_string_and_safe_true
|
||||
result_strict = strict_parse_context.parse_expression('', safe: true)
|
||||
assert_nil(result_strict)
|
||||
|
||||
result_rigid = rigid_parse_context.parse_expression('', safe: true)
|
||||
assert_nil(result_rigid)
|
||||
end
|
||||
|
||||
def test_safe_parse_expression_advances_parser_pointer
|
||||
parser = rigid_parse_context.new_parser('foo, bar')
|
||||
|
||||
# safe_parse_expression consumes "foo"
|
||||
first_result = rigid_parse_context.safe_parse_expression(parser)
|
||||
assert_instance_of(VariableLookup, first_result)
|
||||
assert_equal('foo', first_result.name)
|
||||
|
||||
parser.consume(:comma)
|
||||
|
||||
# safe_parse_expression consumes "bar"
|
||||
second_result = rigid_parse_context.safe_parse_expression(parser)
|
||||
assert_instance_of(VariableLookup, second_result)
|
||||
assert_equal('bar', second_result.name)
|
||||
|
||||
parser.consume(:end_of_string)
|
||||
end
|
||||
|
||||
def test_parse_expression_with_whitespace_in_rigid_mode
|
||||
result = rigid_parse_context.parse_expression(' ', safe: true)
|
||||
assert_nil(result)
|
||||
end
|
||||
|
||||
private
|
||||
|
||||
def strict_parse_context
|
||||
@strict_parse_context ||= ParseContext.new(
|
||||
environment: Environment.build(error_mode: :strict),
|
||||
)
|
||||
end
|
||||
|
||||
def rigid_parse_context
|
||||
@rigid_parse_context ||= ParseContext.new(
|
||||
environment: Environment.build(error_mode: :rigid),
|
||||
)
|
||||
end
|
||||
end
|
||||
Reference in New Issue
Block a user