Add line numbers to warnings

History.md

@@ -3,8 +3,6 @@
 ## 3.0.0 / not yet released / branch "master"
 
 * ...
-* Fixed condition with wrong data types, see #423 [Bogdan Gusiev]
-* Add url_encode to standard filters, see #421 [Derrick Reimer, djreimer]
 * Add uniq to standard filters [Florian Weingarten, fw42]
 * Add exception_handler feature, see #397 and #254 [Bogdan Gusiev, bogdan and Florian Weingarten, fw42]
 * Optimize variable parsing to avoid repeated regex evaluation during template rendering #383 [Jason Hiltz-Laforge, jasonhl]

lib/liquid/condition.rb

@@ -15,9 +15,7 @@ module Liquid
       '>'.freeze  => :>,
       '>='.freeze => :>=,
       '<='.freeze => :<=,
-      'contains'.freeze => lambda { |cond, left, right|
-        left && right && left.respond_to?(:include?) ? left.include?(right) : false
-      }
+      'contains'.freeze => lambda { |cond, left, right| left && right ? left.include?(right) : false }
     }
 
     def self.operators

lib/liquid/context.rb

@@ -16,14 +16,12 @@ module Liquid
     attr_reader :scopes, :errors, :registers, :environments, :resource_limits
     attr_accessor :exception_handler
 
-    def initialize(environments = {}, outer_scope = {}, registers = {}, rethrow_errors = false, resource_limits = nil)
+    def initialize(environments = {}, outer_scope = {}, registers = {}, rethrow_errors = false, resource_limits = {})
       @environments     = [environments].flatten
       @scopes           = [(outer_scope || {})]
       @registers        = registers
       @errors           = []
-      @resource_limits  = resource_limits || Template.default_resource_limits
-      @resource_limits[:render_score_current] = 0
-      @resource_limits[:assign_score_current] = 0
+      @resource_limits  = (resource_limits || {}).merge!({ :render_score_current => 0, :assign_score_current => 0 })
       @parsed_expression = Hash.new{ |cache, markup| cache[markup] = Expression.parse(markup) }
       squash_instance_assigns_with_environments
 
@@ -94,11 +92,8 @@ module Liquid
     end
 
 
-    def handle_error(e, token=nil)
-      if e.is_a?(Liquid::Error)
-        e.set_line_number_from_token(token)
-      end
-
+    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)

lib/liquid/errors.rb

@@ -1,51 +1,34 @@
 module Liquid
   class Error < ::StandardError
     attr_accessor :line_number
-    attr_accessor :markup_context
 
-    def to_s(with_prefix=true)
-      str = ""
-      str << message_prefix if with_prefix
-      str << super()
-
-      if markup_context
-        str << " "
-        str << markup_context
+    def self.render(e)
+      msg = if e.is_a?(Liquid::Error) && e.line_number
+        "#{e.line_number}: #{e.message}"
+      else
+        e.message
       end
 
-      str
+      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
-
-    def self.render(e)
-      if e.is_a?(Liquid::Error)
-        e.to_s
-      else
-        "Liquid error: #{e.to_s}"
-      end
-    end
-
-    private
-
-    def message_prefix
-      str = ""
-      if is_a?(SyntaxError)
-        str << "Liquid syntax error"
-      else
-        str << "Liquid error"
-      end
-
-      if line_number
-        str << " (line #{line_number})"
-      end
-
-      str << ": "
-      str
-    end
   end
 
   class ArgumentError < Error; end

lib/liquid/parser_switching.rb

@@ -8,7 +8,7 @@ module Liquid
         begin
           return strict_parse_with_error_context(markup)
         rescue SyntaxError => e
-          e.set_line_number_from_token(markup)
+          e.line_number = markup.line_number if markup.is_a?(Token)
           @warnings ||= []
           @warnings << e
           return lax_parse(markup)
@@ -20,12 +20,12 @@ module Liquid
     def strict_parse_with_error_context(markup)
       strict_parse(markup)
     rescue SyntaxError => e
