Liquid::Drop: Expect @context to always be set

.github/workflows/liquid.yml

@@ -7,7 +7,7 @@ jobs:
       matrix:
         entry:
           - { ruby: 2.5, allowed-failure: false } # minimum supported
-          - { ruby: 3.1, allowed-failure: false } # latest
+          - { ruby: 3.0, allowed-failure: false } # latest
           - { ruby: ruby-head, allowed-failure: true }
     name: test (${{ matrix.entry.ruby }})
     steps:

History.md

@@ -1,14 +1,13 @@
 # Liquid Change Log
 
-## 5.3.0 (unreleased)
+## 5.2.1 (unreleased)
+
+### Features
 
 ### Fixes
-* StandardFilter: Fix missing @context on iterations (#1525) [Thierry Joyal]
+* Liquid::Drop: Expect @context to always be set (#1528) [Thierry Joyal]
 
-### Deprecation
-* Condition#evaluate to require mandatory context argument in Liquid 6.0.0 (#1527) [Thierry Joyal]
-
-## 5.2.0 2022-03-01
+## 5.2.0 2021-03-01
 
 ### Features
 * Add `remove_last`, and `replace_last` filters (#1422) [Anders Hagbard]
@@ -18,6 +17,7 @@
 * Fix some internal errors in filters from invalid input (#1476) [Dylan Thacker-Smith]
 * Allow dash in filter kwarg name for consistency with Liquid::C (#1518) [CP Clermont]
 
+
 ## 5.1.0 / 2021-09-09
 
 ### Features

lib/liquid/condition.rb

@@ -61,7 +61,7 @@ module Liquid
       @child_condition = nil
     end
 
-    def evaluate(context = deprecated_default_context)
+    def evaluate(context = Context.new)
       condition = self
       result = nil
       loop do
@@ -150,12 +150,6 @@ module Liquid
       end
     end
 
-    def deprecated_default_context
-      warn("DEPRECATION WARNING: Condition#evaluate without a context argument is deprecated" \
-        " and will be removed from Liquid 6.0.0.")
-      Context.new
-    end
-
     class ParseTreeVisitor < Liquid::ParseTreeVisitor
       def children
         [

lib/liquid/drop.rb

@@ -27,7 +27,7 @@ module Liquid
 
     # Catch all for the method
     def liquid_method_missing(method)
-      return nil unless @context&.strict_variables
+      return nil unless @context.strict_variables
       raise Liquid::UndefinedDropMethod, "undefined method #{method}"
     end
 

lib/liquid/standardfilters.rb

@@ -472,12 +472,6 @@ module Liquid
       raise Liquid::FloatDomainError, e.message
     end
 
-    # @public_docs
-    # @syntax {{ number | at_least: number }}
-    # @summary Limits a number to a minimum value.
-    # @type filter
-    # @category Math
-    # @return number
     def at_least(input, n)
       min_value = Utils.to_number(n)
 
@@ -486,12 +480,6 @@ module Liquid
       result.is_a?(BigDecimal) ? result.to_f : result
     end
 
-    # @public_docs
-    # @syntax {{ number | at_most: number }}
-    # @summary Limits a number to a maximum value.
-    # @type filter
-    # @category Math
-    # @return number
     def at_most(input, n)
       max_value = Utils.to_number(n)
 
@@ -594,9 +582,8 @@ module Liquid
 
       def each
         @input.each do |e|
-          e = e.respond_to?(:to_liquid) ? e.to_liquid : e
           e.context = @context if e.respond_to?(:context=)
-          yield(e)
+          yield(e.respond_to?(:to_liquid) ? e.to_liquid : e)
         end
       end
     end

lib/liquid/static_registers.rb

@@ -31,11 +31,7 @@ module Liquid
       if @registers.key?(key)
         @registers.fetch(key)
       elsif default != UNDEFINED
-        if block_given?
-          @static.fetch(key, &block)
-        else
-          @static.fetch(key, default)
-        end
+        @static.fetch(key, default, &block)
       else
         @static.fetch(key, &block)
       end

lib/liquid/version.rb

@@ -2,5 +2,5 @@
 # frozen_string_literal: true
 
 module Liquid
-  VERSION = "5.3.0.alpha"
+  VERSION = "5.2.0"
 end

test/integration/context_test.rb

@@ -24,7 +24,7 @@ class ContextSensitiveDrop < Liquid::Drop
   end
 end
 
-class Category
+class Category < Liquid::Drop
   attr_accessor :name
 
   def initialize(name)
@@ -36,9 +36,8 @@ class Category
   end
 end
 
-class CategoryDrop < Liquid::Drop
+class CategoryDrop
   attr_accessor :category, :context
-
   def initialize(category)
     @category = category
   end
@@ -406,42 +405,45 @@ class ContextTest < Minitest::Test
   end
 
   def test_lambda_is_called_once
-    @global = 0
-
     @context['callcount'] = proc {
-      @global += 1
+      @global ||= 0
+      @global  += 1
       @global.to_s
     }
 
     assert_equal('1', @context['callcount'])
     assert_equal('1', @context['callcount'])
     assert_equal('1', @context['callcount'])
+
+    @global = nil
   end
 
   def test_nested_lambda_is_called_once
-    @global = 0
-
     @context['callcount'] = { "lambda" => proc {
-                                            @global += 1
+                                            @global ||= 0
+                                            @global  += 1
                                             @global.to_s
                                           } }
 
     assert_equal('1', @context['callcount.lambda'])
     assert_equal('1', @context['callcount.lambda'])
     assert_equal('1', @context['callcount.lambda'])
+
+    @global = nil
   end
 
   def test_lambda_in_array_is_called_once
-    @global = 0
-
     @context['callcount'] = [1, 2, proc {
-                                     @global += 1
+                                     @global ||= 0
+                                     @global  += 1
                                      @global.to_s
                                    }, 4, 5]
 
     assert_equal('1', @context['callcount[2]'])
     assert_equal('1', @context['callcount[2]'])
     assert_equal('1', @context['callcount[2]'])
+
+    @global = nil
   end
 
   def test_access_to_context_from_proc

test/integration/profiler_test.rb

@@ -3,27 +3,6 @@
 require 'test_helper'
 
 class ProfilerTest < Minitest::Test
-  class TestDrop < Liquid::Drop
-    def initialize(value)
-      super()
-      @value = value
-    end
-
-    def to_s
-      artificial_execution_time
-
-      @value
-    end
-
-    private
-
-    # Monotonic clock precision fluctuate based on the operating system
-    # By introducing a small sleep we ensure ourselves to register a non zero unit of time
-    def artificial_execution_time
-      sleep(Process.clock_getres(Process::CLOCK_MONOTONIC))
-    end
-  end
-
   include Liquid
 
   class ProfilingFileSystem
@@ -219,22 +198,16 @@ class ProfilerTest < Minitest::Test
 
   def test_profiling_supports_self_time
     t = Template.parse("{% for item in collection %} {{ item }} {% endfor %}", profile: true)
-    collection = [
-      TestDrop.new("one"),
-      TestDrop.new("two"),
-    ]
-    output = t.render!("collection" => collection)
-    assert_equal(" one  two ", output)
-
+    t.render!("collection" => ["one", "two"])
     leaf = t.profiler[0].children[0]
-    assert_operator(leaf.self_time, :>, 0.0)
+
+    assert_operator(leaf.self_time, :>, 0)
   end
 
   def test_profiling_supports_total_time
-    t = Template.parse("{% if true %} {{ test }} {% endif %}", profile: true)
-    output = t.render!("test" => TestDrop.new("one"))
-    assert_equal(" one ", output)
+    t = Template.parse("{% if true %} {% increment test %} {{ test }} {% endif %}", profile: true)
+    t.render!
 
-    assert_operator(t.profiler[0].total_time, :>, 0.0)
+    assert_operator(t.profiler[0].total_time, :>, 0)
   end
 end

test/integration/standard_filter_test.rb

@@ -25,24 +25,8 @@ class TestThing
 end
 
 class TestDrop < Liquid::Drop
-  def initialize(value:)
-    @value = value
-  end
-
-  attr_reader :value
-
-  def registers
-    @context.registers
-  end
-end
-
-class TestModel
-  def initialize(value:)
-    @value = value
-  end
-
-  def to_liquid
-    TestDrop.new(value: @value)
+  def test
+    "testfoo"
   end
 end
 
@@ -378,9 +362,8 @@ class StandardFiltersTest < Minitest::Test
     assert_equal(["foo"], @filters.uniq("foo"))
     assert_equal([1, 3, 2, 4], @filters.uniq([1, 1, 3, 2, 3, 1, 4, 3, 2, 1]))
     assert_equal([{ "a" => 1 }, { "a" => 3 }, { "a" => 2 }], @filters.uniq([{ "a" => 1 }, { "a" => 3 }, { "a" => 1 }, { "a" => 2 }], "a"))
-    test_drop = TestDrop.new(value: "test")
-    test_drop_alternate = TestDrop.new(value: "test")
-    assert_equal([test_drop], @filters.uniq([test_drop, test_drop_alternate], 'value'))
+    testdrop = TestDrop.new
+    assert_equal([testdrop], @filters.uniq([testdrop, TestDrop.new], 'test'))
   end
 
   def test_uniq_empty_array
@@ -439,16 +422,6 @@ class StandardFiltersTest < Minitest::Test
     assert_template_result("woot: 1", '{{ foo | map: "whatever" }}', "foo" => [t])
   end
 
-  def test_map_calls_context=
-    model = TestModel.new(value: "test")
-
-    template = Template.parse('{{ foo | map: "registers" }}')
-    template.registers[:test] = 1234
-    template.assigns['foo'] = [model]
-
-    assert_template_result("{:test=>1234}", template.render!)
-  end
-
   def test_map_on_hashes
     assert_template_result("4217", '{{ thing | map: "foo" | map: "bar" }}',
       "thing" => { "foo" => [{ "bar" => 42 }, { "bar" => 17 }] })
@@ -467,9 +440,9 @@ class StandardFiltersTest < Minitest::Test
   end
 
   def test_map_over_proc
-    drop  = TestDrop.new(value: "testfoo")
+    drop  = TestDrop.new
     p     = proc { drop }
-    templ = '{{ procs | map: "value" }}'
+    templ = '{{ procs | map: "test" }}'
     assert_template_result("testfoo", templ, "procs" => [p])
   end
 
@@ -865,7 +838,7 @@ class StandardFiltersTest < Minitest::Test
   end
 
   def test_all_filters_never_raise_non_liquid_exception
-    test_drop = TestDrop.new(value: "test")
+    test_drop = TestDrop.new
     test_drop.context = Context.new
     test_enum = TestEnumerable.new
     test_enum.context = Context.new

test/unit/condition_unit_test.rb

@@ -10,8 +10,8 @@ class ConditionUnitTest < Minitest::Test
   end
 
   def test_basic_condition
-    assert_equal(false, Condition.new(1, '==', 2).evaluate(Context.new))
-    assert_equal(true,  Condition.new(1, '==', 1).evaluate(Context.new))
+    assert_equal(false, Condition.new(1, '==', 2).evaluate)
+    assert_equal(true,  Condition.new(1, '==', 1).evaluate)
   end
 
   def test_default_operators_evalute_true
@@ -67,11 +67,11 @@ class ConditionUnitTest < Minitest::Test
   end
 
   def test_hash_compare_backwards_compatibility
-    assert_nil(Condition.new({}, '>', 2).evaluate(Context.new))
-    assert_nil(Condition.new(2, '>', {}).evaluate(Context.new))
-    assert_equal(false, Condition.new({}, '==', 2).evaluate(Context.new))
-    assert_equal(true, Condition.new({ 'a' => 1 }, '==', 'a' => 1).evaluate(Context.new))
-    assert_equal(true, Condition.new({ 'a' => 2 }, 'contains', 'a').evaluate(Context.new))
+    assert_nil(Condition.new({}, '>', 2).evaluate)
+    assert_nil(Condition.new(2, '>', {}).evaluate)
+    assert_equal(false, Condition.new({}, '==', 2).evaluate)
+    assert_equal(true, Condition.new({ 'a' => 1 }, '==', 'a' => 1).evaluate)
+    assert_equal(true, Condition.new({ 'a' => 2 }, 'contains', 'a').evaluate)
   end
 
   def test_contains_works_on_arrays
@@ -106,29 +106,30 @@ class ConditionUnitTest < Minitest::Test
 
   def test_or_condition
     condition = Condition.new(1, '==', 2)
-    assert_equal(false, condition.evaluate(Context.new))
+
+    assert_equal(false, condition.evaluate)
 
     condition.or(Condition.new(2, '==', 1))
 
-    assert_equal(false, condition.evaluate(Context.new))
+    assert_equal(false, condition.evaluate)
 
     condition.or(Condition.new(1, '==', 1))
 
-    assert_equal(true, condition.evaluate(Context.new))
+    assert_equal(true, condition.evaluate)
   end
 
   def test_and_condition
     condition = Condition.new(1, '==', 1)
 
-    assert_equal(true, condition.evaluate(Context.new))
+    assert_equal(true, condition.evaluate)
 
     condition.and(Condition.new(2, '==', 2))
 
-    assert_equal(true, condition.evaluate(Context.new))
+    assert_equal(true, condition.evaluate)
 
     condition.and(Condition.new(2, '==', 1))
 
-    assert_equal(false, condition.evaluate(Context.new))
+    assert_equal(false, condition.evaluate)
   end
 
   def test_should_allow_custom_proc_operator
@@ -147,20 +148,6 @@ class ConditionUnitTest < Minitest::Test
     assert_evaluates_true(VariableLookup.new("one"), '==', VariableLookup.new("another"))
   end
 
-  def test_default_context_is_deprecated
-    if Gem::Version.new(Liquid::VERSION) >= Gem::Version.new('6.0.0')
-      flunk("Condition#evaluate without a context argument is to be removed")
-    end
-
-    _out, err = capture_io do
-      assert_equal(true, Condition.new(1, '==', 1).evaluate)
-    end
-
-    expected = "DEPRECATION WARNING: Condition#evaluate without a context argument is deprecated" \
-      " and will be removed from Liquid 6.0.0."
-    assert_includes(err.lines.map(&:strip), expected)
-  end
-
   private
 
   def assert_evaluates_true(left, op, right)