Revert "Merge pull request #458 from Shopify/block-body"

This reverts commit 12d526a05c, reversing
changes made to 263e90e772.

Conflicts:
	lib/liquid/block_body.rb

History.md

@@ -3,7 +3,6 @@
 ## 3.0.0 / not yet released / branch "master"
 
 * ...
-* Block parsing moved to BlockBody class, see #458 [Dylan Thacker-Smith, dylanahsmith]
 * Removed Block#end_tag. Instead, override parse with `super` followed by your code. See #446 [Dylan Thacker-Smith, dylanahsmith]
 * Fixed condition with wrong data types, see #423 [Bogdan Gusiev]
 * Add url_encode to standard filters, see #421 [Derrick Reimer, djreimer]

lib/liquid.rb

@@ -57,7 +57,6 @@ require 'liquid/context'
 require 'liquid/parser_switching'
 require 'liquid/tag'
 require 'liquid/block'
-require 'liquid/block_body'
 require 'liquid/document'
 require 'liquid/variable'
 require 'liquid/variable_lookup'

lib/liquid/block.rb

@@ -1,26 +1,65 @@
 module Liquid
   class Block < Tag
-    def initialize(tag_name, markup, options)
-      super
-      @blank = true
-    end
-
-    def parse(tokens)
-      @body = BlockBody.new
-      while more = parse_body(@body, tokens)
-      end
-    end
-
-    def render(context)
-      @body.render(context)
-    end
+    FullToken         = /\A#{TagStart}\s*(\w+)\s*(.*)?#{TagEnd}\z/om
+    ContentOfVariable = /\A#{VariableStart}(.*)#{VariableEnd}\z/om
+    TAGSTART = "{%".freeze
+    VARSTART = "{{".freeze
 
     def blank?
       @blank
     end
 
-    def nodelist
-      @body.nodelist
+    def parse(tokens)
+      @blank = true
+      @nodelist ||= []
+      @nodelist.clear
+
+      while token = tokens.shift
+        begin
+          unless token.empty?
+            case
+            when token.start_with?(TAGSTART)
+              if token =~ FullToken
+
+                # if we found the proper block delimiter just end parsing here and let the outer block
+                # proceed
+                return if block_delimiter == $1
+
+                # fetch the tag from registered blocks
+                if tag = Template.tags[$1]
+                  markup = token.is_a?(Token) ? token.child($2) : $2
+                  new_tag = tag.parse($1, markup, tokens, @options)
+                  new_tag.line_number = token.line_number if token.is_a?(Token)
+                  @blank &&= new_tag.blank?
+                  @nodelist << new_tag
+                else
+                  # this tag is not registered with the system
+                  # pass it to the current block for special handling or error reporting
+                  unknown_tag($1, $2, tokens)
+                end
+              else
+                raise SyntaxError.new(options[:locale].t("errors.syntax.tag_termination".freeze, :token => token, :tag_end => TagEnd.inspect))
+              end
+            when token.start_with?(VARSTART)
+              new_var = create_variable(token)
+              new_var.line_number = token.line_number if token.is_a?(Token)
+              @nodelist << new_var
+              @blank = false
+            else
+              @nodelist << token
+              @blank &&= (token =~ /\A\s*\z/)
+            end
+          end
+        rescue SyntaxError => e
+          e.set_line_number_from_token(token)
+          raise
+        end
+      end
+
+      # Make sure that it's ok to end parsing in the current block.
+      # Effectively this method will throw an exception unless the current block is
+      # of type Document
+      assert_missing_delimitation!
     end
 
     # warnings of this block and all sub-tags
@@ -57,23 +96,65 @@ module Liquid
       @block_delimiter ||= "end#{block_name}"
     end
 
+    def create_variable(token)
+      token.scan(ContentOfVariable) do |content|
+        markup = token.is_a?(Token) ? token.child(content.first) : content.first
+        return Variable.new(markup, @options)
+      end
+      raise SyntaxError.new(options[:locale].t("errors.syntax.variable_termination".freeze, :token => token, :tag_end => VariableEnd.inspect))
+    end
+
+    def render(context)
+      render_all(@nodelist, context)
+    end
+
     protected
 
-    def parse_body(body, tokens)
-      body.parse(tokens, options) do |end_tag_name, end_tag_params|
-        @blank &&= body.blank?
+    def assert_missing_delimitation!
+      raise SyntaxError.new(options[:locale].t("errors.syntax.tag_never_closed".freeze, :block_name => block_name))
+    end
 
