diff --git a/.gitignore b/.gitignore index 7ac01c1..90bf6dc 100644 --- a/.gitignore +++ b/.gitignore @@ -7,3 +7,4 @@ pkg .ruby-version Gemfile.lock .bundle +.byebug_history diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index 6868a7a..4af3202 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -1,6 +1,6 @@ # This configuration was generated by # `rubocop --auto-gen-config` -# on 2019-03-19 11:04:37 -0400 using RuboCop version 0.53.0. +# on 2019-04-22 19:11:24 -0400 using RuboCop version 0.53.0. # The point is for the user to remove these configuration records # one by one as the offenses are removed from the code base. # Note that changes in the inspected code, or installation of new @@ -46,18 +46,18 @@ Lint/Void: Exclude: - 'lib/liquid/parse_context.rb' -# Offense count: 54 +# Offense count: 53 Metrics/AbcSize: Max: 56 # Offense count: 12 Metrics/CyclomaticComplexity: - Max: 12 + Max: 13 # Offense count: 112 # Configuration parameters: CountComments. Metrics/MethodLength: - Max: 37 + Max: 38 # Offense count: 8 Metrics/PerceivedComplexity: @@ -90,7 +90,7 @@ Naming/UncommunicativeMethodParamName: - 'test/integration/template_test.rb' - 'test/unit/condition_unit_test.rb' -# Offense count: 10 +# Offense count: 12 # Cop supports --auto-correct. # Configuration parameters: EnforcedStyle. # SupportedStyles: prefer_alias, prefer_alias_method @@ -253,7 +253,7 @@ Style/WhileUntilModifier: Exclude: - 'lib/liquid/tags/case.rb' -# Offense count: 640 +# Offense count: 648 # Configuration parameters: AllowHeredoc, AllowURI, URISchemes, IgnoreCopDirectives, IgnoredPatterns. # URISchemes: http, https Metrics/LineLength: diff --git a/lib/liquid/block.rb b/lib/liquid/block.rb index 00c59b2..549a3a3 100644 --- a/lib/liquid/block.rb +++ b/lib/liquid/block.rb @@ -13,6 +13,7 @@ module Liquid end end + # For backwards compatibility def render(context) @body.render(context) end diff --git a/lib/liquid/block_body.rb b/lib/liquid/block_body.rb index d4fab30..c2478ce 100644 --- a/lib/liquid/block_body.rb +++ b/lib/liquid/block_body.rb @@ -67,19 +67,23 @@ module Liquid end def render(context) - output = [] + render_to_output_buffer(context, '') + end + + def render_to_output_buffer(context, output) context.resource_limits.render_score += @nodelist.length idx = 0 while node = @nodelist[idx] + previous_output_size = output.bytesize + case node when String - 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 +92,30 @@ 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 + + raise_if_resource_limits_reached(context, output.bytesize - previous_output_size) 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 - rescue MemoryError => e - raise e + def render_node(context, output, node) + node.render_to_output_buffer(context, output) 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 raise_if_resource_limits_reached(context, length) + context.resource_limits.render_length += length 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..cda166b 100644 --- a/lib/liquid/profiler/hooks.rb +++ b/lib/liquid/profiler/hooks.rb @@ -1,23 +1,23 @@ 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_to_output_buffer_with_profiling(context, output) Profiler.profile_children(context.evaluate(@template_name_expr).to_s) do - render_without_profiling(context) + render_to_output_buffer_without_profiling(context, output) end end - alias_method :render_without_profiling, :render - alias_method :render, :render_with_profiling + alias_method :render_to_output_buffer_without_profiling, :render_to_output_buffer + alias_method :render_to_output_buffer, :render_to_output_buffer_with_profiling end end diff --git a/lib/liquid/tag.rb b/lib/liquid/tag.rb index 06970c1..5099ccb 100644 --- a/lib/liquid/tag.rb +++ b/lib/liquid/tag.rb @@ -36,6 +36,14 @@ module Liquid ''.freeze end + # For backwards compatibility with custom tags. In a future release, the semantics + # of the `render_to_output_buffer` method will become the default and the `render` + # method will be removed. + def render_to_output_buffer(context, output) + output << render(context) + output + end + def blank? false end diff --git a/lib/liquid/tags/assign.rb b/lib/liquid/tags/assign.rb index 3b696dd..767d874 100644 --- a/lib/liquid/tags/assign.rb +++ b/lib/liquid/tags/assign.rb @@ -22,11 +22,11 @@ module Liquid end end - def render(context) + def render_to_output_buffer(context, output) val = @from.render(context) 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..d717b76 100644 --- a/lib/liquid/tags/capture.rb +++ b/lib/liquid/tags/capture.rb @@ -22,11 +22,12 @@ module Liquid end end - def render(context) - output = super + def render_to_output_buffer(context, output) + previous_output_size = output.bytesize + super context.scopes.last[@to] = output - context.resource_limits.assign_score += output.bytesize - ''.freeze + context.resource_limits.assign_score += (output.bytesize - previous_output_size) + output end def blank? diff --git a/lib/liquid/tags/case.rb b/lib/liquid/tags/case.rb index 5036b27..92b2ed0 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_to_output_buffer(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_to_output_buffer(context, output) if execute_else_block elsif block.evaluate(context) execute_else_block = false - output << block.attachment.render(context) + block.attachment.render_to_output_buffer(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..cad3931 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_to_output_buffer(_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..e42244d 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_to_output_buffer(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..08ddd4d 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_to_output_buffer(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 f18fb71..71c2f91 100644 --- a/lib/liquid/tags/for.rb +++ b/lib/liquid/tags/for.rb @@ -70,14 +70,16 @@ module Liquid @else_block = BlockBody.new end - def render(context) + def render_to_output_buffer(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 + + output end protected @@ -150,12 +152,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]) @@ -166,7 +166,7 @@ module Liquid segment.each do |item| context[@variable_name] = item - result << @for_block.render(context) + @for_block.render_to_output_buffer(context, output) loop_vars.send(:increment!) # Handle any interrupts if they exist. @@ -181,7 +181,7 @@ module Liquid end end - result + output end def set_attribute(key, expr) @@ -197,8 +197,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_to_output_buffer(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..25534a9 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_to_output_buffer(context, output) context.stack do @blocks.each do |block| if block.evaluate(context) - return block.attachment.render(context) + return block.attachment.render_to_output_buffer(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..e3040ce 100644 --- a/lib/liquid/tags/ifchanged.rb +++ b/lib/liquid/tags/ifchanged.rb @@ -1,16 +1,17 @@ module Liquid class Ifchanged < Block - def render(context) + def render_to_output_buffer(context, output) context.stack do - output = super + block_output = '' + super(context, block_output) - 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..24acf9d 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_to_output_buffer(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_to_output_buffer(context, output) end else context[context_variable_name] = variable - partial.render(context) + partial.render_to_output_buffer(context, 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..5af1242 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_to_output_buffer(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..4fa75d9 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_to_output_buffer(_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..9532102 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_to_output_buffer(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..18856c3 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_to_output_buffer(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_to_output_buffer(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_to_output_buffer(context, output) end end - - ''.freeze end + + output end end diff --git a/lib/liquid/template.rb b/lib/liquid/template.rb index 31a67e4..35db674 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) @@ -204,10 +207,9 @@ module Liquid begin # 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) + with_profiling(context) do + @root.render_to_output_buffer(context, output || '') end - result.respond_to?(:join) ? result.join : result rescue Liquid::MemoryError => e context.handle_error(e) ensure @@ -220,6 +222,10 @@ module Liquid render(*args) end + def render_to_output_buffer(context, output) + render(context, output: output) + end + private def tokenize(source) diff --git a/lib/liquid/variable.rb b/lib/liquid/variable.rb index 717b1a2..cbf9986 100644 --- a/lib/liquid/variable.rb +++ b/lib/liquid/variable.rb @@ -85,12 +85,23 @@ module Liquid end obj = context.apply_global_filter(obj) - taint_check(context, obj) - obj end + def render_to_output_buffer(context, output) + obj = render(context) + + if obj.is_a?(Array) + output << obj.join + elsif obj.nil? + else + output << obj.to_s + end + + output + end + private def parse_filter_expressions(filter_name, unparsed_args) diff --git a/performance/shopify/comment_form.rb b/performance/shopify/comment_form.rb index d661c31..7b5bd53 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_to_output_buffer(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..0abd11f 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_to_output_buffer(context, output) @context = context context.stack do diff --git a/test/integration/blank_test.rb b/test/integration/blank_test.rb index e9b56df..2b46ad7 100644 --- a/test/integration/blank_test.rb +++ b/test/integration/blank_test.rb @@ -1,11 +1,10 @@ require 'test_helper' class FoobarTag < Liquid::Tag - def render(*args) - " " + def render_to_output_buffer(context, output) + output << ' ' + output end - - Liquid::Template.register_tag('foobar', FoobarTag) end class BlankTestFileSystem @@ -31,7 +30,9 @@ class BlankTest < Minitest::Test end def test_new_tags_are_not_blank_by_default - assert_template_result(" " * N, wrap_in_for("{% foobar %}")) + with_custom_tag('foobar', FoobarTag) do + assert_template_result(" " * N, wrap_in_for("{% foobar %}")) + end end def test_loops_are_blank diff --git a/test/integration/tags/include_tag_test.rb b/test/integration/tags/include_tag_test.rb index 9c188d5..14bb8c3 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_to_output_buffer(context, output) + output << @template_name[1..-2] + output end end diff --git a/test/test_helper.rb b/test/test_helper.rb index ac5ab53..34e7553 100755 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -84,6 +84,13 @@ module Minitest ensure Liquid::Template.error_mode = old_mode end + + def with_custom_tag(tag_name, tag_class) + Liquid::Template.register_tag(tag_name, tag_class) + yield + ensure + Liquid::Template.tags.delete(tag_name) + end end end diff --git a/test/unit/block_unit_test.rb b/test/unit/block_unit_test.rb index 6a27a7d..9f7b94f 100644 --- a/test/unit/block_unit_test.rb +++ b/test/unit/block_unit_test.rb @@ -44,10 +44,47 @@ class BlockUnitTest < Minitest::Test end def test_with_custom_tag - Liquid::Template.register_tag("testtag", Block) - assert Liquid::Template.parse("{% testtag %} {% endtesttag %}") - ensure - Liquid::Template.tags.delete('testtag') + with_custom_tag('testtag', Block) do + assert Liquid::Template.parse("{% testtag %} {% endtesttag %}") + end + end + + def test_custom_block_tags_have_a_default_render_to_output_buffer_method_for_backwards_compatibility + klass1 = Class.new(Block) do + def render(*) + 'hello' + end + end + + with_custom_tag('blabla', klass1) do + template = Liquid::Template.parse("{% blabla %} bla {% endblabla %}") + + assert_equal 'hello', template.render + + buf = '' + output = template.render({}, output: buf) + assert_equal 'hello', output + assert_equal 'hello', buf + assert_equal buf.object_id, output.object_id + end + + klass2 = Class.new(klass1) do + def render(*) + 'foo' + super + 'bar' + end + end + + with_custom_tag('blabla', klass2) do + template = Liquid::Template.parse("{% blabla %} foo {% endblabla %}") + + assert_equal 'foohellobar', template.render + + buf = '' + output = template.render({}, output: buf) + assert_equal 'foohellobar', output + assert_equal 'foohellobar', buf + assert_equal buf.object_id, output.object_id + end end private diff --git a/test/unit/tag_unit_test.rb b/test/unit/tag_unit_test.rb index c4b901b..a3fb40e 100644 --- a/test/unit/tag_unit_test.rb +++ b/test/unit/tag_unit_test.rb @@ -18,4 +18,42 @@ class TagUnitTest < Minitest::Test tag = Tag.parse("some_tag", "", Tokenizer.new(""), ParseContext.new) assert_equal 'some_tag', tag.tag_name end + + def test_custom_tags_have_a_default_render_to_output_buffer_method_for_backwards_compatibility + klass1 = Class.new(Tag) do + def render(*) + 'hello' + end + end + + with_custom_tag('blabla', klass1) do + template = Liquid::Template.parse("{% blabla %}") + + assert_equal 'hello', template.render + + buf = '' + output = template.render({}, output: buf) + assert_equal 'hello', output + assert_equal 'hello', buf + assert_equal buf.object_id, output.object_id + end + + klass2 = Class.new(klass1) do + def render(*) + 'foo' + super + 'bar' + end + end + + with_custom_tag('blabla', klass2) do + template = Liquid::Template.parse("{% blabla %}") + + assert_equal 'foohellobar', template.render + + buf = '' + output = template.render({}, output: buf) + assert_equal 'foohellobar', output + assert_equal 'foohellobar', buf + assert_equal buf.object_id, output.object_id + end + end end