Merge pull request #2065 from Shopify/strict2-assign-capture

Add strict2_parse to assign and capture tags

lib/liquid/tags/assign.rb

@@ -18,6 +18,8 @@ module Liquid
   # @liquid_syntax_keyword variable_name The name of the variable being created.
   # @liquid_syntax_keyword value The value you want to assign to the variable.
   class Assign < Tag
+    include ParserSwitching
+
     Syntax = /(#{VariableSignature}+)\s*=\s*(.*)\s*/om
 
     # @api private
@@ -29,6 +31,10 @@ module Liquid
 
     def initialize(tag_name, markup, parse_context)
       super
+      parse_with_selected_parser(markup)
+    end
+
+    def lax_parse(markup)
       if markup =~ Syntax
         @to   = Regexp.last_match(1)
         @from = Variable.new(Regexp.last_match(2), parse_context)
@@ -37,6 +43,25 @@ module Liquid
       end
     end
 
+    def strict_parse(markup)
+      lax_parse(markup)
+    end
+
+    def strict2_parse(markup)
+      unless markup =~ Syntax
+        self.class.raise_syntax_error(parse_context)
+      end
+
+      lhs = Regexp.last_match(1).strip
+      rhs = Regexp.last_match(2)
+
+      p = @parse_context.new_parser(lhs)
+      @to = p.consume(:id)
+      p.consume(:end_of_string)
+
+      @from = Variable.new(rhs, parse_context)
+    end
+
     def render_to_output_buffer(context, output)
       val = @from.render(context)
       context.scopes.last[@to] = val

lib/liquid/tags/capture.rb

@@ -20,10 +20,18 @@ module Liquid
   # @liquid_syntax_keyword variable The name of the variable being created.
   # @liquid_syntax_keyword value The value you want to assign to the variable.
   class Capture < Block
+    include ParserSwitching
+
     Syntax = /(#{VariableSignature}+)/o
 
+    attr_reader :to
+
     def initialize(tag_name, markup, options)
       super
+      parse_with_selected_parser(markup)
+    end
+
+    def lax_parse(markup)
       if markup =~ Syntax
         @to = Regexp.last_match(1)
       else
@@ -31,6 +39,16 @@ module Liquid
       end
     end
 
+    def strict_parse(markup)
+      lax_parse(markup)
+    end
+
+    def strict2_parse(markup)
+      p = @parse_context.new_parser(markup.strip)
+      @to = p.consume(:id)
+      p.consume(:end_of_string)
+    end
+
     def render_to_output_buffer(context, output)
       context.resource_limits.with_capture do
         capture_output = render(context)

lib/liquid/tags/include.rb

@@ -20,7 +20,8 @@ module Liquid
   class Include < Tag
     prepend Tag::Disableable
 
-    SYNTAX = /(#{QuotedFragment}+)(\s+(?:with|for)\s+(#{QuotedFragment}+))?(\s+(?:as)\s+(#{VariableSegment}+))?/o
+    FOR = 'for'
+    SYNTAX = /(#{QuotedFragment}+)(\s+(with|#{FOR})\s+(#{QuotedFragment}+))?(\s+(?:as)\s+(#{VariableSegment}+))?/o
     Syntax = SYNTAX
 
     attr_reader :template_name_expr, :variable_name_expr, :attributes
@@ -84,12 +85,18 @@ module Liquid
     alias_method :parse_context, :options
     private :parse_context
 
+    def for_loop?
+      @is_for_loop
+    end
+
     def strict2_parse(markup)
       p = @parse_context.new_parser(markup)
 
       @template_name_expr = safe_parse_expression(p)
-      @variable_name_expr = safe_parse_expression(p) if p.id?("for") || p.id?("with")
+      with_or_for         = p.id?("for") || p.id?("with")
+      @variable_name_expr = safe_parse_expression(p) if with_or_for
       @alias_name         = p.consume(:id) if p.id?("as")
+      @is_for_loop        = (with_or_for == FOR)
 
       p.consume?(:comma)
 
@@ -111,11 +118,13 @@ module Liquid
     def lax_parse(markup)
       if markup =~ SYNTAX
         template_name = Regexp.last_match(1)
-        variable_name = Regexp.last_match(3)
+        with_or_for   = Regexp.last_match(3)
+        variable_name = Regexp.last_match(4)
 
-        @alias_name         = Regexp.last_match(5)
+        @alias_name         = Regexp.last_match(6)
         @variable_name_expr = variable_name ? parse_expression(variable_name) : nil
         @template_name_expr = parse_expression(template_name)
+        @is_for_loop        = (with_or_for == FOR)
         @attributes         = {}
 
         markup.scan(TagAttributes) do |key, value|

test/integration/assign_test.rb

@@ -97,6 +97,46 @@ class AssignTest < Minitest::Test
     assert_equal(12, assign_score_of('int' => 123, 'str' => 'abcd'))
   end
 
+  def test_assign_with_valid_identifier_in_strict2
+    assert_template_result("hello", "{% assign my_var = 'hello' %}{{ my_var }}", error_mode: :strict2)
+  end
+
+  def test_assign_with_hyphen_in_strict2
+    assert_template_result("hello", "{% assign my-var = 'hello' %}{{ my-var }}", error_mode: :strict2)
+  end
+
+  def test_assign_rejects_parentheses_in_variable_name_in_strict2
+    assert_raises(Liquid::SyntaxError) do
+      Liquid::Template.parse("{% assign (a(b(c) = 1234 %}", error_mode: :strict2)
+    end
+  end
+
+  def test_assign_rejects_brackets_in_variable_name_in_strict2
+    assert_raises(Liquid::SyntaxError) do
+      Liquid::Template.parse("{% assign [x.y] = 'hello' %}", error_mode: :strict2)
+    end
+  end
+
+  def test_assign_rejects_dot_in_variable_name_in_strict2
+    assert_raises(Liquid::SyntaxError) do
+      Liquid::Template.parse("{% assign a.b = 'hello' %}", error_mode: :strict2)
+    end
+  end
+
+  def test_assign_rejects_numeric_variable_name_in_strict2
+    assert_raises(Liquid::SyntaxError) do
+      Liquid::Template.parse("{% assign 1abc = 'hello' %}", error_mode: :strict2)
+    end
+  end
+
+  def test_assign_allows_invalid_names_in_lax
+    assert_template_result("1234", "{% assign (a(b(c) = 1234 %}{{ self['(a(b(c)'] }}", error_mode: :lax)
+  end
+
+  def test_assign_with_filter_in_strict2
+    assert_template_result("HELLO", "{% assign my_var = 'hello' | upcase %}{{ my_var }}", error_mode: :strict2)
+  end
+
   private
 
   class ObjectWrapperDrop < Liquid::Drop

test/integration/capture_test.rb

@@ -6,7 +6,11 @@ class CaptureTest < Minitest::Test
   include Liquid
 
   def test_captures_block_content_in_variable
-    assert_template_result("test string", "{% capture 'var' %}test string{% endcapture %}{{var}}", {})
+    assert_template_result("test string", "{% capture var %}test string{% endcapture %}{{var}}", {})
+  end
+
+  def test_captures_block_content_in_quoted_variable_in_lax
+    assert_template_result("test string", "{% capture 'var' %}test string{% endcapture %}{{var}}", {}, error_mode: :lax)
   end
 
   def test_capture_with_hyphen_in_variable_name
@@ -49,4 +53,35 @@ class CaptureTest < Minitest::Test
     t.render!
     assert_equal(9, t.resource_limits.assign_score)
   end
+
+  def test_capture_with_valid_identifier_in_strict2
+    assert_template_result("hello", "{% capture my_var %}hello{% endcapture %}{{ my_var }}", error_mode: :strict2)
+  end
+
+  def test_capture_with_hyphen_in_strict2
+    assert_template_result("hello", "{% capture my-var %}hello{% endcapture %}{{ my-var }}", error_mode: :strict2)
+  end
+
+  def test_capture_rejects_parentheses_in_variable_name_in_strict2
+    assert_raises(Liquid::SyntaxError) do
+      Liquid::Template.parse("{% capture (x[y %}hello{% endcapture %}", error_mode: :strict2)
+    end
+  end
+
+  def test_capture_rejects_dot_in_variable_name_in_strict2
+    assert_raises(Liquid::SyntaxError) do
+      Liquid::Template.parse("{% capture a.b %}hello{% endcapture %}", error_mode: :strict2)
+    end
+  end
+
+  def test_capture_rejects_numeric_variable_name_in_strict2
+    assert_raises(Liquid::SyntaxError) do
+      Liquid::Template.parse("{% capture 1abc %}hello{% endcapture %}", error_mode: :strict2)
+    end
+  end
+
+  def test_capture_allows_invalid_names_in_lax
+    t = Liquid::Template.parse("{% capture (x[y %}hello{% endcapture %}", error_mode: :lax)
+    assert_equal("(x[y", t.root.nodelist.first.to)
+  end
 end

test/integration/tags/cycle_tag_test.rb

@@ -105,10 +105,8 @@ class CycleTagTest < Minitest::Test
       error1 = assert_raises(Liquid::SyntaxError) { Template.parse(template1) }
       error2 = assert_raises(Liquid::SyntaxError) { Template.parse(template2) }
 
-      expected_error = /Liquid syntax error: \[:dot, "."\] is not a valid expression/
+      assert_match(/Liquid syntax error:/, error1.message)
-
+      assert_match(/Liquid syntax error: \[:dot, "."\] is not a valid expression/, error2.message)
-      assert_match(expected_error, error1.message)
-      assert_match(expected_error, error2.message)
     end
   end
 

test/integration/tags/include_tag_test.rb

@@ -439,4 +439,49 @@ class IncludeTagTest < Minitest::Test
       assert_match(/Unexpected character =/, error.message)
     end
   end
+
+  def test_include_for_loop_true_with_for_keyword
+    with_error_modes(:lax, :strict, :strict2) do
+      template = Template.parse("{% include 'product' for products %}")
+      include_node = template.root.nodelist.first
+
+      assert(include_node.for_loop?, "Expected for_loop? to be true for 'for' keyword")
+    end
+  end
+
+  def test_include_for_loop_false_with_with_keyword
+    with_error_modes(:lax, :strict, :strict2) do
+      template = Template.parse("{% include 'product' with product %}")
+      include_node = template.root.nodelist.first
+
+      refute(include_node.for_loop?, "Expected for_loop? to be false for 'with' keyword")
+    end
+  end
+
+  def test_include_for_loop_false_without_keyword
+    with_error_modes(:lax, :strict, :strict2) do
+      template = Template.parse("{% include 'header' %}")
+      include_node = template.root.nodelist.first
+
+      refute(include_node.for_loop?, "Expected for_loop? to be false when no keyword")
+    end
+  end
+
+  def test_include_for_loop_with_alias
+    with_error_modes(:lax, :strict, :strict2) do
+      template = Template.parse("{% include 'product' for products as item %}")
+      include_node = template.root.nodelist.first
+
+      assert(include_node.for_loop?, "Expected for_loop? to be true for 'for' with alias")
+    end
+  end
+
+  def test_include_with_keyword_and_alias
+    with_error_modes(:lax, :strict, :strict2) do
+      template = Template.parse("{% include 'product' with products[0] as item %}")
+      include_node = template.root.nodelist.first
+
+      refute(include_node.for_loop?, "Expected for_loop? to be false for 'with' with alias")
+    end
+  end
 end # IncludeTagTest