-        return false if end_tag_name == block_delimiter
-        unless end_tag_name
-          raise SyntaxError.new(@options[:locale].t("errors.syntax.tag_never_closed".freeze, :block_name => block_name))
+    def render_all(list, context)
+      output = []
+      context.resource_limits[:render_length_current] = 0
+      context.resource_limits[:render_score_current] += list.length
+
+      list.each do |token|
+        # Break out if we have any unhanded interrupts.
+        break if context.has_interrupt?
+
+        begin
+          # If we get an Interrupt that means the block must stop processing. An
+          # Interrupt is any command that stops block execution such as {% break %}
+          # or {% continue %}
+          if token.is_a? Continue or token.is_a? Break
+            context.push_interrupt(token.interrupt)
+            break
+          end
+
+          token_output = render_token(token, context)
+
+          unless token.is_a?(Block) && token.blank?
+            output << token_output
+          end
+        rescue MemoryError => e
+          raise e
+        rescue ::StandardError => e
+          output << (context.handle_error(e, token))
         end
-
-        # this tag is not registered with the system
-        # pass it to the current block for special handling or error reporting
-        unknown_tag(end_tag_name, end_tag_params, tokens)
       end
 
-      true
+      output.join
+    end
+
+    def render_token(token, context)
+      token_output = (token.respond_to?(:render) ? token.render(context) : token)
+      context.increment_used_resources(:render_length_current, token_output)
+      if context.resource_limits_reached?
+        context.resource_limits[:reached] = true
+        raise MemoryError.new("Memory limits exceeded".freeze)
+      end
+      token_output
     end
   end
 end

lib/liquid/document.rb

@@ -1,24 +1,17 @@
 module Liquid
-  class Document < BlockBody
-    def self.parse(tokens, options)
-      doc = new
-      doc.parse(tokens, options)
-      doc
+  class Document < Block
+    def self.parse(tokens, options={})
+      # we don't need markup to open this block
+      super(nil, nil, tokens, options)
     end
 
-    def parse(tokens, options)
-      super do |end_tag_name, end_tag_params|
-        unknown_tag(end_tag_name, options) if end_tag_name
-      end
+    # There isn't a real delimiter
+    def block_delimiter
+      []
     end
 
-    def unknown_tag(tag, options)
-      case tag
-      when 'else'.freeze, 'end'.freeze
-        raise SyntaxError.new(options[:locale].t("errors.syntax.unexpected_outer_tag".freeze, :tag => tag))
-      else
-        raise SyntaxError.new(options[:locale].t("errors.syntax.unknown_tag".freeze, :tag => tag))
-      end
+    # Document blocks don't need to be terminated since they are not actually opened
+    def assert_missing_delimitation!
     end
   end
 end

lib/liquid/locales/en.yml

@@ -14,8 +14,7 @@
       include: "Error in tag 'include' - Valid syntax: include '[template]' (with|for) [object|collection]"
       unknown_tag: "Unknown tag '%{tag}'"
       invalid_delimiter: "'end' is not a valid delimiter for %{block_name} tags. use %{block_delimiter}"
-      unexpected_else: "%{block_name} tag does not expect 'else' tag"
-      unexpected_outer_tag: "Unexpected outer '%{tag}' tag"
+      unexpected_else: "%{block_name} tag does not expect else tag"
       tag_termination: "Tag '%{token}' was not properly terminated with regexp: %{tag_end}"
       variable_termination: "Variable '%{token}' was not properly terminated with regexp: %{tag_end}"
       tag_never_closed: "'%{block_name}' tag was never closed"

lib/liquid/profiler/hooks.rb

@@ -1,5 +1,5 @@
 module Liquid
-  class BlockBody
+  class Block < Tag
     def render_token_with_profiling(token, context)
       Profiler.profile_token_render(token) do
         render_token_without_profiling(token, context)

lib/liquid/tags/case.rb

@@ -14,18 +14,12 @@ module Liquid
       end
     end
 
-    def parse(tokens)
-      body = BlockBody.new
-      while more = parse_body(body, tokens)
-        body = @blocks.last.attachment
-      end
-    end
-
     def nodelist
-      @blocks.map(&:attachment)
+      @blocks.flat_map(&:attachment)
     end
 
     def unknown_tag(tag, markup, tokens)