-      e.markup_context = markup_context(markup)
+      e.message << markup_context(markup)
       raise e
     end
 
     def markup_context(markup)
-      "in \"#{markup.strip}\""
+      " in \"#{markup.strip}\"" 
     end
   end
-end
+end

lib/liquid/standardfilters.rb

@@ -42,10 +42,6 @@ module Liquid
       input.to_s.gsub(HTML_ESCAPE_ONCE_REGEXP, HTML_ESCAPE)
     end
 
-    def url_encode(input)
-      CGI.escape(input) rescue input
-    end
-
     def slice(input, offset, length=nil)
       offset = Integer(offset)
       length = length ? Integer(length) : 1

lib/liquid/tags/include.rb

@@ -30,11 +30,6 @@ module Liquid
           @attributes[key] = value
         end
 
-        if partial_parsable_at_parse_time?
-          source = read_template_from_file_system_at_parse
-          @partial = Liquid::Template.parse(source, pass_options)
-        end
-
       else
         raise SyntaxError.new(options[:locale].t("errors.syntax.include".freeze))
       end
@@ -71,21 +66,17 @@ module Liquid
         template_name = context[@template_name]
 
         if cached = cached_partials[template_name]
-          cached
-        else
-          if @partial && context.registers[:file_system].nil?
-            partial = @partial
-          else
-            partial = Liquid::Template.parse(read_template_from_file_system(context), pass_options)
-          end
-          cached_partials[template_name] = partial
-          context.registers[:cached_partials] = cached_partials
-          partial
+          return cached
         end
+        source = read_template_from_file_system(context)
+        partial = Liquid::Template.parse(source)
+        cached_partials[template_name] = partial
+        context.registers[:cached_partials] = cached_partials
+        partial
       end
 
       def read_template_from_file_system(context)
-        file_system = context.registers[:file_system] || parsed_file_system || Liquid::Template.file_system
+        file_system = context.registers[:file_system] || Liquid::Template.file_system
 
         # make read_template_file call backwards-compatible.
         case file_system.method(:read_template_file).arity
@@ -97,33 +88,6 @@ module Liquid
           raise ArgumentError, "file_system.read_template_file expects two parameters: (template_name, context)"
         end
       end
-
-      def read_template_from_file_system_at_parse
-        parsed_file_system.read_template_file(parsed_template_name)
-      end
-
-      def parsed_file_system
-        options[:file_system]
-      end
-
-      def partial_parsable_at_parse_time?
-        template_name_is_string_constant = parsed_template_name.is_a?(String)
-        options[:file_system] && template_name_is_string_constant
-      end
-
-      def parsed_template_name
-        Expression.parse(@template_name)
-      end
-
-      def pass_options
-        dont_pass = @options[:include_options_blacklist]
-        return {locale: @options[:locale]} if dont_pass == true
-        opts = @options.merge(included: true, include_options_blacklist: false)
-        if dont_pass.is_a?(Array)
-          dont_pass.each {|o| opts.delete(o)}
-        end
-        opts
-      end
   end
 
   Template.register_tag('include'.freeze, Include)

lib/liquid/template.rb

@@ -86,10 +86,6 @@ module Liquid
         Strainer.global_filter(mod)
       end
 
-      def default_resource_limits
-        @default_resource_limits ||= {}
-      end
-
       # creates a new <tt>Template</tt> object from liquid source code
       # To enable profiling, pass in <tt>profile: true</tt> as an option.
       # See Liquid::Profiler for more information
@@ -99,16 +95,16 @@ module Liquid
       end
     end
 
+    # creates a new <tt>Template</tt> from an array of tokens. Use <tt>Template.parse</tt> instead
     def initialize
-      @resource_limits = self.class.default_resource_limits.dup
+      @resource_limits = {}
     end
 
     # Parse source code.
     # Returns self for easy chaining
     def parse(source, options = {})
-      @options = options
-      @profiling = options[:profile]
-      @line_numbers = options[:line_numbers] || @profiling
+      @profiling = options.delete(:profile)
+      @line_numbers = options.delete(:line_numbers) || @profiling
       @root = Document.parse(tokenize(source), DEFAULT_OPTIONS.merge(options))
       @warnings = nil
       self
