diff --git a/lib/liquid/variable.rb b/lib/liquid/variable.rb index f355d694..3fd947a7 100644 --- a/lib/liquid/variable.rb +++ b/lib/liquid/variable.rb @@ -66,12 +66,23 @@ module Liquid return if p.look(:end_of_string) @name = parse_context.safe_parse_expression(p) - @filters << strict_parse_filter_expressions(p) while p.consume?(:pipe) + while p.consume?(:pipe) + filtername = p.consume(:id) + filterargs = p.consume?(:colon) ? parse_filterargs(p) : Const::EMPTY_ARRAY + @filters << lax_parse_filter_expressions(filtername, filterargs) + end p.consume(:end_of_string) end def rigid_parse(markup) - strict_parse(markup) + @filters = [] + p = @parse_context.new_parser(markup) + + return if p.look(:end_of_string) + + @name = parse_context.safe_parse_expression(p) + @filters << rigid_parse_filter_expressions(p) while p.consume?(:pipe) + p.consume(:end_of_string) end def parse_filterargs(p) @@ -145,7 +156,7 @@ module Liquid # argument = (positional_argument | keyword_argument) # positional_argument = expression # keyword_argument = id ":" expression - def strict_parse_filter_expressions(p) + def rigid_parse_filter_expressions(p) filtername = p.consume(:id) filter_args = [] keyword_args = {} diff --git a/test/integration/variable_test.rb b/test/integration/variable_test.rb index 19922c19..740ea9f9 100644 --- a/test/integration/variable_test.rb +++ b/test/integration/variable_test.rb @@ -209,4 +209,69 @@ class VariableTest < Minitest::Test end end end + + def test_filter_with_single_trailing_comma + template = '{{ "hello" | append: "world", }}' + + with_error_mode(:strict) do + error = assert_raises(Liquid::SyntaxError) { Template.parse(template) } + assert_match(/is not a valid expression/, error.message) + end + + with_error_mode(:rigid) do + assert_template_result('helloworld', template) + end + end + + def test_multiple_filters_with_trailing_commas + template = '{{ "hello" | append: "1", | append: "2", }}' + + with_error_mode(:strict) do + error = assert_raises(Liquid::SyntaxError) { Template.parse(template) } + assert_match(/is not a valid expression/, error.message) + end + + with_error_mode(:rigid) do + assert_template_result('hello12', template) + end + end + + def test_filter_with_colon_but_no_arguments + template = '{{ "test" | upcase: }}' + + with_error_mode(:strict) do + error = assert_raises(Liquid::SyntaxError) { Template.parse(template) } + assert_match(/is not a valid expression/, error.message) + end + + with_error_mode(:rigid) do + assert_template_result('TEST', template) + end + end + + def test_filter_chain_with_colon_no_args + template = '{{ "test" | append: "x" | upcase: }}' + + with_error_mode(:strict) do + error = assert_raises(Liquid::SyntaxError) { Template.parse(template) } + assert_match(/is not a valid expression/, error.message) + end + + with_error_mode(:rigid) do + assert_template_result('TESTX', template) + end + end + + def test_combining_trailing_comma_and_empty_args + template = '{{ "test" | append: "x", | upcase: }}' + + with_error_mode(:strict) do + error = assert_raises(Liquid::SyntaxError) { Template.parse(template) } + assert_match(/is not a valid expression/, error.message) + end + + with_error_mode(:rigid) do + assert_template_result('TESTX', template) + end + end end diff --git a/test/unit/variable_unit_test.rb b/test/unit/variable_unit_test.rb index c8e572c9..82ac0330 100644 --- a/test/unit/variable_unit_test.rb +++ b/test/unit/variable_unit_test.rb @@ -154,33 +154,43 @@ class VariableUnitTest < Minitest::Test end def test_strict_filter_argument_parsing - # optional colon - var = create_variable(%(n | f1 | f2:), error_mode: :strict) - assert_equal([['f1', []], ['f2', []]], var.filters) + with_error_mode(:strict) do + assert_raises(SyntaxError) do + create_variable(%( number_of_comments | pluralize: 'comment': 'comments' )) + end + end + end - # missing argument throws error - assert_raises(SyntaxError) { create_variable(%(n | f1: ,), error_mode: :strict) } - assert_raises(SyntaxError) { create_variable(%(n | f1: ,| f2), error_mode: :strict) } + def test_rigid_filter_argument_parsing + with_error_mode(:rigid) do + # optional colon + var = create_variable(%(n | f1 | f2:)) + assert_equal([['f1', []], ['f2', []]], var.filters) - # arg requires colon - assert_raises(SyntaxError) { create_variable(%(n | f1 1), error_mode: :strict) } + # missing argument throws error + assert_raises(SyntaxError) { create_variable(%(n | f1: ,)) } + assert_raises(SyntaxError) { create_variable(%(n | f1: ,| f2)) } - # trailing comma doesn't throw - create_variable(%(n | f1: 1, 2, 3, | f2:), error_mode: :strict) + # arg requires colon + assert_raises(SyntaxError) { create_variable(%(n | f1 1)) } - # missing comma throws error - assert_raises(SyntaxError) { create_variable(%(n | filter: 1 2, 3), error_mode: :strict) } + # trailing comma doesn't throw + create_variable(%(n | f1: 1, 2, 3, | f2:)) - # positional and kwargs parsing - var = create_variable(%(n | filter: 1, 2, 3 | filter2: k1: 1, k2: 2), error_mode: :strict) - assert_equal([['filter', [1, 2, 3]], ['filter2', [], { "k1" => 1, "k2" => 2 }]], var.filters) + # missing comma throws error + assert_raises(SyntaxError) { create_variable(%(n | filter: 1 2, 3)) } - # positional and kwargs intermixed (pos1, key1: val1, pos2) - var = create_variable(%(n | link_to: class: "black", "https://example.com", title: "title"), error_mode: :strict) - assert_equal([['link_to', ["https://example.com"], { "class" => "black", "title" => "title" }]], var.filters) + # positional and kwargs parsing + var = create_variable(%(n | filter: 1, 2, 3 | filter2: k1: 1, k2: 2)) + assert_equal([['filter', [1, 2, 3]], ['filter2', [], { "k1" => 1, "k2" => 2 }]], var.filters) - # string key throws - assert_raises(SyntaxError) { create_variable(%(n | pluralize: 'comment': 'comments'), error_mode: :strict) } + # positional and kwargs intermixed (pos1, key1: val1, pos2) + var = create_variable(%(n | link_to: class: "black", "https://example.com", title: "title")) + assert_equal([['link_to', ["https://example.com"], { "class" => "black", "title" => "title" }]], var.filters) + + # string key throws + assert_raises(SyntaxError) { create_variable(%(n | pluralize: 'comment': 'comments')) } + end end def test_output_raw_source_of_variable