Add line numbers to warnings

lib/liquid.rb

@@ -54,6 +54,7 @@ require 'liquid/interrupts'
 require 'liquid/strainer'
 require 'liquid/expression'
 require 'liquid/context'
+require 'liquid/parser_switching'
 require 'liquid/tag'
 require 'liquid/block'
 require 'liquid/document'
@@ -66,11 +67,11 @@ require 'liquid/standardfilters'
 require 'liquid/condition'
 require 'liquid/module_ex'
 require 'liquid/utils'
+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/token'
 require 'liquid/profiler/hooks'

lib/liquid/block.rb

@@ -32,7 +32,8 @@ module Liquid
 
               # fetch the tag from registered blocks
               if tag = Template.tags[$1]
-                new_tag = tag.parse($1, $2, tokens, @options)
+                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
@@ -103,7 +104,8 @@ module Liquid
 
     def create_variable(token)
       token.scan(ContentOfVariable) do |content|
-        return Variable.new(content.first, @options)
+        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
@@ -144,7 +146,7 @@ module Liquid
         rescue MemoryError => e
           raise e
         rescue ::StandardError => e
-          output << (context.handle_error(e))
+          output << (context.handle_error(e, token))
         end
       end
 

lib/liquid/context.rb

@@ -91,17 +91,12 @@ module Liquid
       @interrupts.pop
     end
 
-    def handle_error(e)
+
+    def handle_error(e, token)
+      e = Liquid::Error.error_with_line_number(e, token)
       errors.push(e)
-
       raise if exception_handler && exception_handler.call(e)
-
+      Liquid::Error.render(e)
-      case e
-      when SyntaxError
-        "Liquid syntax error: #{e.message}"
-      else
-        "Liquid error: #{e.message}"
-      end
     end
 
     def invoke(method, *args)

lib/liquid/errors.rb

@@ -1,5 +1,35 @@
 module Liquid
-  class Error < ::StandardError; end
+  class Error < ::StandardError
+    attr_accessor :line_number
+
+    def self.render(e)
+      msg = if e.is_a?(Liquid::Error) && e.line_number
+        "#{e.line_number}: #{e.message}"
+      else
+        e.message
+      end
+
+      case e
+      when SyntaxError
+        "Liquid syntax error: #{msg}"
+      else
+        "Liquid error: #{msg}"
+      end
+    end
+
+    def self.error_with_line_number(e, token)
+      if e.is_a?(Liquid::Error)
+        e.set_line_number_from_token(token)
+      end
+
+      e
+    end
+
+    def set_line_number_from_token(token)
+      return unless token.respond_to?(:line_number)
+      self.line_number = token.line_number
+    end
+  end
 
   class ArgumentError < Error; end
   class ContextError < Error; end

lib/liquid/parser_switching.rb

@@ -0,0 +1,31 @@
+module Liquid
+  module ParserSwitching
+    def parse_with_selected_parser(markup)
+      case @options[:error_mode] || Template.error_mode
+      when :strict then strict_parse_with_error_context(markup)
+      when :lax    then lax_parse(markup)
+      when :warn
+        begin
+          return strict_parse_with_error_context(markup)
+        rescue SyntaxError => e
+          e.line_number = markup.line_number if markup.is_a?(Token)
+          @warnings ||= []
+          @warnings << e
+          return lax_parse(markup)
+        end
+      end
+    end
+
+    private
+    def strict_parse_with_error_context(markup)
+      strict_parse(markup)
+    rescue SyntaxError => e
+      e.message << markup_context(markup)
+      raise e
+    end
+
+    def markup_context(markup)
+      " in \"#{markup.strip}\"" 
+    end
+  end
+end

lib/liquid/tag.rb

@@ -2,6 +2,7 @@ module Liquid
   class Tag
     attr_accessor :options, :line_number
     attr_reader :nodelist, :warnings
+    include ParserSwitching
 
     class << self
       def parse(tag_name, markup, tokens, options)
@@ -37,29 +38,5 @@ module Liquid
     def blank?
       false
     end
-
-    def parse_with_selected_parser(markup)
-      case @options[:error_mode] || Template.error_mode
-      when :strict then strict_parse_with_error_context(markup)
-      when :lax    then lax_parse(markup)
-      when :warn
-        begin
-          return strict_parse_with_error_context(markup)
-        rescue SyntaxError => e
-          @warnings ||= []
-          @warnings << e
-          return lax_parse(markup)
-        end
-      end
-    end
-
-    private
-
-    def strict_parse_with_error_context(markup)
-      strict_parse(markup)
-    rescue SyntaxError => e
-      e.message << " in \"#{markup.strip}\"" 
-      raise e
-    end
   end
 end

lib/liquid/template.rb

@@ -104,6 +104,7 @@ module Liquid
     # Returns self for easy chaining
     def parse(source, options = {})
       @profiling = options.delete(:profile)