@@ -201,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
@@ -239,7 +235,7 @@ module Liquid
     end
 
     def with_profiling
-      if @profiling && !@options[:included]
+      if @profiling
         @profiler = Profiler.new
         @profiler.start
 

lib/liquid/variable.rb

@@ -30,7 +30,7 @@ module Liquid
     end
 
     def markup_context(markup)
-      "in \"{{#{markup}}}\""
+      " in \"{{#{markup}}}\""
     end
 
     def lax_parse(markup)

test/integration/error_handling_test.rb

@@ -40,13 +40,13 @@ class ErrorHandlingTest < Minitest::Test
     expected = <<-TEXT
       Hello,
 
-      Liquid error (line 3): standard error will raise a standard error.
+      Liquid error: 3: standard error will raise a standard error.
 
       Bla bla test.
 
-      Liquid syntax error (line 7): syntax error will raise a syntax error.
+      Liquid syntax error: 7: syntax error will raise a syntax error.
 
-      This is an argument error: Liquid error (line 9): argument error
+      This is an argument error: Liquid error: 9: argument error
 
       Bla.
     TEXT
@@ -104,28 +104,25 @@ class ErrorHandlingTest < Minitest::Test
     err = assert_raises(SyntaxError) do
       Liquid::Template.parse(' {% if 1 =! 2 %}ok{% endif %} ', :error_mode => :strict)
     end
-    assert_equal 'Liquid syntax error: Unexpected character = in "1 =! 2"', err.message
+    assert_equal 'Unexpected character = in "1 =! 2"', err.message
 
     err = assert_raises(SyntaxError) do
       Liquid::Template.parse('{{%%%}}', :error_mode => :strict)
     end
-    assert_equal 'Liquid syntax error: Unexpected character % in "{{%%%}}"', err.message
+    assert_equal 'Unexpected character % in "{{%%%}}"', err.message
   end
 
   def test_warnings
     template = Liquid::Template.parse('{% if ~~~ %}{{%%%}}{% else %}{{ hello. }}{% endif %}', :error_mode => :warn)
     assert_equal 3, template.warnings.size
-    assert_equal 'Unexpected character ~ in "~~~"', template.warnings[0].to_s(false)
-    assert_equal 'Unexpected character % in "{{%%%}}"', template.warnings[1].to_s(false)
-    assert_equal 'Expected id but found end_of_string in "{{ hello. }}"', template.warnings[2].to_s(false)
+    assert_equal 'Unexpected character ~ in "~~~"', template.warnings[0].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
 
   def test_warning_line_numbers
     template = Liquid::Template.parse("{% if ~~~ %}\n{{%%%}}{% else %}\n{{ hello. }}{% endif %}", :error_mode => :warn, :line_numbers => true)
-    assert_equal 'Liquid syntax error (line 1): Unexpected character ~ in "~~~"', template.warnings[0].message
-    assert_equal 'Liquid syntax error (line 2): Unexpected character % in "{{%%%}}"', template.warnings[1].message
-    assert_equal 'Liquid syntax error (line 3): Expected id but found end_of_string in "{{ hello. }}"', template.warnings[2].message
     assert_equal 3, template.warnings.size
     assert_equal [1,2,3], template.warnings.map(&:line_number)
   end

test/integration/render_profiling_test.rb

@@ -48,18 +48,6 @@ class RenderProfilingTest < Minitest::Test
     assert_equal 2, t.profiler[1].line_number
   end
 
-  def test_profiling_includes_line_numbers_of_included_partials
-    t = Template.parse("{% include 'a_template' %}", :profile => true)
-    t.render!
-
-    included_children = t.profiler[0].children
-
-    # {% assign template_name = 'a_template' %}
-    assert_equal 1, included_children[0].line_number
-    # {{ template_name }}
-    assert_equal 2, included_children[1].line_number
-  end
-
   def test_profiling_times_the_rendering_of_tokens
     t = Template.parse("{% include 'a_template' %}", :profile => true)
     t.render!