+      @nodelist = []
       case tag
       when 'when'.freeze
         record_when_condition(markup)
@@ -43,10 +37,10 @@ module Liquid
         output = ''
         @blocks.each do |block|
           if block.else?
-            return block.attachment.render(context) if execute_else_block
+            return render_all(block.attachment, context) if execute_else_block
           elsif block.evaluate(context)
             execute_else_block = false
-            output << block.attachment.render(context)
+            output << render_all(block.attachment, context)
           end
         end
         output
@@ -56,9 +50,8 @@ module Liquid
     private
 
     def record_when_condition(markup)
-      body = BlockBody.new
-
       while markup
+        # Create a new nodelist and assign it to the new block
         if not markup =~ WhenSyntax
           raise SyntaxError.new(options[:locale].t("errors.syntax.case_invalid_when".freeze))
         end
@@ -66,8 +59,8 @@ module Liquid
         markup = $2
 
         block = Condition.new(@left, '=='.freeze, Expression.parse($1))
-        block.attach(body)
-        @blocks << block
+        block.attach(@nodelist)
+        @blocks.push(block)
       end
     end
 
@@ -77,7 +70,7 @@ module Liquid
       end
 
       block = ElseCondition.new
-      block.attach(BlockBody.new)
+      block.attach(@nodelist)
       @blocks << block
     end
   end

lib/liquid/tags/for.rb

@@ -49,22 +49,20 @@ module Liquid
     def initialize(tag_name, markup, options)
       super
       parse_with_selected_parser(markup)
-      @for_block = BlockBody.new
-    end
-
-    def parse(tokens)
-      if more = parse_body(@for_block, tokens)
-        parse_body(@else_block, tokens)
-      end
+      @nodelist = @for_block = []
     end
 
     def nodelist
-      @else_block ? [@for_block, @else_block] : [@for_block]
+      if @else_block
+        @for_block + @else_block
+      else
+        @for_block
+      end
     end
 
     def unknown_tag(tag, markup, tokens)
       return super unless tag == 'else'.freeze
-      @else_block = BlockBody.new
+      @nodelist = @else_block = []
     end
 
     def render(context)
@@ -112,7 +110,7 @@ module Liquid
             'last'.freeze    => (index == length - 1)
           }
 
-          result << @for_block.render(context)
+          result << render_all(@for_block, context)
 
           # Handle any interrupts if they exist.
           if context.has_interrupt?
@@ -177,7 +175,7 @@ module Liquid
     end
 
     def render_else(context)
-      @else_block ? @else_block.render(context) : ''.freeze
+      return @else_block ? [render_all(@else_block, context)] : ''.freeze
     end
 
     def iterable?(collection)

lib/liquid/tags/if.rb

@@ -20,13 +20,8 @@ module Liquid
       push_block('if'.freeze, markup)
     end
 
-    def parse(tokens)
-      while more = parse_body(@blocks.last.attachment, tokens)
-      end
-    end
-
     def nodelist
-      @blocks.map(&:attachment)
+      @blocks.flat_map(&:attachment)
     end
 
     def unknown_tag(tag, markup, tokens)
@@ -41,7 +36,7 @@ module Liquid
       context.stack do
         @blocks.each do |block|
           if block.evaluate(context)
-            return block.attachment.render(context)
+            return render_all(block.attachment, context)
           end
         end
         ''.freeze
@@ -58,7 +53,7 @@ module Liquid
         end
 
         @blocks.push(block)
-        block.attach(BlockBody.new)
+        @nodelist = block.attach(Array.new)
       end
 
       def lax_parse(markup)

lib/liquid/tags/raw.rb

@@ -3,27 +3,16 @@ module Liquid
     FullTokenPossiblyInvalid = /\A(.*)#{TagStart}\s*(\w+)\s*(.*)?#{TagEnd}\z/om
 
     def parse(tokens)
-      @body = ''
+      @nodelist ||= []
+      @nodelist.clear
       while token = tokens.shift
         if token =~ FullTokenPossiblyInvalid
-          @body << $1 if $1 != "".freeze
+          @nodelist << $1 if $1 != "".freeze
           return if block_delimiter == $2
         end
-        @body << token if not token.empty?
+        @nodelist << token if not token.empty?
       end
     end