+      @line_numbers = options.delete(:line_numbers) || @profiling
       @root = Document.parse(tokenize(source), DEFAULT_OPTIONS.merge(options))
       @warnings = nil
       self
@@ -196,7 +197,7 @@ module Liquid
         end
         result.respond_to?(:join) ? result.join : result
       rescue Liquid::MemoryError => e
-        context.handle_error(e)
+        context.handle_error(e, nil)
       ensure
         @errors = context.errors
       end
@@ -223,7 +224,7 @@ module Liquid
     end
 
     def calculate_line_numbers(raw_tokens)
-      return raw_tokens unless @profiling
+      return raw_tokens unless @line_numbers
 
       current_line = 1
       raw_tokens.map do |token|
@@ -247,6 +248,5 @@ module Liquid
         yield
       end
     end
-
   end
 end

lib/liquid/token.rb

@@ -1,6 +1,5 @@
 module Liquid
   class Token < String
-
     attr_reader :line_number
 
     def initialize(content, line_number)
@@ -12,5 +11,8 @@ module Liquid
       "<raw>"
     end
 
+    def child(string)
+      Token.new(string, @line_number)
+    end
   end
 end

lib/liquid/variable.rb

@@ -15,30 +15,24 @@ module Liquid
     EasyParse = /\A *(\w+(?:\.\w+)*) *\z/
     attr_accessor :filters, :name, :warnings
     attr_accessor :line_number
+    include ParserSwitching
 
     def initialize(markup, options = {})
       @markup  = markup
       @name    = nil
       @options = options || {}
 
-      case @options[:error_mode] || Template.error_mode
+      parse_with_selected_parser(markup)
-      when :strict then strict_parse(markup)
-      when :lax    then lax_parse(markup)
-      when :warn
-        begin
-          strict_parse(markup)
-        rescue SyntaxError => e
-          @warnings ||= []
-          @warnings << e
-          lax_parse(markup)
-        end
-      end
     end
 
     def raw
       @markup
     end
 
+    def markup_context(markup)
+      " in \"{{#{markup}}}\""
+    end
+
     def lax_parse(markup)
       @filters = []
       if markup =~ /\s*(#{QuotedFragment})(.*)/om
@@ -74,9 +68,6 @@ module Liquid
         @filters << [filtername, filterargs]
       end
       p.consume(:end_of_string)
-    rescue SyntaxError => e
-      e.message << " in \"{{#{markup}}}\""
-      raise e
     end
 
     def parse_filterargs(p)

test/integration/error_handling_test.rb

@@ -22,6 +22,39 @@ end
 class ErrorHandlingTest < Minitest::Test
   include Liquid
 
+  def test_templates_parsed_with_line_numbers_renders_them_in_errors
+    template = <<-LIQUID
+      Hello,
+
+      {{ errors.standard_error }} will raise a standard error.
+
+      Bla bla test.
+
+      {{ errors.syntax_error }} will raise a syntax error.
+
+      This is an argument error: {{ errors.argument_error }}
+
+      Bla.
+    LIQUID
+
+    expected = <<-TEXT
+      Hello,
+
+      Liquid error: 3: standard error will raise a standard error.
+
+      Bla bla test.
+
+      Liquid syntax error: 7: syntax error will raise a syntax error.
+
+      This is an argument error: Liquid error: 9: argument error
+
+      Bla.
+    TEXT
+
+    output = Liquid::Template.parse(template, line_numbers: true).render('errors' => ErrorDrop.new)
+    assert_equal expected, output
+  end
+
   def test_standard_error
     template = Liquid::Template.parse( ' {{ errors.standard_error }} '  )
     assert_equal ' Liquid error: standard error ', template.render('errors' => ErrorDrop.new)
@@ -59,7 +92,7 @@ class ErrorHandlingTest < Minitest::Test
       end
     end
   end
-  
+
   def test_lax_unrecognized_operator
     template = Liquid::Template.parse(' {% if 1 =! 2 %}ok{% endif %} ', :error_mode => :lax)
     assert_equal ' Liquid error: Unknown operator =! ', template.render
@@ -88,11 +121,17 @@ class ErrorHandlingTest < Minitest::Test
     assert_equal '', template.render
   end
 
+  def test_warning_line_numbers
+    template = Liquid::Template.parse("{% if ~~~ %}\n{{%%%}}{% else %}\n{{ hello. }}{% endif %}", :error_mode => :warn, :line_numbers => true)
+    assert_equal 3, template.warnings.size
+    assert_equal [1,2,3], template.warnings.map(&:line_number)
+  end
+
   # Liquid should not catch Exceptions that are not subclasses of StandardError, like Interrupt and NoMemoryError
   def test_exceptions_propagate
     assert_raises Exception do
-      template = Liquid::Template.parse( ' {{ errors.exception }} '  )
+      template = Liquid::Template.parse('{{ errors.exception }}')
       template.render('errors' => ErrorDrop.new)
     end
   end
-end # ErrorHandlingTest
+end