Merge pull request #244 from Shopify/proper-parse-warnings

Add Better Parse Warnings To recursive-parser Branch

lib/liquid/block.rb

@@ -14,6 +14,9 @@ module Liquid
       @nodelist ||= []
       @nodelist.clear
 
+      # All child tags of the current block.
+      @children = []
+
       while token = tokens.shift
         case token
         when IsTag
@@ -31,6 +34,7 @@ module Liquid
               new_tag = tag.new_with_options($1, $2, tokens, @options || {})
               @blank &&= new_tag.blank?
               @nodelist << new_tag
+              @children << new_tag
             else
               # this tag is not registered with the system
               # pass it to the current block for special handling or error reporting
@@ -40,7 +44,9 @@ module Liquid
             raise SyntaxError, "Tag '#{token}' was not properly terminated with regexp: #{TagEnd.inspect} "
           end
         when IsVariable
-          @nodelist << create_variable(token)
+          new_var = create_variable(token)
+          @nodelist << new_var
+          @children << new_var
           @blank = false
         when ''
           # pass
@@ -56,6 +62,18 @@ module Liquid
       assert_missing_delimitation!
     end
 
+    # warnings of this block and all sub-tags
+    def warnings
+      all_warnings = []
+      all_warnings.concat(@warnings) if @warnings
+
+      @children.each do |node|
+        all_warnings.concat(node.warnings || [])
+      end
+
+      all_warnings
+    end
+
     def end_tag
     end
 

lib/liquid/parser.rb

@@ -13,7 +13,7 @@ module Liquid
     def consume(type = nil)
       token = @tokens[@p]
       if type && token[0] != type
-        raise SyntaxError, "Expected #{type} but found #{@tokens[@p]}"
+        raise SyntaxError, "Expected #{type} but found #{@tokens[@p].first}"
       end
       @p += 1
       token[1]

lib/liquid/tag.rb

@@ -1,6 +1,7 @@
 module Liquid
   class Tag
     attr_accessor :nodelist, :options
+    attr_reader :warnings
 
     def self.new_with_options(tag_name, markup, tokens, options)
       # Forgive me Matz for I have sinned. I know this code is weird

lib/liquid/tags/for.rb

@@ -58,7 +58,6 @@ module Liquid
     end
 
     def render(context)
-      context.errors.concat(@warnings) if @warnings
       context.registers[:for] ||= Hash.new(0)
 
       collection = context[@collection_name]

lib/liquid/tags/if.rb

@@ -29,7 +29,6 @@ module Liquid
     end
 
     def render(context)
-      context.errors.concat(@warnings) if @warnings
       context.stack do
         @blocks.each do |block|
           if block.evaluate(context)

lib/liquid/template.rb

@@ -69,9 +69,15 @@ module Liquid
     # Returns self for easy chaining
     def parse(source, options = {})
       @root = Document.new(tokenize(source), options)
+      @warnings = nil
       self
     end
 
+    def warnings
+      return [] unless @root
+      @warnings ||= @root.warnings
+    end
+
     def registers
       @registers ||= {}
     end

lib/liquid/variable.rb

@@ -13,12 +13,11 @@ module Liquid
   class Variable
     FilterParser = /(?:#{FilterSeparator}|(?:\s*(?:#{QuotedFragment}|#{ArgumentSeparator})\s*)+)/o
     EasyParse = /^ *(\w+(?:\.\w+)*) *$/
-    attr_accessor :filters, :name
+    attr_accessor :filters, :name, :warnings
 
     def initialize(markup, options = {})
       @markup  = markup
       @name    = nil
-      @warning = nil
       @options = options || {}
       
 
@@ -29,7 +28,8 @@ module Liquid
         begin
           strict_parse(markup)
         rescue SyntaxError => e
-          @warning = e
+          @warnings ||= []
+          @warnings << e
           lax_parse(markup)
         end
       end
@@ -87,7 +87,6 @@ module Liquid
 
     def render(context)
       return '' if @name.nil?
-      context.errors << @warning if @warning
       @filters.inject(context[@name]) do |output, filter|
         filterargs = []
         keyword_args = {}

test/liquid/error_handling_test.rb

@@ -92,10 +92,12 @@ class ErrorHandlingTest < Test::Unit::TestCase
   end
 
   def test_warnings
-    template = Liquid::Template.parse('{% if ~~~ %}derp{% else %}wat{% endif %}', :error_mode => :warn)
+    template = Liquid::Template.parse('{% if ~~~ %}{{%%%}}{% else %}{{ hello. }}{% endif %}', :error_mode => :warn)
-    assert_equal 'wat', template.render
+    assert_equal 3, template.warnings.size
-    assert_equal 1, template.errors.size
+    assert_equal 'Unexpected character ~ in "~~~"', template.warnings[0].message
-    assert_equal 'Unexpected character ~ in "~~~"', template.errors.first.message
+    assert_equal 'Unexpected character % in "{{%%%}}"', template.warnings[1].message
+    assert_equal 'Expected id but found end_of_string in "{{ hello. }}"', template.warnings[2].message
+    assert_equal '', template.render
   end
 
   # Liquid should not catch Exceptions that are not subclasses of StandardError, like Interrupt and NoMemoryError