From 4b1835e3c027c8d4d2964907b47319839826b745 Mon Sep 17 00:00:00 2001 From: Florian Weingarten Date: Fri, 7 Nov 2014 01:47:47 +0000 Subject: [PATCH] Revert "Merge pull request #458 from Shopify/block-body" This reverts commit 12d526a05c0654dc67e350f170f91d62fd4875b7, reversing changes made to 263e90e7725586b66339eda953c8e6fbc7af8a6f. Conflicts: lib/liquid/block_body.rb --- History.md | 1 - lib/liquid.rb | 1 - lib/liquid/block.rb | 135 +++++++++++++++++++++------ lib/liquid/document.rb | 25 ++--- lib/liquid/locales/en.yml | 3 +- lib/liquid/profiler/hooks.rb | 2 +- lib/liquid/tags/case.rb | 23 ++--- lib/liquid/tags/for.rb | 20 ++-- lib/liquid/tags/if.rb | 11 +-- lib/liquid/tags/raw.rb | 19 +--- lib/liquid/tags/unless.rb | 4 +- test/integration/document_test.rb | 19 ---- test/unit/tags/case_tag_unit_test.rb | 2 +- test/unit/tags/for_tag_unit_test.rb | 4 +- test/unit/tags/if_tag_unit_test.rb | 2 +- test/unit/template_unit_test.rb | 11 +-- 16 files changed, 154 insertions(+), 128 deletions(-) delete mode 100644 test/integration/document_test.rb diff --git a/History.md b/History.md index 26f1fabb..486eb944 100644 --- a/History.md +++ b/History.md @@ -3,7 +3,6 @@ ## 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 af38cd42..f7191073 100644 --- a/lib/liquid.rb +++ b/lib/liquid.rb @@ -57,7 +57,6 @@ 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 e6db8692..7de23e21 100644 --- a/lib/liquid/block.rb +++ b/lib/liquid/block.rb @@ -1,26 +1,65 @@ module Liquid class Block < Tag - def initialize(tag_name, markup, options) - super - @blank = true - end - - def parse(tokens) - @body = BlockBody.new - while more = parse_body(@body, tokens) - end - end - - def render(context) - @body.render(context) - end + FullToken = /\A#{TagStart}\s*(\w+)\s*(.*)?#{TagEnd}\z/om + ContentOfVariable = /\A#{VariableStart}(.*)#{VariableEnd}\z/om + TAGSTART = "{%".freeze + VARSTART = "{{".freeze def blank? @blank end - def nodelist - @body.nodelist + 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! end # warnings of this block and all sub-tags @@ -57,23 +96,65 @@ 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 parse_body(body, tokens) - body.parse(tokens, options) do |end_tag_name, end_tag_params| - @blank &&= body.blank? + def assert_missing_delimitation! + raise SyntaxError.new(options[:locale].t("errors.syntax.tag_never_closed".freeze, :block_name => block_name)) + end - 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)) + 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)) 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 - true + 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 end end diff --git a/lib/liquid/document.rb b/lib/liquid/document.rb index c5709cb9..1969a3af 100644 --- a/lib/liquid/document.rb +++ b/lib/liquid/document.rb @@ -1,24 +1,17 @@ module Liquid - class Document < BlockBody - def self.parse(tokens, options) - doc = new - doc.parse(tokens, options) - doc + class Document < Block + def self.parse(tokens, options={}) + # we don't need markup to open this block + super(nil, nil, tokens, options) end - def parse(tokens, options) - super do |end_tag_name, end_tag_params| - unknown_tag(end_tag_name, options) if end_tag_name - end + # There isn't a real delimiter + def block_delimiter + [] end - def unknown_tag(tag, options) - 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 + # Document blocks don't need to be terminated since they are not actually opened + def assert_missing_delimitation! end end end diff --git a/lib/liquid/locales/en.yml b/lib/liquid/locales/en.yml index 3e7460c0..09f0ad86 100644 --- a/lib/liquid/locales/en.yml +++ b/lib/liquid/locales/en.yml @@ -14,8 +14,7 @@ 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_outer_tag: "Unexpected outer '%{tag}' tag" + unexpected_else: "%{block_name} tag does not expect else 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 8a6e8083..dfad85d4 100644 --- a/lib/liquid/profiler/hooks.rb +++ b/lib/liquid/profiler/hooks.rb @@ -1,5 +1,5 @@ module Liquid - class BlockBody + class Block < Tag 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 dea0be20..b18893b4 100644 --- a/lib/liquid/tags/case.rb +++ b/lib/liquid/tags/case.rb @@ -14,18 +14,12 @@ module Liquid end end - def parse(tokens) - body = BlockBody.new - while more = parse_body(body, tokens) - body = @blocks.last.attachment - end - end - def nodelist - @blocks.map(&:attachment) + @blocks.flat_map(&:attachment) end def unknown_tag(tag, markup, tokens) + @nodelist = [] case tag when 'when'.freeze record_when_condition(markup) @@ -43,10 +37,10 @@ module Liquid output = '' @blocks.each do |block| if block.else? - return block.attachment.render(context) if execute_else_block + return render_all(block.attachment, context) if execute_else_block elsif block.evaluate(context) execute_else_block = false - output << block.attachment.render(context) + output << render_all(block.attachment, context) end end output @@ -56,9 +50,8 @@ module Liquid private def record_when_condition(markup) - body = BlockBody.new - 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 @@ -66,8 +59,8 @@ module Liquid markup = $2 block = Condition.new(@left, '=='.freeze, Expression.parse($1)) - block.attach(body) - @blocks << block + block.attach(@nodelist) + @blocks.push(block) end end @@ -77,7 +70,7 @@ module Liquid end block = ElseCondition.new - block.attach(BlockBody.new) + block.attach(@nodelist) @blocks << block end end diff --git a/lib/liquid/tags/for.rb b/lib/liquid/tags/for.rb index 31da9d67..9e09af59 100644 --- a/lib/liquid/tags/for.rb +++ b/lib/liquid/tags/for.rb @@ -49,22 +49,20 @@ module Liquid def initialize(tag_name, markup, options) super parse_with_selected_parser(markup) - @for_block = BlockBody.new - end - - def parse(tokens) - if more = parse_body(@for_block, tokens) - parse_body(@else_block, tokens) - end + @nodelist = @for_block = [] end def nodelist - @else_block ? [@for_block, @else_block] : [@for_block] + if @else_block + @for_block + @else_block + else + @for_block + end end def unknown_tag(tag, markup, tokens) return super unless tag == 'else'.freeze - @else_block = BlockBody.new + @nodelist = @else_block = [] end def render(context) @@ -112,7 +110,7 @@ module Liquid 'last'.freeze => (index == length - 1) } - result << @for_block.render(context) + result << render_all(@for_block, context) # Handle any interrupts if they exist. if context.has_interrupt? @@ -177,7 +175,7 @@ module Liquid end def render_else(context) - @else_block ? @else_block.render(context) : ''.freeze + return @else_block ? [render_all(@else_block, context)] : ''.freeze end def iterable?(collection) diff --git a/lib/liquid/tags/if.rb b/lib/liquid/tags/if.rb index ba668f23..f91ba95e 100644 --- a/lib/liquid/tags/if.rb +++ b/lib/liquid/tags/if.rb @@ -20,13 +20,8 @@ module Liquid push_block('if'.freeze, markup) end - def parse(tokens) - while more = parse_body(@blocks.last.attachment, tokens) - end - end - def nodelist - @blocks.map(&:attachment) + @blocks.flat_map(&:attachment) end def unknown_tag(tag, markup, tokens) @@ -41,7 +36,7 @@ module Liquid context.stack do @blocks.each do |block| if block.evaluate(context) - return block.attachment.render(context) + return render_all(block.attachment, context) end end ''.freeze @@ -58,7 +53,7 @@ module Liquid end @blocks.push(block) - block.attach(BlockBody.new) + @nodelist = block.attach(Array.new) end def lax_parse(markup) diff --git a/lib/liquid/tags/raw.rb b/lib/liquid/tags/raw.rb index 41b2ec45..59e52c98 100644 --- a/lib/liquid/tags/raw.rb +++ b/lib/liquid/tags/raw.rb @@ -3,27 +3,16 @@ module Liquid FullTokenPossiblyInvalid = /\A(.*)#{TagStart}\s*(\w+)\s*(.*)?#{TagEnd}\z/om def parse(tokens) - @body = '' + @nodelist ||= [] + @nodelist.clear while token = tokens.shift if token =~ FullTokenPossiblyInvalid - @body << $1 if $1 != "".freeze + @nodelist << $1 if $1 != "".freeze return if block_delimiter == $2 end - @body << token if not token.empty? + @nodelist << 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 7aff42b2..eb8a7314 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 first_block.attachment.render(context) + return render_all(first_block.attachment, context) end # After the first condition unless works just like if @blocks[1..-1].each do |block| if block.evaluate(context) - return block.attachment.render(context) + return render_all(block.attachment, context) end end diff --git a/test/integration/document_test.rb b/test/integration/document_test.rb deleted file mode 100644 index bcc4a21c..00000000 --- a/test/integration/document_test.rb +++ /dev/null @@ -1,19 +0,0 @@ -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/unit/tags/case_tag_unit_test.rb b/test/unit/tags/case_tag_unit_test.rb index 71103081..3c1be2c9 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.map(&:nodelist).flatten + assert_equal ['WHEN', 'ELSE'], template.root.nodelist[0].nodelist end end diff --git a/test/unit/tags/for_tag_unit_test.rb b/test/unit/tags/for_tag_unit_test.rb index b8fc5206..17f88cef 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.map(&:nodelist).flatten + assert_equal ['FOR'], template.root.nodelist[0].nodelist 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.map(&:nodelist).flatten + assert_equal ['FOR', 'ELSE'], template.root.nodelist[0].nodelist end end diff --git a/test/unit/tags/if_tag_unit_test.rb b/test/unit/tags/if_tag_unit_test.rb index 7ecfc409..17f826cc 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.map(&:nodelist).flatten + assert_equal ['IF', 'ELSE'], template.root.nodelist[0].nodelist end end diff --git a/test/unit/template_unit_test.rb b/test/unit/template_unit_test.rb index 3d63c09a..c50b0673 100644 --- a/test/unit/template_unit_test.rb +++ b/test/unit/template_unit_test.rb @@ -5,17 +5,16 @@ class TemplateUnitTest < Minitest::Test def test_sets_default_localization_in_document t = Template.new - t.parse('{%comment%}{%endcomment%}') - assert_instance_of I18n, t.root.nodelist[0].options[:locale] + t.parse('') + assert_instance_of I18n, t.root.options[:locale] end def test_sets_default_localization_in_context_with_quick_initialization t = Template.new - t.parse('{%comment%}{%endcomment%}', :locale => I18n.new(fixture("en_locale.yml"))) + t.parse('{{foo}}', :locale => I18n.new(fixture("en_locale.yml"))) - locale = t.root.nodelist[0].options[:locale] - assert_instance_of I18n, locale - assert_equal fixture("en_locale.yml"), locale.path + assert_instance_of I18n, t.root.options[:locale] + assert_equal fixture("en_locale.yml"), t.root.options[:locale].path end def test_with_cache_classes_tags_returns_the_same_class