Fix variable to keep it backward-compatible in strict mode

* lax_parse    - no changes
  * strict_parse - uses the `lax_parse_filter_expressions` (as it was doing before)
  * rigid_parse  - uses the `rigid_parse_filter_expressions`

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 = {}

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

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