diff --git a/History.md b/History.md index 486eb94..26f1fab 100644 --- a/History.md +++ b/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] diff --git a/lib/liquid.rb b/lib/liquid.rb index f719107..af38cd4 100644 --- a/lib/liquid.rb +++ b/lib/liquid.rb @@ -57,6 +57,7 @@ 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' diff --git a/lib/liquid/block.rb b/lib/liquid/block.rb index 7de23e2..010fa0c 100644 --- a/lib/liquid/block.rb +++ b/lib/liquid/block.rb @@ -1,65 +1,26 @@ module Liquid class Block < Tag - FullToken = /\A#{TagStart}\s*(\w+)\s*(.*)?#{TagEnd}\z/om - ContentOfVariable = /\A#{VariableStart}(.*)#{VariableEnd}\z/om - TAGSTART = "{%".freeze - VARSTART = "{{".freeze + def initialize(tag_name, markup, options) + super + @blank = true + end + + def parse(tokens) + @body = BlockBody.new(options) + while more = parse_body(@body, tokens) + end + end + + def render(context) + @body.render(context) + end def blank? @blank end - def parse(tokens) - @blank = true - @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! + def nodelist + @body.nodelist 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! - raise SyntaxError.new(options[:locale].t("errors.syntax.tag_never_closed".freeze, :block_name => block_name)) - end + def parse_body(body, tokens) + body.parse(tokens) do |end_tag_name, end_tag_params| + @blank &&= body.blank? - def render_all(list, context) - output = [] - context.resource_limits[:render_length_current] = 0 - 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)) + return false if end_tag_name == block_delimiter + unless end_tag_name + raise SyntaxError.new(@options[:locale].t("errors.syntax.tag_never_closed".freeze, :block_name => block_name)) 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 - 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 + true end end end diff --git a/lib/liquid/block_body.rb b/lib/liquid/block_body.rb new file mode 100644 index 0000000..19169dd --- /dev/null +++ b/lib/liquid/block_body.rb @@ -0,0 +1,128 @@ +module Liquid + class BlockBody + FullToken = /\A#{TagStart}\s*(\w+)\s*(.*)?#{TagEnd}\z/om + ContentOfVariable = /\A#{VariableStart}(.*)#{VariableEnd}\z/om + TAGSTART = "{%".freeze + VARSTART = "{{".freeze + + def self.parse(tokens, options) + body = new(options) + body.parse(tokens) + body + end + + attr_reader :nodelist + + def initialize(options) + @nodelist = [] + @blank = true + @options = options + end + + def parse(tokens) + 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 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 + + 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) && node.warnings + end + all_warnings + end + + def render(context) + output = [] + context.resource_limits[:render_length_current] = 0 + context.resource_limits[:render_score_current] += @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 + + 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 + + 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 + end +end diff --git a/lib/liquid/document.rb b/lib/liquid/document.rb index 1969a3a..f7485a4 100644 --- a/lib/liquid/document.rb +++ b/lib/liquid/document.rb @@ -1,17 +1,18 @@ module Liquid - class Document < Block - def self.parse(tokens, options={}) - # we don't need markup to open this block - super(nil, nil, tokens, options) + class Document < BlockBody + def parse(tokens) + super do |end_tag_name, end_tag_params| + unknown_tag(end_tag_name) if end_tag_name + end end - # There isn't a real delimiter - def block_delimiter - [] - end - - # Document blocks don't need to be terminated since they are not actually opened - def assert_missing_delimitation! + def unknown_tag(tag) + 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 diff --git a/lib/liquid/locales/en.yml b/lib/liquid/locales/en.yml index 09f0ad8..3e7460c 100644 --- a/lib/liquid/locales/en.yml +++ b/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" diff --git a/lib/liquid/profiler/hooks.rb b/lib/liquid/profiler/hooks.rb index dfad85d..8a6e808 100644 --- a/lib/liquid/profiler/hooks.rb +++ b/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) diff --git a/lib/liquid/tags/case.rb b/lib/liquid/tags/case.rb index b18893b..f10f53d 100644 --- a/lib/liquid/tags/case.rb +++ b/lib/liquid/tags/case.rb @@ -14,12 +14,18 @@ module Liquid end end + def parse(tokens) + body = BlockBody.new(options) + 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(options) + 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) - @blocks.push(block) + block.attach(body) + @blocks << block end end @@ -70,7 +77,7 @@ module Liquid end block = ElseCondition.new - block.attach(@nodelist) + block.attach(BlockBody.new(options)) @blocks << block end end diff --git a/lib/liquid/tags/for.rb b/lib/liquid/tags/for.rb index 9e09af5..253c3fe 100644 --- a/lib/liquid/tags/for.rb +++ b/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(options) + end + + def parse(tokens) + if more = parse_body(@for_block, tokens) + parse_body(@else_block, tokens) + end end def nodelist - if @else_block - @for_block + @else_block - else - @for_block - end + @else_block ? [@for_block, @else_block] : [@for_block] end def unknown_tag(tag, markup, tokens) return super unless tag == 'else'.freeze - @nodelist = @else_block = [] + @else_block = BlockBody.new(options) 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) diff --git a/lib/liquid/tags/if.rb b/lib/liquid/tags/if.rb index f91ba95..3d4f3db 100644 --- a/lib/liquid/tags/if.rb +++ b/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(options)) end def lax_parse(markup) diff --git a/lib/liquid/tags/raw.rb b/lib/liquid/tags/raw.rb index 59e52c9..41b2ec4 100644 --- a/lib/liquid/tags/raw.rb +++ b/lib/liquid/tags/raw.rb @@ -3,16 +3,27 @@ module Liquid FullTokenPossiblyInvalid = /\A(.*)#{TagStart}\s*(\w+)\s*(.*)?#{TagEnd}\z/om def parse(tokens) - @nodelist ||= [] - @nodelist.clear + @body = '' 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) diff --git a/lib/liquid/tags/unless.rb b/lib/liquid/tags/unless.rb index eb8a731..7aff42b 100644 --- a/lib/liquid/tags/unless.rb +++ b/lib/liquid/tags/unless.rb @@ -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 diff --git a/test/integration/document_test.rb b/test/integration/document_test.rb new file mode 100644 index 0000000..bcc4a21 --- /dev/null +++ b/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 diff --git a/test/integration/error_handling_test.rb b/test/integration/error_handling_test.rb index c21b4b4..f0f96e9 100644 --- a/test/integration/error_handling_test.rb +++ b/test/integration/error_handling_test.rb @@ -151,7 +151,7 @@ class ErrorHandlingTest < Minitest::Test def test_syntax_errors_in_nested_blocks_have_correct_line_number err = assert_raises(SyntaxError) do - Liquid::Template.parse(%q{ + tmpl = Liquid::Template.parse(%q{ foobar {% if 1 != 2 %} diff --git a/test/unit/tags/case_tag_unit_test.rb b/test/unit/tags/case_tag_unit_test.rb index 3c1be2c..7110308 100644 --- a/test/unit/tags/case_tag_unit_test.rb +++ b/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 diff --git a/test/unit/tags/for_tag_unit_test.rb b/test/unit/tags/for_tag_unit_test.rb index 17f88ce..b8fc520 100644 --- a/test/unit/tags/for_tag_unit_test.rb +++ b/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 diff --git a/test/unit/tags/if_tag_unit_test.rb b/test/unit/tags/if_tag_unit_test.rb index 17f826c..7ecfc40 100644 --- a/test/unit/tags/if_tag_unit_test.rb +++ b/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 diff --git a/test/unit/template_unit_test.rb b/test/unit/template_unit_test.rb index c50b067..3d63c09 100644 --- a/test/unit/template_unit_test.rb +++ b/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('') - assert_instance_of I18n, t.root.options[:locale] + t.parse('{%comment%}{%endcomment%}') + 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] - assert_equal fixture("en_locale.yml"), t.root.options[:locale].path + locale = t.root.nodelist[0].options[:locale] + assert_instance_of I18n, locale + assert_equal fixture("en_locale.yml"), locale.path end def test_with_cache_classes_tags_returns_the_same_class