-
-    def render(context)
-      @body
-    end
-
-    def nodelist
-      [@body]
-    end
-
-    def blank?
-      @body.empty?
-    end
   end
 
   Template.register_tag('raw'.freeze, Raw)

lib/liquid/tags/unless.rb

@@ -12,13 +12,13 @@ module Liquid
         # First condition is interpreted backwards ( if not )
         first_block = @blocks.first
         unless first_block.evaluate(context)
-          return first_block.attachment.render(context)
+          return render_all(first_block.attachment, context)
         end
 
         # After the first condition unless works just like if
         @blocks[1..-1].each do |block|
           if block.evaluate(context)
-            return block.attachment.render(context)
+            return render_all(block.attachment, context)
           end
         end
 

test/integration/document_test.rb

@@ -1,19 +0,0 @@
-require 'test_helper'
-
-class DocumentTest < Minitest::Test
-  include Liquid
-
-  def test_unexpected_outer_tag
-    exc = assert_raises(SyntaxError) do
-      Template.parse("{% else %}")
-    end
-    assert_equal exc.message, "Liquid syntax error: Unexpected outer 'else' tag"
-  end
-
-  def test_unknown_tag
-    exc = assert_raises(SyntaxError) do
-      Template.parse("{% foo %}")
-    end
-    assert_equal exc.message, "Liquid syntax error: Unknown tag 'foo'"
-  end
-end

test/unit/tags/case_tag_unit_test.rb

@@ -5,6 +5,6 @@ class CaseTagUnitTest < Minitest::Test
 
   def test_case_nodelist
     template = Liquid::Template.parse('{% case var %}{% when true %}WHEN{% else %}ELSE{% endcase %}')
-    assert_equal ['WHEN', 'ELSE'], template.root.nodelist[0].nodelist.map(&:nodelist).flatten
+    assert_equal ['WHEN', 'ELSE'], template.root.nodelist[0].nodelist
   end
 end

test/unit/tags/for_tag_unit_test.rb

@@ -3,11 +3,11 @@ require 'test_helper'
 class ForTagUnitTest < Minitest::Test
   def test_for_nodelist
     template = Liquid::Template.parse('{% for item in items %}FOR{% endfor %}')
-    assert_equal ['FOR'], template.root.nodelist[0].nodelist.map(&:nodelist).flatten
+    assert_equal ['FOR'], template.root.nodelist[0].nodelist
   end
 
   def test_for_else_nodelist
     template = Liquid::Template.parse('{% for item in items %}FOR{% else %}ELSE{% endfor %}')
-    assert_equal ['FOR', 'ELSE'], template.root.nodelist[0].nodelist.map(&:nodelist).flatten
+    assert_equal ['FOR', 'ELSE'], template.root.nodelist[0].nodelist
   end
 end

test/unit/tags/if_tag_unit_test.rb

@@ -3,6 +3,6 @@ require 'test_helper'
 class IfTagUnitTest < Minitest::Test
   def test_if_nodelist
     template = Liquid::Template.parse('{% if true %}IF{% else %}ELSE{% endif %}')
-    assert_equal ['IF', 'ELSE'], template.root.nodelist[0].nodelist.map(&:nodelist).flatten
+    assert_equal ['IF', 'ELSE'], template.root.nodelist[0].nodelist
   end
 end

test/unit/template_unit_test.rb

@@ -5,17 +5,16 @@ class TemplateUnitTest < Minitest::Test
 
   def test_sets_default_localization_in_document
     t = Template.new
-    t.parse('{%comment%}{%endcomment%}')
-    assert_instance_of I18n, t.root.nodelist[0].options[:locale]
+    t.parse('')
+    assert_instance_of I18n, t.root.options[:locale]
   end
 
   def test_sets_default_localization_in_context_with_quick_initialization
     t = Template.new
-    t.parse('{%comment%}{%endcomment%}', :locale => I18n.new(fixture("en_locale.yml")))
+    t.parse('{{foo}}', :locale => I18n.new(fixture("en_locale.yml")))
 
-    locale = t.root.nodelist[0].options[:locale]
-    assert_instance_of I18n, locale
-    assert_equal fixture("en_locale.yml"), locale.path
+    assert_instance_of I18n, t.root.options[:locale]
+    assert_equal fixture("en_locale.yml"), t.root.options[:locale].path
   end
 
   def test_with_cache_classes_tags_returns_the_same_class