diff --git a/lib/liquid.rb b/lib/liquid.rb index c1fac5f..f719107 100644 --- a/lib/liquid.rb +++ b/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' diff --git a/lib/liquid/block.rb b/lib/liquid/block.rb index 5503913..6d45c71 100644 --- a/lib/liquid/block.rb +++ b/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 diff --git a/lib/liquid/context.rb b/lib/liquid/context.rb index 9ff7260..8e58b1d 100644 --- a/lib/liquid/context.rb +++ b/lib/liquid/context.rb @@ -91,17 +91,15 @@ module Liquid @interrupts.pop end - def handle_error(e) - errors.push(e) - raise if exception_handler && exception_handler.call(e) - - case e - when SyntaxError - "Liquid syntax error: #{e.message}" - else - "Liquid error: #{e.message}" + def handle_error(e, token=nil) + if e.is_a?(Liquid::Error) + e.set_line_number_from_token(token) end + + errors.push(e) + raise if exception_handler && exception_handler.call(e) + Liquid::Error.render(e) end def invoke(method, *args) diff --git a/lib/liquid/errors.rb b/lib/liquid/errors.rb index 85cb373..44f89bc 100644 --- a/lib/liquid/errors.rb +++ b/lib/liquid/errors.rb @@ -1,5 +1,52 @@ module Liquid - class Error < ::StandardError; end + 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 + end + + str + 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 class ContextError < Error; end diff --git a/lib/liquid/parser_switching.rb b/lib/liquid/parser_switching.rb new file mode 100644 index 0000000..5fc0f38 --- /dev/null +++ b/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.set_line_number_from_token(markup) + @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.markup_context = markup_context(markup) + raise e + end + + def markup_context(markup) + "in \"#{markup.strip}\"" + end + end +end diff --git a/lib/liquid/tag.rb b/lib/liquid/tag.rb index b4dfec4..d8d35c5 100644 --- a/lib/liquid/tag.rb +++ b/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 diff --git a/lib/liquid/template.rb b/lib/liquid/template.rb index a1a6f55..5755722 100644 --- a/lib/liquid/template.rb +++ b/lib/liquid/template.rb @@ -105,6 +105,7 @@ module Liquid def parse(source, options = {}) @options = options @profiling = options[:profile] + @line_numbers = options[:line_numbers] || @profiling @root = Document.parse(tokenize(source), DEFAULT_OPTIONS.merge(options)) @warnings = nil self @@ -224,7 +225,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| @@ -248,6 +249,5 @@ module Liquid yield end end - end end diff --git a/lib/liquid/profiler/token.rb b/lib/liquid/token.rb similarity index 76% rename from lib/liquid/profiler/token.rb rename to lib/liquid/token.rb index 8c43da2..acf8ef9 100644 --- a/lib/liquid/profiler/token.rb +++ b/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 "" end + def child(string) + Token.new(string, @line_number) + end end end diff --git a/lib/liquid/variable.rb b/lib/liquid/variable.rb index 144b679..1282a2b 100644 --- a/lib/liquid/variable.rb +++ b/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 - 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 + parse_with_selected_parser(markup) 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) diff --git a/test/integration/error_handling_test.rb b/test/integration/error_handling_test.rb index 4f25f81..ee07204 100644 --- a/test/integration/error_handling_test.rb +++ b/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 (line 3): standard error will raise a standard error. + + Bla bla test. + + Liquid syntax error (line 7): syntax error will raise a syntax error. + + This is an argument error: Liquid error (line 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 @@ -71,28 +104,37 @@ class ErrorHandlingTest < Minitest::Test err = assert_raises(SyntaxError) do Liquid::Template.parse(' {% if 1 =! 2 %}ok{% endif %} ', :error_mode => :strict) end - assert_equal 'Unexpected character = in "1 =! 2"', err.message + assert_equal 'Liquid syntax error: Unexpected character = in "1 =! 2"', err.message err = assert_raises(SyntaxError) do Liquid::Template.parse('{{%%%}}', :error_mode => :strict) end - assert_equal 'Unexpected character % in "{{%%%}}"', err.message + assert_equal 'Liquid syntax error: 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].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 '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 '', 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 + # 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 diff --git a/test/integration/tags/include_tag_test.rb b/test/integration/tags/include_tag_test.rb index 8c45a8f..49267f3 100644 --- a/test/integration/tags/include_tag_test.rb +++ b/test/integration/tags/include_tag_test.rb @@ -132,7 +132,7 @@ class IncludeTagTest < Minitest::Test Liquid::Template.file_system = infinite_file_system.new - assert_raises(Liquid::StackLevelError) do + assert_raises(Liquid::StackLevelError, SystemStackError) do Template.parse("{% include 'loop' %}").render! end