WIP: Add column number

Resource counting perf

.travis.yml

@@ -1,13 +1,16 @@
+language: ruby
+
 rvm:
-  - 1.9
   - 2.0
   - 2.1
-  - jruby-19mode
+  - ruby-head
   - jruby-head
   - rbx-2
+
+sudo: false
+
 matrix:
   allow_failures:
-    - rvm: rbx-2
     - rvm: jruby-head
 
 script: "rake test"

History.md

@@ -3,6 +3,7 @@
 ## 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]

README.md

@@ -1,5 +1,5 @@
-[![Build Status](https://secure.travis-ci.org/Shopify/liquid.png?branch=master)](http://travis-ci.org/Shopify/liquid)
+[![Build Status](https://api.travis-ci.org/Shopify/liquid.svg?branch=master)](http://travis-ci.org/Shopify/liquid)
-[![Inline docs](http://inch-ci.org/github/Shopify/liquid.png)](http://inch-ci.org/github/Shopify/liquid)
+[![Inline docs](http://inch-ci.org/github/Shopify/liquid.svg?branch=master)](http://inch-ci.org/github/Shopify/liquid)
 
 # Liquid template engine
 

lib/liquid.rb

@@ -57,11 +57,13 @@ 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'
 require 'liquid/range_lookup'
 require 'liquid/file_system'
+require 'liquid/resource_limits'
 require 'liquid/template'
 require 'liquid/standardfilters'
 require 'liquid/condition'
@@ -72,6 +74,3 @@ require 'liquid/token'
 # Load all the tags of the standard library
 #
 Dir[File.dirname(__FILE__) + '/liquid/tags/*.rb'].each { |f| require f }
-
-require 'liquid/profiler'
-require 'liquid/profiler/hooks'

lib/liquid/block.rb

@@ -1,65 +1,26 @@
 module Liquid
   class Block < Tag
-    FullToken         = /\A#{TagStart}\s*(\w+)\s*(.*)?#{TagEnd}\z/om
+    def initialize(tag_name, markup, options)
-    ContentOfVariable = /\A#{VariableStart}(.*)#{VariableEnd}\z/om
+      super
-    TAGSTART = "{%".freeze
+      @blank = true
-    VARSTART = "{{".freeze
+    end
+
+    def parse(tokens)
+      @body = BlockBody.new
+      while more = parse_body(@body, tokens)
+      end
+    end
+
+    def render(context)
+      @body.render(context)
+    end
 
     def blank?
       @blank
     end
 
-    def parse(tokens)
+    def nodelist
-      @blank = true
+      @body.nodelist
-      @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
@@ -96,65 +57,23 @@ 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 assert_missing_delimitation!
+    def parse_body(body, tokens)
-      raise SyntaxError.new(options[:locale].t("errors.syntax.tag_never_closed".freeze, :block_name => block_name))
+      body.parse(tokens, options) do |end_tag_name, end_tag_params|
-    end
+        @blank &&= body.blank?
 
-    def render_all(list, context)
+        return false if end_tag_name == block_delimiter
-      output = []
+        unless end_tag_name
-      context.resource_limits[:render_length_current] = 0
+          raise SyntaxError.new(@options[:locale].t("errors.syntax.tag_never_closed".freeze, :block_name => block_name))
-      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
 
-      output.join
+      true
-    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/block_body.rb

@@ -0,0 +1,131 @@
+module Liquid
+  class BlockBody
+    FullToken = /\A#{TagStart}\s*(\w+)\s*(.*)?#{TagEnd}\z/om
+    ContentOfVariable = /\A#{VariableStart}(.*)#{VariableEnd}\z/om
+    TAGSTART = "{%".freeze
+    VARSTART = "{{".freeze
+
+    attr_reader :nodelist
+
+    def initialize
+      @nodelist = []
+      @blank = true
+    end
+
+    def parse(tokens, options)
+      while token = tokens.shift
+        begin
+          unless token.empty?
+            case
+            when token.start_with?(TAGSTART)
+              if token =~ FullToken
+                tag_name = $1
+                markup = $2
+                # fetch the tag from registered blocks
+                if tag = Template.tags[tag_name]
+                  markup = token.child(markup) if token.is_a?(Token)
+                  new_tag = tag.parse(tag_name, markup, tokens, options)
+                  new_tag.line_number = token.line_number if token.is_a?(Token)
+                  @blank &&= new_tag.blank?
+                  @nodelist << new_tag
+                else
+                  # end parsing if we reach an unknown tag and let the caller decide
+                  # determine how to proceed
+                  return yield tag_name, markup
+                end
+              else
+                raise_missing_tag_terminator(token, options)
+              end
+            when token.start_with?(VARSTART)
+              new_var = create_variable(token, options)
+              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
+
+      yield nil, nil
+    end
+
+    def blank?
+      @blank
+    end
+
+    def warnings
+      all_warnings = []
+      nodelist.each do |node|
+        all_warnings.concat(node.warnings || []) if node.respond_to?(:warnings)
+      end
+      all_warnings
+    end
+
+    def render(context)
+      output = []
+      context.resource_limits.render_score += @nodelist.length
+
+      @nodelist.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
+      end
+
+      output.join
+    end
+
+    private
+
+    def render_token(token, context)
+      token_output = (token.respond_to?(:render) ? token.render(context) : token)
+      token_str = token_output.is_a?(Array) ? token_output.join : token_output.to_s
+
+      context.resource_limits.render_length += token_str.length
+      if context.resource_limits.reached?
+        raise MemoryError.new("Memory limits exceeded".freeze)
+      end
+      token_str
+    end
+
+    def create_variable(token, options)
+      token.scan(ContentOfVariable) do |content|
+        markup = token.is_a?(Token) ? token.child(content.first) : content.first
+        return Variable.new(markup, options)
+      end
+      raise_missing_variable_terminator(token, options)
+    end
+
+    def raise_missing_tag_terminator(token, options)
+      raise SyntaxError.new(options[:locale].t("errors.syntax.tag_termination".freeze, :token => token, :tag_end => TagEnd.inspect))
+    end
+
+    def raise_missing_variable_terminator(token, options)
+      raise SyntaxError.new(options[:locale].t("errors.syntax.variable_termination".freeze, :token => token, :tag_end => VariableEnd.inspect))
+    end
+  end
+end

lib/liquid/context.rb

@@ -21,9 +21,7 @@ module Liquid
       @scopes           = [(outer_scope || {})]
       @registers        = registers
       @errors           = []
-      @resource_limits  = resource_limits || Template.default_resource_limits.dup
+      @resource_limits  = resource_limits || ResourceLimits.new(Template.default_resource_limits)
-      @resource_limits[:render_score_current] = 0
-      @resource_limits[:assign_score_current] = 0
       squash_instance_assigns_with_environments
 
       @this_stack_used = false
@@ -36,20 +34,6 @@ module Liquid
       @filters = []
     end
 
-    def increment_used_resources(key, obj)
-      @resource_limits[key] += if obj.kind_of?(String) || obj.kind_of?(Array) || obj.kind_of?(Hash)
-        obj.length
-      else
-        1
-      end
-    end
-
-    def resource_limits_reached?
-      (@resource_limits[:render_length_limit] && @resource_limits[:render_length_current] > @resource_limits[:render_length_limit]) ||
-      (@resource_limits[:render_score_limit]  && @resource_limits[:render_score_current]  > @resource_limits[:render_score_limit] ) ||
-      (@resource_limits[:assign_score_limit]  && @resource_limits[:assign_score_current]  > @resource_limits[:assign_score_limit] )
-    end
-
     def strainer
       @strainer ||= Strainer.create(self, @filters)
     end

lib/liquid/document.rb

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

lib/liquid/lexer.rb

@@ -13,7 +13,7 @@ module Liquid
       '?'.freeze => :question,
       '-'.freeze => :dash
     }
-    IDENTIFIER = /\w+/
+    IDENTIFIER = /[a-zA-Z_][\w-]*\??/
     SINGLE_STRING_LITERAL = /'[^\']*'/
     DOUBLE_STRING_LITERAL = /"[^\"]*"/
     NUMBER_LITERAL = /-?\d+(\.\d+)?/

lib/liquid/locales/en.yml

@@ -14,7 +14,8 @@
       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_else: "%{block_name} tag does not expect 'else' tag"
+      unexpected_outer_tag: "Unexpected outer '%{tag}' 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/parser.rb

@@ -75,13 +75,6 @@ module Liquid
 
     def variable_signature
       str = consume(:id)
-      while consume?(:dash)
-        str << "-".freeze
-        str << consume(:id)
-      end
-      if consume?(:question)
-        str << "?".freeze
-      end
       if look(:open_square)
         str << consume
         str << expression

lib/liquid/profiler.rb

@@ -1,9 +1,12 @@
+require 'liquid/profiler/hooks'
+
 module Liquid
 
   # Profiler enables support for profiling template rendering to help track down performance issues.
   #
-  # To enable profiling, pass the <tt>profile: true</tt> option to <tt>Liquid::Template.parse</tt>. Then, after
+  # To enable profiling, first require 'liquid/profiler'.
-  # <tt>Liquid::Template#render</tt> is called, the template object makes available an instance of this
+  # Then, to profile a parse/render cycle, pass the <tt>profile: true</tt> option to <tt>Liquid::Template.parse</tt>.
+  # After <tt>Liquid::Template#render</tt> is called, the template object makes available an instance of this
   # class via the <tt>Liquid::Template#profiler</tt> method.
   #
   #   template = Liquid::Template.parse(template_content, profile: true)

lib/liquid/profiler/hooks.rb

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

lib/liquid/resource_limits.rb

@@ -0,0 +1,23 @@
+module Liquid
+  class ResourceLimits
+    attr_accessor :render_length, :render_score, :assign_score,
+                  :render_length_limit, :render_score_limit, :assign_score_limit
+
+    def initialize(limits)
+      @render_length_limit = limits[:render_length_limit]
+      @render_score_limit = limits[:render_score_limit]
+      @assign_score_limit = limits[:assign_score_limit]
+      reset
+    end
+
+    def reached?
+      (@render_length_limit && @render_length > @render_length_limit) ||
+      (@render_score_limit  && @render_score  > @render_score_limit ) ||
+      (@assign_score_limit  && @assign_score  > @assign_score_limit )
+    end
+
+    def reset
+      @render_length = @render_score = @assign_score = 0
+    end
+  end
+end

lib/liquid/tags/assign.rb

@@ -25,7 +25,10 @@ module Liquid
     def render(context)
       val = @from.render(context)
       context.scopes.last[@to] = val
-      context.increment_used_resources(:assign_score_current, val)
+
+      inc = val.instance_of?(String) || val.instance_of?(Array) || val.instance_of?(Hash) ? val.length : 1
+      context.resource_limits.assign_score += inc
+
       ''.freeze
     end
 

lib/liquid/tags/capture.rb

@@ -25,7 +25,7 @@ module Liquid
     def render(context)
       output = super
       context.scopes.last[@to] = output
-      context.increment_used_resources(:assign_score_current, output)
+      context.resource_limits.assign_score += output.length
       ''.freeze
     end
 

lib/liquid/tags/case.rb

@@ -14,12 +14,18 @@ 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.flat_map(&:attachment)
+      @blocks.map(&:attachment)
     end
 
     def unknown_tag(tag, markup, tokens)
-      @nodelist = []
       case tag
       when 'when'.freeze
         record_when_condition(markup)
@@ -37,10 +43,10 @@ module Liquid
         output = ''
         @blocks.each do |block|
           if block.else?
-            return render_all(block.attachment, context) if execute_else_block
+            return block.attachment.render(context) if execute_else_block
           elsif block.evaluate(context)
             execute_else_block = false
-            output << render_all(block.attachment, context)
+            output << block.attachment.render(context)
           end
         end
         output
@@ -50,8 +56,9 @@ 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
@@ -59,8 +66,8 @@ module Liquid
         markup = $2
 
         block = Condition.new(@left, '=='.freeze, Expression.parse($1))
-        block.attach(@nodelist)
+        block.attach(body)
-        @blocks.push(block)
+        @blocks << block
       end
     end
 
@@ -70,7 +77,7 @@ module Liquid
       end
 
       block = ElseCondition.new
-      block.attach(@nodelist)
+      block.attach(BlockBody.new)
       @blocks << block
     end
   end

lib/liquid/tags/for.rb

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

lib/liquid/tags/if.rb

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

lib/liquid/tags/raw.rb

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

@@ -3,7 +3,7 @@ require File.dirname(__FILE__) + '/if'
 module Liquid
   # Unless is a conditional just like 'if' but works on the inverse logic.
   #
-  #   {% unless x < 0 %} x is greater than zero {% end %}
+  #   {% unless x < 0 %} x is greater than zero {% endunless %}
   #
   class Unless < If
     def render(context)
@@ -12,13 +12,13 @@ module Liquid
         # First condition is interpreted backwards ( if not )
         first_block = @blocks.first
         unless first_block.evaluate(context)
-          return render_all(first_block.attachment, context)
+          return first_block.attachment.render(context)
         end
 
         # After the first condition unless works just like if
         @blocks[1..-1].each do |block|
           if block.evaluate(context)
-            return render_all(block.attachment, context)
+            return block.attachment.render(context)
           end
         end
 

lib/liquid/template.rb

@@ -18,7 +18,9 @@ module Liquid
       :locale => I18n.new
     }
 
-    attr_accessor :root, :resource_limits
+    attr_accessor :root
+    attr_reader :resource_limits
+
     @@file_system = BlankFileSystem.new
 
     class TagRegistry
@@ -110,7 +112,7 @@ module Liquid
     end
 
     def initialize
-      @resource_limits = self.class.default_resource_limits.dup
+      @resource_limits = ResourceLimits.new(self.class.default_resource_limits)
     end
 
     # Parse source code.
@@ -203,6 +205,9 @@ module Liquid
         context.add_filters(args.pop)
       end
 
+      # Retrying a render resets resource usage
+      context.resource_limits.reset
+
       begin
         # render the nodelist.
         # for performance reasons we get an array back here. join will make a string out of it.
@@ -241,15 +246,24 @@ module Liquid
       return raw_tokens unless @line_numbers
 
       current_line = 1
+      current_column = 1
       raw_tokens.map do |token|
-        Token.new(token, current_line).tap do
+        Token.new(token, current_line, current_column).tap do
-          current_line += token.count("\n")
+          new_line_count = token.count("\n")
+          if new_line_count > 0
+            current_line += new_line_count
+            current_column = token.size - token.rindex("\n") + 1
+          else
+            current_column += token.size
+          end
         end
       end
     end
 
     def with_profiling
       if @profiling && !@options[:included]
+        raise "Profiler not loaded, require 'liquid/profiler' first" unless defined?(Liquid::Profiler)
+
         @profiler = Profiler.new
         @profiler.start
 

lib/liquid/token.rb

@@ -1,10 +1,11 @@
 module Liquid
   class Token < String
-    attr_reader :line_number
+    attr_reader :line_number, :column_number
 
-    def initialize(content, line_number)
+    def initialize(content, line_number, column_number=nil)
       super(content)
       @line_number = line_number
+      @column_number = column_number
     end
 
     def raw

lib/liquid/variable.rb

@@ -12,7 +12,6 @@ module Liquid
   #
   class Variable
     FilterParser = /(?:\s+|#{QuotedFragment}|#{ArgumentSeparator})+/o
-    EasyParse = /\A *(\w+(?:\.\w+)*) *\z/
     attr_accessor :filters, :name, :warnings
     attr_accessor :line_number
     include ParserSwitching
@@ -53,17 +52,10 @@ module Liquid
     end
 
     def strict_parse(markup)
-      # Very simple valid cases
-      if markup =~ EasyParse
-        @name = Expression.parse($1)
-        @filters = []
-        return
-      end
-
       @filters = []
       p = Parser.new(markup)
-      # Could be just filters with no input
+
-      @name = p.look(:pipe) ? nil : Expression.parse(p.expression)
+      @name = Expression.parse(p.expression)
       while p.consume?(:pipe)
         filtername = p.consume(:id)
         filterargs = p.consume?(:colon) ? parse_filterargs(p) : []

performance/profile.rb

@@ -8,10 +8,17 @@ profiler.run
 [:cpu, :object].each do |profile_type|
   puts "Profiling in #{profile_type.to_s} mode..."
   results = StackProf.run(mode: profile_type) do
-    100.times do
+    200.times do
       profiler.run
     end
   end
+
+  if profile_type == :cpu && graph_filename = ENV['GRAPH_FILENAME']
+    File.open(graph_filename, 'w') do |f|
+      StackProf::Report.new(results).print_graphviz(nil, f)
+    end
+  end
+
   StackProf::Report.new(results).print_text(false, 20)
   File.write(ENV['FILENAME'] + "." + profile_type.to_s, Marshal.dump(results)) if ENV['FILENAME']
 end

test/integration/document_test.rb

@@ -0,0 +1,19 @@
+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/integration/parsing_quirks_test.rb

@@ -28,11 +28,14 @@ class ParsingQuirksTest < Minitest::Test
 
   def test_error_on_empty_filter
     assert Template.parse("{{test}}")
-    assert Template.parse("{{|test}}")
+
+    with_error_mode(:lax) do
+      assert Template.parse("{{|test}}")
+    end
+
     with_error_mode(:strict) do
-      assert_raises(SyntaxError) do
+      assert_raises(SyntaxError) { Template.parse("{{|test}}") }
-        Template.parse("{{test |a|b|}}")
+      assert_raises(SyntaxError) { Template.parse("{{test |a|b|}}") }
-      end
     end
   end
 
@@ -100,4 +103,17 @@ class ParsingQuirksTest < Minitest::Test
     end
   end
 
+  def test_invalid_variables_work
+    with_error_mode(:lax) do
+      assert_template_result('bar', "{% assign 123foo = 'bar' %}{{ 123foo }}")
+      assert_template_result('123', "{% assign 123 = 'bar' %}{{ 123 }}")
+    end
+  end
+
+  def test_extra_dots_in_ranges
+    with_error_mode(:lax) do
+      assert_template_result('12345', "{% for i in (1...5) %}{{ i }}{% endfor %}")
+    end
+  end
+
 end # ParsingQuirksTest

test/integration/template_test.rb

@@ -1,4 +1,5 @@
 require 'test_helper'
+require 'timeout'
 
 class TemplateContextDrop < Liquid::Drop
   def before_method(method)
@@ -37,6 +38,16 @@ class TemplateTest < Minitest::Test
     assert_equal 'from instance assigns', t.parse("{{ foo }}").render!
   end
 
+  def test_warnings_is_not_exponential_time
+    str = "false"
+    100.times do
+      str = "{% if true %}true{% else %}#{str}{% endif %}"
+    end
+
+    t = Template.parse(str)
+    assert_equal [], Timeout::timeout(1) { t.warnings }
+  end
+
   def test_instance_assigns_persist_on_same_template_parsing_between_renders
     t = Template.new.parse("{{ foo }}{% assign foo = 'foo' %}{{ foo }}")
     assert_equal 'foo', t.render!
@@ -82,69 +93,92 @@ class TemplateTest < Minitest::Test
 
   def test_resource_limits_works_with_custom_length_method
     t = Template.parse("{% assign foo = bar %}")
-    t.resource_limits = { :render_length_limit => 42 }
+    t.resource_limits.render_length_limit = 42
     assert_equal "", t.render!("bar" => SomethingWithLength.new)
   end
 
   def test_resource_limits_render_length
     t = Template.parse("0123456789")
-    t.resource_limits = { :render_length_limit => 5 }
+    t.resource_limits.render_length_limit = 5
     assert_equal "Liquid error: Memory limits exceeded", t.render()
-    assert t.resource_limits[:reached]
+    assert t.resource_limits.reached?
-    t.resource_limits = { :render_length_limit => 10 }
+
+    t.resource_limits.render_length_limit = 10
     assert_equal "0123456789", t.render!()
-    refute_nil t.resource_limits[:render_length_current]
+    refute_nil t.resource_limits.render_length
   end
 
   def test_resource_limits_render_score
     t = Template.parse("{% for a in (1..10) %} {% for a in (1..10) %} foo {% endfor %} {% endfor %}")
-    t.resource_limits = { :render_score_limit => 50 }
+    t.resource_limits.render_score_limit = 50
     assert_equal "Liquid error: Memory limits exceeded", t.render()
-    assert t.resource_limits[:reached]
+    assert t.resource_limits.reached?
+
     t = Template.parse("{% for a in (1..100) %} foo {% endfor %}")
-    t.resource_limits = { :render_score_limit => 50 }
+    t.resource_limits.render_score_limit = 50
     assert_equal "Liquid error: Memory limits exceeded", t.render()
-    assert t.resource_limits[:reached]
+    assert t.resource_limits.reached?
-    t.resource_limits = { :render_score_limit => 200 }
+
+    t.resource_limits.render_score_limit = 200
     assert_equal (" foo " * 100), t.render!()
-    refute_nil t.resource_limits[:render_score_current]
+    refute_nil t.resource_limits.render_score
   end
 
   def test_resource_limits_assign_score
     t = Template.parse("{% assign foo = 42 %}{% assign bar = 23 %}")
-    t.resource_limits = { :assign_score_limit => 1 }
+    t.resource_limits.assign_score_limit = 1
     assert_equal "Liquid error: Memory limits exceeded", t.render()
-    assert t.resource_limits[:reached]
+    assert t.resource_limits.reached?
-    t.resource_limits = { :assign_score_limit => 2 }
+
+    t.resource_limits.assign_score_limit = 2
     assert_equal "", t.render!()
-    refute_nil t.resource_limits[:assign_score_current]
+    refute_nil t.resource_limits.assign_score
   end
 
   def test_resource_limits_aborts_rendering_after_first_error
     t = Template.parse("{% for a in (1..100) %} foo1 {% endfor %} bar {% for a in (1..100) %} foo2 {% endfor %}")
-    t.resource_limits = { :render_score_limit => 50 }
+    t.resource_limits.render_score_limit = 50
     assert_equal "Liquid error: Memory limits exceeded", t.render()
-    assert t.resource_limits[:reached]
+    assert t.resource_limits.reached?
   end
 
   def test_resource_limits_hash_in_template_gets_updated_even_if_no_limits_are_set
     t = Template.parse("{% for a in (1..100) %} {% assign foo = 1 %} {% endfor %}")
     t.render!()
-    assert t.resource_limits[:assign_score_current] > 0
+    assert t.resource_limits.assign_score > 0
-    assert t.resource_limits[:render_score_current] > 0
+    assert t.resource_limits.render_score > 0
-    assert t.resource_limits[:render_length_current] > 0
+    assert t.resource_limits.render_length > 0
+  end
+
+  def test_render_length_persists_between_blocks
+    t = Template.parse("{% if true %}aaaa{% endif %}")
+    t.resource_limits.render_length_limit = 7
+    assert_equal "Liquid error: Memory limits exceeded", t.render()
+    t.resource_limits.render_length_limit = 8
+    assert_equal "aaaa", t.render()
+
+    t = Template.parse("{% if true %}aaaa{% endif %}{% if true %}bbb{% endif %}")
+    t.resource_limits.render_length_limit = 13
+    assert_equal "Liquid error: Memory limits exceeded", t.render()
+    t.resource_limits.render_length_limit = 14
+    assert_equal "aaaabbb", t.render()
+
+    t = Template.parse("{% if true %}a{% endif %}{% if true %}b{% endif %}{% if true %}a{% endif %}{% if true %}b{% endif %}{% if true %}a{% endif %}{% if true %}b{% endif %}")
+    t.resource_limits.render_length_limit = 5
+    assert_equal "Liquid error: Memory limits exceeded", t.render()
+    t.resource_limits.render_length_limit = 11
+    assert_equal "Liquid error: Memory limits exceeded", t.render()
+    t.resource_limits.render_length_limit = 12
+    assert_equal "ababab", t.render()
   end
 
   def test_default_resource_limits_unaffected_by_render_with_context
     context = Context.new
     t = Template.parse("{% for a in (1..100) %} {% assign foo = 1 %} {% endfor %}")
     t.render!(context)
-    assert context.resource_limits[:assign_score_current] > 0
+    assert context.resource_limits.assign_score > 0
-    assert context.resource_limits[:render_score_current] > 0
+    assert context.resource_limits.render_score > 0
-    assert context.resource_limits[:render_length_current] > 0
+    assert context.resource_limits.render_length > 0
-    refute Template.default_resource_limits.key?(:assign_score_current)
-    refute Template.default_resource_limits.key?(:render_score_current)
-    refute Template.default_resource_limits.key?(:render_length_current)
   end
 
   def test_can_use_drop_as_context

test/test_helper.rb

@@ -5,6 +5,7 @@ require 'spy/integration'
 
 $:.unshift(File.join(File.expand_path(File.dirname(__FILE__)), '..', 'lib'))
 require 'liquid.rb'
+require 'liquid/profiler'
 
 mode = :strict
 if env_mode = ENV['LIQUID_PARSER_MODE']

test/unit/lexer_unit_test.rb

@@ -32,7 +32,10 @@ class LexerUnitTest < Minitest::Test
 
   def test_fancy_identifiers
     tokens = Lexer.new('hi five?').tokenize
-    assert_equal [[:id,'hi'], [:id, 'five'], [:question, '?'], [:end_of_string]], tokens
+    assert_equal [[:id, 'hi'], [:id, 'five?'], [:end_of_string]], tokens
+
+    tokens = Lexer.new('2foo').tokenize
+    assert_equal [[:number, '2'], [:id, 'foo'], [:end_of_string]], tokens
   end
 
   def test_whitespace

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
+    assert_equal ['WHEN', 'ELSE'], template.root.nodelist[0].nodelist.map(&:nodelist).flatten
   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
+    assert_equal ['FOR'], template.root.nodelist[0].nodelist.map(&:nodelist).flatten
   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
+    assert_equal ['FOR', 'ELSE'], template.root.nodelist[0].nodelist.map(&:nodelist).flatten
   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
+    assert_equal ['IF', 'ELSE'], template.root.nodelist[0].nodelist.map(&:nodelist).flatten
   end
 end

test/unit/template_unit_test.rb

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

test/unit/variable_unit_test.rb

@@ -6,9 +6,6 @@ class VariableUnitTest < Minitest::Test
   def test_variable
     var = Variable.new('hello')
     assert_equal VariableLookup.new('hello'), var.name
-
-    var = Variable.new('hello[goodbye ]')
-    assert_equal VariableLookup.new('hello[goodbye]'), var.name
   end
 
   def test_filters
@@ -105,6 +102,17 @@ class VariableUnitTest < Minitest::Test
     assert_equal 1000.01, var.name
   end
 
+  def test_dashes
+    assert_equal VariableLookup.new('foo-bar'), Variable.new('foo-bar').name
+    assert_equal VariableLookup.new('foo-bar-2'), Variable.new('foo-bar-2').name
+
+    with_error_mode :strict do
+      assert_raises(Liquid::SyntaxError) { Variable.new('foo - bar') }
+      assert_raises(Liquid::SyntaxError) { Variable.new('-foo') }
+      assert_raises(Liquid::SyntaxError) { Variable.new('2foo') }
+    end
+  end
+
   def test_string_with_special_chars
     var = Variable.new(%| 'hello! $!@.;"ddasd" ' |)
     assert_equal 'hello! $!@.;"ddasd" ', var.name