diff --git a/lib/liquid/context.rb b/lib/liquid/context.rb index 2dcc6af..1cf1885 100644 --- a/lib/liquid/context.rb +++ b/lib/liquid/context.rb @@ -12,12 +12,12 @@ module Liquid # # context['bob'] #=> nil class Context class Context - attr_reader :scopes, :errors, :registers, :environments, :resource_limits + attr_reader :scope, :errors, :registers, :environments, :resource_limits attr_accessor :exception_renderer, :template_name, :partial, :global_filter, :strict_variables, :strict_filters def initialize(environments = {}, outer_scope = {}, registers = {}, rethrow_errors = false, resource_limits = nil) @environments = [environments].flatten - @scopes = [(outer_scope || {})] + @scope = outer_scope || {} @registers = registers @errors = [] @partial = false @@ -25,8 +25,6 @@ module Liquid @resource_limits = resource_limits || ResourceLimits.new(Template.default_resource_limits) squash_instance_assigns_with_environments - @this_stack_used = false - self.exception_renderer = Template.default_exception_renderer if rethrow_errors self.exception_renderer = ->(e) { raise } @@ -35,6 +33,8 @@ module Liquid @interrupts = [] @filters = [] @global_filter = nil + + @stack_level = 0 end def warnings @@ -86,21 +86,9 @@ module Liquid strainer.invoke(method, *args).to_liquid end - # Push new local scope on the stack. use Context#stack instead - def push(new_scope = {}) - @scopes.unshift(new_scope) - raise StackLevelError, "Nesting too deep".freeze if @scopes.length > Block::MAX_DEPTH - end - # Merge a hash of variables in the current local scope def merge(new_scopes) - @scopes[0].merge!(new_scopes) - end - - # Pop from the stack. use Context#stack instead - def pop - raise ContextError if @scopes.size == 1 - @scopes.shift + @scope.merge!(new_scopes) end # Pushes a new local scope on the stack, pops it at the end of the block @@ -111,32 +99,26 @@ module Liquid # end # # context['var] #=> nil - def stack(new_scope = nil) - old_stack_used = @this_stack_used - if new_scope - push(new_scope) - @this_stack_used = true - else - @this_stack_used = false + def stack(*variable_names) + previous_values = {} + variable_names.each do |variable_name| + previous_values[variable_name] = @scope[variable_name] end - yield - ensure - pop if @this_stack_used - @this_stack_used = old_stack_used - end + @stack_level += 1 + raise StackLevelError, "Nesting too deep".freeze if @stack_level > Block::MAX_DEPTH - def clear_instance_assigns - @scopes[0] = {} + begin + yield + ensure + @scope.merge!(previous_values) + @stack_level -= 1 + end end # Only allow String, Numeric, Hash, Array, Proc, Boolean or Liquid::Drop def []=(key, value) - unless @this_stack_used - @this_stack_used = true - push({}) - end - @scopes[0][key] = value + @scope[key] = value end # Look up variable, either resolve directly after considering the name. We can directly handle @@ -161,26 +143,19 @@ module Liquid # Fetches an object starting at the local scope and then moving up the hierachy def find_variable(key, raise_on_not_found: true) - # This was changed from find() to find_index() because this is a very hot - # path and find_index() is optimized in MRI to reduce object allocation - index = @scopes.find_index { |s| s.key?(key) } - scope = @scopes[index] if index - - variable = nil + scope = @scope if @scope.key?(key) if scope.nil? - @environments.each do |e| + index = @environments.find_index do |e| variable = lookup_and_evaluate(e, key, raise_on_not_found: raise_on_not_found) # When lookup returned a value OR there is no value but the lookup also did not raise # then it is the value we are looking for. - if !variable.nil? || @strict_variables && raise_on_not_found - scope = e - break - end + !variable.nil? || @strict_variables && raise_on_not_found end + + scope = @environments[index || -1] end - scope ||= @environments.last || @scopes.last variable ||= lookup_and_evaluate(scope, key, raise_on_not_found: raise_on_not_found) variable = variable.to_liquid @@ -213,10 +188,10 @@ module Liquid end def squash_instance_assigns_with_environments - @scopes.last.each_key do |k| + @scope.each_key do |k| @environments.each do |env| if env.key?(k) - scopes.last[k] = lookup_and_evaluate(env, k) + @scope[k] = lookup_and_evaluate(env, k) break end end diff --git a/lib/liquid/tags/assign.rb b/lib/liquid/tags/assign.rb index 3b696dd..ca630f6 100644 --- a/lib/liquid/tags/assign.rb +++ b/lib/liquid/tags/assign.rb @@ -24,7 +24,7 @@ module Liquid def render(context) val = @from.render(context) - context.scopes.last[@to] = val + context[@to] = val context.resource_limits.assign_score += assign_score_of(val) ''.freeze end diff --git a/lib/liquid/tags/capture.rb b/lib/liquid/tags/capture.rb index d5b8e29..92f25d8 100644 --- a/lib/liquid/tags/capture.rb +++ b/lib/liquid/tags/capture.rb @@ -24,7 +24,7 @@ module Liquid def render(context) output = super - context.scopes.last[@to] = output + context[@to] = output context.resource_limits.assign_score += output.bytesize ''.freeze end diff --git a/lib/liquid/tags/case.rb b/lib/liquid/tags/case.rb index 5036b27..ed6ab09 100644 --- a/lib/liquid/tags/case.rb +++ b/lib/liquid/tags/case.rb @@ -39,20 +39,19 @@ module Liquid end def render(context) - context.stack do - execute_else_block = true + execute_else_block = true - output = '' - @blocks.each do |block| - if block.else? - return block.attachment.render(context) if execute_else_block - elsif block.evaluate(context) - execute_else_block = false - output << block.attachment.render(context) - end + output = '' + @blocks.each do |block| + if block.else? + return block.attachment.render(context) if execute_else_block + elsif block.evaluate(context) + execute_else_block = false + output << block.attachment.render(context) end - output end + + output end private diff --git a/lib/liquid/tags/cycle.rb b/lib/liquid/tags/cycle.rb index 17aa860..5dcd058 100644 --- a/lib/liquid/tags/cycle.rb +++ b/lib/liquid/tags/cycle.rb @@ -34,15 +34,13 @@ module Liquid def render(context) context.registers[:cycle] ||= {} - context.stack do - key = context.evaluate(@name) - iteration = context.registers[:cycle][key].to_i - result = context.evaluate(@variables[iteration]) - iteration += 1 - iteration = 0 if iteration >= @variables.size - context.registers[:cycle][key] = iteration - result - end + key = context.evaluate(@name) + iteration = context.registers[:cycle][key].to_i + result = context.evaluate(@variables[iteration]) + iteration += 1 + iteration = 0 if iteration >= @variables.size + context.registers[:cycle][key] = iteration + result end private diff --git a/lib/liquid/tags/for.rb b/lib/liquid/tags/for.rb index f18fb71..ef68239 100644 --- a/lib/liquid/tags/for.rb +++ b/lib/liquid/tags/for.rb @@ -156,7 +156,7 @@ module Liquid result = '' - context.stack do + context.stack('forloop', @variable_name) do loop_vars = Liquid::ForloopDrop.new(@name, length, for_stack[-1]) for_stack.push(loop_vars) diff --git a/lib/liquid/tags/if.rb b/lib/liquid/tags/if.rb index 02da42b..419d2bb 100644 --- a/lib/liquid/tags/if.rb +++ b/lib/liquid/tags/if.rb @@ -40,14 +40,12 @@ module Liquid end def render(context) - context.stack do - @blocks.each do |block| - if block.evaluate(context) - return block.attachment.render(context) - end + @blocks.each do |block| + if block.evaluate(context) + return block.attachment.render(context) end - ''.freeze end + ''.freeze end private diff --git a/lib/liquid/tags/ifchanged.rb b/lib/liquid/tags/ifchanged.rb index d70cbe1..9e9a70b 100644 --- a/lib/liquid/tags/ifchanged.rb +++ b/lib/liquid/tags/ifchanged.rb @@ -1,15 +1,13 @@ module Liquid class Ifchanged < Block def render(context) - context.stack do - output = super + output = super - if output != context.registers[:ifchanged] - context.registers[:ifchanged] = output - output - else - ''.freeze - end + if output != context.registers[:ifchanged] + context.registers[:ifchanged] = output + output + else + ''.freeze end end end diff --git a/lib/liquid/tags/include.rb b/lib/liquid/tags/include.rb index c9f2a28..c202144 100644 --- a/lib/liquid/tags/include.rb +++ b/lib/liquid/tags/include.rb @@ -60,7 +60,7 @@ module Liquid begin context.template_name = template_name context.partial = true - context.stack do + context.stack(context_variable_name, *@attributes.keys) do @attributes.each do |key, value| context[key] = context.evaluate(value) end diff --git a/lib/liquid/tags/table_row.rb b/lib/liquid/tags/table_row.rb index 7f391cf..5057a7d 100644 --- a/lib/liquid/tags/table_row.rb +++ b/lib/liquid/tags/table_row.rb @@ -31,7 +31,7 @@ module Liquid cols = context.evaluate(@attributes['cols'.freeze]).to_i result = "\n" - context.stack do + context.stack('tablerowloop', @variable_name) do tablerowloop = Liquid::TablerowloopDrop.new(length, cols) context['tablerowloop'.freeze] = tablerowloop diff --git a/lib/liquid/tags/unless.rb b/lib/liquid/tags/unless.rb index 1d4280d..7c5b688 100644 --- a/lib/liquid/tags/unless.rb +++ b/lib/liquid/tags/unless.rb @@ -7,22 +7,20 @@ module Liquid # class Unless < If def render(context) - 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) - 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) - end - end - - ''.freeze + # First condition is interpreted backwards ( if not ) + first_block = @blocks.first + unless first_block.evaluate(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 block.attachment.render(context) + end + end + + ''.freeze end end diff --git a/performance/shopify/comment_form.rb b/performance/shopify/comment_form.rb index d661c31..37f8c0b 100644 --- a/performance/shopify/comment_form.rb +++ b/performance/shopify/comment_form.rb @@ -15,7 +15,7 @@ class CommentForm < Liquid::Block def render(context) article = context[@variable_name] - context.stack do + context.stack('form') do context['form'] = { 'posted_successfully?' => context.registers[:posted_successfully], 'errors' => context['comment.errors'], diff --git a/performance/shopify/paginate.rb b/performance/shopify/paginate.rb index 38a9a1a..18e5c5b 100644 --- a/performance/shopify/paginate.rb +++ b/performance/shopify/paginate.rb @@ -24,7 +24,7 @@ class Paginate < Liquid::Block def render(context) @context = context - context.stack do + context.stack('paginate') do current_page = context['current_page'].to_i pagination = { diff --git a/test/integration/drop_test.rb b/test/integration/drop_test.rb index 2de4a5a..e51111b 100644 --- a/test/integration/drop_test.rb +++ b/test/integration/drop_test.rb @@ -1,14 +1,6 @@ require 'test_helper' class ContextDrop < Liquid::Drop - def scopes - @context.scopes.size - end - - def scopes_as_array - (1..@context.scopes.size).to_a - end - def loop_pos @context['forloop.index'] end @@ -194,31 +186,6 @@ class DropsTest < Minitest::Test end end - def test_scope - assert_equal '1', Liquid::Template.parse('{{ context.scopes }}').render!('context' => ContextDrop.new) - assert_equal '2', Liquid::Template.parse('{%for i in dummy%}{{ context.scopes }}{%endfor%}').render!('context' => ContextDrop.new, 'dummy' => [1]) - assert_equal '3', Liquid::Template.parse('{%for i in dummy%}{%for i in dummy%}{{ context.scopes }}{%endfor%}{%endfor%}').render!('context' => ContextDrop.new, 'dummy' => [1]) - end - - def test_scope_though_proc - assert_equal '1', Liquid::Template.parse('{{ s }}').render!('context' => ContextDrop.new, 's' => proc{ |c| c['context.scopes'] }) - assert_equal '2', Liquid::Template.parse('{%for i in dummy%}{{ s }}{%endfor%}').render!('context' => ContextDrop.new, 's' => proc{ |c| c['context.scopes'] }, 'dummy' => [1]) - assert_equal '3', Liquid::Template.parse('{%for i in dummy%}{%for i in dummy%}{{ s }}{%endfor%}{%endfor%}').render!('context' => ContextDrop.new, 's' => proc{ |c| c['context.scopes'] }, 'dummy' => [1]) - end - - def test_scope_with_assigns - assert_equal 'variable', Liquid::Template.parse('{% assign a = "variable"%}{{a}}').render!('context' => ContextDrop.new) - assert_equal 'variable', Liquid::Template.parse('{% assign a = "variable"%}{%for i in dummy%}{{a}}{%endfor%}').render!('context' => ContextDrop.new, 'dummy' => [1]) - assert_equal 'test', Liquid::Template.parse('{% assign header_gif = "test"%}{{header_gif}}').render!('context' => ContextDrop.new) - assert_equal 'test', Liquid::Template.parse("{% assign header_gif = 'test'%}{{header_gif}}").render!('context' => ContextDrop.new) - end - - def test_scope_from_tags - assert_equal '1', Liquid::Template.parse('{% for i in context.scopes_as_array %}{{i}}{% endfor %}').render!('context' => ContextDrop.new, 'dummy' => [1]) - assert_equal '12', Liquid::Template.parse('{%for a in dummy%}{% for i in context.scopes_as_array %}{{i}}{% endfor %}{% endfor %}').render!('context' => ContextDrop.new, 'dummy' => [1]) - assert_equal '123', Liquid::Template.parse('{%for a in dummy%}{%for a in dummy%}{% for i in context.scopes_as_array %}{{i}}{% endfor %}{% endfor %}{% endfor %}').render!('context' => ContextDrop.new, 'dummy' => [1]) - end - def test_access_context_from_drop assert_equal '123', Liquid::Template.parse('{%for a in dummy%}{{ context.loop_pos }}{% endfor %}').render!('context' => ContextDrop.new, 'dummy' => [1, 2, 3]) end diff --git a/test/unit/context_unit_test.rb b/test/unit/context_unit_test.rb index fab19b8..2acadf9 100644 --- a/test/unit/context_unit_test.rb +++ b/test/unit/context_unit_test.rb @@ -102,21 +102,6 @@ class ContextUnitTest < Minitest::Test assert_nil @context['does_not_exist'] end - def test_scoping - @context.push - @context.pop - - assert_raises(Liquid::ContextError) do - @context.pop - end - - assert_raises(Liquid::ContextError) do - @context.push - @context.pop - @context.pop - end - end - def test_length_query @context['numbers'] = [1, 2, 3, 4] @@ -170,20 +155,43 @@ class ContextUnitTest < Minitest::Test def test_add_item_in_outer_scope @context['test'] = 'test' - @context.push - assert_equal 'test', @context['test'] - @context.pop + + @context.stack('test') do + assert_equal 'test', @context['test'] + end + assert_equal 'test', @context['test'] end def test_add_item_in_inner_scope - @context.push - @context['test'] = 'test' - assert_equal 'test', @context['test'] - @context.pop + @context.stack('test') do + @context['test'] = 'test' + assert_equal 'test', @context['test'] + end + assert_nil @context['test'] end + def test_nested_scopes + @context['test'] = 1 + + @context.stack('test') do + assert_equal 1, @context['test'] + @context['test'] = 2 + assert_equal 2, @context['test'] + + @context.stack('test') do + assert_equal 2, @context['test'] + @context['test'] = 3 + assert_equal 3, @context['test'] + end + + assert_equal 2, @context['test'] + end + + assert_equal 1, @context['test'] + end + def test_hierachical_data @context['hash'] = { "name" => 'tobi' } assert_equal 'tobi', @context['hash.name']