From 2a1ca3152d1e5726e05ac27c0a6b33a61577cbe1 Mon Sep 17 00:00:00 2001 From: Florian Weingarten Date: Wed, 17 Apr 2019 14:07:10 +0100 Subject: [PATCH] liquid without the garbage --- lib/liquid/block.rb | 4 ++-- lib/liquid/block_body.rb | 24 ++++++++++------------- lib/liquid/profiler/hooks.rb | 12 ++++++------ lib/liquid/tag.rb | 4 ++-- lib/liquid/tags/assign.rb | 6 +++--- lib/liquid/tags/capture.rb | 8 ++++---- lib/liquid/tags/case.rb | 10 +++++----- lib/liquid/tags/comment.rb | 4 ++-- lib/liquid/tags/cycle.rb | 17 +++++++++++++--- lib/liquid/tags/decrement.rb | 5 +++-- lib/liquid/tags/for.rb | 22 +++++++++++---------- lib/liquid/tags/if.rb | 7 ++++--- lib/liquid/tags/ifchanged.rb | 14 ++++++------- lib/liquid/tags/include.rb | 10 ++++++---- lib/liquid/tags/increment.rb | 5 +++-- lib/liquid/tags/raw.rb | 5 +++-- lib/liquid/tags/table_row.rb | 15 ++++++++------ lib/liquid/tags/unless.rb | 10 +++++----- lib/liquid/template.rb | 10 +++++++++- lib/liquid/variable.rb | 17 ++++++++++++++-- performance/shopify/comment_form.rb | 6 ++++-- performance/shopify/paginate.rb | 2 +- test/integration/blank_test.rb | 5 +++-- test/integration/tags/include_tag_test.rb | 5 +++-- test/integration/template_test.rb | 18 ++++++++--------- 25 files changed, 144 insertions(+), 101 deletions(-) diff --git a/lib/liquid/block.rb b/lib/liquid/block.rb index 00c59b2..843b6b7 100644 --- a/lib/liquid/block.rb +++ b/lib/liquid/block.rb @@ -13,8 +13,8 @@ module Liquid end end - def render(context) - @body.render(context) + def render(context, output = '') + @body.render(context, output) end def blank? diff --git a/lib/liquid/block_body.rb b/lib/liquid/block_body.rb index d4fab30..f404d20 100644 --- a/lib/liquid/block_body.rb +++ b/lib/liquid/block_body.rb @@ -66,8 +66,7 @@ module Liquid @blank end - def render(context) - output = [] + def render(context, output = '') context.resource_limits.render_score += @nodelist.length idx = 0 @@ -77,9 +76,9 @@ module Liquid check_resources(context, node) output << node when Variable - render_node_to_output(node, output, context) + render_node(context, output, node) when Block - render_node_to_output(node, output, context, node.blank?) + render_node(context, node.blank? ? '' : output, node) break if context.interrupt? # might have happened in a for-block when Continue, Break # If we get an Interrupt that means the block must stop processing. An @@ -88,34 +87,31 @@ module Liquid context.push_interrupt(node.interrupt) break else # Other non-Block tags - render_node_to_output(node, output, context) + render_node(context, output, node) break if context.interrupt? # might have happened through an include end idx += 1 end - output.join + output end private - def render_node_to_output(node, output, context, skip_output = false) - node_output = node.render(context) - node_output = node_output.is_a?(Array) ? node_output.join : node_output.to_s - check_resources(context, node_output) - output << node_output unless skip_output + def render_node(context, output, node) + node.render(context, output) + check_resources(context, output) rescue MemoryError => e raise e rescue UndefinedVariable, UndefinedDropMethod, UndefinedFilter => e context.handle_error(e, node.line_number) - output << nil rescue ::StandardError => e line_number = node.is_a?(String) ? nil : node.line_number output << context.handle_error(e, line_number) end - def check_resources(context, node_output) - context.resource_limits.render_length += node_output.bytesize + def check_resources(context, output) + context.resource_limits.render_length = output.bytesize return unless context.resource_limits.reached? raise MemoryError.new("Memory limits exceeded".freeze) end diff --git a/lib/liquid/profiler/hooks.rb b/lib/liquid/profiler/hooks.rb index cb11cd7..112a7d3 100644 --- a/lib/liquid/profiler/hooks.rb +++ b/lib/liquid/profiler/hooks.rb @@ -1,19 +1,19 @@ module Liquid class BlockBody - def render_node_with_profiling(node, output, context, skip_output = false) + def render_node_with_profiling(context, output, node) Profiler.profile_node_render(node) do - render_node_without_profiling(node, output, context, skip_output) + render_node_without_profiling(context, output, node) end end - alias_method :render_node_without_profiling, :render_node_to_output - alias_method :render_node_to_output, :render_node_with_profiling + alias_method :render_node_without_profiling, :render_node + alias_method :render_node, :render_node_with_profiling end class Include < Tag - def render_with_profiling(context) + def render_with_profiling(context, output) Profiler.profile_children(context.evaluate(@template_name_expr).to_s) do - render_without_profiling(context) + render_without_profiling(context, output) end end diff --git a/lib/liquid/tag.rb b/lib/liquid/tag.rb index 06970c1..d9d4ff7 100644 --- a/lib/liquid/tag.rb +++ b/lib/liquid/tag.rb @@ -32,8 +32,8 @@ module Liquid self.class.name.downcase end - def render(_context) - ''.freeze + def render(_context, output = '') + output end def blank? diff --git a/lib/liquid/tags/assign.rb b/lib/liquid/tags/assign.rb index 3b696dd..62a4747 100644 --- a/lib/liquid/tags/assign.rb +++ b/lib/liquid/tags/assign.rb @@ -22,11 +22,11 @@ module Liquid end end - def render(context) - val = @from.render(context) + def render(context, output = '') + val = @from.render(context, nil) context.scopes.last[@to] = val context.resource_limits.assign_score += assign_score_of(val) - ''.freeze + output end def blank? diff --git a/lib/liquid/tags/capture.rb b/lib/liquid/tags/capture.rb index d5b8e29..b4d418b 100644 --- a/lib/liquid/tags/capture.rb +++ b/lib/liquid/tags/capture.rb @@ -22,11 +22,11 @@ module Liquid end end - def render(context) - output = super + def render(context, output = '') + super context.scopes.last[@to] = output - context.resource_limits.assign_score += output.bytesize - ''.freeze + context.resource_limits.assign_score = output.bytesize + output end def blank? diff --git a/lib/liquid/tags/case.rb b/lib/liquid/tags/case.rb index 5036b27..385c878 100644 --- a/lib/liquid/tags/case.rb +++ b/lib/liquid/tags/case.rb @@ -38,21 +38,21 @@ module Liquid end end - def render(context) + def render(context, output = '') context.stack do execute_else_block = true - output = '' @blocks.each do |block| if block.else? - return block.attachment.render(context) if execute_else_block + block.attachment.render(context, output) if execute_else_block elsif block.evaluate(context) execute_else_block = false - output << block.attachment.render(context) + block.attachment.render(context, output) end end - output end + + output end private diff --git a/lib/liquid/tags/comment.rb b/lib/liquid/tags/comment.rb index c57c9cd..cb7739f 100644 --- a/lib/liquid/tags/comment.rb +++ b/lib/liquid/tags/comment.rb @@ -1,7 +1,7 @@ module Liquid class Comment < Block - def render(_context) - ''.freeze + def render(_context, output = '') + output end def unknown_tag(_tag, _markup, _tokens) diff --git a/lib/liquid/tags/cycle.rb b/lib/liquid/tags/cycle.rb index 17aa860..cbcff79 100644 --- a/lib/liquid/tags/cycle.rb +++ b/lib/liquid/tags/cycle.rb @@ -31,18 +31,29 @@ module Liquid end end - def render(context) + def render(context, output = '') context.registers[:cycle] ||= {} context.stack do key = context.evaluate(@name) iteration = context.registers[:cycle][key].to_i - result = context.evaluate(@variables[iteration]) + + val = context.evaluate(@variables[iteration]) + + if val.is_a?(Array) + val = val.join + elsif !val.is_a?(String) + val = val.to_s + end + + output << val + iteration += 1 iteration = 0 if iteration >= @variables.size context.registers[:cycle][key] = iteration - result end + + output end private diff --git a/lib/liquid/tags/decrement.rb b/lib/liquid/tags/decrement.rb index b5cdaaa..9cf3073 100644 --- a/lib/liquid/tags/decrement.rb +++ b/lib/liquid/tags/decrement.rb @@ -23,11 +23,12 @@ module Liquid @variable = markup.strip end - def render(context) + def render(context, output = '') value = context.environments.first[@variable] ||= 0 value -= 1 context.environments.first[@variable] = value - value.to_s + output << value.to_s + output end end diff --git a/lib/liquid/tags/for.rb b/lib/liquid/tags/for.rb index b69aa78..01df484 100644 --- a/lib/liquid/tags/for.rb +++ b/lib/liquid/tags/for.rb @@ -70,13 +70,13 @@ module Liquid @else_block = BlockBody.new end - def render(context) + def render(context, output = '') segment = collection_segment(context) if segment.empty? - render_else(context) + render_else(context, output) else - render_segment(context, segment) + render_segment(context, output, segment) end end @@ -141,12 +141,10 @@ module Liquid segment end - def render_segment(context, segment) + def render_segment(context, output, segment) for_stack = context.registers[:for_stack] ||= [] length = segment.length - result = '' - context.stack do loop_vars = Liquid::ForloopDrop.new(@name, length, for_stack[-1]) @@ -157,7 +155,7 @@ module Liquid segment.each do |item| context[@variable_name] = item - result << @for_block.render(context) + @for_block.render(context, output) loop_vars.send(:increment!) # Handle any interrupts if they exist. @@ -172,7 +170,7 @@ module Liquid end end - result + output end def set_attribute(key, expr) @@ -188,8 +186,12 @@ module Liquid end end - def render_else(context) - @else_block ? @else_block.render(context) : ''.freeze + def render_else(context, output) + if @else_block + @else_block.render(context, output) + else + output + end end class ParseTreeVisitor < Liquid::ParseTreeVisitor diff --git a/lib/liquid/tags/if.rb b/lib/liquid/tags/if.rb index 02da42b..a1e4eec 100644 --- a/lib/liquid/tags/if.rb +++ b/lib/liquid/tags/if.rb @@ -39,15 +39,16 @@ module Liquid end end - def render(context) + def render(context, output) context.stack do @blocks.each do |block| if block.evaluate(context) - return block.attachment.render(context) + return block.attachment.render(context, output) end end - ''.freeze end + + output end private diff --git a/lib/liquid/tags/ifchanged.rb b/lib/liquid/tags/ifchanged.rb index d70cbe1..2c026f1 100644 --- a/lib/liquid/tags/ifchanged.rb +++ b/lib/liquid/tags/ifchanged.rb @@ -1,16 +1,16 @@ module Liquid class Ifchanged < Block - def render(context) + def render(context, output) context.stack do - output = super + block_output = super(context, '') - if output != context.registers[:ifchanged] - context.registers[:ifchanged] = output - output - else - ''.freeze + if block_output != context.registers[:ifchanged] + context.registers[:ifchanged] = block_output + output << block_output end end + + output end end diff --git a/lib/liquid/tags/include.rb b/lib/liquid/tags/include.rb index c9f2a28..28a7481 100644 --- a/lib/liquid/tags/include.rb +++ b/lib/liquid/tags/include.rb @@ -42,7 +42,7 @@ module Liquid def parse(_tokens) end - def render(context) + def render(context, output) template_name = context.evaluate(@template_name_expr) raise ArgumentError.new(options[:locale].t("errors.argument.include")) unless template_name @@ -66,19 +66,21 @@ module Liquid end if variable.is_a?(Array) - variable.collect do |var| + variable.each do |var| context[context_variable_name] = var - partial.render(context) + partial.render(context, output: output) end else context[context_variable_name] = variable - partial.render(context) + partial.render(context, output: output) end end ensure context.template_name = old_template_name context.partial = old_partial end + + output end private diff --git a/lib/liquid/tags/increment.rb b/lib/liquid/tags/increment.rb index baa0cbb..4182c46 100644 --- a/lib/liquid/tags/increment.rb +++ b/lib/liquid/tags/increment.rb @@ -20,10 +20,11 @@ module Liquid @variable = markup.strip end - def render(context) + def render(context, output = '') value = context.environments.first[@variable] ||= 0 context.environments.first[@variable] = value + 1 - value.to_s + output << value.to_s + output end end diff --git a/lib/liquid/tags/raw.rb b/lib/liquid/tags/raw.rb index 6b461bd..4f6aa15 100644 --- a/lib/liquid/tags/raw.rb +++ b/lib/liquid/tags/raw.rb @@ -22,8 +22,9 @@ module Liquid raise SyntaxError.new(parse_context.locale.t("errors.syntax.tag_never_closed".freeze, block_name: block_name)) end - def render(_context) - @body + def render(_context, output = '') + output << @body + output end def nodelist diff --git a/lib/liquid/tags/table_row.rb b/lib/liquid/tags/table_row.rb index 7f391cf..09b69e9 100644 --- a/lib/liquid/tags/table_row.rb +++ b/lib/liquid/tags/table_row.rb @@ -18,7 +18,7 @@ module Liquid end end - def render(context) + def render(context, output = '') collection = context.evaluate(@collection_name) or return ''.freeze from = @attributes.key?('offset'.freeze) ? context.evaluate(@attributes['offset'.freeze]).to_i : 0 @@ -30,7 +30,7 @@ module Liquid cols = context.evaluate(@attributes['cols'.freeze]).to_i - result = "\n" + output << "\n" context.stack do tablerowloop = Liquid::TablerowloopDrop.new(length, cols) context['tablerowloop'.freeze] = tablerowloop @@ -38,17 +38,20 @@ module Liquid collection.each do |item| context[@variable_name] = item - result << "" << super << '' + output << "" + super + output << '' if tablerowloop.col_last && !tablerowloop.last - result << "\n" + output << "\n" end tablerowloop.send(:increment!) end end - result << "\n" - result + + output << "\n" + output end class ParseTreeVisitor < Liquid::ParseTreeVisitor diff --git a/lib/liquid/tags/unless.rb b/lib/liquid/tags/unless.rb index 1d4280d..8d4c32e 100644 --- a/lib/liquid/tags/unless.rb +++ b/lib/liquid/tags/unless.rb @@ -6,23 +6,23 @@ module Liquid # {% unless x < 0 %} x is greater than zero {% endunless %} # class Unless < If - def render(context) + def render(context, output = '') context.stack do # First condition is interpreted backwards ( if not ) first_block = @blocks.first unless first_block.evaluate(context) - return first_block.attachment.render(context) + return first_block.attachment.render(context, output) 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 block.attachment.render(context, output) end end - - ''.freeze end + + output end end diff --git a/lib/liquid/template.rb b/lib/liquid/template.rb index 31a67e4..c22f164 100644 --- a/lib/liquid/template.rb +++ b/lib/liquid/template.rb @@ -187,9 +187,12 @@ module Liquid raise ArgumentError, "Expected Hash or Liquid::Context as parameter" end + output = nil + case args.last when Hash options = args.pop + output = options[:output] if options[:output] registers.merge!(options[:registers]) if options[:registers].is_a?(Hash) @@ -205,7 +208,8 @@ module Liquid # render the nodelist. # for performance reasons we get an array back here. join will make a string out of it. result = with_profiling(context) do - @root.render(context) + output ||= self.class.output_buffer.clear + @root.render(context, output) end result.respond_to?(:join) ? result.join : result rescue Liquid::MemoryError => e @@ -215,6 +219,10 @@ module Liquid end end + def self.output_buffer + @output_buffer ||= String.new(capacity: 1024) + end + def render!(*args) @rethrow_errors = true render(*args) diff --git a/lib/liquid/variable.rb b/lib/liquid/variable.rb index 717b1a2..5e03706 100644 --- a/lib/liquid/variable.rb +++ b/lib/liquid/variable.rb @@ -78,7 +78,7 @@ module Liquid filterargs end - def render(context) + def render(context, output = '') obj = @filters.inject(context.evaluate(@name)) do |output, (filter_name, filter_args, filter_kwargs)| filter_args = evaluate_filter_expressions(context, filter_args, filter_kwargs) context.invoke(filter_name, output, *filter_args) @@ -88,7 +88,20 @@ module Liquid taint_check(context, obj) - obj + if output + if obj.is_a?(Array) + output << obj.join + elsif obj.nil? + elsif !obj.is_a?(String) + output << obj.to_s + else + output << obj + end + + output + else + obj + end end private diff --git a/performance/shopify/comment_form.rb b/performance/shopify/comment_form.rb index d661c31..ce33a2c 100644 --- a/performance/shopify/comment_form.rb +++ b/performance/shopify/comment_form.rb @@ -12,7 +12,7 @@ class CommentForm < Liquid::Block end end - def render(context) + def render(context, output = '') article = context[@variable_name] context.stack do @@ -23,7 +23,9 @@ class CommentForm < Liquid::Block 'email' => context['comment.email'], 'body' => context['comment.body'] } - wrap_in_form(article, render_all(@nodelist, context)) + + output << wrap_in_form(article, render_all(@nodelist, context, output)) + output end end diff --git a/performance/shopify/paginate.rb b/performance/shopify/paginate.rb index 38a9a1a..875e58b 100644 --- a/performance/shopify/paginate.rb +++ b/performance/shopify/paginate.rb @@ -21,7 +21,7 @@ class Paginate < Liquid::Block end end - def render(context) + def render(context, output = '') @context = context context.stack do diff --git a/test/integration/blank_test.rb b/test/integration/blank_test.rb index e9b56df..cc09781 100644 --- a/test/integration/blank_test.rb +++ b/test/integration/blank_test.rb @@ -1,8 +1,9 @@ require 'test_helper' class FoobarTag < Liquid::Tag - def render(*args) - " " + def render(context, output = '') + output << ' ' + output end Liquid::Template.register_tag('foobar', FoobarTag) diff --git a/test/integration/tags/include_tag_test.rb b/test/integration/tags/include_tag_test.rb index 9c188d5..cf83f59 100644 --- a/test/integration/tags/include_tag_test.rb +++ b/test/integration/tags/include_tag_test.rb @@ -66,8 +66,9 @@ class CustomInclude < Liquid::Tag def parse(tokens) end - def render(context) - @template_name[1..-2] + def render(context, output = '') + output << @template_name[1..-2] + output end end diff --git a/test/integration/template_test.rb b/test/integration/template_test.rb index 0dc0ae5..c8b7770 100644 --- a/test/integration/template_test.rb +++ b/test/integration/template_test.rb @@ -177,31 +177,31 @@ class TemplateTest < Minitest::Test def test_render_length_persists_between_blocks t = Template.parse("{% if true %}aaaa{% endif %}") - t.resource_limits.render_length_limit = 7 + t.resource_limits.render_length_limit = 3 assert_equal "Liquid error: Memory limits exceeded", t.render - t.resource_limits.render_length_limit = 8 + t.resource_limits.render_length_limit = 4 assert_equal "aaaa", t.render t = Template.parse("{% if true %}aaaa{% endif %}{% if true %}bbb{% endif %}") - t.resource_limits.render_length_limit = 13 + t.resource_limits.render_length_limit = 6 assert_equal "Liquid error: Memory limits exceeded", t.render - t.resource_limits.render_length_limit = 14 + t.resource_limits.render_length_limit = 7 assert_equal "aaaabbb", t.render t = Template.parse("{% if true %}a{% endif %}{% if true %}b{% endif %}{% if true %}a{% endif %}{% if true %}b{% endif %}{% if true %}a{% endif %}{% if true %}b{% endif %}") + t.resource_limits.render_length_limit = 4 + assert_equal "Liquid error: Memory limits exceeded", t.render t.resource_limits.render_length_limit = 5 assert_equal "Liquid error: Memory limits exceeded", t.render - t.resource_limits.render_length_limit = 11 - assert_equal "Liquid error: Memory limits exceeded", t.render - t.resource_limits.render_length_limit = 12 + t.resource_limits.render_length_limit = 6 assert_equal "ababab", t.render end def test_render_length_uses_number_of_bytes_not_characters t = Template.parse("{% if true %}すごい{% endif %}") - t.resource_limits.render_length_limit = 10 + t.resource_limits.render_length_limit = 8 assert_equal "Liquid error: Memory limits exceeded", t.render - t.resource_limits.render_length_limit = 18 + t.resource_limits.render_length_limit = 9 assert_equal "すごい", t.render end