Update liquid-spec adapters

Gemfile

@@ -32,7 +32,6 @@ group :test do
 end
 
 group :spec do
-  # Using feature branch until https://github.com/Shopify/liquid-spec/pull/144 is merged
-  gem 'liquid-spec', github: 'Shopify/liquid-spec', branch: 'self-drop-env-lookup-specs'
+  gem 'liquid-spec', github: 'Shopify/liquid-spec', branch: 'main'
   gem 'activesupport', require: false
 end

History.md

@@ -1,17 +1,5 @@
 # Liquid Change Log
 
-## 5.13.0
-
-* Add TruffleRuby in CI [Benoit Daloze]
-* Skip slow test raising many exceptions on non-CRuby [Benoit Daloze]
-* Reject bare-bracket syntax in strict2 and introduce `self` keyword by [Alok Swamy]
-* Add strict2_parse to assign and capture tags by [Alok Swamy]
-* Add strict2_parse to increment and decrement tags by [Alok Swamy]
-* Update liquid-spec adapters for `missing_features` [Ian Ker-Seymer]
-* Prevent `SelfDrop` context mutation across render boundaries [Guilherme Carreiro]
-* Fix `SelfDrop` equality [Guilherme Carreiro]
-* Let environment `self` shadow `SelfDrop` [Ian Ker-Seymer]
-
 ## 5.11.0
 * Revert the Inline Snippets tag (#2001), treat its inclusion in the latest Liquid release as a bug, and allow for feedback on RFC#1916 to better support Liquid developers [Guilherme Carreiro]
 * Rename the `:rigid` error mode to `:strict2` and display a warning when users attempt to use the `:rigid` mode [Guilherme Carreiro]

lib/liquid/block_body.rb

@@ -99,9 +99,7 @@ module Liquid
         context.handle_error(exc, line_number)
       else
         error_message = context.handle_error(exc, line_number)
-        error_mode = context.registers.static[:template_error_mode]
-        suppress_error_text = blank_tag && error_mode != :strict2 && error_mode != :rigid
-        unless suppress_error_text # blank-tag suppression is kept for backwards compatibility outside strict2
+        unless blank_tag # conditional for backwards compatibility
           output << error_message
         end
       end

lib/liquid/context.rb

@@ -206,21 +206,16 @@ module Liquid
       # path and find_index() is optimized in MRI to reduce object allocation
       index = @scopes.find_index { |s| s.key?(key) }
 
-      fallback_to_self_drop = key == Expression::SELF && index.nil?
+      # `self` resolves to a SelfDrop (enabling `self['var']` lookups),
+      # but only when it hasn't been explicitly assigned as a local variable.
+      return SelfDrop.new(self) if key == Expression::SELF && !index
 
       variable = if index
         lookup_and_evaluate(@scopes[index], key, raise_on_not_found: raise_on_not_found)
       else
-        try_variable_find_in_environments(
-          key,
-          raise_on_not_found: raise_on_not_found && !fallback_to_self_drop,
-        )
+        try_variable_find_in_environments(key, raise_on_not_found: raise_on_not_found)
       end
 
-      # `self` resolves to a SelfDrop (enabling `self['var']` lookups),
-      # but only after the normal environment lookup doesn't find a value.
-      return @self_drop ||= SelfDrop.new(self) if fallback_to_self_drop && variable.nil?
-
       # update variable's context before invoking #to_liquid
       variable.context = self if variable.respond_to?(:context=)
 

lib/liquid/self_drop.rb

@@ -16,39 +16,23 @@ module Liquid
   #   then the local value takes precedence over the `self` object.
   # @liquid_access global
   class SelfDrop < Drop
-    def initialize(self_context)
+    def initialize(context)
       super()
-      @self_context = self_context
+      @context = context
     end
 
     def [](key)
-      @self_context.find_variable(key)
+      @context.find_variable(key)
     rescue UndefinedVariable
       nil
     end
 
     def key?(key)
-      @self_context.variable_defined?(key)
+      @context.variable_defined?(key)
     end
 
     def to_liquid
       self
     end
-
-    def ==(other)
-      other.is_a?(SelfDrop) && other.self_context.equal?(@self_context)
-    end
-
-    alias_method :eql?, :==
-
-    def hash
-      @self_context.object_id.hash
-    end
-
-    protected
-
-    attr_reader :self_context
-
-    undef context=
   end
 end

lib/liquid/template.rb

@@ -151,10 +151,8 @@ module Liquid
 
         c
       when Liquid::Drop
-        drop = args.shift
-        c    = Context.new([drop, assigns], instance_assigns, registers, @rethrow_errors, @resource_limits, {}, @environment)
-        drop.context = c if drop.respond_to?(:context=)
-        c
+        drop         = args.shift
+        drop.context = Context.new([drop, assigns], instance_assigns, registers, @rethrow_errors, @resource_limits, {}, @environment)
       when Hash
         Context.new([args.shift, assigns], instance_assigns, registers, @rethrow_errors, @resource_limits, {}, @environment)
       when nil
@@ -189,20 +187,12 @@ module Liquid
 
       context.template_name ||= name
 
-      previous_error_mode = context.registers.static[:template_error_mode]
-      context.registers.static[:template_error_mode] = @error_mode
-
       begin
         # render the nodelist.
         @root.render_to_output_buffer(context, output || +'')
       rescue Liquid::MemoryError => e
         context.handle_error(e)
       ensure
-        if previous_error_mode
-          context.registers.static[:template_error_mode] = previous_error_mode
-        else
-          context.registers.static.delete(:template_error_mode)
-        end
         @errors = context.errors
       end
     end
@@ -234,7 +224,6 @@ module Liquid
       end
 
       @warnings = parse_context.warnings
-      @error_mode = parse_context.error_mode
       parse_context
     end
 

lib/liquid/version.rb

@@ -2,5 +2,5 @@
 # frozen_string_literal: true
 
 module Liquid
-  VERSION = "5.13.0"
+  VERSION = "5.12.0"
 end

test/integration/blank_body_error_handling_test.rb

@@ -1,85 +0,0 @@
-# frozen_string_literal: true
-
-require 'test_helper'
-
-class BlankBodyErrorHandlingTest < Minitest::Test
-  COMPARISON_ERROR = 'Liquid error (line 1): comparison of Integer with String failed'
-  INVALID_INTEGER_ERROR = 'Liquid error (line 1): invalid integer'
-
-  def render_inline(source, error_mode:, assigns: {})
-    Liquid::Template.parse(source, line_numbers: true, error_mode: error_mode).render(assigns, render_errors: true)
-  end
-
-  def assert_render_raises(source, error_mode:, assigns: {}, message: nil)
-    error = assert_raises(Liquid::ArgumentError) do
-      Liquid::Template.parse(source, line_numbers: true, error_mode: error_mode).render!(assigns)
-    end
-    assert_includes error.message, message if message
-  end
-
-  def test_blank_if_body_suppresses_inline_error_text_in_lax_and_strict
-    [:lax, :strict].each do |mode|
-      assert_equal '', render_inline('{% if 5 > "x" %}{% endif %}', error_mode: mode)
-    end
-  end
-
-  def test_blank_unless_body_suppresses_inline_error_text_in_lax_and_strict
-    [:lax, :strict].each do |mode|
-      assert_equal '', render_inline('{% unless 5 > "x" %} {% endunless %}', error_mode: mode)
-    end
-  end
-
-  def test_blank_for_body_suppresses_inline_error_text_in_lax_and_strict
-    [:lax, :strict].each do |mode|
-      assert_equal '', render_inline('{% for i in (1..3) offset: xs %}{% endfor %}', error_mode: mode, assigns: { 'xs' => 'bad' })
-    end
-  end
-
-  def test_strict2_blank_if_body_shows_inline_error_text
-    assert_equal COMPARISON_ERROR, render_inline('{% if 5 > "x" %}{% endif %}', error_mode: :strict2)
-  end
-
-  def test_strict2_whitespace_if_body_shows_inline_error_text
-    assert_equal COMPARISON_ERROR, render_inline('{% if 5 > "x" %}   {% endif %}', error_mode: :strict2)
-  end
-
-  def test_strict2_assign_if_body_shows_inline_error_text
-    assert_equal COMPARISON_ERROR, render_inline('{% if 5 > "x" %}{% assign a = 1 %}{% endif %}', error_mode: :strict2)
-  end
-
-  def test_strict2_comment_if_body_shows_inline_error_text
-    assert_equal COMPARISON_ERROR, render_inline('{% if 5 > "x" %}{% comment %}c{% endcomment %}{% endif %}', error_mode: :strict2)
-  end
-
-  def test_strict2_capture_if_body_shows_inline_error_text
-    assert_equal COMPARISON_ERROR, render_inline('{% if 5 > "x" %}{% capture c %}text{% endcapture %}{% endif %}', error_mode: :strict2)
-  end
-
-  def test_strict2_blank_unless_body_shows_inline_error_text
-    assert_equal COMPARISON_ERROR, render_inline('{% unless 5 > "x" %} {% endunless %}', error_mode: :strict2)
-  end
-
-  def test_strict2_blank_for_body_shows_inline_error_text
-    assert_equal INVALID_INTEGER_ERROR, render_inline('{% for i in (1..3) offset: xs %}{% endfor %}', error_mode: :strict2, assigns: { 'xs' => 'bad' })
-  end
-
-  def test_nonblank_bodies_show_inline_error_text_in_all_modes
-    [:lax, :strict, :strict2].each do |mode|
-      assert_equal COMPARISON_ERROR, render_inline('{% if 5 > "x" %}{% echo 1 %}{% endif %}', error_mode: mode)
-      assert_equal COMPARISON_ERROR, render_inline('{% if 5 > "x" %}{{ "" }}{% endif %}', error_mode: mode)
-      assert_equal COMPARISON_ERROR, render_inline('{% if 5 > "x" %}{% else %}E{% endif %}', error_mode: mode)
-    end
-  end
-
-  def test_raised_errors_are_not_swallowed_by_blank_if_body
-    [:lax, :strict, :strict2].each do |mode|
-      assert_render_raises('{% if 5 > "x" %}{% endif %}', error_mode: mode, message: 'comparison of Integer with String failed')
-    end
-  end
-
-  def test_raised_errors_are_not_swallowed_by_blank_for_body
-    [:lax, :strict, :strict2].each do |mode|
-      assert_render_raises('{% for i in (1..3) offset: xs %}{% endfor %}', error_mode: mode, assigns: { 'xs' => 'bad' }, message: 'invalid integer')
-    end
-  end
-end

test/integration/self_drop_context_test.rb

@@ -1,120 +0,0 @@
-# frozen_string_literal: true
-
-require 'test_helper'
-
-class SelfDropContextTest < Minitest::Test
-  include Liquid
-
-  def test_self_drop_passed_as_render_param_preserves_original_scope
-    source = <<~LIQUID
-      {%- assign var = 42 -%}
-      {%- assign s = self -%}
-      {%- render "snippet1", other_self: s -%}
-    LIQUID
-
-    partials = {
-      'snippet1' => <<~LIQUID,
-        {%- assign var = 43 -%}
-        {{- other_self.var }}|{{ self.var -}}
-      LIQUID
-    }
-
-    assert_template_result('42|43', source, partials: partials)
-  end
-
-  def test_self_drop_in_render_without_passing_resolves_inner_scope
-    source = <<~LIQUID
-      {%- assign var = 42 -%}
-      {%- render "snippet1" -%}
-    LIQUID
-
-    partials = {
-      'snippet1' => <<~LIQUID,
-        {%- assign var = 99 -%}
-        {{- self.var -}}
-      LIQUID
-    }
-
-    assert_template_result('99', source, partials: partials)
-  end
-
-  def test_self_drop_passed_to_nested_renders_preserves_each_level
-    source = <<~LIQUID
-      {%- assign a = 1 -%}
-      {%- assign s1 = self -%}
-      {%- render "snippet1", outer: s1 -%}
-    LIQUID
-
-    partials = {
-      'snippet1' => <<~LIQUID,
-        {%- assign a = 2 -%}
-        {%- assign s2 = self -%}
-        {%- render "snippet2", outer: outer, middle: s2 -%}
-      LIQUID
-      'snippet2' => <<~LIQUID,
-        {%- assign a = 3 -%}
-        {{- outer.a }}|{{ middle.a }}|{{ self.a -}}
-      LIQUID
-    }
-
-    assert_template_result('1|2|3', source, partials: partials)
-  end
-
-  def test_self_drop_reflects_variables_assigned_after_creation
-    source = <<~LIQUID
-      {%- assign s = self -%}
-      {%- assign x = 42 %}{{ s.x -}}
-    LIQUID
-
-    assert_template_result('42', source)
-  end
-
-  def test_self_drop_context_setter_is_undefined
-    context = Context.new
-    drop = SelfDrop.new(context)
-    refute(drop.respond_to?(:context=))
-
-    assert_template_result('42', '{{ self.x }}', { 'x' => 42 })
-  end
-
-  def test_self_drop_repeated_lookups_compare_equal_for_same_context
-    context = Context.new
-    drop = context.find_variable("self")
-    cached_drop = context.find_variable("self")
-
-    assert_same(drop, cached_drop)
-    assert_equal(drop.object_id, cached_drop.object_id)
-    assert_equal(drop, cached_drop)
-  end
-
-  def test_assigned_self_drop_compares_equal_to_itself
-    assert_template_result('T', '{% assign s = self %}{% if s == s %}T{% else %}F{% endif %}')
-  end
-
-  def test_distinct_self_assignments_compare_equal_for_same_context
-    assert_template_result('T', '{% assign a = self %}{% assign b = self %}{% if a == b %}T{% else %}F{% endif %}')
-  end
-
-  def test_bare_self_compares_equal_to_bare_self
-    assert_template_result('T', '{% if self == self %}T{% else %}F{% endif %}')
-  end
-
-  def test_self_drop_with_strict_variables_does_not_raise_for_defined_var
-    t = Template.parse('{{ self.x }}')
-    result = t.render({ 'x' => 42 }, strict_variables: true)
-    assert_equal('42', result)
-  end
-
-  def test_self_drop_with_strict_variables_returns_nil_for_undefined_var
-    t = Template.parse('{{ self.x }}')
-    result = t.render({}, strict_variables: true)
-    assert_equal('', result)
-  end
-
-  def test_self_drop_can_be_passed_as_bare_drop_to_render
-    t = Template.parse('{{ self.x }}')
-    drop = SelfDrop.new(Context.new({ 'x' => 42 }))
-    result = t.render(drop)
-    assert_equal('42', result)
-  end
-end