test/integration/standard_filter_test.rb

@@ -118,11 +118,6 @@ class StandardFiltersTest < Minitest::Test
     assert_equal '&lt;strong&gt;Hulk&lt;/strong&gt;', @filters.escape_once('&lt;strong&gt;Hulk</strong>')
   end
 
-  def test_url_encode
-    assert_equal 'foo%2B1%40example.com', @filters.url_encode('foo+1@example.com')
-    assert_equal nil, @filters.url_encode(nil)
-  end
-
   def test_truncatewords
     assert_equal 'one two three', @filters.truncatewords('one two three', 4)
     assert_equal 'one two...', @filters.truncatewords('one two three', 2)

test/integration/tags/include_tag_test.rb

@@ -33,13 +33,6 @@ class TestFileSystem
   end
 end
 
-# FileSystems at parse time don't have a context
-class ParseFileSystem
-  def read_template_file(template_path)
-    'from ParseFileSystem'
-  end
-end
-
 class OtherFileSystem
   def read_template_file(template_path, context)
     'from OtherFileSystem'
@@ -84,20 +77,6 @@ class IncludeTagTest < Minitest::Test
       Template.parse("{% include 'pick_a_source' %}").render!({}, :registers => {:file_system => OtherFileSystem.new})
   end
 
-  def test_include_tag_can_use_file_system_at_parse_so_it_can_be_parsed_before_rendered
-    assert_equal 'from ParseFileSystem',
-      Template.parse("{% include 'pick_a_source' %}",file_system: ParseFileSystem.new).render!({})
-  end
-
-  def test_include_tag_looks_at_file_system_passed_in_registers_over_parse_options
-    assert_equal 'from OtherFileSystem',
-      Template.parse("{% include 'pick_a_source' %}",file_system: ParseFileSystem.new).render!({}, :registers => {:file_system => OtherFileSystem.new})
-  end
-
-  def test_include_tag_use_parse_option_file_system_even_if_partial_can_be_preparsed
-    assert_equal 'from ParseFileSystem',
-      Template.parse("{% include template %}",file_system: ParseFileSystem.new).render!({})
-  end 
 
   def test_include_tag_with
     assert_template_result "Product: Draft 151cm ",
@@ -153,7 +132,7 @@ class IncludeTagTest < Minitest::Test
 
     Liquid::Template.file_system = infinite_file_system.new
 
-    assert_raises(Liquid::StackLevelError, SystemStackError) do
+    assert_raises(Liquid::StackLevelError) do
       Template.parse("{% include 'loop' %}").render!
     end
 
@@ -230,19 +209,4 @@ class IncludeTagTest < Minitest::Test
     a.render!
     assert_empty a.errors
   end
-
-  def test_passing_options_to_included_templates
-    assert_raises(Liquid::SyntaxError) do
-      Template.parse("{% include template %}", error_mode: :strict).render!("template" => '{{ "X" || downcase }}')
-    end
-    with_error_mode(:lax) do
-      assert_equal 'x', Template.parse("{% include template %}", error_mode: :strict, include_options_blacklist: true).render!("template" => '{{ "X" || downcase }}')
-    end
-    assert_raises(Liquid::SyntaxError) do
-      Template.parse("{% include template %}", error_mode: :strict, include_options_blacklist: [:locale]).render!("template" => '{{ "X" || downcase }}')
-    end
-    with_error_mode(:lax) do
-      assert_equal 'x', Template.parse("{% include template %}", error_mode: :strict, include_options_blacklist: [:error_mode]).render!("template" => '{{ "X" || downcase }}')
-    end
-  end
 end # IncludeTagTest

test/unit/condition_unit_test.rb

@@ -80,10 +80,6 @@ class ConditionUnitTest < Minitest::Test
     assert_evalutes_false "0", 'contains', 'not_assigned'
   end
 
-  def test_contains_return_false_on_wrong_data_type
-    assert_evalutes_false "1", 'contains', '0'
-  end
-
   def test_or_condition
     condition = Condition.new('1', '==', '2')